diff --git a/.credo.exs b/.credo.exs index a94fead..3a4f8dc 100644 --- a/.credo.exs +++ b/.credo.exs @@ -114,7 +114,6 @@ {Credo.Check.Readability.RedundantBlankLines, []}, {Credo.Check.Readability.Semicolons, []}, {Credo.Check.Readability.SpaceAfterCommas, []}, - {Credo.Check.Readability.StrictModuleLayout, []}, {Credo.Check.Readability.StringSigils, []}, {Credo.Check.Readability.TrailingBlankLine, []}, {Credo.Check.Readability.TrailingWhiteSpace, []}, @@ -167,19 +166,13 @@ {Credo.Check.Warning.UnusedTupleOperation, []}, {Credo.Check.Warning.WrongTestFileExtension, []}, # Module documentation check (enabled after adding @moduledoc to all modules) - {Credo.Check.Readability.ModuleDoc, []}, - - # Promoted in the cleanup ratchet (each currently at zero violations): - {Credo.Check.Refactor.DoubleBooleanNegation, []}, - {Credo.Check.Refactor.FilterReject, []}, - {Credo.Check.Refactor.RejectFilter, []}, - {Credo.Check.Refactor.UtcNowTruncate, []}, - {Credo.Check.Warning.LazyLogging, []}, - {Credo.Check.Warning.LeakyEnvironment, []}, - {Credo.Check.Warning.MapGetUnsafePass, []}, - {Credo.Check.Warning.MixEnv, []} + {Credo.Check.Readability.ModuleDoc, []} ], disabled: [ + # + # Checks scheduled for next check update (opt-in for now) + {Credo.Check.Refactor.UtcNowTruncate, []}, + # # Controversial and experimental checks (opt-in, just move the check to `:enabled` # and be sure to use `mix credo --strict` to see low priority checks) @@ -190,7 +183,6 @@ {Credo.Check.Design.SkipTestWithoutComment, []}, {Credo.Check.Readability.AliasAs, []}, {Credo.Check.Readability.BlockPipe, []}, - # ImplTrue: ~269 violations; deferred to a follow-up. {Credo.Check.Readability.ImplTrue, []}, {Credo.Check.Readability.MultiAlias, []}, {Credo.Check.Readability.NestedFunctionCalls, []}, @@ -200,20 +192,24 @@ {Credo.Check.Readability.SingleFunctionToBlockPipe, []}, {Credo.Check.Readability.SinglePipe, []}, {Credo.Check.Readability.Specs, []}, + {Credo.Check.Readability.StrictModuleLayout, []}, {Credo.Check.Readability.WithCustomTaggedTuple, []}, {Credo.Check.Refactor.ABCSize, []}, - # AppendSingleItem: ~10 violations (mostly tests); deferred to a follow-up. {Credo.Check.Refactor.AppendSingleItem, []}, - # IoPuts: 3 violations in Mv.Release seed output; deferred to a follow-up. + {Credo.Check.Refactor.DoubleBooleanNegation, []}, + {Credo.Check.Refactor.FilterReject, []}, {Credo.Check.Refactor.IoPuts, []}, - # MapMap: ~8 violations; deferred to a follow-up. {Credo.Check.Refactor.MapMap, []}, {Credo.Check.Refactor.ModuleDependencies, []}, - # NegatedIsNil: ~63 violations; deferred to a follow-up. {Credo.Check.Refactor.NegatedIsNil, []}, {Credo.Check.Refactor.PassAsyncInTestCases, []}, {Credo.Check.Refactor.PipeChainStart, []}, + {Credo.Check.Refactor.RejectFilter, []}, {Credo.Check.Refactor.VariableRebinding, []}, + {Credo.Check.Warning.LazyLogging, []}, + {Credo.Check.Warning.LeakyEnvironment, []}, + {Credo.Check.Warning.MapGetUnsafePass, []}, + {Credo.Check.Warning.MixEnv, []}, {Credo.Check.Warning.UnsafeToAtom, []} # {Credo.Check.Refactor.MapInto, []}, diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b2a57d..adbe7e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [1.3.0] - 2026-06-16 +## [Unreleased] ### Added - **GDPR/DSGVO join-form description** – Custom fields can carry a "join form description" that is shown as the field's label on the public join form, with clickable external links (whole URLs and Markdown `[text](url)`). Useful for presenting a GDPR confirmation with a link to an externally hosted privacy declaration before sign-up. @@ -15,8 +15,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **CSV import – membership fee type column** – A `Fee Type`/`Beitragsart` column assigns each member's membership fee type; an unknown name falls back to the default fee type and is flagged in the preview with a link to create it. - **CSV import – mapping preview** – After uploading a file, a preview shows how every column maps (with sample rows and warnings for ignored or unknown columns) and the import only starts once you confirm. - **Dynamic CSV import templates** – The EN and DE import-template downloads now include the association's current custom fields instead of a fixed column set. -- **Deactivate and reactivate members** – Members can be deactivated directly from the member page: a dialog picks the exit date (prefilled to today, future dates allowed); deactivated members can be reactivated, which clears the exit date. -- **Tooltips and OIDC explanation** – Icon-only action buttons (including the Vereinfacht sync control) now carry tooltips and accessible labels, and the OIDC settings section explains that it enables single sign-on. ### Changed - **Member bulk actions in one menu** – The actions above the member overview (open in email program, copy email addresses, export to CSV, export to PDF) are now collected in a single "Aktionen" dropdown instead of separate buttons. Without a selection they apply to all members, or to the currently filtered members; the trigger shows the active scope. Opening the email program is disabled when too many recipients are selected, with a hint to copy the addresses or use the export instead. @@ -24,13 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Default GDPR custom field** – The seeded GDPR field was shortened from "Datenschutzerklärung akzeptiert" to "DSGVO" and now ships with a default join-form description (with a placeholder link to replace). ### Fixed -- **Authentication background workers start correctly** – The token-cleanup (Expunger) and audit-log batching workers now boot under the application's real configuration instead of an unused OTP app, so they run as intended in production rather than silently doing nothing. - **CSV date round-trip** – Date custom-field values are now exported as ISO-8601 (`YYYY-MM-DD`), so an exported CSV can be re-imported without date-parsing errors. - **CSV import – fee-status columns ignored** – Columns such as `Bezahlstatus` / `Membership Fee Status` are always ignored on import and never stored as a custom-field value, even when a custom field of the same name exists. -- **Column-header tooltips clipped** – Tooltips on the members-overview column headers are no longer clipped by the sticky table header. -- **Text selection opens member** – Dragging to select text in a members-overview row (for example to copy an email) no longer opens the member details; a plain click still opens them. -- **Sort by custom date** – Sorting the member list or member export by a custom date field now orders rows chronologically instead of like text, so e.g. 29.01.1981 correctly comes before 01.03.1982. -- **Concurrent member creation no longer deadlocks** – Creating members in parallel (e.g. simultaneous sign-ups, or batch operations) could intermittently fail with a PostgreSQL deadlock; the affected foreign keys are now deferrable, so concurrent member creation succeeds reliably. ## [1.2.0] - 2026-05-08 diff --git a/DESIGN_GUIDELINES.md b/DESIGN_GUIDELINES.md index 15fbbae..34c71b8 100644 --- a/DESIGN_GUIDELINES.md +++ b/DESIGN_GUIDELINES.md @@ -12,7 +12,7 @@ This document defines Mila’s **UI system** to ensure **UX consistency**, **acc - standard page skeletons (index/detail/form) - microcopy conventions (German “du” tone) -> Engineering practices (LiveView load budget, testing, security, etc.) are defined in `CODE_GUIDELINES.md`. +> Engineering practices (LiveView load budget, testing, security, etc.) are defined in `docs/CODE_GUIDELINES.md`. > This document focuses on **visual + UX** consistency and references engineering rules where needed. --- diff --git a/Justfile b/Justfile index a16bf8b..9b0be65 100644 --- a/Justfile +++ b/Justfile @@ -12,10 +12,6 @@ MIX_QUIET := "1" run: install-dependencies start-database migrate-database seed-database mix phx.server -# Dev web server + its database only — no mailcrab/rauthy. -server: install-dependencies start-test-db migrate-database seed-database - mix phx.server - install-dependencies: mix deps.get @@ -33,9 +29,6 @@ seed-database: start-database: docker compose up -d -start-test-db: - docker compose up -d db - # Full check suite: lint + audit + the fast tests (slow/ui excluded). No Dialyzer. ci-dev: install-dependencies lint audit test-fast diff --git a/assets/js/app.js b/assets/js/app.js index a003e27..4c7e3c5 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -103,29 +103,6 @@ Hooks.TableRowKeydown = { } } -// RowSelectionGuard: distinguish drag-to-select-text from a plain click on the members table. -// LiveView fires the row navigation push (select_row_and_navigate) on any click. When the user -// drags across a cell to select text (e.g. an email to copy) and releases, the mouseup produces a -// non-empty text selection; in that case we swallow the click in the capture phase so navigation is -// suppressed. A plain click leaves the selection collapsed and navigates as before. -Hooks.RowSelectionGuard = { - mounted() { - this.handleClickCapture = (e) => { - const selection = window.getSelection() - if (selection && !selection.isCollapsed && selection.toString().trim() !== "") { - e.preventDefault() - e.stopPropagation() - } - } - // Capture phase so this runs before LiveView's bubbling phx-click handler. - this.el.addEventListener("click", this.handleClickCapture, true) - }, - - destroyed() { - this.el.removeEventListener("click", this.handleClickCapture, true) - } -} - // FocusRestore hook: WCAG 2.4.3 — when a modal closes, focus returns to the trigger element (e.g. "Delete member" button) Hooks.FocusRestore = { mounted() { diff --git a/config/dev.exs b/config/dev.exs index d96bd7e..139b816 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -5,7 +5,7 @@ config :mv, Mv.Repo, username: "postgres", password: "postgres", hostname: "localhost", - port: String.to_integer(System.get_env("DB_PORT") || "5000"), + port: 5000, database: "mv_dev", stacktrace: true, show_sensitive_data_on_connection_error: true, @@ -97,9 +97,9 @@ config :mv, :token_signing_secret, "IwUwi65TrEeExwBXXFPGm2I7889NsL" # do not set defaults here so the SSO button stays hidden and no MissingSecret occurs. # config :mv, :oidc, # client_id: "mv", -# base_url: "http://localhost:#{System.get_env("RAUTHY_PORT") || "8080"}/auth/v1", +# base_url: "http://localhost:8080/auth/v1", # client_secret: System.get_env("OIDC_CLIENT_SECRET"), -# redirect_uri: "http://localhost:#{System.get_env("PORT") || "4000"}/auth/user/oidc/callback" +# redirect_uri: "http://localhost:4000/auth/user/oidc/callback" # AshAuthentication development configuration config :mv, :session_identifier, :jti diff --git a/config/test.exs b/config/test.exs index 10ab4e8..7343a6a 100644 --- a/config/test.exs +++ b/config/test.exs @@ -9,7 +9,7 @@ config :mv, Mv.Repo, username: "postgres", password: "postgres", hostname: System.get_env("TEST_POSTGRES_HOST", "localhost"), - port: System.get_env("TEST_POSTGRES_PORT") || System.get_env("DB_PORT") || "5000", + port: System.get_env("TEST_POSTGRES_PORT", "5000"), database: "mv_test#{System.get_env("MIX_TEST_PARTITION")}", pool: Ecto.Adapters.SQL.Sandbox, pool_size: System.schedulers_online() * 8, diff --git a/docker-compose.yml b/docker-compose.yml index 44db148..cbd2e9e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -12,21 +12,19 @@ services: volumes: - postgres-data:/var/lib/postgresql ports: - - "${DB_PORT:-5000}:5432" + - "5000:5432" networks: - local mailcrab: image: marlonb/mailcrab:latest ports: - - "${MAILCRAB_PORT:-1080}:1080" + - "1080:1080" networks: - rauthy-dev rauthy: - # No fixed container_name — Compose derives it from COMPOSE_PROJECT_NAME so - # several isolated stacks coexist (e.g. mv--rauthy-1). A plain - # checkout gets -rauthy-1. + container_name: rauthy-dev image: ghcr.io/sebadob/rauthy:0.35.2 environment: - LOCAL_TEST=true @@ -34,8 +32,7 @@ services: - SMTP_PORT=1025 - SMTP_DANGER_INSECURE=true - LISTEN_SCHEME=http - # Advertised URL must match the host-mapped port below. - - PUB_URL=localhost:${RAUTHY_PORT:-8080} + - PUB_URL=localhost:8080 - BOOTSTRAP_ADMIN_PASSWORD_PLAIN=RauthyTest12345 # Disable strict IP validation to allow access from multiple Docker networks - SESSION_VALIDATE_IP=false @@ -43,7 +40,7 @@ services: # Re-runs after `docker compose down -v` because the DB is empty again. - BOOTSTRAP_DIR=/app/bootstrap ports: - - "${RAUTHY_PORT:-8080}:8080" + - "8080:8080" depends_on: - mailcrab - db diff --git a/docs/README.md b/docs/README.md deleted file mode 100644 index db22d58..0000000 --- a/docs/README.md +++ /dev/null @@ -1,46 +0,0 @@ -# Documentation index - -Project documentation for Mila (Mitgliederverwaltung). Each area below lists a coarse entry point first, then the deeper references. For engineering conventions see the repo-root `CODE_GUIDELINES.md`; for UI conventions see `DESIGN_GUIDELINES.md`. - -## Roles & permissions -- [roles-and-permissions-overview.md](roles-and-permissions-overview.md) — coarse entry: the RBAC concept, evaluated approaches, permission sets and scopes. -- [roles-and-permissions-architecture.md](roles-and-permissions-architecture.md) — deep reference: per-resource policies, permission matrix, page-permission plug, authorization bootstrap patterns. -- [roles-and-permissions-implementation-plan.md](roles-and-permissions-implementation-plan.md) — historical record of how the MVP was built. -- [policy-bypass-vs-haspermission.md](policy-bypass-vs-haspermission.md) — the two-tier Ash pattern (bypass + `expr()` for reads, `HasPermission` for writes). -- [page-permission-route-coverage.md](page-permission-route-coverage.md) — route → permission-set access matrix and the page-permission plug behaviour. - -## Membership & fees -- [membership-fee-overview.md](membership-fee-overview.md) — coarse entry: terminology (DE↔EN), worked cycle examples, UI mockups. -- [membership-fee-architecture.md](membership-fee-architecture.md) — deep reference: design decisions, data model, cycle generation, atomicity. -- [vereinfacht-api.md](vereinfacht-api.md) — the Vereinfacht finance-contact sync integration. - -## Members: import, onboarding, custom fields -- [csv-member-import-v1.md](csv-member-import-v1.md) — CSV member import: column mapping, validation, error handling, templates. -- [onboarding-join-concept.md](onboarding-join-concept.md) — public join flow (double opt-in, JoinRequest), plus unimplemented approval/invite/OIDC design. -- [custom-fields-search-performance.md](custom-fields-search-performance.md) — performance of custom-field values in the member search vector. - -## Groups -- [groups-architecture.md](groups-architecture.md) — groups design, hierarchy extension path, search integration, A11y notes. - -## Database -- [database-schema-readme.md](database-schema-readme.md) — schema overview: tables, relationships, indexes, full-text & fuzzy search. -- [database_schema.dbml](database_schema.dbml) — machine-readable DBML schema (for dbdiagram.io / dbdocs.io). - -## Accounts, OIDC & email -- [admin-bootstrap-and-oidc-role-sync.md](admin-bootstrap-and-oidc-role-sync.md) — production admin bootstrap and OIDC group → Admin role sync (ENV contract). -- [oidc-account-linking.md](oidc-account-linking.md) — secure account linking between password and OIDC accounts. -- [email-sync.md](email-sync.md) — email synchronization rules between linked User and Member. -- [email-validation.md](email-validation.md) — the dual `:html_input` + `:pow` email-validation strategy. -- [email-layout-mockup.md](email-layout-mockup.md) — shared transactional-email layout structure. -- [smtp-configuration-concept.md](smtp-configuration-concept.md) — SMTP config precedence, TLS handling, operational caveats. - -## UI & components -- [settings-authentication-mockup.txt](settings-authentication-mockup.txt) — Settings → Authentication section layout. -- [daisyui-drawer-pattern.md](daisyui-drawer-pattern.md) — the project's sidebar drawer pattern (implemented in `lib/mv_web/components/layouts/sidebar.ex`). -- [badge-wcag-phase1-analysis.md](badge-wcag-phase1-analysis.md) — `<.badge>` component API and WCAG contrast design notes. -- [pdf-generation-imprintor.md](pdf-generation-imprintor.md) — PDF generation via Imprintor + Typst templates (decision vs. Chromium). - -## Project history & planning -- [development-progress-log.md](development-progress-log.md) — coarse implementation history, architecture decisions, migration index, gotchas. -- [feature-roadmap.md](feature-roadmap.md) — per-area status matrix and the open/missing backlog. -- [test-performance-optimization.md](test-performance-optimization.md) — test-suite speed-up: seeds-test coverage mapping and the `@tag :slow` model. diff --git a/docs/admin-bootstrap-and-oidc-role-sync.md b/docs/admin-bootstrap-and-oidc-role-sync.md index de34d47..5413f91 100644 --- a/docs/admin-bootstrap-and-oidc-role-sync.md +++ b/docs/admin-bootstrap-and-oidc-role-sync.md @@ -26,7 +26,7 @@ ### Seeds (Dev/Test) -- priv/repo/seeds_bootstrap.exs – Uses ADMIN_PASSWORD or ADMIN_PASSWORD_FILE when set; otherwise fallback "testpassword" only in dev/test. +- priv/repo/seeds.exs – Uses ADMIN_PASSWORD or ADMIN_PASSWORD_FILE when set; otherwise fallback "testpassword" only in dev/test. ## OIDC Role Sync (Part B) @@ -34,7 +34,7 @@ - `OIDC_ADMIN_GROUP_NAME` – OIDC group name that maps to the Admin role. If unset, no role sync. - `OIDC_GROUPS_CLAIM` – JWT claim name for group list (default "groups"). -- Module: Mv.Config (oidc_admin_group_name/0, oidc_groups_claim/0). +- Module: Mv.OidcRoleSyncConfig (oidc_admin_group_name/0, oidc_groups_claim/0). ### Sign-in page (OIDC-only mode) diff --git a/docs/badge-wcag-phase1-analysis.md b/docs/badge-wcag-phase1-analysis.md index 911cfbd..5b6a834 100644 --- a/docs/badge-wcag-phase1-analysis.md +++ b/docs/badge-wcag-phase1-analysis.md @@ -1,53 +1,88 @@ -# Badge Component Design Notes (WCAG) +# Phase 1 — Badge WCAG Analysis & Migration -Design rationale for the central `<.badge>` core component -(`MvWeb.CoreComponents`) and the WCAG-driven theme overrides in -`assets/css/app.css`. Before it, badges were raw `` -markup with no central component. +## 1) Repo-Analyse (Stand vor Änderungen) -## `<.badge>` API contract +### Badge-Verwendungen (alle Fundstellen) -- `attr :variant` — `:neutral | :primary | :info | :success | :warning | :error` -- `attr :style` — `:soft | :solid | :outline` (default: `:soft`) -- `attr :size` — `:sm | :md` (default: `:md`) -- `attr :sr_label` — optional screen-reader label for icon-only badges -- `slot :icon` — optional -- `slot :inner_block` — badge text +| Datei | Kontext | Markup | +|-------|---------|--------| +| `lib/mv_web/live/member_field_live/index_component.ex` | Tabelle (show_in_overview) | `` / `` | +| `lib/mv_web/live/components/member_filter_component.ex` | Filter-Chips (Anzahl) | `` (2×) | +| `lib/mv_web/live/role_live/index.html.heex` | Tabelle (System Role, Permission Set, Custom) | `badge-warning`, `permission_set_badge_class()`, `badge-ghost` (User Count) | +| `lib/mv_web/helpers/membership_fee_helpers.ex` | Helper | `status_color/1` → "badge-success" \| "badge-error" \| "badge-ghost" | +| `lib/mv_web/live/member_live/show.ex` | Mitgliedsdetail (Beiträge) | ``, `badge-ghost` (No cycles) | +| `lib/mv_web/live/membership_fee_settings_live.ex` | Settings (Fee Types) | `badge-outline`, `badge-ghost` (member count) | +| `lib/mv_web/live/membership_fee_type_live/index.ex` | Index (Fee Types) | `badge-outline`, `badge-ghost` (member count) | +| `lib/mv_web/live/role_live/index.ex` | (Helper-Import) | `permission_set_badge_class/1` | +| `lib/mv_web/live/member_live/show/membership_fees_component.ex` | Mitgliedsbeiträge | `badge-outline`, `["badge", status_color]` | +| `lib/mv_web/live/custom_field_live/index_component.ex` | Tabelle (show_in_overview) | `badge-success`, `badge-ghost` | +| `lib/mv_web/member_live/index/membership_fee_status.ex` | Helper | `format_cycle_status_badge/1` → map mit `color`, `icon`, `label` | +| `lib/mv_web/live/global_settings_live.ex` | Form (label-text-alt) | `badge badge-ghost` "(set)" (2×) | +| `lib/mv_web/live/member_live/index.html.heex` | Tabelle (Status) | `format_cycle_status_badge` + ``, `badge-ghost` (No cycle), `badge-outline badge-primary` (Filter-Chip) | +| `lib/mv_web/live/role_live/helpers.ex` | Helper | `permission_set_badge_class/1` → "badge badge-* badge-sm" | +| `lib/mv_web/live/group_live/show.ex` | Card | `badge badge-outline badge` | +| `lib/mv_web/live/role_live/show.ex` | Detail | `permission_set_badge_class`, `badge-warning` (System), `badge-ghost` (No) | -## Design rules +### DaisyUI/Tailwind Config -- `:soft` and `:solid` use a visible background; `:soft` is the default. No - transparent ghost as a default. -- `:outline` **always** sets a background (e.g. `bg-base-100`) so the border - stays visible on grey (`base-200`/`base-300`) surfaces. -- Ghost only as an explicit opt-in, and then with `bg-base-100` for visibility - (plain DaisyUI `badge-ghost` is `base-200` on `base-200` — nearly invisible). -- Clickable chips keep `<.badge>` as a plain container with a button in the - `inner_block`. +- **Tailwind:** `assets/tailwind.config.js` — erweitert nur `theme.extend.colors.brand`; kein DaisyUI hier. +- **DaisyUI:** wird in `assets/css/app.css` per `@plugin "../vendor/daisyui"` mit `themes: false` geladen. +- **Themes:** Zwei Custom-Themes in `app.css`: + - `@plugin "../vendor/daisyui-theme"` mit `name: "dark"` (default: false) + - `@plugin "../vendor/daisyui-theme"` mit `name: "light"` (default: true) +- **Theme-Umschaltung:** `lib/mv_web/components/layouts/root.html.heex` — Inline-Script setzt `document.documentElement.setAttribute("data-theme", "light"|"dark")` aus `localStorage["phx:theme"]` oder `prefers-color-scheme`. Sidebar enthält Theme-Toggle (`<.theme_toggle />`). -## WCAG contrast overrides (`app.css`) +### Core Components -DaisyUI defaults do not reach the WCAG 2.2 AA **4.5:1** text-contrast ratio for -badges, so `app.css` adds per-theme overrides on top of the custom `light` / -`dark` themes: +- **Modul:** `lib/mv_web/components/core_components.ex` (MvWeb.CoreComponents). +- **Vorhanden:** flash, button, dropdown_menu, form_section, input, header, table, icon, link, etc. +- **Badge:** Bisher keine zentrale `<.badge>`-Komponente. -- **Light theme:** darker `--badge-fg` for all solid variants; darker text on - `badge-soft`'s tinted background; uniform dark text for `badge-outline` on - `base-100`. -- **Dark theme:** slightly darkened solid-badge backgrounds so the light - `*-content` colors reach 4.5:1; lighter, readable variant tints for - `badge-soft`; light `--badge-fg` for `badge-outline` on `base-100`. +### DaisyUI Badge (Vendor) -Related: contrast overrides for the member-filter join buttons -(`.member-filter-dropdown .join .btn`) under the same 4.5:1 rule. +- **Default:** `--badge-bg: var(--badge-color, var(--color-base-100))`, `--badge-fg: var(--color-base-content)`. +- **badge-outline:** `--badge-bg: "#0000"` (transparent) → Kontrastproblem auf base-200/base-300. +- **badge-ghost:** `background-color: var(--color-base-200)`, `color: var(--color-base-content)` → auf base-200-Flächen kaum sichtbar. +- **badge-soft:** color-mix 8% Variante mit base-100 → sichtbar; Text ist Variantenfarbe (Kontrast prüfen). -## Variant helpers +--- -- `MembershipFeeHelpers.status_variant/1` → `:success | :error | :warning`. - `status_variant(:suspended) -> :warning` (yellow) deliberately matches the - Edit button (`btn-warning`), keeping the "suspended" badge the same color as - its action. -- `RoleLive.Helpers.permission_set_badge_variant/1` → - `:neutral | :info | :success | :error`. -- `MembershipFeeStatus.format_cycle_status_badge/1` additionally returns a - `:variant` for `<.badge>`. +## 2) Core Component <.badge> API (geplant) + +- **attr :variant** — `:neutral | :primary | :info | :success | :warning | :error` +- **attr :style** — `:soft | :solid | :outline` (Default: `:soft`) +- **attr :size** — `:sm | :md` (Default: `:md`) +- **slot :inner_block** — Badge-Text +- **attr :sr_label** — optional, für Icon-only (Screen Reader) +- **slot :icon** — optional + +Regeln: + +- `:soft` und `:solid` nutzen sichtbaren Hintergrund (kein transparenter Ghost als Default). +- `:outline` setzt immer einen Hintergrund (z. B. `bg-base-100`), damit der Rand auf grauen Flächen sichtbar bleibt. +- Ghost nur als explizites Opt-in; dann mit `bg-base-100` für Sichtbarkeit. + +--- + +## 3) Theme-Overrides (WCAG) + +- In `app.css` sind bereits Custom-Themes für `light` und `dark` mit eigenen Tokens. +- **Badge-Kontrast (WCAG 2.2 AA 4.5:1):** Zusätzliche Overrides in `app.css`: + - **Light theme:** Dunkle `--badge-fg` für alle Varianten (primary, success, error, warning, info, neutral); für `badge-soft` dunklere Textfarbe (`color`) auf getöntem Hintergrund; für `badge-outline` einheitlich dunkle Schrift auf base-100. + - **Dark theme:** Leicht abgedunkelte Badge-Hintergründe für Solid-Badges, damit die hellen *-content-Farben 4.5:1 erreichen; für `badge-soft` hellere, gut lesbare Variantentöne; für `badge-outline` heller Text (`--badge-fg`) auf base-100. + +--- + +## 4) Migration (erledigt) + +- Alle `` durch `<.badge variant="..." style="...">...` ersetzt. +- Klickbare Chips (z. B. Group Show „Remove“) bleiben als <.badge> mit Button im inner_block (Badge ist nur Container). +- **Neue Helper:** `MembershipFeeHelpers.status_variant/1` (→ :success | :error | :warning; suspended = :warning wie Edit-Button), `RoleLive.Helpers.permission_set_badge_variant/1` (→ :neutral | :info | :success | :error). +- **Angepasst:** `MembershipFeeStatus.format_cycle_status_badge/1` liefert zusätzlich `:variant` für <.badge>. +- **Migrierte Stellen:** member_field_live, member_filter_component, role_live (index + show), member_live (show, index, membership_fees_component), membership_fee_settings_live, membership_fee_type_live, custom_field_live, global_settings_live, group_live/show. + +## 5) Weitere Anpassungen (nach Phase 1) + +- **Filter Join-Buttons (WCAG):** In `app.css` Kontrast-Overrides für `.member-filter-dropdown .join .btn` (inaktiv: base-100/base-200 + dunkle/helle Schrift; aktiv: success/error mit 4.5:1). +- **Badge „Pausiert“ (suspended):** `status_variant(:suspended)` → `:warning` (gelb), damit Badge dieselbe Farbe wie der Edit-Button (btn-warning) hat. +- **Filter-Dropdown schließen:** `phx-click-away` vom inneren Panel auf den äußeren Wrapper (`member-filter-dropdown`) verschoben; Klick auf den Filter-Button schließt das Dropdown (konsistent mit Spalten/Ausblenden). diff --git a/docs/csv-member-import-v1.md b/docs/csv-member-import-v1.md index 9f4fe8c..1a717c6 100644 --- a/docs/csv-member-import-v1.md +++ b/docs/csv-member-import-v1.md @@ -1,172 +1,796 @@ -# CSV Member Import +# CSV Member Import v1 - Implementation Plan -Reference for how the CSV member import actually behaves. The end-to-end -LiveView test (`test/mv_web/live/import_live_test.exs`) and future maintenance -depend on the rules documented here. +**Version:** 1.0 +**Last Updated:** 2026-01-13 +**Status:** In Progress (Backend Complete, UI Complete, Tests Pending) +**Related Documents:** +- [Feature Roadmap](./feature-roadmap.md) - Overall feature planning -**Status:** implemented (backend + LiveView UI). +## Implementation Status -Implementation: +**Completed Issues:** +- ✅ Issue #1: CSV Specification & Static Template Files +- ✅ Issue #2: Import Service Module Skeleton +- ✅ Issue #3: CSV Parsing + Delimiter Auto-Detection + BOM Handling +- ✅ Issue #4: Header Normalization + Per-Header Mapping +- ✅ Issue #5: Validation (Required Fields) + Error Formatting +- ✅ Issue #6: Persistence via Ash Create + Per-Row Error Capture (with Error-Capping) +- ✅ Issue #7: Admin Global Settings LiveView UI (Upload + Start Import + Results + Template Links) +- ✅ Issue #8: Authorization + Limits +- ✅ Issue #11: Custom Field Import (Backend + UI) -- `lib/mv/membership/import/csv_parser.ex` — BOM stripping, delimiter detection, physical line numbering -- `lib/mv/membership/import/header_mapper.ex` — header normalization + column mapping -- `lib/mv/membership/import/column_resolver.ex` — read-only resolution of groups + fee-type columns (preview) -- `lib/mv/membership/import/member_csv.ex` — `prepare/2`, `process_chunk/4`, validation, member creation -- `lib/mv/membership/import/import_runner.ex` — orchestration glue -- `lib/mv_web/live/import_live.ex` (+ `import_live/components.ex`) — UI, state machine, chunk driving -- `lib/mv_web/controllers/import_template_controller.ex` — on-the-fly template generation +**In Progress / Pending:** +- ⏳ Issue #9: End-to-End LiveView Tests + Fixtures +- ⏳ Issue #10: Documentation Polish -## Scope +**Latest Update:** CSV Import UI fully implemented in GlobalSettingsLive with chunk processing, progress tracking, error display, and custom field support (2026-01-13) -Admin-only bulk creation of members from an uploaded CSV. +--- -- **Create only** — no upsert/update of existing members. -- **No deduplication** — a duplicate email fails its row (unique constraint) and is reported as an error. -- **Best-effort, row-by-row** — no transactional rollback; a failed row does not abort the import. -- **No background jobs** — progress is driven via LiveView `handle_info` chunk messages. -- **Errors shown in UI only** — no error-CSV export. +## Table of Contents -Out of scope: upsert, mapping wizard, transactional all-or-nothing, error export, import history/audit. +- [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) -## UI Flow +--- -- **Route:** `/admin/import` (LiveView `MvWeb.ImportLive`). Template downloads: - `/admin/import/template/en` and `/admin/import/template/de` (dynamic controller, not static files). -- **Authorization:** requires `can?(:create, Mv.Membership.Member)`. Non-admins are - redirected with a "don't have permission" flash. The import section, the template - controller, and the `start_import` event all enforce this. -- **Upload:** `allow_upload(:csv_file, accept: .csv, max_entries: 1, auto_upload: true)`. - File size limit enforced by `max_file_size`. -- **State machine** (`@import_status`): `idle → preview → running → done|error`. - - **start_import** parses + resolves the file and transitions to **preview**. This step - is **read-only**: no members are created yet. The preview shows the column mapping, - sample rows, groups that exist vs. would be created, and fee-type/unknown-column warnings. - - **confirm_import** begins processing and creates members chunk by chunk. -- **Results:** success count, failure count, error list (each with CSV line number, message, - optional field), warnings, and a truncation notice when errors exceed the cap. +## Overview & Scope -## Limits +### What We're Building -- **Max file size:** configurable via `config :mv, csv_import: [max_file_size_mb: ...]` (enforced by `allow_upload`). -- **Max rows:** configurable via `config :mv, csv_import: [max_rows: ...]`, default **1000**, excluding header. Enforced in `MemberCSV.prepare/2`; exceeding it yields an error containing `"exceeds"`. -- **Chunk size:** 200 rows per chunk. -- **Error cap:** 50 errors collected per import overall (`failed` count stays accurate; `errors_truncated?` flag set when exceeded). +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. -## Parsing (`CsvParser.parse/1`) +**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`, `street`, `postal_code`, `city`, `country`) +- **Import custom field values** - Map CSV columns to existing custom fields by name (unknown custom field columns will be ignored with a warning) +- Validate each row (required field: `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) -- Content must be **valid UTF-8** (else error). Empty content / empty header row are errors. -- **UTF-8 BOM is stripped first**, before any header handling. -- Line endings normalized: `\r\n`, `\r`, `\n` all handled. -- **Delimiter auto-detection:** parse the header with both `;` and `,` parsers (NimbleCSV, - quote-aware), count non-empty fields each yields, pick the higher; **`;` wins ties**; - default `;`. -- **Quoting:** double-quote quoting; `""` inside a quoted field is a literal `"`. Newlines - inside quoted fields are supported — the record keeps its **start** line number. -- **Physical line numbers:** rows are returned as `{csv_line_number, values}` where the line - number is the physical 1-based line in the file (header is line 1, first data row is line 2). - **Empty lines are skipped but do not shift numbering** — downstream code must use the - parser's line numbers, never recompute from row index. (Test asserts an invalid row after a - skipped empty line still reports its true physical line, e.g. `Line 4`.) -- Completely empty rows are skipped. An unparsable row produces an error naming its line number. +**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) -## Header Mapping & Normalization (`HeaderMapper`) +### Out of Scope (v1) -**`normalize_header/1`** (applied identically to incoming headers, mapping variants, custom -field names, group names, and fee-type names): +**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 +- ❌ Batch validation preview before import +- ❌ Dynamic template generation +- ❌ Import history/audit log +- ❌ Import templates for other entities -1. trim, lowercase -2. transliterate German chars: `ß → ss`, `ä → ae`, `ö → oe`, `ü → ue` (and uppercase forms) -3. unify hyphen variants (en dash U+2013, em dash U+2014, minus U+2212 → `-`) -4. punctuation to spaces: `_`, `()[]{}`, `/`, `\` → space -5. **remove all whitespace** (so `first name` == `firstname`) -6. final trim +--- -Matching is on the fully normalized string. +## UX Flow -**Required field:** `email`. Missing it aborts `prepare` with a "Missing required header" error. +### Access & Location -**Unknown member-field columns:** ignored (no error). If an unknown column looks like it -could be a custom field that does not exist, a **warning** is emitted (import continues). +**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) -**Duplicate headers** mapping to the same canonical field (or same custom field) are an error. +### User Journey -### Supported member fields and header variants +1. **Navigate to Global Settings** +2. **Access Import Section** + - **Important notice:** Custom fields should be created in Mila before importing CSV files with custom field columns (unknown columns will be ignored with a warning) + - Upload area (drag & drop or file picker) + - Template download links (English / German) + - Help text explaining CSV format and custom field requirements +3. **Ensure Custom Fields Exist (if importing custom fields)** + - Navigate to Custom Fields section and create required custom fields + - Note the name/identifier for each custom field (used as CSV header) +4. **Download Template (Optional)** +5. **Prepare CSV File** + - Include custom field columns using the custom field name as header (e.g., `membership_number`, `birth_date`) +6. **Upload CSV** +7. **Start Import** + - Runs server-side via LiveView messages (may take up to ~30 seconds for large files) + - Warning messages if custom field columns reference non-existent custom fields (columns will be ignored) +8. **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) -Source of truth is `@member_field_variants_raw` in `header_mapper.ex`. Variants below are -illustrative; matching is via normalization, so casing/hyphen/whitespace differences all collapse. +### Error Handling -| Canonical | Example accepted headers (EN / DE) | Notes | +- **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 (all importable):** +- `email` / `E-Mail` (required) +- `first_name` / `Vorname` (optional) +- `last_name` / `Nachname` (optional) +- `join_date` / `Beitrittsdatum` (optional, ISO-8601 date) +- `exit_date` / `Austrittsdatum` (optional, ISO-8601 date) +- `notes` / `Notizen` (optional) +- `country` / `Land` / `Staat` (optional) +- `city` / `Stadt` (optional) +- `street` / `Straße` (optional) +- `house_number` / `Hausnummer` / `Nr.` (optional) +- `postal_code` / `PLZ` / `Postleitzahl` (optional) +- `membership_fee_start_date` / `Beitragsbeginn` (optional, ISO-8601 date) + +Address column order in import/export matches the members overview: country, city, street, house number, postal code. + +**Not supported for import (by design):** +- **membership_fee_status** – Computed field (from fee cycles). Not stored; export-only. +- **groups** – Many-to-many relationship. Would require resolving group names to IDs; not in current scope. +- **membership_fee_type_id** – Foreign key; could be added later (e.g. resolve type name to ID). + +**Custom Fields:** +- Any custom field column using the custom field's **name** as the header (e.g., `membership_number`, `birth_date`) +- **Important:** Custom fields must be created in Mila before importing. The CSV header must match the custom field name exactly (same normalization as member fields). +- **Behavior:** If the CSV contains custom field columns that don't exist in Mila, a warning message will be shown and those columns will be ignored during import. +- **Value Validation:** Custom field values are validated according to the custom field type: + - **string**: Any text value (trimmed) + - **integer**: Must be a valid integer (e.g., `42`, `-10`). Invalid values will cause a row error with the custom field name and reason. + - **boolean**: Accepts `true`, `false`, `1`, `0`, `yes`, `no`, `ja`, `nein` (case-insensitive). Invalid values will cause a row error. + - **date**: Must be in ISO-8601 format (YYYY-MM-DD, e.g., `2024-01-15`). Invalid values will cause a row error. + - **email**: Must be a valid email format (contains `@`, 5-254 characters, valid format). Invalid values will cause a row error. +- **Error Messages:** Custom field validation errors are included in the import error list with format: `custom_field: – ` (e.g., `custom_field: Alter – expected integer, got: abc`) + +**Member Field Header Mapping:** + +| Canonical Field | English Variants | German Variants | |---|---|---| -| `email` (required) | email, e-mail, e_mail, mail, e-mail-adresse / E-Mail | | -| `first_name` | first name, firstname / Vorname | | -| `last_name` | last name, lastname, surname / Nachname, Familienname | | -| `join_date` | join date / Beitrittsdatum | ISO-8601 date | -| `exit_date` | exit date / Austrittsdatum | ISO-8601 date | -| `notes` | notes / Notizen, Bemerkungen | | -| `street` | street, address / Straße, Strasse | | -| `house_number` | house number, house no / Hausnummer, Nr, Nr., Nummer | | -| `postal_code` | postal code, zip, postcode / PLZ, Postleitzahl | | -| `city` | city, town / Stadt, Ort | | -| `country` | country / Land, Staat | | -| `membership_fee_start_date` | membership fee start date, fee start / Beitragsbeginn | ISO-8601 date | +| `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` | +| `join_date` | `join date`, `join_date` | `Beitrittsdatum`, `beitritts-datum` | +| `exit_date` | `exit date`, `exit_date` | `Austrittsdatum`, `austritts-datum` | +| `notes` | `notes` | `Notizen`, `bemerkungen` | +| `street` | `street`, `address` | `Straße`, `strasse`, `Strasse` | +| `house_number` | `house number`, `house_number`, `house no` | `Hausnummer`, `Nr`, `Nr.`, `Nummer` | +| `postal_code` | `postal_code`, `zip`, `postcode` | `PLZ`, `plz`, `Postleitzahl`, `postleitzahl` | +| `city` | `city`, `town` | `Stadt`, `stadt`, `Ort` | +| `country` | `country` | `Land`, `land`, `Staat`, `staat` | +| `membership_fee_start_date` | `membership fee start date`, `membership_fee_start_date`, `fee start` | `Beitragsbeginn`, `beitrags-beginn` | -### Special relationship columns +**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 -- **groups** (headers `Groups` / `Gruppen` / `Gruppe`) — comma-separated group names. Names - matched case-insensitively against existing groups; **missing groups are auto-created** during - processing. A group-assignment failure fails that row (the member was already created). -- **membership_fee_type** (headers `Fee Type`, `fee_type`, `membership_fee_type` / `Beitragsart`) - — name matched to an existing `MembershipFeeType`. **Empty cell → default fee type** (no warning). - **Matched name → that fee type.** **Unmatched name → default fee type + warning** naming the value. +**Unknown columns:** ignored (no error) -These columns are resolved against the DB read-only in `prepare` (`ColumnResolver`) for the -preview; the actual writes happen in `process_chunk`. +**Required fields:** `email` -### Fields not importable (explicitly ignored) +**Custom Field Columns:** +- Custom field columns are identified by matching the normalized CSV header to the custom field `name` (not slug) +- Same normalization rules apply as for member fields (trim, lowercase, Unicode normalization, underscore replacement) +- Unknown custom field columns (non-existent names) will be ignored with a warning message -- **membership_fee_status** — computed from fee cycles; not stored. Fee-status header variants - (`Membership Fee Status`, `Bezahlstatus`, `Mitgliedsbeitragsstatus`) and the DE export label - `Startdatum Mitgliedsbeitrag` are placed in the `ignored` list and never mapped. (The UI notice - names `Groups`/`Gruppen`, `Fee Type`/`Beitragsart`, and the always-ignored `Bezahlstatus`.) +### CSV Template Files -## Custom Fields +**Location:** +- `priv/static/templates/member_import_en.csv` +- `priv/static/templates/member_import_de.csv` -- Custom field columns are matched by the custom field **name** (not slug), using the same - normalization. Member fields take priority on a name collision. -- **Custom fields must exist in Mila before import.** Unknown custom-field columns are ignored - with a warning; the import still runs. -- Empty custom-field cells create no value. Values are trimmed; type-validated per the custom - field's `value_type`: - - **string** — any text (trimmed). - - **integer** — must parse fully (`Integer.parse` with no remainder); e.g. `42`, `-10`. - - **boolean** — case-insensitive `true/false`, `1/0`, `yes/no`, `ja/nein`. - - **date** — ISO-8601 `YYYY-MM-DD`. - - **email** — validated with `EctoCommons.EmailValidator` (same checks as member email). -- A value failing type validation fails the row. Error message format: - `custom_field: – expected , got: ` (type label is the human-readable - `FieldTypes.label/1`, with format hints for boolean/date). +**Content:** +- Header row with required + common optional fields +- **Note:** Custom field columns are not included in templates by default (users add them based on their custom field configuration) +- One example row +- Uses semicolon delimiter (`;`) +- UTF-8 encoding **with BOM** (Excel compatibility) -## Validation & Member Creation (`process_chunk/4` → `process_row`) +**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). -Per row: validate → create member → create custom-field values → assign groups. Sequential. +**Example Usage in LiveView Templates:** -- **Email** is required and format-validated (`EctoCommons.EmailValidator`, `Mv.Constants.email_validator_checks()`) on a trimmed value. All string member values are trimmed. -- **Date fields** (`join_date`, `exit_date`, `membership_fee_start_date`): empty/blank strings are converted to `nil` so Ash accepts them. -- Member created via `Mv.Membership.create_member/2`. Custom field values are passed as - `custom_field_values` (Ash union `_union_type`/`_union_value` format), omitted when none. -- **Errors** are `%MemberCSV.Error{csv_line_number, field, message}`: - - `csv_line_number` is the physical line (1-based); never recomputed in this layer. - - Validation errors get `field: :email`; Ash errors prefer the field-level error. - - **Duplicate email** (unique constraint) is surfaced as a friendly - `"email has already been taken"` message. -- **Error capping** (`max_errors`, default 50, tracked across chunks via `existing_error_count`): - once the cap is hit, no further errors are collected but **all rows are still processed** and - the `failed` count stays accurate; `errors_truncated?` is set and the UI shows a truncation notice. +```heex + +<.link href={~p"/templates/member_import_en.csv"} download> + <%= gettext("Download English Template") %> + -## Templates (`ImportTemplateController`) +<.link href={~p"/templates/member_import_de.csv"} download> + <%= gettext("Download German Template") %> + -- Generated on the fly (not static files), gated by `can?(:create, Member)`. -- One header row: standard member columns (localized EN/DE) + `Groups`/`Gruppen` + - `Fee Type`/`Beitragsart` + **every existing custom field name** appended, then one example row. -- Semicolon-delimited, RFC-4180 quoting; fields run through `MembersCSV.safe_cell/1` to - neutralize spreadsheet formula injection (e.g. a custom-field name like `=HYPERLINK(...)`). + +<.link href={Routes.static_path(MvWeb.Endpoint, "/templates/member_import_en.csv")} download> + <%= gettext("Download English Template") %> + +``` + +**Note:** The `templates` directory must be included in `MvWeb.static_paths()` (configured in `lib/mv_web.ex`) for the files to be served. + +### 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 + custom field handling +- `lib/mv/membership/import/csv_parser.ex` - delimiter detection + parsing + BOM handling +- `lib/mv/membership/import/header_mapper.ex` - normalization + header mapping (core fields + custom fields) + +**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 (core member fields) + - **Query existing custom fields and map custom field columns by name** (using same normalization as member fields) + - **Warn about unknown custom field columns** (non-existent names will be ignored with warning) + - Early abort if required headers missing + - Row count check + - Return `import_state` containing chunks, column_map, and custom_field_map +4. **Process:** LiveView drives chunk processing via `handle_info` + - For each chunk: validate + create member + create custom field values + 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 + +**Status:** ✅ **COMPLETED** + +**Goal:** Define CSV contract and add static templates. + +**Tasks:** +- [x] Finalize header mapping variants +- [x] Document normalization rules +- [x] Document delimiter detection strategy +- [x] Create templates in `priv/static/templates/` (UTF-8 with BOM) + - `member_import_en.csv` with English headers + - `member_import_de.csv` with German headers +- [x] Document template URLs and how to link them from LiveView +- [x] Document line number semantics (physical CSV line numbers) +- [x] Templates included in `MvWeb.static_paths()` configuration + +**Definition of Done:** +- [x] Templates open cleanly in Excel/LibreOffice +- [x] CSV spec section complete + +--- + +### Issue #2: Import Service Module Skeleton + +**Dependencies:** None + +**Status:** ✅ **COMPLETED** + +**Goal:** Create service API and error types. + +**API (recommended):** +- `prepare/2` — parse + map + limit checks, returns import_state +- `process_chunk/4` — process one chunk (pure-ish), returns per-chunk results + +**Tasks:** +- [x] Create `lib/mv/membership/import/member_csv.ex` +- [x] Define public function: `prepare/2 (file_content, opts \\ [])` +- [x] Define public function: `process_chunk/4 (chunk_rows_with_lines, column_map, custom_field_map, opts \\ [])` +- [x] Define error struct: `%MemberCSV.Error{csv_line_number: integer, field: atom | nil, message: String.t}` +- [x] Document module + API + +--- + +### Issue #3: CSV Parsing + Delimiter Auto-Detection + BOM Handling + +**Dependencies:** Issue #2 + +**Status:** ✅ **COMPLETED** + +**Goal:** Parse CSV robustly with correct delimiter detection and BOM handling. + +**Tasks:** +- [x] Verify/add NimbleCSV dependency (`{:nimble_csv, "~> 1.0"}`) +- [x] Create `lib/mv/membership/import/csv_parser.ex` +- [x] Implement `strip_bom/1` and apply it **before** any header handling +- [x] Handle `\r\n` and `\n` line endings (trim `\r` on header record) +- [x] Detect delimiter via header recognition (try `;` and `,`) +- [x] Parse CSV and return: + - `headers :: [String.t()]` + - `rows :: [{csv_line_number, [String.t()]}]` with correct physical line numbers +- [x] Skip completely empty records (but preserve correct physical line numbers) +- [x] Return `{:ok, headers, rows}` or `{:error, reason}` + +**Definition of Done:** +- [x] BOM handling works (Excel exports) +- [x] Delimiter detection works reliably +- [x] Rows carry correct `csv_line_number` + +--- + +### Issue #4: Header Normalization + Per-Header Mapping (No Language Detection) + +**Dependencies:** Issue #3 + +**Status:** ✅ **COMPLETED** + +**Goal:** Map each header individually to canonical fields (normalized comparison). + +**Tasks:** +- [x] Create `lib/mv/membership/import/header_mapper.ex` +- [x] Implement `normalize_header/1` +- [x] Normalize mapping variants once and compare normalized strings +- [x] Build `column_map` (canonical field -> column index) +- [x] **Early abort if required headers missing** (`email`) +- [x] Ignore unknown columns (member fields only) +- [x] **Separate custom field column detection** (by name, with normalization) + +**Definition of Done:** +- [x] English/German headers map correctly +- [x] Missing required columns fails fast + +--- + +### Issue #5: Validation (Required Fields) + Error Formatting + +**Dependencies:** Issue #4 + +**Status:** ✅ **COMPLETED** + +**Goal:** Validate each row and return structured, translatable errors. + +**Tasks:** +- [x] Implement `validate_row/3 (row_map, csv_line_number, opts)` +- [x] Required field presence (`email`) +- [x] Email format validation (EctoCommons.EmailValidator) +- [x] Trim values before validation +- [x] Gettext-backed error messages + +--- + +### Issue #6: Persistence via Ash Create + Per-Row Error Capture (Chunked Processing) + +**Dependencies:** Issue #5 + +**Status:** ✅ **COMPLETED** + +**Goal:** Create members and capture errors per row with correct CSV line numbers. + +**Tasks:** +- [x] Implement `process_chunk/4` in service: + - Input: `[{csv_line_number, row_map}]` + - Validate + create sequentially + - Collect counts + first 50 errors (per import overall; LiveView enforces cap across chunks) + - **Error-Capping:** Supports `existing_error_count` and `max_errors` in opts (default: 50) + - **Error-Capping:** Only collects errors if under limit, but continues processing all rows + - **Error-Capping:** `failed` count is always accurate, even when errors are capped +- [x] 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 +- [x] Map row_map to Ash attrs (`%{first_name: ..., ...}`) +- [x] Custom field value processing and creation + +**Important:** **Do not recompute line numbers** in this layer—use the ones provided by the parser. + +**Implementation Notes:** +- `process_chunk/4` accepts `opts` with `existing_error_count` and `max_errors` for error capping across chunks +- Error capping respects the limit per import overall (not per chunk) +- Processing continues even after error limit is reached (for accurate counts) + +--- + +### Issue #7: Admin Global Settings LiveView UI (Upload + Start Import + Results + Template Links) + +**Dependencies:** Issue #6 + +**Status:** ✅ **COMPLETED** + +**Goal:** UI section with upload, progress, results, and template links. + +**Tasks:** +- [x] Render import section only for admins +- [x] **Add prominent UI notice about custom fields:** + - Display alert/info box: "Custom fields must be created in Mila before importing CSV files with custom field columns" + - Explain: "Use the custom field name as the CSV column header (same normalization as member fields applies)" + - Add link to custom fields management section +- [x] Configure `allow_upload/3`: + - `.csv` only, `max_entries: 1`, `max_file_size: 10MB`, `auto_upload: true` (auto-upload enabled for better UX) +- [x] `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})` +- [x] `handle_info({:process_chunk, idx}, socket)`: + - Fetch chunk from `import_state` + - Call `MemberCSV.process_chunk/4` with error capping support + - Merge counts/errors into progress assigns (cap errors at 50 overall) + - Schedule next chunk (or finish and show results) + - Async task processing with SQL sandbox support for tests +- [x] Results UI: + - Success count + - Failure count + - Error list (line number + message + field) + - **Warning messages for unknown custom field columns** (non-existent names) shown in results + - Progress indicator during import + - Error truncation notice when errors exceed limit + +**Template links:** +- [x] Link `/templates/member_import_en.csv` and `/templates/member_import_de.csv` via Phoenix static path helpers. + +**Definition of Done:** +- [x] Upload area with drag & drop support +- [x] Template download links (EN/DE) +- [x] Progress tracking during import +- [x] Results display with success/error counts +- [x] Error list with line numbers and field information +- [x] Warning display for unknown custom field columns +- [x] Admin-only access control +- [x] Async chunk processing with proper error handling + +--- + +### Issue #8: Authorization + Limits + +**Dependencies:** None (can be parallelized) + +**Status:** ✅ **COMPLETED** + +**Goal:** Ensure admin-only access and enforce limits. + +**Tasks:** +- [x] Admin check in start import event handler (via `Authorization.can?/3`) +- [x] File size enforced in upload config (`max_file_size: 10MB`) +- [x] Row limit enforced in `MemberCSV.prepare/2` (max_rows: 1000, configurable via opts) +- [x] Chunk size limit (200 rows per chunk) +- [x] Error limit (50 errors per import) +- [x] UI-level authorization check (import section only visible to admins) +- [x] Event-level authorization check (prevents unauthorized import attempts) + +**Implementation Notes:** +- File size limit: 10 MB (10,485,760 bytes) enforced via `allow_upload/3` +- Row limit: 1,000 rows (excluding header) enforced in `MemberCSV.prepare/2` +- Chunk size: 200 rows per chunk (configurable via opts) +- Error limit: 50 errors per import (configurable via `@max_errors`) +- Authorization uses `MvWeb.Authorization.can?/3` with `:create` permission on `Mv.Membership.Member` + +**Definition of Done:** +- [x] Admin-only access enforced at UI and event level +- [x] File size limit enforced +- [x] Row count limit enforced +- [x] Chunk processing with size limits +- [x] Error capping implemented + +--- + +### Issue #9: End-to-End LiveView Tests + Fixtures + +**Dependencies:** Issue #7 and #8 + +**Tasks:** +- [ ] Fixtures: + - valid EN/DE (core fields only) + - valid with custom fields + - invalid + - unknown custom field name (non-existent, should show warning) + - 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 + - custom field import success + - custom field import warning (non-existent name, column ignored) + +--- + +### 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 + +**Dependencies:** Issue #6 (Persistence) + +**Priority:** High (Core v1 Feature) + +**Status:** ✅ **COMPLETED** (Backend + UI Implementation) + +**Goal:** Support importing custom field values from CSV columns. Custom fields should exist in Mila before import for best results. + +**Important Requirements:** +- **Custom fields should be created in Mila first** - Unknown custom field columns will be ignored with a warning message +- CSV headers for custom fields must match the custom field **name** exactly (same normalization as member fields applies) +- Custom field values are validated according to the custom field type (string, integer, boolean, date, email) +- Unknown custom field columns (non-existent names) will be ignored with a warning - import continues + +**Tasks:** +- [x] Extend `header_mapper.ex` to detect custom field columns by name (using same normalization as member fields) +- [x] Query existing custom fields during `prepare/2` to map custom field columns +- [x] Collect unknown custom field columns and add warning messages (don't fail import) +- [x] Map custom field CSV values to `CustomFieldValue` creation in `process_chunk/4` +- [x] Handle custom field type validation (string, integer, boolean, date, email) with proper error messages +- [x] Create `CustomFieldValue` records linked to members during import +- [x] Validate custom field values and return structured errors with custom field name and reason +- [x] UI help text and link to custom field management (implemented in Issue #7) +- [x] Update error messages to include custom field validation errors (format: `custom_field: – expected , got: `) +- [x] Add UI help text explaining custom field requirements (completed in Issue #7): + - "Custom fields must be created in Mila before importing" + - "Use the custom field name as the CSV column header (same normalization as member fields)" + - Link to custom fields management section +- [x] Update CSV templates documentation to explain custom field columns (documented in Issue #1) +- [x] Add tests for custom field import (valid, invalid name, type validation, warning for unknown) + +**Definition of Done:** +- [x] Custom field columns are recognized by name (with normalization) +- [x] Warning messages shown for unknown custom field columns (import continues) +- [x] Custom field values are created and linked to members +- [x] Type validation works for all custom field types (string, integer, boolean, date, email) +- [x] UI clearly explains custom field requirements (completed in Issue #7) +- [x] Tests cover custom field import scenarios (including warning for unknown names) +- [x] Error messages include custom field validation errors with proper formatting + +**Implementation Notes:** +- Custom field lookup is built in `prepare/2` and passed via `custom_field_lookup` in opts +- Custom field values are formatted according to type in `format_custom_field_value/2` +- Unknown custom field columns generate warnings in `import_state.warnings` + +--- + +## 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 +│ ├── import_runner.ex # orchestration: file read, progress merge, chunk process, error format +│ ├── csv_parser.ex # delimiter detection + parsing + BOM handling +│ └── header_mapper.ex # normalization + header mapping +└── mv_web/ + └── live/ + ├── import_export_live.ex # mount / handle_event / handle_info + glue only + └── import_export_live/ + └── components.ex # UI: custom_fields_notice, template_links, import_form, import_progress, import_results + +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** diff --git a/docs/custom-fields-search-performance.md b/docs/custom-fields-search-performance.md index 47de308..3987c85 100644 --- a/docs/custom-fields-search-performance.md +++ b/docs/custom-fields-search-performance.md @@ -2,88 +2,242 @@ ## Current Implementation -The member `search_vector` includes custom field values via database triggers that aggregate all of a member's custom field values, extract the value from each JSONB record (`value->>'_union_value'`), and add them at weight `C`. +The search vector includes custom field values via database triggers that: +1. Aggregate all custom field values for a member +2. Extract values from JSONB format +3. Add them to the search_vector with weight 'C' -Two triggers maintain the vector: +## Performance Considerations -- `members_search_vector_trigger()` — fires on `members` INSERT/UPDATE; runs a subquery `SELECT string_agg(...) FROM custom_field_values WHERE member_id = NEW.id`. -- `update_member_search_vector_from_custom_field_value()` — fires on `custom_field_values` INSERT/UPDATE/DELETE; re-aggregates and updates the member's `search_vector`. +### 1. Trigger Performance on Member Updates -Both rely on `custom_field_values_member_id_idx`, so the per-member aggregation is an indexed lookup. +**Current Implementation:** +- `members_search_vector_trigger()` executes a subquery on every INSERT/UPDATE: + ```sql + SELECT string_agg(...) FROM custom_field_values WHERE member_id = NEW.id + ``` -## Applied Trigger Optimizations +**Performance Impact:** +- ✅ **Good:** Index on `member_id` exists (`custom_field_values_member_id_idx`) +- ✅ **Good:** Subquery only runs for the affected member +- ⚠️ **Potential Issue:** With many custom fields per member (e.g., 50+), aggregation could be slower +- ⚠️ **Potential Issue:** JSONB extraction (`value->>'_union_value'`) is relatively fast but adds overhead -`update_member_search_vector_from_custom_field_value()` was optimized: +**Expected Performance:** +- **Small scale (< 10 custom fields per member):** Negligible impact (< 5ms per operation) +- **Medium scale (10-30 custom fields):** Minor impact (5-20ms per operation) +- **Large scale (30+ custom fields):** Noticeable impact (20-50ms+ per operation) -- **Fetch only required member fields** (first_name, last_name, email, etc.) instead of the full record — reduces per-call overhead by roughly 30–50%. -- **Early return on UPDATE when the value is unchanged** — skips the expensive re-aggregation entirely. +### 2. Trigger Performance on Custom Field Value Changes -Measured effect per custom-field-value change: +**Current Implementation:** +- `update_member_search_vector_from_custom_field_value()` executes on every INSERT/UPDATE/DELETE on `custom_field_values` +- **Optimized:** Only fetches required member fields (not full record) to reduce overhead +- **Optimized:** Skips re-aggregation on UPDATE if value hasn't actually changed +- Aggregates all custom field values, then updates member search_vector -| Case | Before | After | -|------|--------|-------| -| Value changed | 5–15 ms | 3–10 ms | -| Value unchanged (UPDATE) | 5–15 ms | < 1 ms (early return) | +**Performance Impact:** +- ✅ **Good:** Index on `member_id` ensures fast lookup +- ✅ **Optimized:** Only required fields are fetched (first_name, last_name, email, etc.) instead of full record +- ✅ **Optimized:** UPDATE operations that don't change the value skip expensive re-aggregation (early return) +- ⚠️ **Note:** Re-aggregation is still necessary when values change (required for search_vector consistency) +- ⚠️ **Critical:** Bulk operations (e.g., importing 1000 members with custom fields) will trigger this for each row -Re-aggregation is still required whenever a value actually changes — that is necessary for `search_vector` consistency. +**Expected Performance:** +- **Single operation (value changed):** 3-10ms per custom field value change (improved from 5-15ms) +- **Single operation (value unchanged):** <1ms (early return, no aggregation) +- **Bulk operations:** Could be slow (consider disabling trigger temporarily) -## Search Vector Size +### 3. Search Vector Size -- String custom field values are capped at **10,000 characters each**; there is no cap on the number of custom fields per member. -- `tsvector` has no hard size limit, but very large vectors (> ~100 KB) degrade GIN index maintenance, tsvector operations, and trigger time. Worst case: 100 fields × 10,000 chars ≈ 1 MB of aggregated text for one member. -- **Recommendation:** monitor `search_vector` size in production; consider capping total custom-field content per member if large vectors appear. +**Current Constraints:** +- String values: max 10,000 characters per custom field +- No limit on number of custom fields per member +- tsvector has no explicit size limit, but very large vectors can cause issues -## Bulk Imports +**Potential Issues:** +- **Theoretical maximum:** If a member has 100 custom fields with 10,000 char strings each, the aggregated text could be ~1MB +- **Practical concern:** Very large search vectors (> 100KB) can slow down: + - Index updates (GIN index maintenance) + - Search queries (tsvector operations) + - Trigger execution time -The custom-field-value trigger fires once per row, so importing many members with custom fields is expensive. For bulk imports, **temporarily disable the `custom_field_values` trigger**, then re-aggregate `search_vector` in a batch after the import. The initial backfill migration also updates all members in a single transaction (table lock); for > 10,000 members, batch the backfill and run during a maintenance window. +**Recommendation:** +- Monitor search_vector size in production +- Consider limiting total custom field content per member if needed +- PostgreSQL can handle large tsvectors, but performance degrades gradually -## Search Query Structure +### 4. Initial Migration Performance -Full-text search uses the GIN index on `search_vector` (fast). Substring/custom-field matching adds `EXISTS (SELECT 1 FROM custom_field_values WHERE member_id = id AND ... LIKE ...)` subqueries, which are **not indexed** on the JSONB value (sequential scan) and run even when the FTS branch already matches. This is the main known weakness; it is acceptable at the current scale (< 30 custom fields/member, < 10,000 members) but is the first thing to revisit if search slows. +**Current Implementation:** +- Updates ALL members in a single transaction: + ```sql + UPDATE members m SET search_vector = ... (subquery for each member) + ``` -## Search Filter Functions +**Performance Impact:** +- ⚠️ **Potential Issue:** With 10,000+ members, this could take minutes +- ⚠️ **Potential Issue:** Single transaction locks the members table +- ⚠️ **Potential Issue:** If migration fails, entire rollback required -The search query in `lib/membership/member.ex` is split into modular filter builders, combined as a single OR-chain in priority order: +**Recommendation:** +- For large datasets (> 10,000 members), consider: + - Batch updates (e.g., 1000 members at a time) + - Run during maintenance window + - Monitor progress -1. `build_fts_filter/1` — full-text search (highest priority, GIN-indexed, fastest). -2. `build_substring_filter/2` — `ILIKE` substring matching on structured fields (postal_code, house_number, email, city, country). -3. `build_custom_field_filter/1` — JSONB custom-field value matching via `EXISTS` subquery. -4. `build_fuzzy_filter/2` — trigram fuzzy matching on first_name, last_name, street (pg_trgm). +### 5. Search Query Performance -Priority: **FTS > Substring > Custom Fields > Fuzzy**. +**Current Implementation:** +- Full-text search uses GIN index on `search_vector` (fast) +- Additional LIKE queries on `custom_field_values` for substring matching: + ```sql + EXISTS (SELECT 1 FROM custom_field_values WHERE member_id = id AND ... LIKE ...) + ``` + +**Performance Impact:** +- ✅ **Good:** GIN index on `search_vector` is very fast +- ⚠️ **Potential Issue:** LIKE queries on JSONB are not indexed (sequential scan) +- ⚠️ **Potential Issue:** EXISTS subquery runs for every search, even if search_vector match is found +- ⚠️ **Potential Issue:** With many custom fields, the LIKE queries could be slow + +**Expected Performance:** +- **With GIN index match:** Very fast (< 10ms for typical queries) +- **Without GIN index match (fallback to LIKE):** Slower (10-100ms depending on data size) +- **Worst case:** Sequential scan of all custom_field_values for all members + +## Recommendations + +### Short-term (Current Implementation) + +1. **Monitor Performance:** + - Add logging for trigger execution time + - Monitor search_vector size distribution + - Track search query performance + +2. **Index Verification:** + - Ensure `custom_field_values_member_id_idx` exists and is used + - Verify GIN index on `search_vector` is maintained + +3. **Bulk Operations:** + - For bulk imports, consider temporarily disabling the custom_field_values trigger + - Re-enable and update search_vectors in batch after import + +### Medium-term Optimizations + +1. **✅ Optimize Trigger Function (FULLY IMPLEMENTED):** + - ✅ Only fetch required member fields instead of full record (reduces overhead) + - ✅ Skip re-aggregation on UPDATE if value hasn't actually changed (early return optimization) + +2. **Limit Search Vector Size:** + - Truncate very long custom field values (e.g., first 1000 chars) + - Add warning if aggregated text exceeds threshold + +3. **Optimize LIKE Queries:** + - Consider adding a generated column for searchable text + - Or use a materialized view for custom field search + +### Long-term Considerations + +1. **Alternative Approaches:** + - Separate search index table for custom fields + - Use Elasticsearch or similar for advanced search + - Materialized view for search optimization + +2. **Scaling Strategy:** + - If performance becomes an issue with 100+ custom fields per member: + - Consider limiting which custom fields are searchable + - Use a separate search service + - Implement search result caching + +## Performance Benchmarks (Estimated) + +Based on typical PostgreSQL performance: + +| Scenario | Members | Custom Fields/Member | Expected Impact | +|----------|---------|---------------------|-----------------| +| Small | < 1,000 | < 10 | Negligible (< 5ms per operation) | +| Medium | 1,000-10,000 | 10-30 | Minor (5-20ms per operation) | +| Large | 10,000-100,000 | 30-50 | Noticeable (20-50ms per operation) | +| Very Large | > 100,000 | 50+ | Significant (50-200ms+ per operation) | ## Monitoring Queries ```sql --- search_vector size distribution -SELECT - pg_size_pretty(octet_length(search_vector::text)) AS size, - COUNT(*) AS member_count +-- Check search_vector size distribution +SELECT + pg_size_pretty(octet_length(search_vector::text)) as size, + COUNT(*) as member_count FROM members WHERE search_vector IS NOT NULL GROUP BY octet_length(search_vector::text) ORDER BY octet_length(search_vector::text) DESC LIMIT 20; --- average / max custom fields per member -SELECT - AVG(custom_field_count) AS avg_custom_fields, - MAX(custom_field_count) AS max_custom_fields +-- Check average custom fields per member +SELECT + AVG(custom_field_count) as avg_custom_fields, + MAX(custom_field_count) as max_custom_fields FROM ( - SELECT member_id, COUNT(*) AS custom_field_count + SELECT member_id, COUNT(*) as custom_field_count FROM custom_field_values GROUP BY member_id ) subq; --- trigger execution time (requires pg_stat_statements) -SELECT mean_exec_time, calls, query +-- Check trigger execution time (requires pg_stat_statements) +SELECT + mean_exec_time, + calls, + query FROM pg_stat_statements WHERE query LIKE '%members_search_vector_trigger%' ORDER BY mean_exec_time DESC; ``` -## Future Options (if scale demands) +## Code Quality Improvements (Post-Review) + +### Refactored Search Implementation + +The search query has been refactored for better maintainability and clarity: + +**Before:** Single large OR-chain with mixed search types (hard to maintain) + +**After:** Modular functions grouped by search type: +- `build_fts_filter/1` - Full-text search (highest priority, fastest) +- `build_substring_filter/2` - Substring matching on structured fields +- `build_custom_field_filter/1` - Custom field value search (JSONB LIKE) +- `build_fuzzy_filter/2` - Trigram/fuzzy matching for names and streets + +**Benefits:** +- ✅ Clear separation of concerns +- ✅ Easier to maintain and test +- ✅ Better documentation of search priority +- ✅ Easier to optimize individual search types + +**Search Priority Order:** +1. **FTS (Full-Text Search)** - Fastest, uses GIN index on search_vector +2. **Substring** - For structured fields (postal_code, phone_number, etc.) +3. **Custom Fields** - JSONB LIKE queries (fallback for substring matching) +4. **Fuzzy Matching** - Trigram similarity for names and streets + +## Conclusion + +The current implementation is **well-optimized for typical use cases** (< 30 custom fields per member, < 10,000 members). For larger scales, monitoring and potential optimizations may be needed. + +**Key Strengths:** +- Indexed lookups (member_id index) +- Efficient GIN index for search +- Trigger-based automatic updates +- Modular, maintainable search code structure + +**Key Weaknesses:** +- LIKE queries on JSONB (not indexed) +- Re-aggregation on every custom field change (necessary for consistency) +- Potential size issues with many/large custom fields +- Substring searches (contains/ILIKE) not index-optimized + +**Recent Optimizations:** +- ✅ Trigger function optimized to fetch only required fields (reduces overhead by ~30-50%) +- ✅ Early return on UPDATE when value hasn't changed (skips expensive re-aggregation, <1ms vs 3-10ms) +- ✅ Improved performance for custom field value updates (3-10ms vs 5-15ms when value changes) -- Generated/searchable text column or materialized view for custom-field substring search (to escape the unindexed JSONB `LIKE`). -- Limit which custom fields are searchable, or truncate long values. -- External search service (e.g., Elasticsearch) for advanced search. diff --git a/docs/daisyui-drawer-pattern.md b/docs/daisyui-drawer-pattern.md index 85690c9..dec599d 100644 --- a/docs/daisyui-drawer-pattern.md +++ b/docs/daisyui-drawer-pattern.md @@ -1,21 +1,533 @@ -# Sidebar Drawer Pattern (project note) +# DaisyUI Drawer Pattern - Standard Implementation -The app's responsive sidebar uses DaisyUI's drawer in the **combined -mobile-overlay + desktop-persistent** configuration. The drawer container, -the `mobile-drawer` toggle checkbox and the mobile header live in the `app/1` -layout (`lib/mv_web/components/layouts.ex`); the sidebar body and the -tap-to-close `drawer-overlay` live in `lib/mv_web/components/layouts/sidebar.ex`. +This document describes the standard DaisyUI drawer pattern for implementing responsive sidebars. It covers mobile overlay drawers, desktop persistent sidebars, and their combination. -## Chosen pattern +## Core Concept + +DaisyUI's drawer component uses a **checkbox-based toggle mechanism** combined with CSS to create accessible, responsive sidebars without custom JavaScript. + +### Key Components + +1. **`drawer`** - Container element +2. **`drawer-toggle`** - Hidden checkbox that controls open/close state +3. **`drawer-content`** - Main content area +4. **`drawer-side`** - Sidebar content (menu, navigation) +5. **`drawer-overlay`** - Optional overlay for mobile (closes drawer on click) + +## HTML Structure + +```html +
+ + + + +
+ + +
+ + +
+ + +
+
+``` + +## How drawer-toggle Works + +### Mechanism + +The `drawer-toggle` is a **hidden checkbox** that serves as the state controller: + +```html + +``` + +### Toggle Behavior + +1. **Label Connection**: Any `