From def399122c354e4b6d7534ba3c290ec4b10511cd Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 16:14:58 +0100 Subject: [PATCH 1/8] fix tests with async true --- test/mv_web/member_live/index_field_visibility_test.exs | 3 ++- test/mv_web/user_live/form_test.exs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/mv_web/member_live/index_field_visibility_test.exs b/test/mv_web/member_live/index_field_visibility_test.exs index 6e1642a..05fa768 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -10,7 +10,8 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do - Integration with member list display - Custom fields visibility """ - use MvWeb.ConnCase, async: true + # async: false to prevent PostgreSQL deadlocks when creating members and custom fields + use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/user_live/form_test.exs b/test/mv_web/user_live/form_test.exs index b8f7313..334dedd 100644 --- a/test/mv_web/user_live/form_test.exs +++ b/test/mv_web/user_live/form_test.exs @@ -1,5 +1,6 @@ defmodule MvWeb.UserLive.FormTest do - use MvWeb.ConnCase, async: true + # async: false to prevent PostgreSQL deadlocks when creating members and users + use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest # Helper to setup authenticated connection and live view From 5718a37aca64d5988674a4c9fc465dbf22ed1d68 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 16:15:22 +0100 Subject: [PATCH 2/8] fix: show custom field input fields also when empty --- lib/mv_web/live/member_live/show.ex | 63 +++++++++++++++++++---------- 1 file changed, 41 insertions(+), 22 deletions(-) diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index d84fca4..52c009a 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -131,15 +131,26 @@ defmodule MvWeb.MemberLive.Show do <%!-- Custom Fields Section --%> - <%= if Enum.any?(@member.custom_field_values) do %> + <%= if Enum.any?(@custom_fields_with_values) do %>
<.section_box title={gettext("Custom Fields")}>
- <%= for cfv <- sort_custom_field_values(@member.custom_field_values) do %> - <% custom_field = cfv.custom_field %> - <% value_type = custom_field && custom_field.value_type %> - <.data_field label={custom_field && custom_field.name}> - {format_custom_field_value(cfv.value, value_type)} + <%= for {custom_field, cfv} <- @custom_fields_with_values do %> + <.data_field label={custom_field.name}> + <%= if cfv && cfv.value do %> + <%= if custom_field.value_type == :email && cfv.value.value && String.trim(cfv.value.value) != "" do %> + + {cfv.value.value} + + <% else %> + {format_custom_field_value(cfv.value, custom_field.value_type)} + <% end %> + <% else %> + {format_custom_field_value(nil, custom_field.value_type)} + <% end %> <% end %>
@@ -175,7 +186,7 @@ defmodule MvWeb.MemberLive.Show do @impl true def mount(_params, _session, socket) do - {:ok, socket} + {:ok, assign(socket, :custom_fields_with_values, [])} end @impl true @@ -187,10 +198,28 @@ defmodule MvWeb.MemberLive.Show do member = Ash.read_one!(query) + # Load all custom fields to display all of them, even if they have no values + {:ok, custom_fields} = Mv.Membership.list_custom_fields() + + # Create a map of custom_field_id -> custom_field_value for quick lookup + custom_field_values_map = + member.custom_field_values + |> Enum.map(fn cfv -> {cfv.custom_field_id, cfv} end) + |> Map.new() + + # Match all custom fields with their values (if they exist) + custom_fields_with_values = + Enum.map(custom_fields, fn cf -> + cfv = Map.get(custom_field_values_map, cf.id) + {cf, cfv} + end) + |> Enum.sort_by(fn {cf, _cfv} -> cf.name end) + {:noreply, socket |> assign(:page_title, page_title(socket.assigns.live_action)) - |> assign(:member, member)} + |> assign(:member, member) + |> assign(:custom_fields_with_values, custom_fields_with_values)} end defp page_title(:show), do: gettext("Show Member") @@ -272,19 +301,13 @@ defmodule MvWeb.MemberLive.Show do defp format_date(date), do: to_string(date) - # Sorts custom field values by custom field name - defp sort_custom_field_values(custom_field_values) do - Enum.sort_by(custom_field_values, fn cfv -> - (cfv.custom_field && cfv.custom_field.name) || "" - end) - end - # Formats custom field value based on type + # Returns empty string for nil/empty values (consistent with member fields behavior) defp format_custom_field_value(%Ash.Union{value: value, type: type}, _expected_type) do format_custom_field_value(value, type) end - defp format_custom_field_value(nil, _type), do: "—" + defp format_custom_field_value(nil, _type), do: "" defp format_custom_field_value(value, :boolean) when is_boolean(value) do if value, do: gettext("Yes"), else: gettext("No") @@ -295,11 +318,7 @@ defmodule MvWeb.MemberLive.Show do end defp format_custom_field_value(value, :email) when is_binary(value) do - assigns = %{email: value} - - ~H""" - {@email} - """ + if String.trim(value) == "", do: "", else: value end defp format_custom_field_value(value, :integer) when is_integer(value) do @@ -307,7 +326,7 @@ defmodule MvWeb.MemberLive.Show do end defp format_custom_field_value(value, _type) when is_binary(value) do - if String.trim(value) == "", do: "—", else: value + if String.trim(value) == "", do: "", else: value end defp format_custom_field_value(value, _type), do: to_string(value) From 8e58829e952624023498d34ab262ada14808e8ae Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 17:07:38 +0100 Subject: [PATCH 3/8] fix: improve performance loading custom fields --- lib/mv_web/live/member_live/show.ex | 119 +++++++++++++++++----------- 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index 52c009a..479f170 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -73,12 +73,7 @@ defmodule MvWeb.MemberLive.Show do <%!-- Email --%>
<.data_field label={gettext("Email")}> - - {@member.email} - + <.mailto_link email={@member.email} display={@member.email} />
@@ -131,26 +126,14 @@ defmodule MvWeb.MemberLive.Show do
<%!-- Custom Fields Section --%> - <%= if Enum.any?(@custom_fields_with_values) do %> + <%= if Enum.any?(@custom_fields) do %>
<.section_box title={gettext("Custom Fields")}>
- <%= for {custom_field, cfv} <- @custom_fields_with_values do %> + <%= for custom_field <- @custom_fields do %> + <% cfv = find_custom_field_value(@member.custom_field_values, custom_field.id) %> <.data_field label={custom_field.name}> - <%= if cfv && cfv.value do %> - <%= if custom_field.value_type == :email && cfv.value.value && String.trim(cfv.value.value) != "" do %> - - {cfv.value.value} - - <% else %> - {format_custom_field_value(cfv.value, custom_field.value_type)} - <% end %> - <% else %> - {format_custom_field_value(nil, custom_field.value_type)} - <% end %> + {format_custom_field_value(cfv, custom_field.value_type)} <% end %>
@@ -186,11 +169,18 @@ defmodule MvWeb.MemberLive.Show do @impl true def mount(_params, _session, socket) do - {:ok, assign(socket, :custom_fields_with_values, [])} + {:ok, socket} end @impl true def handle_params(%{"id" => id}, _, socket) do + # Load custom fields once using assign_new to avoid repeated queries + socket = assign_new(socket, :custom_fields, fn -> + Mv.Membership.CustomField + |> Ash.Query.sort(name: :asc) + |> Ash.read!() + end) + query = Mv.Membership.Member |> filter(id == ^id) @@ -198,28 +188,10 @@ defmodule MvWeb.MemberLive.Show do member = Ash.read_one!(query) - # Load all custom fields to display all of them, even if they have no values - {:ok, custom_fields} = Mv.Membership.list_custom_fields() - - # Create a map of custom_field_id -> custom_field_value for quick lookup - custom_field_values_map = - member.custom_field_values - |> Enum.map(fn cfv -> {cfv.custom_field_id, cfv} end) - |> Map.new() - - # Match all custom fields with their values (if they exist) - custom_fields_with_values = - Enum.map(custom_fields, fn cf -> - cfv = Map.get(custom_field_values_map, cf.id) - {cf, cfv} - end) - |> Enum.sort_by(fn {cf, _cfv} -> cf.name end) - {:noreply, socket |> assign(:page_title, page_title(socket.assigns.live_action)) - |> assign(:member, member) - |> assign(:custom_fields_with_values, custom_fields_with_values)} + |> assign(:member, member)} end defp page_title(:show), do: gettext("Show Member") @@ -265,6 +237,30 @@ defmodule MvWeb.MemberLive.Show do """ end + # Renders a mailto link if email is present, otherwise renders nothing + attr :email, :string, required: true + attr :display, :string, default: nil + + defp mailto_link(assigns) do + display_text = assigns.display || assigns.email + + if assigns.email && String.trim(assigns.email) != "" do + assigns = %{email: assigns.email, display: display_text} + ~H""" + + {@display} + + """ + else + ~H""" + — + """ + end + end + # ----------------------------------------------------------------- # Helper Functions # ----------------------------------------------------------------- @@ -301,14 +297,28 @@ defmodule MvWeb.MemberLive.Show do defp format_date(date), do: to_string(date) + # Finds custom field value for a given custom field id + defp find_custom_field_value(nil, _custom_field_id), do: nil + defp find_custom_field_value(custom_field_values, custom_field_id) when is_list(custom_field_values) do + Enum.find(custom_field_values, fn cfv -> + cfv.custom_field_id == custom_field_id or + (cfv.custom_field && cfv.custom_field.id == custom_field_id) + end) + end + defp find_custom_field_value(_custom_field_values, _custom_field_id), do: nil + # Formats custom field value based on type - # Returns empty string for nil/empty values (consistent with member fields behavior) + # Handles both CustomFieldValue structs and direct values + defp format_custom_field_value(nil, _type), do: render_empty_value() + + defp format_custom_field_value(%Mv.Membership.CustomFieldValue{} = cfv, value_type) do + format_custom_field_value(cfv.value, value_type) + end + defp format_custom_field_value(%Ash.Union{value: value, type: type}, _expected_type) do format_custom_field_value(value, type) end - defp format_custom_field_value(nil, _type), do: "" - defp format_custom_field_value(value, :boolean) when is_boolean(value) do if value, do: gettext("Yes"), else: gettext("No") end @@ -318,7 +328,14 @@ defmodule MvWeb.MemberLive.Show do end defp format_custom_field_value(value, :email) when is_binary(value) do - if String.trim(value) == "", do: "", else: value + if String.trim(value) == "" do + render_empty_value() + else + assigns = %{email: value} + ~H""" + <.mailto_link email={@email} display={@email} /> + """ + end end defp format_custom_field_value(value, :integer) when is_integer(value) do @@ -326,8 +343,16 @@ defmodule MvWeb.MemberLive.Show do end defp format_custom_field_value(value, _type) when is_binary(value) do - if String.trim(value) == "", do: "", else: value + if String.trim(value) == "", do: render_empty_value(), else: value end defp format_custom_field_value(value, _type), do: to_string(value) + + # Renders placeholder for empty custom field values + defp render_empty_value do + assigns = %{} + ~H""" + — + """ + end end From 398a63a98f3b8ace8105bb39d382b3ee43278486 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 17:07:52 +0100 Subject: [PATCH 4/8] add tests for empty custom field section --- test/mv_web/member_live/show_test.exs | 174 ++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 test/mv_web/member_live/show_test.exs diff --git a/test/mv_web/member_live/show_test.exs b/test/mv_web/member_live/show_test.exs new file mode 100644 index 0000000..6ee9b50 --- /dev/null +++ b/test/mv_web/member_live/show_test.exs @@ -0,0 +1,174 @@ +defmodule MvWeb.MemberLive.ShowTest do + @moduledoc """ + Tests for the member show page. + + Tests cover: + - Displaying member information + - Custom Fields section visibility (Issue #282 regression test) + - Custom field values formatting + + ## Note on async: false + Tests use `async: false` (not `async: true`) to prevent PostgreSQL deadlocks + when creating members and custom fields concurrently. This is intentional and + documented here to avoid confusion in commit messages. + """ + # async: false to prevent PostgreSQL deadlocks when creating members and custom fields + use MvWeb.ConnCase, async: false + import Phoenix.LiveViewTest + require Ash.Query + + alias Mv.Membership.{CustomField, CustomFieldValue, Member} + + setup do + # Create test member + {:ok, member} = + Member + |> Ash.Changeset.for_create(:create_member, %{ + first_name: "Alice", + last_name: "Anderson", + email: "alice@example.com" + }) + |> Ash.create() + + %{member: member} + end + + describe "custom fields section visibility (Issue #282)" do + test "displays Custom Fields section even when member has no custom field values", %{ + conn: conn, + member: member + } do + # Create a custom field but no value for the member + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "phone_mobile", + value_type: :string + }) + |> Ash.create() + + conn = conn_with_oidc_user(conn) + {:ok, view, html} = live(conn, ~p"/members/#{member}") + + # Custom Fields section should be visible + assert html =~ gettext("Custom Fields") + + # Custom field label should be visible + assert html =~ custom_field.name + + # Value should show placeholder for empty value + assert html =~ "—" or html =~ gettext("Not set") + end + + test "displays Custom Fields section with multiple custom fields, some without values", %{ + conn: conn, + member: member + } do + # Create multiple custom fields + {:ok, field1} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "phone_mobile", + value_type: :string + }) + |> Ash.create() + + {:ok, field2} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "membership_number", + value_type: :integer + }) + |> Ash.create() + + # Create value only for first field + {:ok, _cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: field1.id, + value: %{"_union_type" => "string", "_union_value" => "+49123456789"} + }) + |> Ash.create() + + conn = conn_with_oidc_user(conn) + {:ok, view, html} = live(conn, ~p"/members/#{member}") + + # Custom Fields section should be visible + assert html =~ gettext("Custom Fields") + + # Both field labels should be visible + assert html =~ field1.name + assert html =~ field2.name + + # First field should show value + assert html =~ "+49123456789" + + # Second field should show placeholder + assert html =~ "—" or html =~ gettext("Not set") + end + + test "does not display Custom Fields section when no custom fields exist", %{ + conn: conn, + member: member + } do + conn = conn_with_oidc_user(conn) + {:ok, view, html} = live(conn, ~p"/members/#{member}") + + # Custom Fields section should NOT be visible + refute html =~ gettext("Custom Fields") + end + end + + describe "custom field value formatting" do + test "formats string custom field values", %{conn: conn, member: member} do + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "phone_mobile", + value_type: :string + }) + |> Ash.create() + + {:ok, _cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "+49123456789"} + }) + |> Ash.create() + + conn = conn_with_oidc_user(conn) + {:ok, view, html} = live(conn, ~p"/members/#{member}") + + assert html =~ "+49123456789" + end + + test "formats email custom field values as mailto links", %{conn: conn, member: member} do + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "private_email", + value_type: :email + }) + |> Ash.create() + + {:ok, _cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "email", "_union_value" => "private@example.com"} + }) + |> Ash.create() + + conn = conn_with_oidc_user(conn) + {:ok, view, html} = live(conn, ~p"/members/#{member}") + + # Should contain mailto link + assert html =~ ~s(href="mailto:private@example.com") + assert html =~ "private@example.com" + end + end +end From 33652265b82dec2a330bef7727b8bf1c567082e4 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 17:10:52 +0100 Subject: [PATCH 5/8] feat: add accessible empty value also to member fields --- lib/mv_web/live/member_live/show.ex | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index 479f170..c9c92f0 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -237,7 +237,7 @@ defmodule MvWeb.MemberLive.Show do """ end - # Renders a mailto link if email is present, otherwise renders nothing + # Renders a mailto link if email is present, otherwise renders empty value placeholder attr :email, :string, required: true attr :display, :string, default: nil @@ -255,9 +255,7 @@ defmodule MvWeb.MemberLive.Show do """ else - ~H""" - — - """ + render_empty_value() end end @@ -265,8 +263,8 @@ defmodule MvWeb.MemberLive.Show do # Helper Functions # ----------------------------------------------------------------- - defp display_value(nil), do: "" - defp display_value(""), do: "" + defp display_value(nil), do: render_empty_value() + defp display_value(""), do: render_empty_value() defp display_value(value), do: value defp format_address(member) do @@ -348,11 +346,16 @@ defmodule MvWeb.MemberLive.Show do defp format_custom_field_value(value, _type), do: to_string(value) - # Renders placeholder for empty custom field values + # Renders accessible placeholder for empty values + # Uses translated text for screen readers while maintaining visual consistency + # The visual "—" is hidden from screen readers, while the translated text is only visible to screen readers defp render_empty_value do - assigns = %{} + assigns = %{text: gettext("Not set")} ~H""" - — + + + {@text} + """ end end From 828e09dce14480831d0ba1ab1cd826d72489e1b9 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 18:12:05 +0100 Subject: [PATCH 6/8] docs: adds implementation plan --- docs/csv-member-import-v1.md | 611 +++++++++++++++++++++++++++++++++++ 1 file changed, 611 insertions(+) create mode 100644 docs/csv-member-import-v1.md diff --git a/docs/csv-member-import-v1.md b/docs/csv-member-import-v1.md new file mode 100644 index 0000000..30409b8 --- /dev/null +++ b/docs/csv-member-import-v1.md @@ -0,0 +1,611 @@ +# CSV Member Import v1 - Implementation Plan + +**Version:** 1.0 +**Date:** 2025-01-XX +**Status:** Ready for Implementation +**Related Documents:** +- [Feature Roadmap](./feature-roadmap.md) - Overall feature planning + +--- + +## Table of Contents + +- [Overview & Scope](#overview--scope) +- [UX Flow](#ux-flow) +- [CSV Specification](#csv-specification) +- [Technical Design Notes](#technical-design-notes) +- [Implementation Issues](#implementation-issues) +- [Rollout & Risks](#rollout--risks) + +--- + +## Overview & Scope + +### What We're Building + +A **basic CSV member import feature** that allows administrators to upload a CSV file and import new members into the system. This is a **v1 minimal implementation** focused on establishing the import structure without advanced features. + +**Core Functionality (v1 Minimal):** +- Upload CSV file via LiveView file upload +- Parse CSV with bilingual header support for core member fields (English/German) +- Auto-detect delimiter (`;` or `,`) using header recognition +- Map CSV columns to core member fields (`first_name`, `last_name`, `email`, `phone_number`, `street`, `postal_code`, `city`) +- Validate each row (required fields: `first_name`, `last_name`, `email`) +- Create members via Ash resource (one-by-one, **no background jobs**, processed in chunks of 200 rows via LiveView messages) +- Display import results: success count, error count, and error details +- Provide static CSV templates (EN/DE) + +**Optional Enhancement (v1.1 - Last Issue):** +- Custom field import (if time permits, otherwise defer to v2) + +**Key Constraints (v1):** +- ✅ **Admin-only feature** +- ✅ **No upsert** (create only) +- ✅ **No deduplication** (duplicate emails fail and show as errors) +- ✅ **No mapping wizard** (fixed header mapping via bilingual variants) +- ✅ **No background jobs** (progress via LiveView `handle_info`) +- ✅ **Best-effort import** (row-by-row, no rollback) +- ✅ **UI-only error display** (no error CSV export) +- ✅ **Safety limits** (10 MB, 1,000 rows, chunks of 200) + +### Out of Scope (v1) + +**Deferred to Future Versions:** +- ❌ Upsert/update existing members +- ❌ Advanced deduplication strategies +- ❌ Column mapping wizard UI +- ❌ Background job processing (Oban/GenStage) +- ❌ Transactional all-or-nothing import +- ❌ Error CSV export/download +- ⚠️ Custom field import (optional, last issue - defer to v2 if scope is tight) +- ❌ Batch validation preview before import +- ❌ Date/boolean field parsing +- ❌ Dynamic template generation +- ❌ Import history/audit log +- ❌ Import templates for other entities + +--- + +## UX Flow + +### Access & Location + +**Entry Point:** +- **Location:** Global Settings page (`/settings`) +- **UI Element:** New section "Import Members (CSV)" below "Custom Fields" section +- **Access Control:** Admin-only (enforced at LiveView event level, not entire `/settings` route) + +### User Journey + +1. **Navigate to Global Settings** +2. **Access Import Section** + - Upload area (drag & drop or file picker) + - Template download links (English / German) + - Help text explaining CSV format +3. **Download Template (Optional)** +4. **Prepare CSV File** +5. **Upload CSV** +6. **Start Import** + - Runs server-side via LiveView messages (may take up to ~30 seconds for large files) +7. **View Results** + - Success count + - Error count + - First 50 errors, each with: + - **CSV line number** (header is line 1, first data record begins at line 2) + - Error message + - Field name (if applicable) + +### Error Handling + +- **File too large:** Flash error before upload starts +- **Too many rows:** Flash error before import starts +- **Invalid CSV format:** Error shown in results +- **Partial success:** Results show both success and error counts + +--- + +## CSV Specification + +### Delimiter + +**Recommended:** Semicolon (`;`) +**Supported:** `;` and `,` + +**Auto-Detection (Header Recognition):** +- Remove UTF-8 BOM *first* +- Extract header record and try parsing with both delimiters +- For each delimiter, count how many recognized headers are present (via normalized variants) +- Choose delimiter with higher recognition; prefer `;` if tied +- If neither yields recognized headers, default to `;` + +### Quoting Rules + +- Fields may be quoted with double quotes (`"`) +- Escaped quotes: `""` inside quoted field represents a single `"` +- **v1 assumption:** CSV records do **not** contain embedded newlines inside quoted fields. (If they do, parsing may fail or line numbers may be inaccurate.) + +### Column Headers + +**v1 Supported Fields (Core Member Fields Only):** +- `first_name` / `Vorname` (required) +- `last_name` / `Nachname` (required) +- `email` / `E-Mail` (required) +- `phone_number` / `Telefon` (optional) +- `street` / `Straße` (optional) +- `postal_code` / `PLZ` / `Postleitzahl` (optional) +- `city` / `Stadt` (optional) + +**Member Field Header Mapping:** + +| Canonical Field | English Variants | German Variants | +|---|---|---| +| `first_name` | `first_name`, `firstname` | `Vorname`, `vorname` | +| `last_name` | `last_name`, `lastname`, `surname` | `Nachname`, `nachname`, `Familienname` | +| `email` | `email`, `e-mail`, `e_mail` | `E-Mail`, `e-mail`, `e_mail` | +| `phone_number` | `phone_number`, `phone`, `telephone` | `Telefon`, `telefon` | +| `street` | `street`, `address` | `Straße`, `strasse`, `Strasse` | +| `postal_code` | `postal_code`, `zip`, `postcode` | `PLZ`, `plz`, `Postleitzahl`, `postleitzahl` | +| `city` | `city`, `town` | `Stadt`, `stadt`, `Ort` | + +**Header Normalization (used consistently for both input headers AND mapping variants):** +- Trim whitespace +- Convert to lowercase +- Normalize Unicode: `ß` → `ss` (e.g., `Straße` → `strasse`) +- Replace hyphens/whitespace with underscores: `E-Mail` → `e_mail`, `phone number` → `phone_number` +- Collapse multiple underscores: `e__mail` → `e_mail` +- Case-insensitive matching + +**Unknown columns:** ignored (no error) + +**Required fields:** `first_name`, `last_name`, `email` + +### CSV Template Files + +**Location:** +- `priv/static/templates/member_import_en.csv` +- `priv/static/templates/member_import_de.csv` + +**Content:** +- Header row with required + common optional fields +- One example row +- Uses semicolon delimiter (`;`) +- UTF-8 encoding **with BOM** (Excel compatibility) + +**Template Access:** +- Templates are static files in `priv/static/templates/` +- Served at: + - `/templates/member_import_en.csv` + - `/templates/member_import_de.csv` +- In LiveView, link them using Phoenix static path helpers (e.g. `~p` or `Routes.static_path/2`, depending on Phoenix version). + +### File Limits + +- **Max file size:** 10 MB +- **Max rows:** 1,000 rows (excluding header) +- **Processing:** chunks of 200 (via LiveView messages) +- **Encoding:** UTF-8 (BOM handled) + +--- + +## Technical Design Notes + +### Architecture Overview + +``` +┌─────────────────┐ +│ LiveView UI │ (GlobalSettingsLive or component) +│ - Upload area │ +│ - Progress │ +│ - Results │ +└────────┬────────┘ + │ prepare + ▼ +┌─────────────────────────────┐ +│ Import Service │ (Mv.Membership.Import.MemberCSV) +│ - parse + map + limit checks│ -> returns import_state +│ - process_chunk(chunk) │ -> returns chunk results +└────────┬────────────────────┘ + │ create + ▼ +┌─────────────────┐ +│ Ash Resource │ (Mv.Membership.Member) +│ - Create │ +└─────────────────┘ +``` + +### Technology Stack + +- **Phoenix LiveView:** file upload via `allow_upload/3` +- **NimbleCSV:** CSV parsing (add explicit dependency if missing) +- **Ash Resource:** member creation via `Membership.create_member/1` +- **Gettext:** bilingual UI/error messages + +### Module Structure + +**New Modules:** +- `lib/mv/membership/import/member_csv.ex` - import orchestration + chunk processing +- `lib/mv/membership/import/csv_parser.ex` - delimiter detection + parsing + BOM handling +- `lib/mv/membership/import/header_mapper.ex` - normalization + header mapping + +**Modified Modules:** +- `lib/mv_web/live/global_settings_live.ex` - render import section, handle upload/events/messages + +### Data Flow + +1. **Upload:** LiveView receives file via `allow_upload` +2. **Consume:** `consume_uploaded_entries/3` reads file content +3. **Prepare:** `MemberCSV.prepare/2` + - Strip BOM + - Detect delimiter (header recognition) + - Parse header + rows + - Map headers to canonical fields + - Early abort if required headers missing + - Row count check + - Return `import_state` containing chunks and metadata +4. **Process:** LiveView drives chunk processing via `handle_info` + - For each chunk: validate + create + collect errors +5. **Results:** LiveView shows progress + final summary + +### Types & Key Consistency + +- **Raw CSV parsing:** returns headers as list of strings, and rows **with csv line numbers** +- **Header mapping:** operates on normalized strings; mapping table variants are normalized once +- **Ash attrs:** built as atom-keyed map (`%{first_name: ..., ...}`) + +### Error Model + +```elixir +%{ + csv_line_number: 5, # physical line number in the CSV file + field: :email, # optional + message: "is not a valid email" +} +``` + +### CSV Line Numbers (Important) + +To keep error reporting user-friendly and accurate, **row errors must reference the physical line number in the original file**, even if empty lines are skipped. + +**Design decision:** the parser returns rows as: + +```elixir +rows :: [{csv_line_number :: pos_integer(), row_map :: map()}] +``` + +Downstream logic must **not** recompute line numbers from row indexes. + +### Authorization + +**Enforcement points:** +1. **LiveView event level:** check admin permission in `handle_event("start_import", ...)` +2. **UI level:** render import section only for admin users +3. **Static templates:** public assets (no authorization needed) + +Use `Mv.Authorization.PermissionSets` (preferred) instead of hard-coded string checks where possible. + +### Safety Limits + +- File size enforced by `allow_upload` (`max_file_size`) +- Row count enforced in `MemberCSV.prepare/2` before processing starts +- Chunking is done via **LiveView `handle_info` loop** (sequential, cooperative scheduling) + +--- + +## Implementation Issues + +### Issue #1: CSV Specification & Static Template Files + +**Dependencies:** None + +**Goal:** Define CSV contract and add static templates. + +**Tasks:** +- [ ] Finalize header mapping variants +- [ ] Document normalization rules +- [ ] Document delimiter detection strategy +- [ ] Create templates in `priv/static/templates/` (UTF-8 with BOM) +- [ ] Document template URLs and how to link them from LiveView +- [ ] Document line number semantics (physical CSV line numbers) + +**Definition of Done:** +- [ ] Templates open cleanly in Excel/LibreOffice +- [ ] CSV spec section complete + +--- + +### Issue #2: Import Service Module Skeleton + +**Dependencies:** None + +**Goal:** Create service API and error types. + +**API (recommended):** +- `prepare/2` — parse + map + limit checks, returns import_state +- `process_chunk/3` — process one chunk (pure-ish), returns per-chunk results + +**Tasks:** +- [ ] Create `lib/mv/membership/import/member_csv.ex` +- [ ] Define public function: `prepare/2 (file_content, opts \\ [])` +- [ ] Define public function: `process_chunk/3 (chunk_rows_with_lines, column_map, opts \\ [])` +- [ ] Define error struct: `%MemberCSV.Error{csv_line_number: integer, field: atom | nil, message: String.t}` +- [ ] Document module + API + +--- + +### Issue #3: CSV Parsing + Delimiter Auto-Detection + BOM Handling + +**Dependencies:** Issue #2 + +**Goal:** Parse CSV robustly with correct delimiter detection and BOM handling. + +**Tasks:** +- [ ] Verify/add NimbleCSV dependency (`{:nimble_csv, "~> 1.0"}`) +- [ ] Create `lib/mv/membership/import/csv_parser.ex` +- [ ] Implement `strip_bom/1` and apply it **before** any header handling +- [ ] Handle `\r\n` and `\n` line endings (trim `\r` on header record) +- [ ] Detect delimiter via header recognition (try `;` and `,`) +- [ ] Parse CSV and return: + - `headers :: [String.t()]` + - `rows :: [{csv_line_number, [String.t()]}]` or directly `[{csv_line_number, row_map}]` +- [ ] Skip completely empty records (but preserve correct physical line numbers) +- [ ] Return `{:ok, headers, rows}` or `{:error, reason}` + +**Definition of Done:** +- [ ] BOM handling works (Excel exports) +- [ ] Delimiter detection works reliably +- [ ] Rows carry correct `csv_line_number` + +--- + +### Issue #4: Header Normalization + Per-Header Mapping (No Language Detection) + +**Dependencies:** Issue #3 + +**Goal:** Map each header individually to canonical fields (normalized comparison). + +**Tasks:** +- [ ] Create `lib/mv/membership/import/header_mapper.ex` +- [ ] Implement `normalize_header/1` +- [ ] Normalize mapping variants once and compare normalized strings +- [ ] Build `column_map` (canonical field -> column index) +- [ ] **Early abort if required headers missing** (`first_name`, `last_name`, `email`) +- [ ] Ignore unknown columns + +**Definition of Done:** +- [ ] English/German headers map correctly +- [ ] Missing required columns fails fast + +--- + +### Issue #5: Validation (Required Fields) + Error Formatting + +**Dependencies:** Issue #4 + +**Goal:** Validate each row and return structured, translatable errors. + +**Tasks:** +- [ ] Implement `validate_row/3 (row_map, csv_line_number, opts)` +- [ ] Required field presence (`first_name`, `last_name`, `email`) +- [ ] Email format validation (EctoCommons.EmailValidator) +- [ ] Trim values before validation +- [ ] Gettext-backed error messages + +--- + +### Issue #6: Persistence via Ash Create + Per-Row Error Capture (Chunked Processing) + +**Dependencies:** Issue #5 + +**Goal:** Create members and capture errors per row with correct CSV line numbers. + +**Tasks:** +- [ ] Implement `process_chunk/3` in service: + - Input: `[{csv_line_number, row_map}]` + - Validate + create sequentially + - Collect counts + first 50 errors (per import overall; LiveView enforces cap across chunks) +- [ ] Implement Ash error formatter helper: + - Convert `Ash.Error.Invalid` into `%MemberCSV.Error{}` + - Prefer field-level errors where possible (attach `field` atom) + - Handle unique email constraint error as user-friendly message +- [ ] Map row_map to Ash attrs (`%{first_name: ..., ...}`) + +**Important:** **Do not recompute line numbers** in this layer—use the ones provided by the parser. + +--- + +### Issue #7: Admin Global Settings LiveView UI (Upload + Start Import + Results + Template Links) + +**Dependencies:** Issue #6 + +**Goal:** UI section with upload, progress, results, and template links. + +**Tasks:** +- [ ] Render import section only for admins +- [ ] Configure `allow_upload/3`: + - `.csv` only, `max_entries: 1`, `max_file_size: 10MB`, `auto_upload: false` +- [ ] `handle_event("start_import", ...)`: + - Admin permission check + - Consume upload -> read file content + - Call `MemberCSV.prepare/2` + - Store `import_state` in assigns (chunks + column_map + metadata) + - Initialize progress assigns + - `send(self(), {:process_chunk, 0})` +- [ ] `handle_info({:process_chunk, idx}, socket)`: + - Fetch chunk from `import_state` + - Call `MemberCSV.process_chunk/3` + - Merge counts/errors into progress assigns (cap errors at 50 overall) + - Schedule next chunk (or finish and show results) +- [ ] Results UI: + - Success count + - Failure count + - Error list (line number + message + field) + +**Template links:** +- Link `/templates/member_import_en.csv` and `/templates/member_import_de.csv` via Phoenix static path helpers. + +--- + +### Issue #8: Authorization + Limits + +**Dependencies:** None (can be parallelized) + +**Goal:** Ensure admin-only access and enforce limits. + +**Tasks:** +- [ ] Admin check in start import event handler +- [ ] File size enforced in upload config +- [ ] Row limit enforced in `MemberCSV.prepare/2` (max_rows from config) +- [ ] Configuration: + ```elixir + config :mv, csv_import: [ + max_file_size_mb: 10, + max_rows: 1000 + ] + ``` + +--- + +### Issue #9: End-to-End LiveView Tests + Fixtures + +**Dependencies:** Issue #7 and #8 + +**Tasks:** +- [ ] Fixtures: + - valid EN/DE + - invalid + - too many rows (1,001) + - BOM + `;` delimiter fixture + - fixture with empty line(s) to validate correct line numbers +- [ ] LiveView tests: + - admin sees section, non-admin does not + - upload + start import + - success + error rendering + - row limit + file size errors + +--- + +### Issue #10: Documentation Polish (Inline Help Text + Docs) + +**Dependencies:** Issue #9 + +**Tasks:** +- [ ] UI help text + translations +- [ ] CHANGELOG entry +- [ ] Ensure moduledocs/docs + +--- + +### Issue #11: Custom Field Import (Optional - v1.1) + +**Dependencies:** Issue #10 +**Status:** Optional + +*(unchanged — intentionally deferred)* + +--- + +## Rollout & Risks + +### Rollout Strategy +- Dev → Staging → Production (with anonymized real-world CSV tests) + +### Risks & Mitigations + +| Risk | Impact | Likelihood | Mitigation | +|---|---:|---:|---| +| Large import timeout | High | Medium | 10 MB + 1,000 rows, chunking via `handle_info` | +| Encoding issues | Medium | Medium | BOM stripping, templates with BOM | +| Invalid CSV format | Medium | High | Clear errors + templates | +| Duplicate emails | Low | High | Ash constraint error -> user-friendly message | +| Performance (no background jobs) | Medium | Low | Small limits, sequential chunk processing | +| Admin access bypass | High | Low | Event-level auth + UI hiding | +| Data corruption | High | Low | Per-row validation + best-effort | + +--- + +## Appendix + +### Module File Structure + +``` +lib/ +├── mv/ +│ └── membership/ +│ └── import/ +│ ├── member_csv.ex # prepare + process_chunk +│ ├── csv_parser.ex # delimiter detection + parsing + BOM handling +│ └── header_mapper.ex # normalization + header mapping +└── mv_web/ + └── live/ + └── global_settings_live.ex # add import section + LV message loop + +priv/ +└── static/ + └── templates/ + ├── member_import_en.csv + └── member_import_de.csv + +test/ +├── mv/ +│ └── membership/ +│ └── import/ +│ ├── member_csv_test.exs +│ ├── csv_parser_test.exs +│ └── header_mapper_test.exs +└── fixtures/ + ├── member_import_en.csv + ├── member_import_de.csv + ├── member_import_invalid.csv + ├── member_import_large.csv + └── member_import_empty_lines.csv +``` + +### Example Usage (LiveView) + +```elixir +def handle_event("start_import", _params, socket) do + assert_admin!(socket.assigns.current_user) + + [{_name, content}] = + consume_uploaded_entries(socket, :csv_file, fn %{path: path}, _entry -> + {:ok, File.read!(path)} + end) + + case Mv.Membership.Import.MemberCSV.prepare(content) do + {:ok, import_state} -> + socket = + socket + |> assign(:import_state, import_state) + |> assign(:import_progress, %{processed: 0, inserted: 0, failed: 0, errors: []}) + |> assign(:importing?, true) + + send(self(), {:process_chunk, 0}) + {:noreply, socket} + + {:error, reason} -> + {:noreply, put_flash(socket, :error, reason)} + end +end + +def handle_info({:process_chunk, idx}, socket) do + %{chunks: chunks, column_map: column_map} = socket.assigns.import_state + + case Enum.at(chunks, idx) do + nil -> + {:noreply, assign(socket, importing?: false)} + + chunk_rows_with_lines -> + {:ok, chunk_result} = + Mv.Membership.Import.MemberCSV.process_chunk(chunk_rows_with_lines, column_map) + + socket = merge_progress(socket, chunk_result) # caps errors at 50 overall + + send(self(), {:process_chunk, idx + 1}) + {:noreply, socket} + end +end +``` + +--- + +**End of Implementation Plan** \ No newline at end of file From 3cf8244cd676f7ce37a4eccc88c2443f16d30757 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 18:14:59 +0100 Subject: [PATCH 7/8] fix linting errors --- lib/mv_web/live/member_live/show.ex | 19 +++++++++++++------ priv/gettext/de/LC_MESSAGES/default.po | 11 +++++------ priv/gettext/default.pot | 5 +++++ priv/gettext/en/LC_MESSAGES/default.po | 10 +++++----- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index c9c92f0..7cdc875 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -175,11 +175,12 @@ defmodule MvWeb.MemberLive.Show do @impl true def handle_params(%{"id" => id}, _, socket) do # Load custom fields once using assign_new to avoid repeated queries - socket = assign_new(socket, :custom_fields, fn -> - Mv.Membership.CustomField - |> Ash.Query.sort(name: :asc) - |> Ash.read!() - end) + socket = + assign_new(socket, :custom_fields, fn -> + Mv.Membership.CustomField + |> Ash.Query.sort(name: :asc) + |> Ash.read!() + end) query = Mv.Membership.Member @@ -246,6 +247,7 @@ defmodule MvWeb.MemberLive.Show do if assigns.email && String.trim(assigns.email) != "" do assigns = %{email: assigns.email, display: display_text} + ~H""" cfv.custom_field_id == custom_field_id or (cfv.custom_field && cfv.custom_field.id == custom_field_id) end) end + defp find_custom_field_value(_custom_field_values, _custom_field_id), do: nil # Formats custom field value based on type @@ -330,6 +335,7 @@ defmodule MvWeb.MemberLive.Show do render_empty_value() else assigns = %{email: value} + ~H""" <.mailto_link email={@email} display={@email} /> """ @@ -351,6 +357,7 @@ defmodule MvWeb.MemberLive.Show do # The visual "—" is hidden from screen readers, while the translated text is only visible to screen readers defp render_empty_value do assigns = %{text: gettext("Not set")} + ~H""" diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index ec6812a..1c298e9 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -1422,6 +1422,11 @@ msgstr "Jährliches Intervall – Beitrittszeitraum nicht einbezogen" msgid "Yearly Interval - Joining Cycle Included" msgstr "Jährliches Intervall – Beitrittszeitraum einbezogen" +#: lib/mv_web/live/member_live/show.ex +#, elixir-autogen, elixir-format +msgid "Not set" +msgstr "Nicht gesetzt" + #~ #: lib/mv_web/live/custom_field_live/show.ex #~ #, elixir-autogen, elixir-format #~ msgid "Auto-generated identifier (immutable)" @@ -1494,12 +1499,6 @@ msgstr "Jährliches Intervall – Beitrittszeitraum einbezogen" #~ msgid "New Custom field" #~ msgstr "Benutzerdefiniertes Feld speichern" -#~ #: lib/mv_web/live/user_live/form.ex -#~ #: lib/mv_web/live/user_live/show.ex -#~ #, elixir-autogen, elixir-format -#~ msgid "Not set" -#~ msgstr "Nicht gesetzt" - #~ #: lib/mv_web/live/contribution_settings_live.ex #~ #, elixir-autogen, elixir-format #~ msgid "Quarterly Interval - Joining Period Excluded" diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index e2bbf32..5450ee0 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -1422,3 +1422,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Yearly Interval - Joining Cycle Included" msgstr "" + +#: lib/mv_web/live/member_live/show.ex +#, elixir-autogen, elixir-format +msgid "Not set" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index d3ee646..095ec30 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -1423,6 +1423,11 @@ msgstr "" msgid "Yearly Interval - Joining Cycle Included" msgstr "" +#: lib/mv_web/live/member_live/show.ex +#, elixir-autogen, elixir-format, fuzzy +msgid "Not set" +msgstr "" + #~ #: lib/mv_web/live/custom_field_live/show.ex #~ #, elixir-autogen, elixir-format #~ msgid "Auto-generated identifier (immutable)" @@ -1495,11 +1500,6 @@ msgstr "" #~ msgid "New Custom field" #~ msgstr "" -#~ #: lib/mv_web/live/user_live/show.ex -#~ #, elixir-autogen, elixir-format, fuzzy -#~ msgid "Not set" -#~ msgstr "" - #~ #: lib/mv_web/live/contribution_settings_live.ex #~ #, elixir-autogen, elixir-format #~ msgid "Quarterly Interval - Joining Period Excluded" From f87e6d3e1d13e019e2dc1734cdcbff8e6021124e Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 18:21:15 +0100 Subject: [PATCH 8/8] fix tests --- test/mv_web/member_live/show_test.exs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/mv_web/member_live/show_test.exs b/test/mv_web/member_live/show_test.exs index 6ee9b50..1e04559 100644 --- a/test/mv_web/member_live/show_test.exs +++ b/test/mv_web/member_live/show_test.exs @@ -16,6 +16,7 @@ defmodule MvWeb.MemberLive.ShowTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query + use Gettext, backend: MvWeb.Gettext alias Mv.Membership.{CustomField, CustomFieldValue, Member} @@ -48,7 +49,7 @@ defmodule MvWeb.MemberLive.ShowTest do |> Ash.create() conn = conn_with_oidc_user(conn) - {:ok, view, html} = live(conn, ~p"/members/#{member}") + {:ok, _view, html} = live(conn, ~p"/members/#{member}") # Custom Fields section should be visible assert html =~ gettext("Custom Fields") @@ -92,7 +93,7 @@ defmodule MvWeb.MemberLive.ShowTest do |> Ash.create() conn = conn_with_oidc_user(conn) - {:ok, view, html} = live(conn, ~p"/members/#{member}") + {:ok, _view, html} = live(conn, ~p"/members/#{member}") # Custom Fields section should be visible assert html =~ gettext("Custom Fields") @@ -113,7 +114,7 @@ defmodule MvWeb.MemberLive.ShowTest do member: member } do conn = conn_with_oidc_user(conn) - {:ok, view, html} = live(conn, ~p"/members/#{member}") + {:ok, _view, html} = live(conn, ~p"/members/#{member}") # Custom Fields section should NOT be visible refute html =~ gettext("Custom Fields") @@ -140,7 +141,7 @@ defmodule MvWeb.MemberLive.ShowTest do |> Ash.create() conn = conn_with_oidc_user(conn) - {:ok, view, html} = live(conn, ~p"/members/#{member}") + {:ok, _view, html} = live(conn, ~p"/members/#{member}") assert html =~ "+49123456789" end @@ -164,7 +165,7 @@ defmodule MvWeb.MemberLive.ShowTest do |> Ash.create() conn = conn_with_oidc_user(conn) - {:ok, view, html} = live(conn, ~p"/members/#{member}") + {:ok, _view, html} = live(conn, ~p"/members/#{member}") # Should contain mailto link assert html =~ ~s(href="mailto:private@example.com")