refactor: reduce complexity

This commit is contained in:
carla 2026-01-15 17:00:17 +01:00
parent 67072f0c52
commit 6dc398fa5a
3 changed files with 92 additions and 78 deletions

View file

@ -97,17 +97,33 @@ defmodule Mv.Membership.Import.HeaderMapper do
} }
# Build reverse map: normalized_variant -> canonical_field # Build reverse map: normalized_variant -> canonical_field
# This is computed at runtime on first access and cached # Cached on first access for performance
defp normalized_to_canonical do defp normalized_to_canonical do
cached = Process.get({__MODULE__, :normalized_to_canonical})
if cached do
cached
else
map = build_normalized_to_canonical_map()
Process.put({__MODULE__, :normalized_to_canonical}, map)
map
end
end
# Builds the normalized variant -> canonical field map
defp build_normalized_to_canonical_map do
@member_field_variants_raw @member_field_variants_raw
|> Enum.flat_map(fn {canonical, variants} -> |> Enum.flat_map(&map_variants_to_normalized/1)
Enum.map(variants, fn variant ->
{normalize_header(variant), canonical}
end)
end)
|> Map.new() |> Map.new()
end end
# Maps a canonical field and its variants to normalized tuples
defp map_variants_to_normalized({canonical, variants}) do
Enum.map(variants, fn variant ->
{normalize_header(variant), canonical}
end)
end
@doc """ @doc """
Normalizes a CSV header string for comparison. Normalizes a CSV header string for comparison.
@ -208,9 +224,10 @@ defmodule Mv.Membership.Import.HeaderMapper do
|> String.replace(<<0x2212::utf8>>, "-") |> String.replace(<<0x2212::utf8>>, "-")
end end
# Normalizes punctuation: parentheses, slashes become spaces # Normalizes punctuation: parentheses, slashes, underscores become spaces
defp normalize_punctuation(str) do defp normalize_punctuation(str) do
str str
|> String.replace("_", " ")
|> String.replace(~r/[()\[\]{}]/, " ") |> String.replace(~r/[()\[\]{}]/, " ")
|> String.replace(~r/[\/\\]/, " ") |> String.replace(~r/[\/\\]/, " ")
end end
@ -228,18 +245,18 @@ defmodule Mv.Membership.Import.HeaderMapper do
result = result =
headers headers
|> Enum.with_index() |> Enum.with_index()
|> Enum.reduce_while({%{}, [], %{}}, fn {header, index}, {acc_map, acc_unknown, acc_seen} -> |> Enum.reduce_while({%{}, []}, fn {header, index}, {acc_map, acc_unknown} ->
normalized = normalize_header(header) normalized = normalize_header(header)
case process_member_header(header, index, normalized, acc_map, acc_seen) do case process_member_header(header, index, normalized, acc_map, %{}) do
{:error, reason} -> {:error, reason} ->
{:halt, {:error, reason}} {:halt, {:error, reason}}
{:ok, new_map, new_seen} -> {:ok, new_map, _} ->
{:cont, {new_map, acc_unknown, new_seen}} {:cont, {new_map, acc_unknown}}
{:unknown} -> {:unknown} ->
{:cont, {acc_map, [index | acc_unknown], acc_seen}} {:cont, {acc_map, [index | acc_unknown]}}
end end
end) end)
@ -247,7 +264,7 @@ defmodule Mv.Membership.Import.HeaderMapper do
{:error, reason} -> {:error, reason} ->
{:error, reason} {:error, reason}
{member_map, unknown_indices, _normalized_seen} -> {member_map, unknown_indices} ->
validate_required_fields(member_map, unknown_indices) validate_required_fields(member_map, unknown_indices)
end end
end end
@ -258,17 +275,17 @@ defmodule Mv.Membership.Import.HeaderMapper do
{:ok, acc_map, acc_seen} {:ok, acc_map, acc_seen}
end end
defp process_member_header(_header, index, normalized, acc_map, acc_seen) do defp process_member_header(_header, index, normalized, acc_map, _acc_seen) do
if Map.has_key?(normalized_to_canonical(), normalized) do case Map.get(normalized_to_canonical(), normalized) do
canonical = normalized_to_canonical()[normalized] nil ->
{:unknown}
if Map.has_key?(acc_map, canonical) do canonical ->
{:error, "duplicate header for #{canonical} (normalized: #{normalized})"} if Map.has_key?(acc_map, canonical) do
else {:error, "duplicate header for #{canonical} (normalized: #{normalized})"}
{:ok, Map.put(acc_map, canonical, index), Map.put(acc_seen, normalized, canonical)} else
end {:ok, Map.put(acc_map, canonical, index), %{}}
else end
{:unknown}
end end
end end
@ -295,7 +312,7 @@ defmodule Mv.Membership.Import.HeaderMapper do
result = result =
unknown_indices unknown_indices
|> Enum.reduce_while({%{}, [], %{}}, fn index, {acc_map, acc_unknown, acc_seen} -> |> Enum.reduce_while({%{}, []}, fn index, {acc_map, acc_unknown} ->
header = Enum.at(headers, index) header = Enum.at(headers, index)
normalized = normalize_header(header) normalized = normalize_header(header)
@ -305,16 +322,16 @@ defmodule Mv.Membership.Import.HeaderMapper do
normalized, normalized,
custom_field_lookup, custom_field_lookup,
acc_map, acc_map,
acc_seen %{}
) do ) do
{:error, reason} -> {:error, reason} ->
{:halt, {:error, reason}} {:halt, {:error, reason}}
{:ok, new_map, new_seen} -> {:ok, new_map, _} ->
{:cont, {new_map, acc_unknown, new_seen}} {:cont, {new_map, acc_unknown}}
{:unknown} -> {:unknown} ->
{:cont, {acc_map, [index | acc_unknown], acc_seen}} {:cont, {acc_map, [index | acc_unknown]}}
end end
end) end)
@ -322,7 +339,7 @@ defmodule Mv.Membership.Import.HeaderMapper do
{:error, reason} -> {:error, reason} ->
{:error, reason} {:error, reason}
{custom_map, remaining_unknown, _normalized_seen} -> {custom_map, remaining_unknown} ->
{:ok, custom_map, Enum.reverse(remaining_unknown)} {:ok, custom_map, Enum.reverse(remaining_unknown)}
end end
end end
@ -350,10 +367,10 @@ defmodule Mv.Membership.Import.HeaderMapper do
normalized, normalized,
_custom_field_lookup, _custom_field_lookup,
acc_map, acc_map,
acc_seen _acc_seen
) )
when normalized == "" do when normalized == "" do
{:ok, acc_map, acc_seen} {:ok, acc_map, %{}}
end end
defp process_custom_field_header( defp process_custom_field_header(
@ -362,7 +379,7 @@ defmodule Mv.Membership.Import.HeaderMapper do
normalized, normalized,
custom_field_lookup, custom_field_lookup,
acc_map, acc_map,
acc_seen _acc_seen
) do ) do
if Map.has_key?(custom_field_lookup, normalized) do if Map.has_key?(custom_field_lookup, normalized) do
custom_field_id = custom_field_lookup[normalized] custom_field_id = custom_field_lookup[normalized]
@ -370,8 +387,7 @@ defmodule Mv.Membership.Import.HeaderMapper do
if Map.has_key?(acc_map, custom_field_id) do if Map.has_key?(acc_map, custom_field_id) do
{:error, "duplicate custom field header (normalized: #{normalized})"} {:error, "duplicate custom field header (normalized: #{normalized})"}
else else
{:ok, Map.put(acc_map, custom_field_id, index), {:ok, Map.put(acc_map, custom_field_id, index), %{}}
Map.put(acc_seen, normalized, custom_field_id)}
end end
else else
{:unknown} {:unknown}

