diff --git a/.gitignore b/.gitignore index 14620df..b37fa85 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,7 @@ mv-*.tar # In case you use Node.js/npm, you want to ignore these. npm-debug.log /assets/node_modules/ +/node_modules/ .cursor diff --git a/.opencode/screenshots/01_mitglieder.png b/.opencode/screenshots/01_mitglieder.png new file mode 100644 index 0000000..7cf25af Binary files /dev/null and b/.opencode/screenshots/01_mitglieder.png differ diff --git a/.opencode/screenshots/02_statistik.png b/.opencode/screenshots/02_statistik.png new file mode 100644 index 0000000..675c036 Binary files /dev/null and b/.opencode/screenshots/02_statistik.png differ diff --git a/.opencode/screenshots/03_beitraege.png b/.opencode/screenshots/03_beitraege.png new file mode 100644 index 0000000..5918953 Binary files /dev/null and b/.opencode/screenshots/03_beitraege.png differ diff --git a/.opencode/screenshots/04_aufnahmeantraege.png b/.opencode/screenshots/04_aufnahmeantraege.png new file mode 100644 index 0000000..13bb316 Binary files /dev/null and b/.opencode/screenshots/04_aufnahmeantraege.png differ diff --git a/.tool-versions b/.tool-versions index e72ed5f..e815bde 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,3 +1,4 @@ elixir 1.18.3-otp-27 erlang 27.3.4 -just 1.50.0 +just 1.51.0 +nodejs 26.2.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c8032c..adbe7e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ 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). +## [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. +- **Join-form description tooltip in member details** – Custom fields that have a join-form description show an info tooltip (prefixed "Beitrittsformular:") on their label in the member detail view. +- **Editable join-form description** – Admins can set a field's join-form description in the custom-field settings, with an inline hint about the supported link syntax. +- **CSV import – groups column** – Members can be assigned to groups during CSV import via a `Groups`/`Gruppen` column; group names that do not exist yet are created automatically, and re-importing the same file does not create duplicate groups. +- **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. + +### 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. +- **Dropdown buttons** – Dropdown buttons (actions, filter, column visibility) now show a chevron so they are recognizable as menus. +- **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 +- **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. + ## [1.2.0] - 2026-05-08 ### Changed diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 2b378ef..ccd16f4 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -1363,6 +1363,8 @@ mix gettext.merge priv/gettext --on-obsolete=mark_as_obsolete ### 3.13 Task Runner: Just +The `Justfile` prepends `~/.asdf/shims`, `~/.asdf/bin`, and `~/.asdf` to `PATH` for all recipes (`set export := true`), so `mix` / `elixir` resolve from `.tool-versions` without shell init. The caller's `PATH` is kept (e.g. Homebrew `asdf`, Docker). Run `asdf install` once per machine; no extra `source` is required for `just run`. + **Common Commands:** ```bash diff --git a/Justfile b/Justfile index 54c395f..9b0be65 100644 --- a/Justfile +++ b/Justfile @@ -1,11 +1,11 @@ set dotenv-load := true set export := true -# Non-interactive shells do not source .bashrc, -# PATH includes asdf shims so that mix / elixir / iex resolve without per-shell -# `source ~/.asdf/asdf.sh`. Recipes inherit this via `set export := true`. -home := env_var('HOME') -PATH := home + "/.asdf/shims:" + home + "/.asdf:" + home + "/.local/bin:/usr/local/bin:/usr/bin:/bin" +# Prepend asdf paths so recipes work without sourcing ~/.asdf/asdf.sh in the shell. +# Caller PATH is preserved (Homebrew asdf, docker CLI, etc.). See CODE_GUIDELINES §3.13. +home := env_var("HOME") +asdf_paths := home + "/.asdf/shims:" + home + "/.asdf/bin:" + home + "/.asdf:" +PATH := asdf_paths + env_var("PATH") MIX_QUIET := "1" @@ -29,7 +29,12 @@ seed-database: start-database: docker compose up -d -ci-dev: lint audit test-fast +# Full check suite: lint + audit + the fast tests (slow/ui excluded). No Dialyzer. +ci-dev: install-dependencies lint audit test-fast + +# Fast pre-commit check: lint + sobelow + only the affected tests (mix test --stale) +# with reduced property runs. Run the full `ci-dev` before pushing. +check: install-dependencies lint sobelow test-stale # Build the Dialyzer PLT. Idempotent — no-op once the PLT is up to date. # First build takes 5–15 min; subsequent runs are seconds. PLT files live in @@ -58,19 +63,28 @@ lint: @bash -c 'for file in priv/gettext/de/LC_MESSAGES/*.po; do awk "/^msgid \"\"$/{header=1; next} /^msgid /{header=0} /^msgstr \"\"$/ && !header{print FILENAME\":\"NR\": \" \$0; exit 1}" "$file" || exit 1; done' mix gettext.extract --check-up-to-date -audit: +# Static security scan (Sobelow). +sobelow: mix sobelow --config + +# Full security audit: Sobelow + dependency advisory scans. +audit: sobelow mix deps.audit --ignore-file .deps_audit_ignore mix hex.audit -# Run all tests -test *args: install-dependencies +# Run all tests. No install-dependencies prerequisite so single-file runs stay +# fast; run `just install-dependencies` once on a fresh checkout. +test *args: mix test {{args}} -# Run only fast tests (excludes slow/performance and UI tests) -test-fast *args: install-dependencies +# Fast tests only (excludes slow/performance and UI tests). +test-fast *args: mix test --exclude slow --exclude ui {{args}} +# Affected fast tests only (mix test --stale) with reduced property runs. +test-stale *args: + PROPERTY_RUNS=25 mix test --stale --exclude slow --exclude ui {{args}} + # Run only UI tests ui *args: install-dependencies mix test --only ui {{args}} diff --git a/config/test.exs b/config/test.exs index ef54982..7343a6a 100644 --- a/config/test.exs +++ b/config/test.exs @@ -62,3 +62,7 @@ config :mv, :join_rate_limit, scale_ms: 60_000, limit: 2 # Ash: silence "after_transaction hooks in surrounding transaction" warning when using # Ecto sandbox (tests run in a transaction; create_member after_transaction is expected). config :ash, warn_on_transaction_hooks?: false + +# StreamData property tests: generated cases per property, overridable via PROPERTY_RUNS +# (the `just check` recipe sets it low for speed; default 100 otherwise). +config :stream_data, max_runs: String.to_integer(System.get_env("PROPERTY_RUNS") || "100") diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 37f9552..98d4053 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -33,7 +33,7 @@ services: restart: unless-stopped db-prod: - image: postgres:18.3-alpine + image: postgres:18.4-alpine container_name: mv-prod-db environment: POSTGRES_USER: postgres diff --git a/docker-compose.yml b/docker-compose.yml index 01a0bd2..cbd2e9e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,7 +4,7 @@ networks: services: db: - image: postgres:18.3-alpine + image: postgres:18.4-alpine environment: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres @@ -25,7 +25,7 @@ services: rauthy: container_name: rauthy-dev - image: ghcr.io/sebadob/rauthy:0.35.1 + image: ghcr.io/sebadob/rauthy:0.35.2 environment: - LOCAL_TEST=true - SMTP_URL=mailcrab diff --git a/lib/membership/custom_field.ex b/lib/membership/custom_field.ex index ef6c79a..5f4dd0e 100644 --- a/lib/membership/custom_field.ex +++ b/lib/membership/custom_field.ex @@ -12,6 +12,8 @@ defmodule Mv.Membership.CustomField do - `slug` - URL-friendly, immutable identifier automatically generated from name (e.g., "phone-mobile") - `value_type` - Data type constraint (`:string`, `:integer`, `:boolean`, `:date`, `:email`). Immutable after creation. - `description` - Optional human-readable description + - `join_description` - Optional label shown for this field on the public join form + (e.g., a GDPR confirmation text); supports inline external links. Falls back to `name` when nil. - `required` - If true, all members must have this custom field (future feature) - `show_in_overview` - If true, this custom field will be displayed in the member overview table and can be sorted @@ -61,7 +63,14 @@ defmodule Mv.Membership.CustomField do end actions do - default_accept [:name, :value_type, :description, :required, :show_in_overview] + default_accept [ + :name, + :value_type, + :description, + :join_description, + :required, + :show_in_overview + ] read :read do primary? true @@ -69,13 +78,13 @@ defmodule Mv.Membership.CustomField do end create :create do - accept [:name, :value_type, :description, :required, :show_in_overview] + accept [:name, :value_type, :description, :join_description, :required, :show_in_overview] change Mv.Membership.Changes.GenerateSlug validate string_length(:slug, min: 1) end update :update do - accept [:name, :description, :required, :show_in_overview] + accept [:name, :description, :join_description, :required, :show_in_overview] require_atomic? false validate fn changeset, _context -> @@ -139,6 +148,15 @@ defmodule Mv.Membership.CustomField do trim?: true ] + attribute :join_description, :string, + allow_nil?: true, + public?: true, + description: "Label shown for this field on the public join form; supports external links", + constraints: [ + max_length: 1000, + trim?: true + ] + attribute :required, :boolean, default: false, allow_nil?: false diff --git a/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex b/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex index 5de15c8..8dae2d1 100644 --- a/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex +++ b/lib/membership/join_request/changes/filter_form_data_by_allowlist.ex @@ -17,16 +17,10 @@ defmodule Mv.Membership.JoinRequest.Changes.FilterFormDataByAllowlist do form_data = Ash.Changeset.get_attribute(changeset, :form_data) || %{} allowlist_ids = - case Membership.get_join_form_allowlist() do - list when is_list(list) -> - list - |> Enum.map(fn item -> item.id end) - |> MapSet.new() - |> MapSet.difference(MapSet.new(@typed_fields)) - - _ -> - MapSet.new() - end + Membership.get_join_form_allowlist() + |> Enum.map(fn item -> item.id end) + |> MapSet.new() + |> MapSet.difference(MapSet.new(@typed_fields)) filtered = form_data diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 85f5562..cddc23f 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -51,6 +51,9 @@ defmodule Mv.Membership.Member do require Logger + @typedoc "An `Mv.Membership.Member` resource record." + @type t :: %__MODULE__{} + # Module constants @member_search_limit 10 @@ -791,7 +794,7 @@ defmodule Mv.Membership.Member do # nil/[] when membership_fee_type is missing. @doc false - @spec get_current_cycle(Member.t()) :: MembershipFeeCycle.t() | nil + @spec get_current_cycle(t()) :: MembershipFeeCycle.t() | nil def get_current_cycle(member) do today = Date.utc_today() @@ -821,7 +824,7 @@ defmodule Mv.Membership.Member do end @doc false - @spec get_last_completed_cycle(Member.t()) :: MembershipFeeCycle.t() | nil + @spec get_last_completed_cycle(t()) :: MembershipFeeCycle.t() | nil def get_last_completed_cycle(member) do today = Date.utc_today() @@ -867,7 +870,7 @@ defmodule Mv.Membership.Member do end @doc false - @spec get_overdue_cycles(Member.t()) :: [MembershipFeeCycle.t()] + @spec get_overdue_cycles(t()) :: [MembershipFeeCycle.t()] def get_overdue_cycles(member) do today = Date.utc_today() @@ -939,7 +942,7 @@ defmodule Mv.Membership.Member do # Already in transaction: use advisory lock directly # Returns {:ok, notifications} - notifications should be returned to after_action hook defp regenerate_cycles_in_transaction(member, today, lock_key) do - EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + _ = EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) end @@ -947,7 +950,7 @@ defmodule Mv.Membership.Member do # Returns {:ok, notifications} - notifications should be sent by caller (e.g., via after_action) defp regenerate_cycles_new_transaction(member, today, lock_key) do Repo.transaction(fn -> - EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + _ = EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) do {:ok, notifications} -> @@ -1093,7 +1096,7 @@ defmodule Mv.Membership.Member do initiator: initiator ) do {:ok, cycles, notifications} -> - send_notifications_if_any(notifications) + _ = send_notifications_if_any(notifications) log_cycle_generation_success(member, cycles, notifications, sync: true, @@ -1112,7 +1115,7 @@ defmodule Mv.Membership.Member do initiator: initiator ) do {:ok, cycles, notifications} -> - send_notifications_if_any(notifications) + _ = send_notifications_if_any(notifications) log_cycle_generation_success(member, cycles, notifications, sync: false, @@ -1231,8 +1234,6 @@ defmodule Mv.Membership.Member do |> String.replace("_", "\\_") end - defp sanitize_search_query(_), do: "" - # ============================================================================ # Search Filter Builders # ============================================================================ diff --git a/lib/membership/member/changes/unrelate_user_when_argument_nil.ex b/lib/membership/member/changes/unrelate_user_when_argument_nil.ex index dc4d097..da8a291 100644 --- a/lib/membership/member/changes/unrelate_user_when_argument_nil.ex +++ b/lib/membership/member/changes/unrelate_user_when_argument_nil.ex @@ -37,9 +37,10 @@ defmodule Mv.Membership.Member.Changes.UnrelateUserWhenArgumentNil do {:ok, %{user: user}} when not is_nil(user) -> # User's :update action only accepts [:email]; use :update_user so # manage_relationship(:member, ..., on_missing: :unrelate) runs and clears member_id. - user - |> Ash.Changeset.for_update(:update_user, %{member: nil}, domain: Mv.Accounts) - |> Ash.update(domain: Mv.Accounts, actor: actor, authorize?: false) + _ = + user + |> Ash.Changeset.for_update(:update_user, %{member: nil}, domain: Mv.Accounts) + |> Ash.update(domain: Mv.Accounts, actor: actor, authorize?: false) changeset diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 7fa35dc..72be69b 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -836,7 +836,10 @@ defmodule Mv.Membership do - `{:ok, rejected_request}` - Rejected JoinRequest - `{:error, error}` - Status error or authorization error """ - @spec reject_join_request(String.t(), keyword()) :: {:ok, JoinRequest.t()} | {:error, term()} + @spec reject_join_request(String.t(), keyword()) :: + {:ok, JoinRequest.t()} + | {:ok, JoinRequest.t(), [Ash.Notifier.Notification.t()]} + | {:error, term()} def reject_join_request(id, opts \\ []) do actor = Keyword.get(opts, :actor) diff --git a/lib/membership_fees/changes/set_membership_fee_start_date.ex b/lib/membership_fees/changes/set_membership_fee_start_date.ex index 0e9cf00..8f5aa56 100644 --- a/lib/membership_fees/changes/set_membership_fee_start_date.ex +++ b/lib/membership_fees/changes/set_membership_fee_start_date.ex @@ -26,8 +26,6 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do """ use Ash.Resource.Change - require Logger - alias Mv.MembershipFees.CalendarCycles @impl true @@ -83,11 +81,6 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do field: :membership_fee_type_id, message: "not found" ) - - {:error, reason} -> - # Log warning for other unexpected errors - Logger.warning("Could not auto-set membership_fee_start_date: #{inspect(reason)}") - changeset end end diff --git a/lib/mv/authorization/permission_sets.ex b/lib/mv/authorization/permission_sets.ex index 3ffae93..ae84cdb 100644 --- a/lib/mv/authorization/permission_sets.ex +++ b/lib/mv/authorization/permission_sets.ex @@ -43,6 +43,7 @@ defmodule Mv.Authorization.PermissionSets do pattern matches and map lookups with no database queries or external calls. """ + @type permission_set_name :: :own_data | :read_only | :normal_user | :admin @type scope :: :own | :linked | :all @type action :: :read | :create | :update | :destroy @@ -88,7 +89,7 @@ defmodule Mv.Authorization.PermissionSets do iex> PermissionSets.all_permission_sets() [:own_data, :read_only, :normal_user, :admin] """ - @spec all_permission_sets() :: [atom()] + @spec all_permission_sets() :: [permission_set_name(), ...] def all_permission_sets do [:own_data, :read_only, :normal_user, :admin] end @@ -107,7 +108,7 @@ defmodule Mv.Authorization.PermissionSets do iex> PermissionSets.get_permissions(:invalid) ** (ArgumentError) invalid permission set: :invalid. Must be one of: [:own_data, :read_only, :normal_user, :admin] """ - @spec get_permissions(atom()) :: permission_set() + @spec get_permissions(permission_set_name()) :: permission_set() def get_permissions(set) when set not in [:own_data, :read_only, :normal_user, :admin] do raise ArgumentError, diff --git a/lib/mv/config.ex b/lib/mv/config.ex index 870d1d3..750a7db 100644 --- a/lib/mv/config.ex +++ b/lib/mv/config.ex @@ -207,8 +207,6 @@ defmodule Mv.Config do end end - defp derive_app_url_from_api_url(_), do: nil - @doc """ Returns true if Vereinfacht is fully configured (URL, API key, and club ID all set). """ @@ -251,7 +249,6 @@ defmodule Mv.Config do case System.get_env(key) do nil -> false v when is_binary(v) -> String.trim(v) != "" - _ -> false end end @@ -270,9 +267,6 @@ defmodule Mv.Config do value when is_binary(value) -> v = String.trim(value) |> String.downcase() v in ["true", "1", "yes"] - - _ -> - false end end @@ -328,7 +322,6 @@ defmodule Mv.Config do defp present?(nil), do: false defp present?(s) when is_binary(s), do: String.trim(s) != "" - defp present?(_), do: false # --------------------------------------------------------------------------- # OIDC authentication @@ -409,7 +402,7 @@ defmodule Mv.Config do @doc """ Returns the OIDC groups claim name (default "groups"). ENV first, then Settings. """ - @spec oidc_groups_claim() :: String.t() | nil + @spec oidc_groups_claim() :: String.t() def oidc_groups_claim do case env_or_setting("OIDC_GROUPS_CLAIM", :oidc_groups_claim) do nil -> "groups" @@ -492,7 +485,7 @@ defmodule Mv.Config do - ENV-only mode (`SMTP_HOST` set): read from ENV `SMTP_PORT` - Settings mode: read from Settings only """ - @spec smtp_port() :: non_neg_integer() | nil + @spec smtp_port() :: pos_integer() | nil def smtp_port do if smtp_env_mode?() do parse_smtp_port_env(System.get_env("SMTP_PORT")) @@ -638,9 +631,15 @@ defmodule Mv.Config do """ @spec mail_from_name() :: String.t() def mail_from_name do - case System.get_env("MAIL_FROM_NAME") do - nil -> get_from_settings(:smtp_from_name) || "Mila" - value -> trim_nil(value) || "Mila" + name = + case System.get_env("MAIL_FROM_NAME") do + nil -> get_from_settings(:smtp_from_name) + value -> trim_nil(value) + end + + case name do + nil -> "Mila" + name -> name end end diff --git a/lib/mv/constants.ex b/lib/mv/constants.ex index 4d09c89..657aa9b 100644 --- a/lib/mv/constants.ex +++ b/lib/mv/constants.ex @@ -40,6 +40,8 @@ defmodule Mv.Constants do @max_boolean_filters 50 + @max_mailto_bulk_recipients 50 + @max_uuid_length 36 @email_validator_checks [:html_input, :pow] @@ -173,6 +175,21 @@ defmodule Mv.Constants do """ def max_boolean_filters, do: @max_boolean_filters + @doc """ + Returns the maximum number of mailto recipients before the bulk "open in email + program" action is disabled. + + The mailto link carries every recipient in its BCC; browsers cannot reliably + hand a too-long mailto URI to the mail program. At or above this count the + action is disabled in the UI (Copy and Export have no such limit). + + ## Examples + + iex> Mv.Constants.max_mailto_bulk_recipients() + 50 + """ + def max_mailto_bulk_recipients, do: @max_mailto_bulk_recipients + @doc """ Returns the maximum length of a UUID string (36 characters including hyphens). diff --git a/lib/mv/helpers/system_actor.ex b/lib/mv/helpers/system_actor.ex index 8cd93d2..7b86a3c 100644 --- a/lib/mv/helpers/system_actor.ex +++ b/lib/mv/helpers/system_actor.ex @@ -225,7 +225,10 @@ defmodule Mv.Helpers.SystemActor do # This allows configuration via SYSTEM_ACTOR_EMAIL env var @spec system_user_email_config() :: String.t() defp system_user_email_config do - System.get_env("SYSTEM_ACTOR_EMAIL") || "system@mila.local" + case System.get_env("SYSTEM_ACTOR_EMAIL") do + nil -> "system@mila.local" + email -> email + end end # Loads the system actor from the database @@ -257,7 +260,7 @@ defmodule Mv.Helpers.SystemActor do end # Handles database error when loading system user - @spec handle_system_user_error(term()) :: Mv.Accounts.User.t() | no_return() + @spec handle_system_user_error({:error, Ash.Error.t()}) :: Mv.Accounts.User.t() | no_return() defp handle_system_user_error(error) do case load_admin_user_fallback() do {:ok, admin_user} -> @@ -393,15 +396,18 @@ defmodule Mv.Helpers.SystemActor do # 1. Only creates system user with known email # 2. Only called during system actor initialization (bootstrap) # 3. Once created, all subsequent operations use proper authorization - Accounts.create_user!(%{email: system_user_email_config()}, - upsert?: true, - upsert_identity: :unique_email, - authorize?: false - ) - |> Ash.Changeset.for_update(:update_internal, %{}) - |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) - |> Ash.update!(authorize?: false) - |> Ash.load!(:role, domain: Mv.Accounts, authorize?: false) + user = + Accounts.create_user!(%{email: system_user_email_config()}, + upsert?: true, + upsert_identity: :unique_email, + authorize?: false + ) + |> Ash.Changeset.for_update(:update_internal, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + |> Ash.load!(:role, domain: Mv.Accounts, authorize?: false) + + %Accounts.User{} = user end # Finds a user by email address diff --git a/lib/mv/mailer.ex b/lib/mv/mailer.ex index ec8f357..1e55b6e 100644 --- a/lib/mv/mailer.ex +++ b/lib/mv/mailer.ex @@ -190,6 +190,4 @@ defmodule Mv.Mailer do defp valid_email?(email) when is_binary(email) do Regex.match?(@email_regex, String.trim(email)) end - - defp valid_email?(_), do: false end diff --git a/lib/mv/membership/custom_field_value_formatter.ex b/lib/mv/membership/custom_field_value_formatter.ex index 9709353..9ba9c42 100644 --- a/lib/mv/membership/custom_field_value_formatter.ex +++ b/lib/mv/membership/custom_field_value_formatter.ex @@ -4,13 +4,13 @@ defmodule Mv.Membership.CustomFieldValueFormatter do Same logic as the member overview Formatter but without Gettext or web helpers, so it can be used from the Membership context. For boolean: "Yes"/"No"; - for date: European format (dd.mm.yyyy). + for date: ISO-8601 (YYYY-MM-DD) so exported values can be re-imported. """ @doc """ Formats a custom field value for plain text (e.g. CSV). Handles nil, Ash.Union, JSONB map, and direct values. Uses custom_field.value_type - for typing. Boolean -> "Yes"/"No", Date -> dd.mm.yyyy. + for typing. Boolean -> "Yes"/"No", Date -> ISO-8601 (YYYY-MM-DD). """ def format_custom_field_value(nil, _custom_field), do: "" @@ -18,6 +18,10 @@ defmodule Mv.Membership.CustomFieldValueFormatter do format_value_by_type(value, type, custom_field) end + def format_custom_field_value(%Date{} = value, custom_field) do + format_value_by_type(value, :date, custom_field) + end + def format_custom_field_value(value, custom_field) when is_map(value) do type = Map.get(value, "type") || Map.get(value, "_union_type") val = Map.get(value, "value") || Map.get(value, "_union_value") @@ -41,12 +45,12 @@ defmodule Mv.Membership.CustomFieldValueFormatter do defp format_value_by_type(value, :boolean, _), do: to_string(value) defp format_value_by_type(%Date{} = date, :date, _) do - Calendar.strftime(date, "%d.%m.%Y") + Date.to_iso8601(date) end defp format_value_by_type(value, :date, _) when is_binary(value) do case Date.from_iso8601(value) do - {:ok, date} -> Calendar.strftime(date, "%d.%m.%Y") + {:ok, date} -> Date.to_iso8601(date) _ -> value end end diff --git a/lib/mv/membership/import/column_resolver.ex b/lib/mv/membership/import/column_resolver.ex new file mode 100644 index 0000000..2edb540 --- /dev/null +++ b/lib/mv/membership/import/column_resolver.ex @@ -0,0 +1,258 @@ +defmodule Mv.Membership.Import.ColumnResolver do + @moduledoc """ + Read-only resolution of CSV import columns against the database. + + Given the `HeaderMapper.build_maps/2` result, the raw numbered rows, and an + actor, `resolve/3` determines: + + - which group names in the groups column already exist (`groups_found`) and + which would have to be created (`groups_to_create`); + - a small set of preview rows for the mapping preview UI. + + No database writes happen here; the resolver only reads. Group creation and + member-group assignment happen during processing via `create_or_find_group/3`. + + This module has no Phoenix or web dependencies. + """ + + require Logger + + alias Mv.Membership.Import.HeaderMapper + + @preview_row_limit 3 + + @type numbered_row :: {pos_integer(), [String.t()]} + + @type resolution :: %{ + groups_found: [%{id: String.t(), name: String.t()}], + groups_to_create: [String.t()], + fee_type_map: %{String.t() => String.t()}, + fee_type_warnings: [String.t()], + has_empty_fee_type_cells?: boolean(), + preview_rows: [[String.t()]] + } + + @doc """ + Resolves the group and fee-type columns of an import against the database and + extracts preview rows. + + Returns a map with `:groups_found`, `:groups_to_create`, `:fee_type_map`, + `:fee_type_warnings`, `:has_empty_fee_type_cells?`, and `:preview_rows`. + """ + @spec resolve(map(), [numbered_row()], term()) :: resolution() + def resolve(header_maps, rows, actor) do + %{ + groups_found: groups_found, + groups_to_create: groups_to_create + } = resolve_groups(header_maps, rows, actor) + + %{ + fee_type_map: fee_type_map, + fee_type_warnings: fee_type_warnings, + has_empty_fee_type_cells?: has_empty_fee_type_cells? + } = resolve_fee_types(header_maps, rows, actor) + + %{ + groups_found: groups_found, + groups_to_create: groups_to_create, + fee_type_map: fee_type_map, + fee_type_warnings: fee_type_warnings, + has_empty_fee_type_cells?: has_empty_fee_type_cells?, + preview_rows: preview_rows(rows) + } + end + + defp resolve_groups(%{groups_column_index: nil}, _rows, _actor) do + %{groups_found: [], groups_to_create: []} + end + + defp resolve_groups(%{groups_column_index: index}, rows, actor) do + existing_groups = list_groups(actor) + lookup = build_group_lookup(existing_groups) + + names = unique_group_names(rows, index) + + {found, to_create} = + Enum.reduce(names, {[], []}, fn name, {found, to_create} -> + case Map.get(lookup, normalize_name(name)) do + nil -> {found, [name | to_create]} + group -> {[%{id: group.id, name: group.name} | found], to_create} + end + end) + + %{groups_found: Enum.reverse(found), groups_to_create: Enum.reverse(to_create)} + end + + defp resolve_fee_types(%{fee_type_column_index: nil}, _rows, _actor) do + %{fee_type_map: %{}, fee_type_warnings: [], has_empty_fee_type_cells?: false} + end + + defp resolve_fee_types(%{fee_type_column_index: index}, rows, actor) do + lookup = build_fee_type_lookup(actor) + + cells = Enum.map(rows, fn {_line, values} -> Enum.at(values, index) end) + + has_empty? = Enum.any?(cells, &blank?/1) + + {fee_type_map, warnings} = + cells + |> Enum.reject(&blank?/1) + |> Enum.uniq_by(&normalize_fee_type_name/1) + |> Enum.reduce({%{}, []}, fn name, {map, warnings} -> + case Map.get(lookup, normalize_fee_type_name(name)) do + nil -> {map, [String.trim(name) | warnings]} + id -> {Map.put(map, normalize_fee_type_name(name), id), warnings} + end + end) + + %{ + fee_type_map: fee_type_map, + fee_type_warnings: Enum.reverse(warnings), + has_empty_fee_type_cells?: has_empty? + } + end + + @doc """ + Normalizes a fee-type name using the same rules as CSV header normalization + (trim, lowercase, transliterate, drop hyphens and whitespace). + """ + @spec normalize_fee_type_name(String.t() | nil) :: String.t() + def normalize_fee_type_name(name) when is_binary(name), do: HeaderMapper.normalize_header(name) + def normalize_fee_type_name(_), do: "" + + defp build_fee_type_lookup(actor) do + actor + |> list_fee_types() + |> Enum.reduce(%{}, fn fee_type, acc -> + normalized = normalize_fee_type_name(fee_type.name) + + if Map.has_key?(acc, normalized) do + Logger.warning( + "Multiple membership fee types normalize to #{inspect(normalized)}; using the first match for CSV import." + ) + + acc + else + Map.put(acc, normalized, fee_type.id) + end + end) + end + + defp list_fee_types(actor) do + Mv.MembershipFees.list_membership_fee_types!(actor: actor) + end + + defp blank?(nil), do: true + defp blank?(value) when is_binary(value), do: String.trim(value) == "" + defp blank?(_), do: false + + @doc """ + Finds an existing group by name (case-insensitive) or creates it. + + Looks first in the pre-fetched `groups` list, then in the database (to catch + groups created earlier in the same import), and only creates a new group when + none is found. This keeps group resolution idempotent across re-imports. + """ + @spec create_or_find_group(String.t(), [Mv.Membership.Group.t()], term()) :: + {:ok, Mv.Membership.Group.t()} | {:error, term()} + def create_or_find_group(name, groups, actor) when is_binary(name) do + trimmed = String.trim(name) + normalized = normalize_name(trimmed) + + case find_group_in_list(groups, normalized) do + nil -> find_or_create_group(trimmed, normalized, actor) + group -> {:ok, group} + end + end + + defp find_group_in_list(groups, normalized) do + Enum.find(groups, fn group -> normalize_name(group.name) == normalized end) + end + + defp find_or_create_group(trimmed, normalized, actor) do + case fetch_group_by_normalized_name(normalized, actor) do + nil -> create_group(trimmed, normalized, actor) + group -> {:ok, group} + end + end + + # Normalizes the Ash code-interface return to a two-shape result. + # + # On a create failure the group may have been created concurrently by another + # import session between our read and our write (the DB unique index is the + # final arbiter, and the name validation is fail-open). Re-fetch by normalized + # name and link to the existing group rather than failing the row. + defp create_group(name, normalized, actor) do + case Mv.Membership.create_group(%{name: name}, actor: actor) do + {:ok, %Mv.Membership.Group{} = group} -> + {:ok, group} + + {:error, reason} -> + case fetch_group_by_normalized_name(normalized, actor) do + nil -> {:error, reason} + group -> {:ok, group} + end + end + end + + # Fetches a single group by case-insensitive name using a name-filtered query + # rather than reading the whole groups table. `normalized` is the trimmed, + # lower-cased name; the DB comparison uses LOWER(name) consistent with the + # Group resource's case-insensitive uniqueness constraint. + defp fetch_group_by_normalized_name(normalized, actor) do + require Ash.Query + + Mv.Membership.Group + |> Ash.Query.filter(fragment("LOWER(?) = ?", name, ^normalized)) + |> Ash.read(actor: actor, domain: Mv.Membership) + |> case do + {:ok, [group | _]} -> group + _ -> nil + end + end + + @doc """ + Splits a raw groups-cell value into trimmed, non-empty group names. + """ + @spec split_group_names(String.t() | nil) :: [String.t()] + def split_group_names(nil), do: [] + + def split_group_names(cell) when is_binary(cell) do + cell + |> String.split(",") + |> Enum.map(&String.trim/1) + |> Enum.reject(&(&1 == "")) + end + + defp unique_group_names(rows, index) do + rows + |> Enum.flat_map(fn {_line, values} -> + values + |> Enum.at(index) + |> split_group_names() + end) + |> Enum.uniq_by(&normalize_name/1) + end + + defp preview_rows(rows) do + rows + |> Enum.take(@preview_row_limit) + |> Enum.map(fn {_line, values} -> values end) + end + + defp list_groups(actor) do + Mv.Membership.list_groups!(actor: actor) + end + + defp build_group_lookup(groups) do + Enum.reduce(groups, %{}, fn group, acc -> + Map.put(acc, normalize_name(group.name), group) + end) + end + + # Case-insensitive comparison consistent with the Group resource's + # case-insensitive name uniqueness. + defp normalize_name(name) when is_binary(name) do + name |> String.trim() |> String.downcase() + end +end diff --git a/lib/mv/membership/import/csv_parser.ex b/lib/mv/membership/import/csv_parser.ex index 2de75ee..142450f 100644 --- a/lib/mv/membership/import/csv_parser.ex +++ b/lib/mv/membership/import/csv_parser.ex @@ -100,7 +100,8 @@ defmodule Mv.Membership.Import.CsvParser do |> String.replace("\r", "\n") end - @spec get_parser(String.t()) :: module() + @spec get_parser(String.t()) :: + Mv.Membership.Import.CsvParserSemicolon | Mv.Membership.Import.CsvParserComma defp get_parser(";"), do: Mv.Membership.Import.CsvParserSemicolon defp get_parser(","), do: Mv.Membership.Import.CsvParserComma defp get_parser(_), do: Mv.Membership.Import.CsvParserSemicolon @@ -116,7 +117,10 @@ defmodule Mv.Membership.Import.CsvParser do if semicolon_score >= comma_score, do: ";", else: "," end - @spec header_field_count(module(), binary()) :: non_neg_integer() + @spec header_field_count( + Mv.Membership.Import.CsvParserSemicolon | Mv.Membership.Import.CsvParserComma, + binary() + ) :: non_neg_integer() defp header_field_count(parser, header_record) do case parse_single_record(parser, header_record, nil) do {:ok, fields} -> Enum.count(fields, &(String.trim(&1) != "")) diff --git a/lib/mv/membership/import/header_mapper.ex b/lib/mv/membership/import/header_mapper.ex index d96d96e..be90ca6 100644 --- a/lib/mv/membership/import/header_mapper.ex +++ b/lib/mv/membership/import/header_mapper.ex @@ -29,12 +29,21 @@ defmodule Mv.Membership.Import.HeaderMapper do Supports English and German header variants (e.g. "Email" / "E-Mail", "Join Date" / "Beitrittsdatum"). + ## Special columns + + - **groups** – Many-to-many relationship (through member_groups). Recognized via the + `groups_column_index` key (headers `Groups`, `Gruppen`, `Gruppe`). Comma-separated + names are resolved during processing; missing groups are auto-created. + - **membership_fee_type** – Recognized via the `fee_type_column_index` key (headers + `Fee Type`, `fee_type`, `membership_fee_type`, `Beitragsart`). Names are matched to + existing fee types; unknown names fall back to the default fee type. + ## Fields not supported for import - **membership_fee_status** – Computed (calculation from membership fee cycles). Not stored; - cannot be set via CSV. Export can include it. - - **groups** – Many-to-many relationship (through member_groups). Import would require - resolving group names/slugs to IDs and creating associations; not in current import scope. + cannot be set via CSV. Export can include it. Fee-status header variants + (`Membership Fee Status`, `Bezahlstatus`, `Mitgliedsbeitragsstatus`) are explicitly + placed in the `ignored` list and never mapped. ## Custom Field Detection @@ -47,10 +56,10 @@ defmodule Mv.Membership.Import.HeaderMapper do "e-mail" iex> HeaderMapper.build_maps(["Email", "First Name"], []) - {:ok, %{member: %{email: 0, first_name: 1}, custom: %{}, unknown: []}} + {:ok, %{member: %{email: 0, first_name: 1}, custom: %{}, unknown: [], ignored: [], groups_column_index: nil, fee_type_column_index: nil}} iex> HeaderMapper.build_maps(["Email", "CustomField"], [%{id: "cf1", name: "CustomField"}]) - {:ok, %{member: %{email: 0}, custom: %{"cf1" => 1}, unknown: []}} + {:ok, %{member: %{email: 0}, custom: %{"cf1" => 1}, unknown: [], ignored: [], groups_column_index: nil, fee_type_column_index: nil}} """ @type column_map :: %{atom() => non_neg_integer()} @@ -60,6 +69,33 @@ defmodule Mv.Membership.Import.HeaderMapper do # Required member fields @required_member_fields [:email] + # Fee-status header variants that must never be imported (computed/read-only field). + # Stored already-normalized; checked before member, custom, groups, and fee-type mapping. + # Maintain this list when new locale translations for fee-status are added. + @ignored_normalized [ + "membershipfeestatus", + "mitgliedsbeitragsstatus", + "bezahlstatus", + # DE export label for membership_fee_start_date — system-managed, not importable + "startdatummitgliedsbeitrag" + ] + + # Normalized header variants for the groups column. The column is resolved to + # group associations during import; it is never a member or custom field. + @groups_column_normalized [ + "groups", + "gruppen", + "gruppe" + ] + + # Normalized header variants for the membership fee-type column. The column is + # resolved to a MembershipFeeType during import; it is never a member or custom field. + @fee_type_column_normalized [ + "membershipfeetype", + "feetype", + "beitragsart" + ] + # Canonical member fields with their raw variants # These will be normalized at runtime when building the lookup map @member_field_variants_raw %{ @@ -239,30 +275,79 @@ defmodule Mv.Membership.Import.HeaderMapper do ## Returns - - `{:ok, %{member: column_map, custom: custom_field_map, unknown: unknown_headers}}` on success + - `{:ok, %{member: column_map, custom: custom_field_map, unknown: unknown_headers, + ignored: [non_neg_integer], groups_column_index: non_neg_integer | nil, + fee_type_column_index: non_neg_integer | nil}}` on success - `{:error, reason}` on error (missing required field, duplicate headers) + The `ignored` list holds the indices of fee-status columns (computed/read-only), + which are never mapped to member or custom fields. + ## Examples iex> build_maps(["Email", "First Name"], []) - {:ok, %{member: %{email: 0, first_name: 1}, custom: %{}, unknown: []}} + {:ok, %{member: %{email: 0, first_name: 1}, custom: %{}, unknown: [], ignored: [], groups_column_index: nil, fee_type_column_index: nil}} iex> build_maps(["Email", "CustomField"], [%{id: "cf1", name: "CustomField"}]) - {:ok, %{member: %{email: 0}, custom: %{"cf1" => 1}, unknown: []}} + {:ok, %{member: %{email: 0}, custom: %{"cf1" => 1}, unknown: [], ignored: [], groups_column_index: nil, fee_type_column_index: nil}} """ @spec build_maps([String.t()], [map()]) :: - {:ok, %{member: column_map(), custom: custom_field_map(), unknown: unknown_headers()}} + {:ok, + %{ + member: column_map(), + custom: custom_field_map(), + unknown: unknown_headers(), + ignored: [non_neg_integer()], + groups_column_index: non_neg_integer() | nil, + fee_type_column_index: non_neg_integer() | nil + }} | {:error, String.t()} def build_maps(headers, custom_fields) when is_list(headers) and is_list(custom_fields) do - with {:ok, member_map, unknown_after_member} <- build_member_map(headers), + ignored = ignored_indices(headers) + groups_column_index = first_matching_index(headers, @groups_column_normalized) + fee_type_column_index = first_matching_index(headers, @fee_type_column_normalized) + + reserved = + [groups_column_index, fee_type_column_index | ignored] + |> Enum.reject(&is_nil/1) + |> MapSet.new() + + with {:ok, member_map, unknown_after_member} <- build_member_map(headers, reserved), {:ok, custom_map, unknown_after_custom} <- build_custom_field_map(headers, unknown_after_member, custom_fields, member_map) do unknown = Enum.map(unknown_after_custom, &Enum.at(headers, &1)) - {:ok, %{member: member_map, custom: custom_map, unknown: unknown}} + + {:ok, + %{ + member: member_map, + custom: custom_map, + unknown: unknown, + ignored: ignored, + groups_column_index: groups_column_index, + fee_type_column_index: fee_type_column_index + }} end end + # Returns the index of the first header whose normalized form is in `variants`, + # or nil if none match. + defp first_matching_index(headers, variants) do + headers + |> Enum.with_index() + |> Enum.find_value(fn {header, index} -> + if normalize_header(header) in variants, do: index + end) + end + + # Returns the column indices whose normalized header is in the fee-status ignore list. + defp ignored_indices(headers) do + headers + |> Enum.with_index() + |> Enum.filter(fn {header, _index} -> normalize_header(header) in @ignored_normalized end) + |> Enum.map(fn {_header, index} -> index end) + end + # --- Private Functions --- # Transliterates German umlauts and special characters @@ -304,13 +389,14 @@ defmodule Mv.Membership.Import.HeaderMapper do |> String.replace(" ", "") end - # Builds member field column map - defp build_member_map(headers) do + # Builds member field column map, skipping reserved (e.g. ignored) indices. + defp build_member_map(headers, reserved) do result = headers |> Enum.with_index() |> Enum.reduce_while({%{}, []}, fn {header, index}, {acc_map, acc_unknown} -> - normalized = normalize_header(header) + normalized = + if MapSet.member?(reserved, index), do: "", else: normalize_header(header) case process_member_header(header, index, normalized, acc_map, %{}) do {:error, reason} -> diff --git a/lib/mv/membership/import/import_runner.ex b/lib/mv/membership/import/import_runner.ex index eccd75f..28893a3 100644 --- a/lib/mv/membership/import/import_runner.ex +++ b/lib/mv/membership/import/import_runner.ex @@ -26,14 +26,8 @@ defmodule Mv.Membership.Import.ImportRunner do {:ok, content} -> {:ok, content} - {:error, reason} when is_atom(reason) -> - {:error, :file.format_error(reason)} - - {:error, %File.Error{reason: reason}} -> - {:error, :file.format_error(reason)} - {:error, reason} -> - {:error, Exception.message(reason)} + {:error, to_string(:file.format_error(reason))} end end @@ -86,7 +80,7 @@ defmodule Mv.Membership.Import.ImportRunner do all_errors = progress.errors ++ chunk_result.errors new_errors = Enum.take(all_errors, max_errors) errors_truncated? = length(all_errors) > max_errors - new_warnings = progress.warnings ++ Map.get(chunk_result, :warnings, []) + new_warnings = Enum.uniq(progress.warnings ++ Map.get(chunk_result, :warnings, [])) chunks_processed = current_chunk_idx + 1 new_status = if chunks_processed >= progress.total_chunks, do: :done, else: :running @@ -103,6 +97,20 @@ defmodule Mv.Membership.Import.ImportRunner do } end + @doc """ + Carries the in-memory group snapshot grown by a chunk back into `import_state` + so the next chunk reuses groups created earlier instead of re-reading the + Group table. When the chunk result omits `groups_found`, the state is returned + unchanged. + """ + @spec carry_groups_forward(map(), map()) :: map() + def carry_groups_forward(import_state, chunk_result) do + case Map.fetch(chunk_result, :groups_found) do + {:ok, groups_found} -> Map.put(import_state, :groups_found, groups_found) + :error -> import_state + end + end + @doc """ Returns the next action after processing a chunk: send the next chunk index or done. """ diff --git a/lib/mv/membership/import/member_csv.ex b/lib/mv/membership/import/member_csv.ex index 23e0d93..31dea59 100644 --- a/lib/mv/membership/import/member_csv.ex +++ b/lib/mv/membership/import/member_csv.ex @@ -6,7 +6,7 @@ defmodule Mv.Membership.Import.MemberCSV do This module provides the core API for CSV member import functionality: - `prepare/2` - Parses and validates CSV content, returns import state - - `process_chunk/3` - Processes a chunk of rows and creates members + - `process_chunk/4` - Processes a chunk of rows and creates members ## Error Handling @@ -22,13 +22,24 @@ defmodule Mv.Membership.Import.MemberCSV do - `column_map` - Map of canonical field names to column indices - `custom_field_map` - Map of custom field names to column indices - `warnings` - List of warning messages (e.g., unknown custom field columns) + - `headers` - The raw CSV header row + - `ignored` - Header names of ignored (fee-status) columns + - `groups_column_index` / `fee_type_column_index` - Indices for resolved columns (or nil) + - `groups_found` / `groups_to_create` - Existing and to-be-created groups from the preview + - `fee_type_map` - Normalized fee-type name to id, for matched fee types + - `fee_type_warnings` - Unmatched fee-type names surfaced in the preview + - `has_empty_fee_type_cells?` - Whether any fee-type cell is blank (default applies) + - `preview_rows` - Up to 3 sample data rows for the mapping preview ## Chunk Results - The `chunk_result` returned by `process_chunk/3` contains: + The `chunk_result` returned by `process_chunk/4` contains: - `inserted` - Number of successfully created members - `failed` - Number of failed member creations - `errors` - List of `%MemberCSV.Error{}` structs (capped at 50 per import) + - `groups_found` - The in-memory group snapshot grown while processing this + chunk; thread it into the next chunk's `:groups_found` opt so groups created + in an earlier chunk are reused without re-reading the Group table ## Examples @@ -37,7 +48,9 @@ defmodule Mv.Membership.Import.MemberCSV do # Process first chunk chunk = Enum.at(import_state.chunks, 0) - {:ok, result} = MemberCSV.process_chunk(chunk, import_state.column_map) + + {:ok, result} = + MemberCSV.process_chunk(chunk, import_state.column_map, import_state.custom_field_map, []) """ defmodule Error do @@ -66,16 +79,29 @@ defmodule Mv.Membership.Import.MemberCSV do custom_field_lookup: %{ String.t() => %{id: String.t(), value_type: atom(), name: String.t()} }, - warnings: list(String.t()) + warnings: list(String.t()), + headers: list(String.t()), + ignored: list(String.t()), + groups_column_index: non_neg_integer() | nil, + fee_type_column_index: non_neg_integer() | nil, + groups_found: list(%{id: String.t(), name: String.t()}), + groups_to_create: list(String.t()), + fee_type_map: %{String.t() => String.t()}, + fee_type_warnings: list(String.t()), + has_empty_fee_type_cells?: boolean(), + preview_rows: list(list(String.t())) } @type chunk_result :: %{ inserted: non_neg_integer(), failed: non_neg_integer(), errors: list(Error.t()), - errors_truncated?: boolean() + errors_truncated?: boolean(), + warnings: list(String.t()), + groups_found: list(Mv.Membership.Group.t() | %{id: String.t(), name: String.t()}) } + alias Mv.Membership.Import.ColumnResolver alias Mv.Membership.Import.CsvParser alias Mv.Membership.Import.HeaderMapper @@ -139,13 +165,27 @@ defmodule Mv.Membership.Import.MemberCSV do # Build custom field lookup for efficient value processing custom_field_lookup = build_custom_field_lookup(custom_fields) + # Resolve DB-backed columns (groups, fee types) read-only for the preview. + resolution = ColumnResolver.resolve(maps, rows, actor) + ignored_headers = Enum.map(maps.ignored, &Enum.at(headers, &1)) + {:ok, %{ chunks: chunks, column_map: maps.member, custom_field_map: maps.custom, custom_field_lookup: custom_field_lookup, - warnings: warnings + warnings: warnings, + headers: headers, + ignored: ignored_headers, + groups_column_index: maps.groups_column_index, + fee_type_column_index: maps.fee_type_column_index, + groups_found: resolution.groups_found, + groups_to_create: resolution.groups_to_create, + fee_type_map: resolution.fee_type_map, + fee_type_warnings: resolution.fee_type_warnings, + has_empty_fee_type_cells?: resolution.has_empty_fee_type_cells?, + preview_rows: resolution.preview_rows }} end end @@ -180,7 +220,7 @@ defmodule Mv.Membership.Import.MemberCSV do end) case HeaderMapper.build_maps(headers, custom_field_maps) do - {:ok, %{member: member_map, custom: custom_map, unknown: unknown}} -> + {:ok, %{unknown: unknown} = maps} -> # Build warnings for unknown custom field columns warnings = unknown @@ -197,7 +237,7 @@ defmodule Mv.Membership.Import.MemberCSV do ) end) - {:ok, %{member: member_map, custom: custom_map}, warnings} + {:ok, maps, warnings} {:error, reason} -> {:error, reason} @@ -210,8 +250,6 @@ defmodule Mv.Membership.Import.MemberCSV do MapSet.member?(HeaderMapper.known_member_fields(), normalized) end - defp member_field?(_), do: false - # Validates that row count doesn't exceed limit defp validate_row_count(rows, max_rows) do if length(rows) > max_rows do @@ -252,9 +290,20 @@ defmodule Mv.Membership.Import.MemberCSV do Map.put(acc, custom_field_id, value) end) - %{member: member_map, custom: custom_map} + %{ + member: member_map, + custom: custom_map, + fee_type: cell_at(row_tuple, tuple_size, maps.fee_type_column_index), + groups: cell_at(row_tuple, tuple_size, maps.groups_column_index) + } end + # Returns the raw cell at the given index, or nil if the column is absent. + defp cell_at(_row_tuple, _size, nil), do: nil + + defp cell_at(row_tuple, size, index) when index < size, do: elem(row_tuple, index) + defp cell_at(_row_tuple, _size, _index), do: "" + @doc """ Processes a chunk of CSV rows and creates members. @@ -270,12 +319,18 @@ defmodule Mv.Membership.Import.MemberCSV do - `chunk_rows_with_lines` - List of tuples `{csv_line_number, row_map}` where: - `csv_line_number` - Physical line number in CSV (1-based) - `row_map` - Map with `:member` and `:custom` keys containing field values - - `column_map` - Map of canonical field names (atoms) to column indices (for reference) - - `custom_field_map` - Map of custom field IDs (strings) to column indices (for reference) + - `column_map` - Unused; kept for backward-compatible call sites. Field values are + read from each row's pre-built `:member`/`:custom` maps, not from this argument. + - `custom_field_map` - Unused; kept for backward-compatible call sites (see above). - `opts` - Optional keyword list for processing options: - `:custom_field_lookup` - Map of custom field IDs to metadata (default: `%{}`) - `:existing_error_count` - Number of errors already collected in previous chunks (default: `0`) - `:max_errors` - Maximum number of errors to collect per import overall (default: `50`) + - `:actor` - Actor used for all writes (default: the system actor) + - `:fee_type_map` - Map of normalized fee-type name to fee-type id, used to resolve + each row's fee-type cell (default: `%{}`) + - `:groups_found` - List of pre-fetched `Group` structs seeding in-memory group + resolution; the snapshot grows as groups are auto-created (default: `[]`) ## Error Capping @@ -314,27 +369,49 @@ defmodule Mv.Membership.Import.MemberCSV do existing_error_count = Keyword.get(opts, :existing_error_count, 0) max_errors = Keyword.get(opts, :max_errors, @default_max_errors) actor = Keyword.get(opts, :actor, SystemActor.get_system_actor()) + fee_type_map = Keyword.get(opts, :fee_type_map, %{}) + groups_found = Keyword.get(opts, :groups_found, []) - {inserted, failed, errors, _collected_error_count, truncated?} = - Enum.reduce(chunk_rows_with_lines, {0, 0, [], 0, false}, fn {line_number, row_map}, - {acc_inserted, acc_failed, - acc_errors, acc_error_count, - acc_truncated?} -> + base_row_opts = %{ + custom_field_lookup: custom_field_lookup, + fee_type_map: fee_type_map, + actor: actor + } + + {inserted, failed, errors, _collected_error_count, truncated?, warnings, groups_acc} = + Enum.reduce(chunk_rows_with_lines, {0, 0, [], 0, false, [], groups_found}, fn {line_number, + row_map}, + {acc_inserted, + acc_failed, + acc_errors, + acc_error_count, + acc_truncated?, + acc_warnings, + acc_groups} -> current_error_count = existing_error_count + acc_error_count + row_opts = Map.put(base_row_opts, :groups_found, acc_groups) - case process_row(row_map, line_number, custom_field_lookup, actor) do - {:ok, _member} -> - update_inserted( - {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?} - ) + case process_row(row_map, line_number, row_opts) do + {:ok, _member, row_warnings, new_groups} -> + {new_inserted, new_failed, new_errors, new_error_count, new_truncated?} = + update_inserted( + {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?} + ) - {:error, error} -> - handle_row_error( - {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}, - error, - current_error_count, - max_errors - ) + {new_inserted, new_failed, new_errors, new_error_count, new_truncated?, + acc_warnings ++ row_warnings, new_groups} + + {:error, error, new_groups} -> + {new_inserted, new_failed, new_errors, new_error_count, new_truncated?} = + handle_row_error( + {acc_inserted, acc_failed, acc_errors, acc_error_count, acc_truncated?}, + error, + current_error_count, + max_errors + ) + + {new_inserted, new_failed, new_errors, new_error_count, new_truncated?, acc_warnings, + new_groups} end end) @@ -343,7 +420,9 @@ defmodule Mv.Membership.Import.MemberCSV do inserted: inserted, failed: failed, errors: Enum.reverse(errors), - errors_truncated?: truncated? + errors_truncated?: truncated?, + warnings: warnings, + groups_found: groups_acc }} end @@ -507,18 +586,27 @@ defmodule Mv.Membership.Import.MemberCSV do defp gettext_error_message(_), do: gettext("Email is invalid.") - # Processes a single row and creates member with custom field values + # Processes a single row and creates member with custom field values. + # On success returns {:ok, member, warnings, groups}; warnings carry non-fatal + # notices such as an unresolved fee-type name. The returned groups list is the + # accumulated in-memory group snapshot (seeded from the chunk, grown with any + # group created while linking this row) so later rows reuse it instead of + # re-reading the whole Group table per row. defp process_row( row_map, line_number, - custom_field_lookup, - actor + %{ + custom_field_lookup: custom_field_lookup, + fee_type_map: fee_type_map, + groups_found: groups_found, + actor: actor + } = _row_opts ) do # Validate row before database insertion case validate_row(row_map, line_number, []) do {:error, error} -> # Return validation error immediately, no DB insert attempted - {:error, error} + {:error, error, groups_found} {:ok, %{member: trimmed_member_attrs, custom: custom_attrs}} -> # Prepare custom field values for Ash @@ -526,20 +614,119 @@ defmodule Mv.Membership.Import.MemberCSV do {:error, validation_errors} -> # Custom field validation errors - return first error first_error = List.first(validation_errors) - {:error, %Error{csv_line_number: line_number, field: nil, message: first_error}} + + {:error, %Error{csv_line_number: line_number, field: nil, message: first_error}, + groups_found} {:ok, custom_field_values} -> - create_member_with_custom_fields( - trimmed_member_attrs, + {fee_attrs, warnings} = + resolve_fee_type_attrs(Map.get(row_map, :fee_type), fee_type_map) + + create_member_and_assign_groups( + Map.merge(trimmed_member_attrs, fee_attrs), custom_field_values, + Map.get(row_map, :groups), + groups_found, line_number, - actor + actor, + warnings ) end end rescue e -> - {:error, %Error{csv_line_number: line_number, field: nil, message: Exception.message(e)}} + {:error, %Error{csv_line_number: line_number, field: nil, message: Exception.message(e)}, + groups_found} + end + + # Creates the member, then assigns groups as a post-creation step. A group + # assignment failure fails the row (the member was already created, but the + # row is reported as failed so the operator can act on it). + defp create_member_and_assign_groups( + member_attrs, + custom_field_values, + groups_cell, + groups_found, + line_number, + actor, + warnings + ) do + case create_member_with_custom_fields( + member_attrs, + custom_field_values, + line_number, + actor, + warnings + ) do + {:ok, member, member_warnings} -> + assign_groups(member, groups_cell, groups_found, line_number, actor, member_warnings) + + {:error, error} -> + {:error, error, groups_found} + end + end + + # Assigns the member to all groups listed in the cell, creating missing groups. + # Returns the (possibly grown) group snapshot so the caller can reuse it. + defp assign_groups(member, groups_cell, groups_found, line_number, actor, warnings) do + names = ColumnResolver.split_group_names(groups_cell) + + Enum.reduce_while(names, {:ok, member, warnings, groups_found}, fn name, + {:ok, _m, _w, acc_groups} -> + case link_member_to_group(member, name, acc_groups, actor) do + {:ok, group} -> + {:cont, {:ok, member, warnings, add_group(acc_groups, group)}} + + {:error, reason} -> + {:halt, + {:error, + %Error{ + csv_line_number: line_number, + field: nil, + message: gettext("Group assignment failed: %{reason}", reason: inspect(reason)) + }, acc_groups}} + end + end) + end + + defp add_group(groups, group) do + if Enum.any?(groups, &(&1.id == group.id)), do: groups, else: [group | groups] + end + + defp link_member_to_group(member, name, groups_found, actor) do + with {:ok, group} <- ColumnResolver.create_or_find_group(name, groups_found, actor), + {:ok, _member_group} <- + Mv.Membership.create_member_group( + %{member_id: member.id, group_id: group.id}, + actor: actor + ) do + {:ok, group} + end + end + + # Resolves the fee-type cell into member attrs plus optional warnings. + # Empty cell -> default fee type (SetDefaultMembershipFeeType), no warning. + # Matched name -> membership_fee_type_id attr. + # Unmatched name -> no attr (default applies), warning naming the value. + defp resolve_fee_type_attrs(nil, _fee_type_map), do: {%{}, []} + + defp resolve_fee_type_attrs(cell, fee_type_map) when is_binary(cell) do + trimmed = String.trim(cell) + + if trimmed == "" do + {%{}, []} + else + case Map.get(fee_type_map, ColumnResolver.normalize_fee_type_name(trimmed)) do + nil -> + {%{}, + [ + gettext("Fee type '%{name}' not found; using the default fee type.", name: trimmed) + ]} + + fee_type_id -> + {%{membership_fee_type_id: fee_type_id}, []} + end + end end # Creates a member with custom field values, handling errors appropriately @@ -547,7 +734,8 @@ defmodule Mv.Membership.Import.MemberCSV do trimmed_member_attrs, custom_field_values, line_number, - actor + actor, + warnings ) do # Convert empty strings to nil for date fields so Ash accepts them member_attrs = sanitize_date_fields(trimmed_member_attrs) @@ -567,7 +755,7 @@ defmodule Mv.Membership.Import.MemberCSV do case Mv.Membership.create_member(final_attrs, actor: actor) do {:ok, member} -> - {:ok, member} + {:ok, member, warnings} {:error, %Ash.Error.Invalid{} = error} -> # Extract email from final_attrs for better error messages diff --git a/lib/mv/membership/member/validations/email_change_permission.ex b/lib/mv/membership/member/validations/email_change_permission.ex index 2b1c041..073da07 100644 --- a/lib/mv/membership/member/validations/email_change_permission.ex +++ b/lib/mv/membership/member/validations/email_change_permission.ex @@ -59,7 +59,7 @@ defmodule Mv.Membership.Member.Validations.EmailChangePermission do # Ash stores actor in changeset.context.private.actor; validation context has .actor; some callsites use context.actor defp resolve_actor(changeset, context) do - ctx = changeset.context || %{} + ctx = changeset.context get_in(ctx, [:private, :actor]) || Map.get(ctx, :actor) || diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex index 16341c4..a98b125 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -16,6 +16,21 @@ defmodule Mv.Membership.MemberExport do alias MvWeb.MemberLive.Index alias MvWeb.MemberLive.Index.MembershipFeeStatus + @typedoc "Validated export parameters produced by `parse_params/1`." + @type parsed_params :: %{ + selected_ids: [String.t()], + member_fields: [String.t()], + selectable_member_fields: [String.t()], + computed_fields: [String.t()], + custom_field_ids: [String.t()], + query: String.t() | nil, + sort_field: String.t() | nil, + sort_order: String.t() | nil, + show_current_cycle: boolean(), + cycle_status_filter: :paid | :unpaid | nil, + boolean_filters: %{optional(String.t()) => boolean()} + } + @member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++ ["membership_fee_type", "membership_fee_status", "groups"] @computed_export_fields ["membership_fee_status"] @@ -305,7 +320,7 @@ defmodule Mv.Membership.MemberExport do :computed_fields, :custom_field_ids, :query, :sort_field, :sort_order, :show_current_cycle, :cycle_status_filter, :boolean_filters. """ - @spec parse_params(map()) :: map() + @spec parse_params(map()) :: parsed_params() def parse_params(params) do # DB fields come from "member_fields" raw_member_fields = extract_list(params, "member_fields") @@ -458,9 +473,6 @@ defmodule Mv.Membership.MemberExport do computed_fields, member_fields ) do - computed_fields = computed_fields || [] - member_fields = member_fields || [] - db_with_insert = Enum.flat_map(db_fields_ordered, fn f -> expand_field_with_computed(f, member_fields, computed_fields) @@ -507,6 +519,4 @@ defmodule Mv.Membership.MemberExport do other -> other end) end - - defp normalize_computed_fields(_), do: [] end diff --git a/lib/mv/membership/members_csv.ex b/lib/mv/membership/members_csv.ex index 6331893..0a19810 100644 --- a/lib/mv/membership/members_csv.ex +++ b/lib/mv/membership/members_csv.ex @@ -21,7 +21,7 @@ defmodule Mv.Membership.MembersCSV do Returns iodata suitable for `IO.iodata_to_binary/1` or sending as response body. RFC 4180 escaping and formula-injection safe_cell are applied. """ - @spec export([struct() | map()], [map()]) :: iodata() + @spec export([struct() | map()], [map()]) :: [iodata()] | Enumerable.t() def export(members, columns) when is_list(members) do header = build_header(columns) rows = Enum.map(members, fn member -> build_row(member, columns) end) diff --git a/lib/mv/membership/members_pdf.ex b/lib/mv/membership/members_pdf.ex index b2989ca..a1c8418 100644 --- a/lib/mv/membership/members_pdf.ex +++ b/lib/mv/membership/members_pdf.ex @@ -143,7 +143,7 @@ defmodule Mv.Membership.MembersPDF do defp convert_to_template_format(export_data, locale, club_name) do # Set locale for translations - Gettext.put_locale(MvWeb.Gettext, locale) + _ = Gettext.put_locale(MvWeb.Gettext, locale) headers = Enum.map(export_data.columns, & &1.label) column_count = length(export_data.columns) @@ -211,9 +211,6 @@ defmodule Mv.Membership.MembersPDF do {:ok, datetime, _offset} -> format_datetime(datetime, locale) - {:ok, datetime} -> - format_datetime(datetime, locale) - {:error, _} -> # Try NaiveDateTime if DateTime parsing fails case NaiveDateTime.from_iso8601(iso8601_string) do @@ -257,8 +254,6 @@ defmodule Mv.Membership.MembersPDF do end end - defp format_date(_, _), do: "" - defp format_dates_in_rows(rows, columns, locale) do date_indices = find_date_column_indices(columns) @@ -321,7 +316,7 @@ defmodule Mv.Membership.MembersPDF do defp format_cell_date_datetime(cell_value, locale) do case DateTime.from_iso8601(cell_value) do - {:ok, datetime} -> format_datetime(datetime, locale) + {:ok, datetime, _offset} -> format_datetime(datetime, locale) _ -> format_cell_date_naive(cell_value, locale) end end diff --git a/lib/mv/membership_fees/cycle_generation_job.ex b/lib/mv/membership_fees/cycle_generation_job.ex index 71a3158..b38886c 100644 --- a/lib/mv/membership_fees/cycle_generation_job.ex +++ b/lib/mv/membership_fees/cycle_generation_job.ex @@ -58,7 +58,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do {:ok, %{success: 45, failed: 0, total: 45}} """ - @spec run() :: {:ok, map()} | {:error, term()} + @spec run() :: {:ok, CycleGenerator.results_summary()} | {:error, Ash.Error.t()} def run do Logger.info("Starting membership fee cycle generation job") start_time = System.monotonic_time(:millisecond) @@ -98,7 +98,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do Mv.MembershipFees.CycleGenerationJob.run(batch_size: 5) """ - @spec run(keyword()) :: {:ok, map()} | {:error, term()} + @spec run(keyword()) :: {:ok, CycleGenerator.results_summary()} | {:error, Ash.Error.t()} def run(opts) when is_list(opts) do Logger.info("Starting membership fee cycle generation job with opts: #{inspect(opts)}") start_time = System.monotonic_time(:millisecond) @@ -135,7 +135,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do - `{:error, reason}` - Error with reason """ - @spec pending_members_count() :: {:ok, non_neg_integer()} | {:error, term()} + @spec pending_members_count() :: {:ok, non_neg_integer()} | {:error, Ash.Error.t()} def pending_members_count do today = Date.utc_today() @@ -166,7 +166,7 @@ defmodule Mv.MembershipFees.CycleGenerationJob do - `{:error, reason}` - Error with reason """ - @spec run_for_member(String.t()) :: {:ok, [map()]} | {:error, term()} + @spec run_for_member(String.t()) :: CycleGenerator.generate_result() def run_for_member(member_id) when is_binary(member_id) do Logger.info("Generating cycles for member #{member_id}") CycleGenerator.generate_cycles_for_member(member_id) diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 8f1bc7c..189f40a 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -1,4 +1,11 @@ defmodule Mv.MembershipFees.CycleGenerator do + @typedoc "Aggregate counts returned by a batch cycle-generation run." + @type results_summary :: %{ + success: non_neg_integer(), + failed: non_neg_integer(), + total: non_neg_integer() + } + @moduledoc """ Module for generating membership fee cycles for members. @@ -115,7 +122,7 @@ defmodule Mv.MembershipFees.CycleGenerator do lock_key = Member.advisory_lock_key_for_member_id(member.id) Repo.transaction(fn -> - EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + _ = EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) case do_generate_cycles(member, today, opts) do {:ok, cycles, notifications} -> @@ -159,7 +166,8 @@ defmodule Mv.MembershipFees.CycleGenerator do - `{:error, reason}` - Error with reason """ - @spec generate_cycles_for_all_members(keyword()) :: {:ok, map()} | {:error, term()} + @spec generate_cycles_for_all_members(keyword()) :: + {:ok, results_summary()} | {:error, Ash.Error.t()} def generate_cycles_for_all_members(opts \\ []) do today = Keyword.get(opts, :today, Date.utc_today()) batch_size = Keyword.get(opts, :batch_size, 10) @@ -212,7 +220,7 @@ defmodule Mv.MembershipFees.CycleGenerator do defp process_member_cycle_generation(member, today) do case generate_cycles_for_member(member, today: today) do {:ok, _cycles, notifications} = ok -> - send_notifications_for_batch_job(notifications) + _ = send_notifications_for_batch_job(notifications) {member.id, ok} {:error, _reason} = err -> diff --git a/lib/mv/oidc_role_sync.ex b/lib/mv/oidc_role_sync.ex index a13748a..0f6467c 100644 --- a/lib/mv/oidc_role_sync.ex +++ b/lib/mv/oidc_role_sync.ex @@ -87,8 +87,6 @@ defmodule Mv.OidcRoleSync do ArgumentError -> nil end - defp safe_get_atom(_map, _key), do: nil - defp peek_jwt_claims(token) do parts = String.split(token, ".") diff --git a/lib/mv/oidc_role_sync_config.ex b/lib/mv/oidc_role_sync_config.ex index 2a8574c..bbb5770 100644 --- a/lib/mv/oidc_role_sync_config.ex +++ b/lib/mv/oidc_role_sync_config.ex @@ -15,6 +15,6 @@ defmodule Mv.OidcRoleSyncConfig do @doc "Returns the JWT/user_info claim name for groups; defaults to \"groups\"." def oidc_groups_claim do - Mv.Config.oidc_groups_claim() || "groups" + Mv.Config.oidc_groups_claim() end end diff --git a/lib/mv/release.ex b/lib/mv/release.ex index 116b276..5db4751 100644 --- a/lib/mv/release.ex +++ b/lib/mv/release.ex @@ -22,7 +22,7 @@ defmodule Mv.Release do require Logger def migrate do - load_app() + _ = load_app() for repo <- repos() do {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :up, all: true)) @@ -75,14 +75,14 @@ defmodule Mv.Release do dev_path = Path.join(priv, "repo/seeds_dev.exs") prev = Code.compiler_options() - Code.compiler_options(ignore_module_conflict: true) + _ = Code.compiler_options(ignore_module_conflict: true) try do - Code.eval_file(bootstrap_path) + _ = Code.eval_file(bootstrap_path) IO.puts("✅ Bootstrap seeds completed.") if System.get_env("RUN_DEV_SEEDS") == "true" do - Code.eval_file(dev_path) + _ = Code.eval_file(dev_path) IO.puts("✅ Dev seeds completed.") end after @@ -92,7 +92,7 @@ defmodule Mv.Release do end def rollback(repo, version) do - load_app() + _ = load_app() {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :down, to: version)) end @@ -139,10 +139,11 @@ defmodule Mv.Release do {:ok, %Role{} = admin_role} -> case get_user_by_email(email) do {:ok, %User{} = user} -> - user - |> Ash.Changeset.for_update(:update, %{}) - |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) - |> Ash.update!(authorize?: false) + _ = + user + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) :ok @@ -189,15 +190,16 @@ defmodule Mv.Release do defp create_admin_user(email, password, admin_role) do case Accounts.create_user(%{email: email}, authorize?: false) do {:ok, user} -> - user - |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) - |> Ash.update!(authorize?: false) - |> then(fn u -> - u - |> Ash.Changeset.for_update(:update, %{}) - |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + _ = + user + |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) |> Ash.update!(authorize?: false) - end) + |> then(fn u -> + u + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + end) :ok @@ -207,15 +209,16 @@ defmodule Mv.Release do end defp update_admin_user(user, password, admin_role) do - user - |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) - |> Ash.update!(authorize?: false) - |> then(fn u -> - u - |> Ash.Changeset.for_update(:update, %{}) - |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + _ = + user + |> Ash.Changeset.for_update(:admin_set_password, %{password: password}) |> Ash.update!(authorize?: false) - end) + |> then(fn u -> + u + |> Ash.Changeset.for_update(:update, %{}) + |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove) + |> Ash.update!(authorize?: false) + end) :ok end diff --git a/lib/mv/repo.ex b/lib/mv/repo.ex index 0a4a04d..183c54f 100644 --- a/lib/mv/repo.ex +++ b/lib/mv/repo.ex @@ -19,4 +19,12 @@ defmodule Mv.Repo do def min_pg_version do %Version{major: 17, minor: 2, patch: 0} end + + # This app does not use schema-based multitenancy, so there are no tenant + # schemas to migrate. Returning [] keeps the AshPostgres callback total + # rather than raising the default "not defined" error. + @impl true + def all_tenants do + [] + end end diff --git a/lib/mv/vereinfacht/client.ex b/lib/mv/vereinfacht/client.ex index 3cbba71..999bd44 100644 --- a/lib/mv/vereinfacht/client.ex +++ b/lib/mv/vereinfacht/client.ex @@ -8,6 +8,12 @@ defmodule Mv.Vereinfacht.Client do """ require Logger + @typedoc "Error reasons returned by Vereinfacht API calls." + @type error_reason :: + :not_configured + | {:request_failed, map()} + | {:http, non_neg_integer(), :html_response | binary()} + @content_type "application/vnd.api+json" @doc """ @@ -31,7 +37,7 @@ defmodule Mv.Vereinfacht.Client do {:error, :not_configured} """ @spec test_connection(String.t() | nil, String.t() | nil, String.t() | nil) :: - {:ok, :connected} | {:error, term()} + {:ok, :connected} | {:error, error_reason()} def test_connection(api_url, api_key, club_id) do if blank?(api_url) or blank?(api_key) or blank?(club_id) do {:error, :not_configured} @@ -92,13 +98,12 @@ defmodule Mv.Vereinfacht.Client do @sync_timeout_ms 5_000 - # Resolved at compile time so Mix is never called at runtime (Mix is not available in releases). - @env Mix.env() - # In test, skip retries so sync fails fast when no API is running (avoids log spam and long waits). + # `sql_sandbox?/0` reads runtime config (true only in test) and avoids calling Mix at runtime, + # which is unavailable in releases. defp req_http_options do opts = [receive_timeout: @sync_timeout_ms] - if @env == :test, do: [retry: false] ++ opts, else: opts + if Mv.Config.sql_sandbox?(), do: [retry: false] ++ opts, else: opts end defp post_and_parse_contact(url, body, api_key) do @@ -230,7 +235,7 @@ defmodule Mv.Vereinfacht.Client do Returns the full response body (decoded JSON) for debugging/display. """ - @spec get_contact(String.t()) :: {:ok, map()} | {:error, term()} + @spec get_contact(String.t()) :: {:ok, map()} | {:error, error_reason()} def get_contact(contact_id) when is_binary(contact_id) do fetch_contact(contact_id, []) end diff --git a/lib/mv/vereinfacht/sync_flash.ex b/lib/mv/vereinfacht/sync_flash.ex index 874a717..5c643b6 100644 --- a/lib/mv/vereinfacht/sync_flash.ex +++ b/lib/mv/vereinfacht/sync_flash.ex @@ -37,9 +37,10 @@ defmodule Mv.Vereinfacht.SyncFlash do def create_table! do # :public so any process can write (SyncContact runs in LiveView/Ash transaction process, # not the process that created the table). :protected would restrict writes to the creating process. - if :ets.whereis(@table) == :undefined do - :ets.new(@table, [:set, :public, :named_table]) - end + _ = + if :ets.whereis(@table) == :undefined do + :ets.new(@table, [:set, :public, :named_table]) + end :ok end diff --git a/lib/mv/vereinfacht/vereinfacht.ex b/lib/mv/vereinfacht/vereinfacht.ex index 83492b7..4d58f8d 100644 --- a/lib/mv/vereinfacht/vereinfacht.ex +++ b/lib/mv/vereinfacht/vereinfacht.ex @@ -26,7 +26,7 @@ defmodule Mv.Vereinfacht do - `{:error, {:http, status, message}}` – API returned an error (e.g. 401, 403) - `{:error, {:request_failed, reason}}` – network/transport error """ - @spec test_connection() :: {:ok, :connected} | {:error, term()} + @spec test_connection() :: {:ok, :connected} | {:error, Mv.Vereinfacht.Client.error_reason()} def test_connection do Client.test_connection( Mv.Config.vereinfacht_api_url(), diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex index d821416..de009b6 100644 --- a/lib/mv_web/authorization.ex +++ b/lib/mv_web/authorization.ex @@ -113,8 +113,7 @@ defmodule MvWeb.Authorization do iex> can_access_page?(mitglied, "/members") false """ - @spec can_access_page?(map() | nil, String.t() | Phoenix.VerifiedRoutes.unverified_path()) :: - boolean() + @spec can_access_page?(map() | nil, String.t()) :: boolean() def can_access_page?(nil, _page_path), do: false def can_access_page?(user, page_path) do diff --git a/lib/mv_web/components/bulk_actions_dropdown.ex b/lib/mv_web/components/bulk_actions_dropdown.ex new file mode 100644 index 0000000..d0b6172 --- /dev/null +++ b/lib/mv_web/components/bulk_actions_dropdown.ex @@ -0,0 +1,243 @@ +defmodule MvWeb.Components.BulkActionsDropdown do + @moduledoc """ + Single "Aktionen" dropdown bundling the four member bulk actions, flattened to + one level: open in email program (mailto), copy email addresses, export to CSV, + export to PDF. + + It keeps the CSRF-protected `
` POST export items unchanged (CSV/PDF) and + adds the mailto and copy items that previously lived as standalone header + buttons next to a separate export dropdown. + + ## Scope and trigger badge + + The trigger reads `Aktionen` followed by a scope badge: an emphasized + (`primary`) count `N` when `N` members are selected, and a muted (`neutral`) + badge otherwise — `gefiltert` when a search term or filter narrows the list, + `alle` when nothing is selected and no search/filter is active. Only an actual + selection is emphasized. The badge sits inside the shared `dropdown_menu/1` + trigger via its `trigger_badge` slot, matching the member-filter dropdown's + count badge. The `scope`, `selected_count`, `mailto_bcc`, `recipient_count` + and `mailto_disabled?` are computed by the parent LiveView and passed in. + + ## Recipient handling (mailto / copy) + + The parent already excludes members without an email when building + `mailto_bcc` and `recipient_count` (defensive filter preserved verbatim from + the previous behaviour). Export, by contrast, still includes every member in + scope regardless of email — its payload is unchanged. + + ## Mailto recipient cap + + A mailto URI carries every recipient in its BCC; browsers cannot reliably hand + a very long mailto over to the mail program. When `mailto_disabled?` is true + (recipient count at or above `Mv.Constants.max_mailto_bulk_recipients/0`) the + mailto item is rendered disabled (`aria-disabled`, `tabindex="-1"`, href + dropped) with an explanatory tooltip. Copy and Export have no such cap. + + ## Event routing + + `dropdown_menu/1` sends `toggle_dropdown`/`close_dropdown` to `@myself`, so the + component owns its own `:open` state. The copy item carries an *un-targeted* + `phx-click="copy_emails"`, which therefore reaches the parent LiveView's + `handle_event/3` (which keeps access to `@members`), plus the + `CopyToClipboard` hook. + """ + use MvWeb, :live_component + use Gettext, backend: MvWeb.Gettext + + # Same focus ring as CoreComponents button/dropdown (WCAG 2.4.7) + defp dropdown_item_class do + focus = + MvWeb.CoreComponents.button_focus_classes() + |> Kernel.++(["focus-visible:ring-inset"]) + |> Enum.join(" ") + + "flex items-center gap-2 px-2 py-1 rounded cursor-pointer hover:bg-base-200 w-full text-left whitespace-nowrap #{focus}" + end + + @impl true + def mount(socket) do + {:ok, assign(socket, :open, false)} + end + + @impl true + def update(assigns, socket) do + socket = + socket + |> assign(:id, assigns.id) + |> assign(:export_payload_json, assigns[:export_payload_json] || "") + |> assign(:selected_count, assigns[:selected_count] || 0) + |> assign(:scope, assigns[:scope] || :all) + |> assign(:mailto_bcc, assigns[:mailto_bcc] || "") + |> assign(:recipient_count, assigns[:recipient_count] || 0) + |> assign(:mailto_disabled?, assigns[:mailto_disabled?] || false) + + # The parent never sets :open (the component owns it via toggle/close). + # Honouring an explicit :open assign keeps the component renderable in + # isolation (render_component/2) for structural tests. + socket = + case Map.fetch(assigns, :open) do + {:ok, open} -> assign(socket, :open, open) + :error -> socket + end + + {:ok, socket} + end + + @impl true + def render(assigns) do + assigns = + assigns + |> assign(:scope_label, scope_label(assigns)) + |> assign(:scope_variant, scope_variant(assigns)) + + ~H""" +
+ <.dropdown_menu + id={"#{@id}-menu"} + button_label={gettext("Actions")} + icon="hero-bolt" + open={@open} + phx_target={@myself} + menu_width="w-70" + menu_align="left" + button_class="btn-secondary gap-2" + testid="bulk-actions-dropdown" + button_testid="bulk-actions-button" + menu_testid="bulk-actions-menu" + > + <:trigger_badge> + <.badge variant={@scope_variant} size="sm" data-testid="bulk-actions-scope-badge"> + {@scope_label} + + +
  • + <.mailto_item mailto_bcc={@mailto_bcc} disabled={@mailto_disabled?} /> +
    • +
  • + +
    • +
  • + + + + +
    • + +
  • +
    + + + +
    +
    • + +
    + """ + end + + # The mailto item is an anchor menu item. When over the recipient cap it is + # rendered disabled following the same a11y pattern as a disabled CoreComponents + # link button (href dropped, tabindex=-1, aria-disabled=true) and exposes the + # explanatory tooltip via title. + attr :mailto_bcc, :string, required: true + attr :disabled, :boolean, required: true + + defp mailto_item(%{disabled: true} = assigns) do + assigns = assign(assigns, :item_class, dropdown_item_class()) + + ~H""" + + <.icon name="hero-envelope" class="h-4 w-4" /> + {gettext("Open in email program")} + + """ + end + + defp mailto_item(%{disabled: false} = assigns) do + assigns = assign(assigns, :item_class, dropdown_item_class()) + + ~H""" + @mailto_bcc} + class={@item_class} + aria-label={gettext("Open in email program")} + data-testid="bulk-actions-mailto" + > + <.icon name="hero-envelope" class="h-4 w-4" /> + {gettext("Open in email program")} + + """ + end + + defp over_threshold_tooltip do + gettext("Too many recipients for this function. Copy the addresses or export the list.") + end + + # The trigger scope is shown as a badge after the "Aktionen" label. Only an + # actual selection is emphasized (primary); both the "filtered" and "all" + # scopes are muted (neutral), since neither means members are selected. + defp scope_label(assigns) do + case assigns.scope do + :selection -> to_string(assigns.selected_count) + :filtered -> gettext("filtered") + _ -> gettext("all") + end + end + + defp scope_variant(assigns) do + case assigns.scope do + :selection -> "primary" + _ -> "neutral" + end + end + + @impl true + def handle_event("toggle_dropdown", _params, socket) do + {:noreply, assign(socket, :open, !socket.assigns.open)} + end + + def handle_event("close_dropdown", _params, socket) do + {:noreply, assign(socket, :open, false)} + end +end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 465d41a..13c69a8 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -464,6 +464,9 @@ defmodule MvWeb.CoreComponents do slot :inner_block, doc: "Custom content for the dropdown menu (e.g., forms)" + slot :trigger_badge, + doc: "Optional badge rendered in the trigger after the label (e.g. a scope badge)" + def dropdown_menu(assigns) do menu_testid = assigns.menu_testid || "#{assigns.testid}-menu" @@ -498,6 +501,8 @@ defmodule MvWeb.CoreComponents do <.icon name={@icon} /> <% end %> {@button_label} + {render_slot(@trigger_badge)} + <.icon name="hero-chevron-down" class="size-4" />