From 8b3cc6a6b2100bd0e5847ccd4b834175b7aecf81 Mon Sep 17 00:00:00 2001 From: carla Date: Mon, 19 Jan 2026 11:22:11 +0100 Subject: [PATCH] feat: adds row validation --- lib/mv/membership/import/member_csv.ex | 138 ++++++++++--- test/mv/membership/import/member_csv_test.exs | 189 +++++++++++++++++- 2 files changed, 300 insertions(+), 27 deletions(-) diff --git a/lib/mv/membership/import/member_csv.ex b/lib/mv/membership/import/member_csv.ex index 26756b4..6a1ba0f 100644 --- a/lib/mv/membership/import/member_csv.ex +++ b/lib/mv/membership/import/member_csv.ex @@ -76,6 +76,8 @@ defmodule Mv.Membership.Import.MemberCSV do alias Mv.Membership.Import.CsvParser alias Mv.Membership.Import.HeaderMapper + use Gettext, backend: MvWeb.Gettext + @doc """ Prepares CSV content for import by parsing, mapping headers, and validating limits. @@ -295,38 +297,122 @@ defmodule Mv.Membership.Import.MemberCSV do {:ok, %{inserted: inserted, failed: failed, errors: Enum.reverse(errors)}} end + @doc """ + Validates a single CSV row before database insertion. + + This function: + 1. Trims all string values in the member map + 2. Validates that email is present and not empty after trimming + 3. Validates email format using EctoCommons.EmailValidator + 4. Returns structured errors with Gettext-backed messages + + ## Parameters + + - `row_map` - Map with `:member` and `:custom` keys containing field values + - `csv_line_number` - Physical line number in CSV (1-based, header is line 1) + - `opts` - Optional keyword list (for future extensions) + + ## Returns + + - `{:ok, trimmed_row_map}` - Successfully validated row with trimmed values + - `{:error, %Error{}}` - Validation error with structured error information + + ## Examples + + iex> row_map = %{member: %{email: " john@example.com "}, custom: %{}} + iex> MemberCSV.validate_row(row_map, 2, []) + {:ok, %{member: %{email: "john@example.com"}, custom: %{}}} + + iex> row_map = %{member: %{}, custom: %{}} + iex> MemberCSV.validate_row(row_map, 3, []) + {:error, %MemberCSV.Error{csv_line_number: 3, field: :email, message: "Email is required."}} + """ + @spec validate_row(map(), pos_integer(), keyword()) :: + {:ok, map()} | {:error, Error.t()} + def validate_row(row_map, csv_line_number, _opts \\ []) do + # Safely get member map (handle missing key) + member_attrs = Map.get(row_map, :member, %{}) + custom_attrs = Map.get(row_map, :custom, %{}) + + # Trim all string values in member map + trimmed_member = trim_string_values(member_attrs) + + # Validate email presence (after trim) + email = Map.get(trimmed_member, :email) + + cond do + is_nil(email) or email == "" -> + {:error, + %Error{ + csv_line_number: csv_line_number, + field: :email, + message: gettext("Email is required.") + }} + + not valid_email_format?(email) -> + {:error, + %Error{ + csv_line_number: csv_line_number, + field: :email, + message: gettext("Email is invalid.") + }} + + true -> + {:ok, %{member: trimmed_member, custom: custom_attrs}} + end + end + + # Validates email format using EctoCommons.EmailValidator + defp valid_email_format?(email) when is_binary(email) do + changeset = + {%{}, %{email: :string}} + |> Ecto.Changeset.cast(%{email: email}, [:email]) + |> EctoCommons.EmailValidator.validate_email(:email, checks: [:html_input, :pow]) + + changeset.valid? + end + + defp valid_email_format?(_), do: false + # Processes a single row and creates member with custom field values defp process_row( - %{member: member_attrs, custom: custom_attrs}, + row_map, line_number, custom_field_lookup ) do - # Prepare custom field values for Ash - custom_field_values = prepare_custom_field_values(custom_attrs, custom_field_lookup) - - # Create member with custom field values - member_attrs_with_cf = - member_attrs - |> Map.put(:custom_field_values, custom_field_values) - |> trim_string_values() - - # Only include custom_field_values if not empty - final_attrs = - if Enum.empty?(custom_field_values) do - Map.delete(member_attrs_with_cf, :custom_field_values) - else - member_attrs_with_cf - end - - case Mv.Membership.create_member(final_attrs) do - {:ok, member} -> - {:ok, member} - - {:error, %Ash.Error.Invalid{} = error} -> - {:error, format_ash_error(error, line_number)} - + # Validate row before database insertion + case validate_row(row_map, line_number, []) do {:error, error} -> - {:error, %Error{csv_line_number: line_number, field: nil, message: inspect(error)}} + # Return validation error immediately, no DB insert attempted + {:error, error} + + {:ok, %{member: trimmed_member_attrs, custom: custom_attrs}} -> + # Prepare custom field values for Ash + custom_field_values = prepare_custom_field_values(custom_attrs, custom_field_lookup) + + # Create member with custom field values + member_attrs_with_cf = + trimmed_member_attrs + |> Map.put(:custom_field_values, custom_field_values) + + # Only include custom_field_values if not empty + final_attrs = + if Enum.empty?(custom_field_values) do + Map.delete(member_attrs_with_cf, :custom_field_values) + else + member_attrs_with_cf + end + + case Mv.Membership.create_member(final_attrs) do + {:ok, member} -> + {:ok, member} + + {:error, %Ash.Error.Invalid{} = error} -> + {:error, format_ash_error(error, line_number)} + + {:error, error} -> + {:error, %Error{csv_line_number: line_number, field: nil, message: inspect(error)}} + end end rescue e -> diff --git a/test/mv/membership/import/member_csv_test.exs b/test/mv/membership/import/member_csv_test.exs index 356cc19..5482354 100644 --- a/test/mv/membership/import/member_csv_test.exs +++ b/test/mv/membership/import/member_csv_test.exs @@ -124,7 +124,52 @@ defmodule Mv.Membership.Import.MemberCSVTest do error = List.first(chunk_result.errors) assert error.csv_line_number == 2 assert error.field == :email - assert error.message =~ "email" + # Error message should come from validate_row (Gettext-backed) + assert is_binary(error.message) + assert error.message != "" + end + + test "returns error for missing email" do + chunk_rows_with_lines = [ + {2, %{member: %{}, custom: %{}}} + ] + + column_map = %{} + custom_field_map = %{} + opts = [] + + assert {:ok, chunk_result} = + MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) + + assert chunk_result.inserted == 0 + assert chunk_result.failed == 1 + assert length(chunk_result.errors) == 1 + + error = List.first(chunk_result.errors) + assert error.csv_line_number == 2 + assert error.field == :email + assert is_binary(error.message) + end + + test "returns error for whitespace-only email" do + chunk_rows_with_lines = [ + {3, %{member: %{email: " "}, custom: %{}}} + ] + + column_map = %{email: 0} + custom_field_map = %{} + opts = [] + + assert {:ok, chunk_result} = + MemberCSV.process_chunk(chunk_rows_with_lines, column_map, custom_field_map, opts) + + assert chunk_result.inserted == 0 + assert chunk_result.failed == 1 + assert length(chunk_result.errors) == 1 + + error = List.first(chunk_result.errors) + assert error.csv_line_number == 3 + assert error.field == :email end test "returns error for duplicate email" do @@ -218,6 +263,9 @@ defmodule Mv.Membership.Import.MemberCSVTest do error = List.first(chunk_result.errors) assert error.csv_line_number == 3 + assert error.field == :email + # Error should come from validate_row, not from DB insert + assert is_binary(error.message) end test "preserves CSV line numbers in errors" do @@ -279,6 +327,145 @@ defmodule Mv.Membership.Import.MemberCSVTest do end end + describe "validate_row/3" do + test "returns error when email is missing" do + row_map = %{member: %{}, custom: %{}} + csv_line_number = 5 + opts = [] + + assert {:error, error} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert %MemberCSV.Error{} = error + assert error.csv_line_number == 5 + assert error.field == :email + assert error.message != nil + assert error.message != "" + end + + test "returns error when email is only whitespace" do + row_map = %{member: %{email: " "}, custom: %{}} + csv_line_number = 3 + opts = [] + + assert {:error, error} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert %MemberCSV.Error{} = error + assert error.csv_line_number == 3 + assert error.field == :email + assert error.message != nil + end + + test "returns error when email is nil" do + row_map = %{member: %{email: nil}, custom: %{}} + csv_line_number = 7 + opts = [] + + assert {:error, error} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert %MemberCSV.Error{} = error + assert error.csv_line_number == 7 + assert error.field == :email + end + + test "returns error when email format is invalid" do + row_map = %{member: %{email: "invalid-email"}, custom: %{}} + csv_line_number = 4 + opts = [] + + assert {:error, error} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert %MemberCSV.Error{} = error + assert error.csv_line_number == 4 + assert error.field == :email + assert error.message != nil + end + + test "returns {:ok, trimmed_row_map} when email is valid with whitespace" do + row_map = %{member: %{email: " john@example.com "}, custom: %{}} + csv_line_number = 2 + opts = [] + + assert {:ok, trimmed_row_map} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert trimmed_row_map.member.email == "john@example.com" + end + + test "returns {:ok, trimmed_row_map} when email is valid without whitespace" do + row_map = %{member: %{email: "john@example.com"}, custom: %{}} + csv_line_number = 2 + opts = [] + + assert {:ok, trimmed_row_map} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert trimmed_row_map.member.email == "john@example.com" + end + + test "trims all string values in member map" do + row_map = %{ + member: %{ + email: " john@example.com ", + first_name: " John ", + last_name: " Doe " + }, + custom: %{} + } + csv_line_number = 2 + opts = [] + + assert {:ok, trimmed_row_map} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert trimmed_row_map.member.email == "john@example.com" + assert trimmed_row_map.member.first_name == "John" + assert trimmed_row_map.member.last_name == "Doe" + end + + test "preserves custom map unchanged" do + row_map = %{ + member: %{email: "john@example.com"}, + custom: %{"field1" => "value1"} + } + csv_line_number = 2 + opts = [] + + assert {:ok, trimmed_row_map} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert trimmed_row_map.custom == %{"field1" => "value1"} + end + + test "uses Gettext for error messages" do + row_map = %{member: %{}, custom: %{}} + csv_line_number = 5 + opts = [] + + # Test with default locale (should work) + assert {:error, error} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert is_binary(error.message) + + # Test with German locale + Gettext.put_locale(MvWeb.Gettext, "de") + assert {:error, error_de} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert is_binary(error_de.message) + + # Test with English locale + Gettext.put_locale(MvWeb.Gettext, "en") + assert {:error, error_en} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert is_binary(error_en.message) + + # Reset to default + Gettext.put_locale(MvWeb.Gettext, "en") + end + + test "handles empty opts gracefully" do + row_map = %{member: %{email: "john@example.com"}, custom: %{}} + csv_line_number = 2 + opts = [] + + assert {:ok, _} = MemberCSV.validate_row(row_map, csv_line_number, opts) + end + + test "handles missing member key gracefully" do + row_map = %{custom: %{}} + csv_line_number = 3 + opts = [] + + assert {:error, error} = MemberCSV.validate_row(row_map, csv_line_number, opts) + assert %MemberCSV.Error{} = error + assert error.csv_line_number == 3 + end + end + describe "module documentation" do test "module has @moduledoc" do # Check that the module exists and has documentation