View file

@ -63,6 +63,7 @@ defmodule Mv.Membership.Import.MemberCSV do
chunks: list(list({pos_integer(), map()})), chunks: list(list({pos_integer(), map()})),
column_map: %{atom() => non_neg_integer()}, column_map: %{atom() => non_neg_integer()},
custom_field_map: %{String.t() => non_neg_integer()}, custom_field_map: %{String.t() => non_neg_integer()},
custom_field_lookup: %{String.t() => %{id: String.t(), value_type: atom()}},
warnings: list(String.t()) warnings: list(String.t())
} }
@ -118,11 +119,15 @@ defmodule Mv.Membership.Import.MemberCSV do
:ok <- validate_row_count(rows, max_rows) do :ok <- validate_row_count(rows, max_rows) do
chunks = chunk_rows(rows, maps, chunk_size) chunks = chunk_rows(rows, maps, chunk_size)
# Build custom field lookup for efficient value processing
custom_field_lookup = build_custom_field_lookup(custom_fields)
{:ok, {:ok,
%{ %{
chunks: chunks, chunks: chunks,
column_map: maps.member, column_map: maps.member,
custom_field_map: maps.custom, custom_field_map: maps.custom,
custom_field_lookup: custom_field_lookup,
warnings: warnings warnings: warnings
}} }}
end end
@ -140,6 +145,15 @@ defmodule Mv.Membership.Import.MemberCSV do
{:error, "Failed to load custom fields: #{Exception.message(e)}"} {:error, "Failed to load custom fields: #{Exception.message(e)}"}
end end
# Builds custom field lookup map for efficient value processing
defp build_custom_field_lookup(custom_fields) do
custom_fields
|> Enum.reduce(%{}, fn cf, acc ->
id_str = to_string(cf.id)
Map.put(acc, id_str, %{id: cf.id, value_type: cf.value_type})
end)
end
# Builds header maps using HeaderMapper and collects warnings for unknown custom fields # Builds header maps using HeaderMapper and collects warnings for unknown custom fields
defp build_header_maps(headers, custom_fields) do defp build_header_maps(headers, custom_fields) do
# Convert custom fields to maps with id and name # Convert custom fields to maps with id and name
@ -263,11 +277,13 @@ defmodule Mv.Membership.Import.MemberCSV do
%{String.t() => non_neg_integer()}, %{String.t() => non_neg_integer()},
keyword() keyword()
) :: {:ok, chunk_result()} | {:error, String.t()} ) :: {:ok, chunk_result()} | {:error, String.t()}
def process_chunk(chunk_rows_with_lines, _column_map, _custom_field_map, _opts \\ []) do def process_chunk(chunk_rows_with_lines, _column_map, _custom_field_map, opts \\ []) do
custom_field_lookup = Keyword.get(opts, :custom_field_lookup, %{})
{inserted, failed, errors} = {inserted, failed, errors} =
Enum.reduce(chunk_rows_with_lines, {0, 0, []}, fn {line_number, row_map}, Enum.reduce(chunk_rows_with_lines, {0, 0, []}, fn {line_number, row_map},
{acc_inserted, acc_failed, acc_errors} -> {acc_inserted, acc_failed, acc_errors} ->
case process_row(row_map, line_number) do case process_row(row_map, line_number, custom_field_lookup) do
{:ok, _member} -> {:ok, _member} ->
{acc_inserted + 1, acc_failed, acc_errors} {acc_inserted + 1, acc_failed, acc_errors}
@ -280,9 +296,13 @@ defmodule Mv.Membership.Import.MemberCSV do
end end
# Processes a single row and creates member with custom field values # Processes a single row and creates member with custom field values
defp process_row(%{member: member_attrs, custom: custom_attrs}, line_number) do defp process_row(
%{member: member_attrs, custom: custom_attrs},
line_number,
custom_field_lookup
) do
# Prepare custom field values for Ash # Prepare custom field values for Ash
custom_field_values = prepare_custom_field_values(custom_attrs) custom_field_values = prepare_custom_field_values(custom_attrs, custom_field_lookup)
# Create member with custom field values # Create member with custom field values
member_attrs_with_cf = member_attrs_with_cf =
@ -314,53 +334,26 @@ defmodule Mv.Membership.Import.MemberCSV do
end end
# Prepares custom field values from row map for Ash # Prepares custom field values from row map for Ash
defp prepare_custom_field_values(custom_attrs) when is_map(custom_attrs) do defp prepare_custom_field_values(custom_attrs, custom_field_lookup) when is_map(custom_attrs) do
custom_attrs custom_attrs
|> Enum.filter(fn {_id, value} -> value != nil && value != "" end) |> Enum.filter(fn {_id, value} -> value != nil && value != "" end)
|> Enum.map(fn {custom_field_id_str, value} -> |> Enum.map(fn {custom_field_id_str, value} ->
# Load custom field to get value_type and ensure ID is correct case Map.get(custom_field_lookup, custom_field_id_str) do
case load_custom_field_by_id(custom_field_id_str) do nil ->
{:ok, custom_field} -> # Custom field not found, skip
# Use the actual custom_field.id (UUID) from the database
%{
"custom_field_id" => to_string(custom_field.id),
"value" => format_custom_field_value(value, custom_field.value_type)
}
{:error, _} ->
# Skip if custom field not found
nil nil
%{id: custom_field_id, value_type: value_type} ->
%{
"custom_field_id" => to_string(custom_field_id),
"value" => format_custom_field_value(value, value_type)
}
end end
end) end)
|> Enum.filter(&(&1 != nil)) |> Enum.filter(&(&1 != nil))
end end
defp prepare_custom_field_values(_), do: [] defp prepare_custom_field_values(_, _), do: []
# Loads a custom field by ID (string or UUID)
defp load_custom_field_by_id(id) when is_binary(id) do
require Ash.Query
try do
# Try to parse as UUID first
uuid_id =
case Ecto.UUID.cast(id) do
{:ok, uuid} -> uuid
:error -> id
end
custom_field =
Mv.Membership.CustomField
|> Ash.Query.filter(id == ^uuid_id)
|> Ash.read_one!()
{:ok, custom_field}
rescue
_ -> {:error, :not_found}
end
end
defp load_custom_field_by_id(_), do: {:error, :invalid_id}
# Formats a custom field value according to its type # Formats a custom field value according to its type
# Uses _union_type and _union_value format as expected by Ash # Uses _union_type and _union_value format as expected by Ash

View file

@ -72,7 +72,7 @@ defmodule Mv.Membership.Import.MemberCSVTest do
end end
end end
describe "process_chunk/3" do describe "process_chunk/4" do
test "function exists and accepts chunk_rows_with_lines, column_map, custom_field_map, and opts" do test "function exists and accepts chunk_rows_with_lines, column_map, custom_field_map, and opts" do
chunk_rows_with_lines = [{2, %{member: %{email: "john@example.com"}, custom: %{}}}] chunk_rows_with_lines = [{2, %{member: %{email: "john@example.com"}, custom: %{}}}]
column_map = %{email: 0} column_map = %{email: 0}
@ -173,7 +173,12 @@ defmodule Mv.Membership.Import.MemberCSVTest do
column_map = %{email: 0} column_map = %{email: 0}
custom_field_map = %{to_string(custom_field.id) => 1} custom_field_map = %{to_string(custom_field.id) => 1}
opts = []
custom_field_lookup = %{
to_string(custom_field.id) => %{id: custom_field.id, value_type: custom_field.value_type}
}
opts = [custom_field_lookup: custom_field_lookup]
assert {:ok, chunk_result} = assert {:ok, chunk_result} =
MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts)