Implements row validation closes #333 #355

Merged
carla merged 4 commits from feature/333_validation into main 2026-01-19 11:47:03 +01:00
9 changed files with 455 additions and 29 deletions

62
docs/email-validation.md Normal file
View file

@ -0,0 +1,62 @@
# Email Validation Strategy
We use `EctoCommons.EmailValidator` with both `:html_input` and `:pow` checks, defined centrally in `Mv.Constants.email_validator_checks/0`.
## Checks Used
- `:html_input` - Pragmatic validation matching browser `<input type="email">` behavior
- `:pow` - Stricter validation following email spec, supports internationalization (Unicode)
## Rationale
Using both checks ensures:
- **Compatibility with common email providers** (`:html_input`) - Matches what users expect from web forms
- **Compliance with email standards** (`:pow`) - Follows RFC 5322 and related specifications
- **Support for international email addresses** (`:pow`) - Allows Unicode characters in email addresses
This dual approach provides a balance between user experience (accepting common email formats) and technical correctness (validating against email standards).
## Usage
The checks are used consistently across all email validation points:
- `Mv.Membership.Import.MemberCSV.validate_row/3` - CSV import validation
- `Mv.Membership.Member` validations - Member resource validation
- `Mv.Accounts.User` validations - User resource validation
All three locations use `Mv.Constants.email_validator_checks()` to ensure consistency.
## Implementation Details
### CSV Import Validation
The CSV import uses a schemaless changeset for email validation:
```elixir
changeset =
{%{}, %{email: :string}}
|> Ecto.Changeset.cast(%{email: Map.get(member_attrs, :email)}, [:email])
|> Ecto.Changeset.update_change(:email, &String.trim/1)
|> Ecto.Changeset.validate_required([:email])
|> EctoCommons.EmailValidator.validate_email(:email, checks: Mv.Constants.email_validator_checks())
```
This approach:
- Trims whitespace before validation
- Validates email is required
- Validates email format using the centralized checks
- Provides consistent error messages via Gettext
### Resource Validations
Both `Member` and `User` resources use similar schemaless changesets within their Ash validations, ensuring consistent validation behavior across the application.
## Changing the Validation Strategy
To change the email validation checks, update the `@email_validator_checks` constant in `Mv.Constants`. This will automatically apply to all validation points.
**Note:** Changing the validation strategy may affect existing data. Consider:
- Whether existing emails will still be valid
- Migration strategy for invalid emails
- User communication if validation becomes stricter

View file

@ -290,7 +290,9 @@ defmodule Mv.Accounts.User do
changeset2 =
{%{}, %{email: :string}}
|> Ecto.Changeset.cast(%{email: email_string}, [:email])
|> EctoCommons.EmailValidator.validate_email(:email, checks: [:html_input, :pow])
|> EctoCommons.EmailValidator.validate_email(:email,
checks: Mv.Constants.email_validator_checks()
)
if changeset2.valid? do
:ok

View file

@ -453,7 +453,9 @@ defmodule Mv.Membership.Member do
changeset2 =
{%{}, %{email: :string}}
|> Ecto.Changeset.cast(%{email: email}, [:email])
|> EctoCommons.EmailValidator.validate_email(:email, checks: [:html_input, :pow])
|> EctoCommons.EmailValidator.validate_email(:email,
checks: Mv.Constants.email_validator_checks()
)
if changeset2.valid? do
:ok

View file

@ -19,6 +19,8 @@ defmodule Mv.Constants do
@custom_field_prefix "custom_field_"
@email_validator_checks [:html_input, :pow]
def member_fields, do: @member_fields
@doc """
@ -30,4 +32,23 @@ defmodule Mv.Constants do
"custom_field_"
"""
def custom_field_prefix, do: @custom_field_prefix
@doc """
Returns the email validator checks used for EctoCommons.EmailValidator.
We use both `:html_input` and `:pow` checks:
- `:html_input` - Pragmatic validation matching browser `<input type="email">` behavior
- `:pow` - Stricter validation following email spec, supports internationalization (Unicode)
Using both ensures:
- Compatibility with common email providers (html_input)
- Compliance with email standards (pow)
- Support for international email addresses (pow)
## Examples
iex> Mv.Constants.email_validator_checks()
[:html_input, :pow]
"""
def email_validator_checks, do: @email_validator_checks
end

View file

@ -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,157 @@ 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, %{})
# Validate email using schemaless changeset
changeset =
{%{}, %{email: :string}}
|> Ecto.Changeset.cast(%{email: Map.get(member_attrs, :email)}, [:email])
|> Ecto.Changeset.update_change(:email, &String.trim/1)
|> Ecto.Changeset.validate_required([:email])
|> EctoCommons.EmailValidator.validate_email(:email,
checks: Mv.Constants.email_validator_checks()
)
if changeset.valid? do
# Apply trimmed email back to member_attrs
trimmed_email = Ecto.Changeset.get_change(changeset, :email)
trimmed_member = Map.put(member_attrs, :email, trimmed_email) |> trim_string_values()
{:ok, %{member: trimmed_member, custom: custom_attrs}}
else
# Extract first error
error = extract_changeset_error(changeset, csv_line_number)
{:error, error}
end
end
# Extracts the first error from a changeset and converts it to a MemberCSV.Error struct
defp extract_changeset_error(changeset, csv_line_number) do
case Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} ->
Enum.reduce(opts, msg, fn {key, value}, acc ->
String.replace(acc, "%{#{key}}", to_string(value))
end)
end) do
%{email: [message | _]} ->
# Email-specific error
%Error{
csv_line_number: csv_line_number,
field: :email,
message: gettext_error_message(message)
}
errors when map_size(errors) > 0 ->
# Get first error (any field)
{field, [message | _]} = Enum.at(Enum.to_list(errors), 0)
%Error{
csv_line_number: csv_line_number,
field: String.to_existing_atom(to_string(field)),
message: gettext_error_message(message)
}
_ ->
# Fallback
%Error{
csv_line_number: csv_line_number,
field: :email,
message: gettext("Email is invalid.")
}
end
end
# Maps changeset error messages to appropriate Gettext messages
defp gettext_error_message(message) when is_binary(message) do
cond do
String.contains?(String.downcase(message), "required") or
String.contains?(String.downcase(message), "can't be blank") ->
gettext("Email is required.")
String.contains?(String.downcase(message), "invalid") or
String.contains?(String.downcase(message), "not a valid") ->
gettext("Email is invalid.")
true ->
message
end
end
defp gettext_error_message(_), do: gettext("Email is invalid.")
# 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 ->

View file

@ -2182,6 +2182,16 @@ msgstr "Mitglied wurde erfolgreich erstellt"
msgid "Member updated successfully"
msgstr "Mitglied wurde erfolgreich aktualisiert"
#: lib/mv/membership/import/member_csv.ex
#, elixir-autogen, elixir-format
msgid "Email is invalid."
msgstr "E-Mail ist ungültig."
#: lib/mv/membership/import/member_csv.ex
#, elixir-autogen, elixir-format
msgid "Email is required."
msgstr "E-Mail ist erforderlich."
#: lib/mv_web/components/layouts/sidebar.ex
#, elixir-autogen, elixir-format, fuzzy
msgid "Roles"

View file

@ -2183,6 +2183,15 @@ msgstr ""
msgid "Member updated successfully"
msgstr ""
#: lib/mv/membership/import/member_csv.ex
#, elixir-autogen, elixir-format
msgid "Email is invalid."
msgstr ""
#: lib/mv/membership/import/member_csv.ex
#, elixir-autogen, elixir-format
msgid "Email is required."
#: lib/mv_web/components/layouts/sidebar.ex
#, elixir-autogen, elixir-format
msgid "Roles"

View file

@ -2183,6 +2183,16 @@ msgstr "Member created successfully"
msgid "Member updated successfully"
msgstr "Member updated successfully"
#: lib/mv/membership/import/member_csv.ex
#, elixir-autogen, elixir-format
msgid "Email is invalid."
msgstr ""
#: lib/mv/membership/import/member_csv.ex
#, elixir-autogen, elixir-format
msgid "Email is required."
msgstr ""
#: lib/mv_web/components/layouts/sidebar.ex
#, elixir-autogen, elixir-format, fuzzy
msgid "Roles"

View file

@ -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,147 @@ 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