From b7a83d92985e767f7aa40119c537735e602fac66 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 10 Mar 2026 12:18:36 +0100 Subject: [PATCH 1/4] test: add tests for join form settings --- docs/development-progress-log.md | 3 + test/membership/setting_join_form_test.exs | 281 +++++++++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 test/membership/setting_join_form_test.exs diff --git a/docs/development-progress-log.md b/docs/development-progress-log.md index a86efe6..b2a814b 100644 --- a/docs/development-progress-log.md +++ b/docs/development-progress-log.md @@ -809,6 +809,9 @@ end - **PR review follow-ups (Join confirmation):** Join confirmation email uses `Mailer.deliver/1` and returns `{:ok, email}` \| `{:error, reason}`; domain logs delivery errors but still returns `{:ok, request}` so the user sees success. Comment in `submit_join_request/2` clarifies that the raw token is hashed by `JoinRequest.Changes.SetConfirmationToken`. Cleanup task uses `Ash.bulk_destroy` and logs partial errors without halting. Layout uses assigns `app_name` and `locale` (from config/Gettext) instead of hardcoded "Mila" and `lang="de"`. Production `runtime.exs` sets `:mail_from` from ENV (`MAIL_FROM_NAME`, `MAIL_FROM_EMAIL`). Layout reference unified to `"layout.html"`; redundant `put_layout` removed from senders. - Tests: `join_request_test.exs`, `join_request_submit_email_test.exs`, `join_confirm_controller_test.exs` – all pass. +**Subtask 3 – Admin: Join form settings (TDD tests only):** +- Test file: `test/membership/setting_join_form_test.exs` – TDD tests for join form settings (persistence, validation, allowlist, defaults, robustness). Tests are red until Setting gains `join_form_enabled`, `join_form_field_ids`, `join_form_field_required` and `Mv.Membership.get_join_form_allowlist/0` is implemented. No functionality implemented yet. + ### Test Data Management **Seed Data:** diff --git a/test/membership/setting_join_form_test.exs b/test/membership/setting_join_form_test.exs new file mode 100644 index 0000000..9b15ca4 --- /dev/null +++ b/test/membership/setting_join_form_test.exs @@ -0,0 +1,281 @@ +defmodule Mv.Membership.SettingJoinFormTest do + @moduledoc """ + TDD tests for Join Form Settings (onboarding-join-concept subtask 3). + + These tests define the expected API and behaviour for the "Onboarding / Join" section + in global settings. No functionality is implemented yet; tests are expected to fail + (red) until: + + - Setting resource has attributes: `join_form_enabled`, `join_form_field_ids`, + `join_form_field_required`, plus validations and accept in the update action. + - `Mv.Membership.get_join_form_allowlist/0` is implemented and returns the allowlist + for the public join form (subtask 4). + """ + use Mv.DataCase, async: false + + alias Mv.Membership + alias Mv.Helpers.SystemActor + alias Mv.Constants + + setup do + {:ok, settings} = Membership.get_settings() + saved_enabled = Map.get(settings, :join_form_enabled) + saved_ids = Map.get(settings, :join_form_field_ids) + saved_required = Map.get(settings, :join_form_field_required) + + on_exit(fn -> + {:ok, s} = Membership.get_settings() + attrs = %{} + attrs = if saved_enabled != nil, do: Map.put(attrs, :join_form_enabled, saved_enabled), else: attrs + attrs = if saved_ids != nil, do: Map.put(attrs, :join_form_field_ids, saved_ids || []), else: attrs + attrs = + if saved_required != nil, + do: Map.put(attrs, :join_form_field_required, saved_required || %{}), + else: attrs + + if attrs != %{} do + Membership.update_settings(s, attrs) + end + end) + + :ok + end + + defp update_join_form_settings(settings, attrs) do + Membership.update_settings(settings, attrs) + end + + defp error_message(errors, field) when is_atom(field) do + errors + |> Enum.filter(fn err -> Map.get(err, :field) == field end) + |> Enum.map(&Map.get(&1, :message, "")) + |> List.first() || "" + end + + # ---- 1. Persistence and loading ---- + + describe "join form settings persistence and loading" do + test "save and load join_form_enabled plus field selection and required flags returns same config" do + {:ok, settings} = Membership.get_settings() + attrs = %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name"], + join_form_field_required: %{"email" => true, "first_name" => false} + } + + assert {:ok, updated} = update_join_form_settings(settings, attrs) + assert updated.join_form_enabled == true + assert updated.join_form_field_ids == ["email", "first_name"] + assert updated.join_form_field_required["email"] == true + assert updated.join_form_field_required["first_name"] == false + + {:ok, reloaded} = Membership.get_settings() + assert reloaded.join_form_enabled == true + assert reloaded.join_form_field_ids == ["email", "first_name"] + assert reloaded.join_form_field_required["email"] == true + assert reloaded.join_form_field_required["first_name"] == false + end + + test "repeated save with changed field list overwrites config without leftovers" do + {:ok, settings} = Membership.get_settings() + assert {:ok, _} = update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name"], + join_form_field_required: %{"email" => true, "first_name" => false} + }) + + assert {:ok, updated} = update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "last_name"], + join_form_field_required: %{"email" => true, "last_name" => false} + }) + + assert updated.join_form_field_ids == ["email", "last_name"] + assert Map.has_key?(updated.join_form_field_required, "last_name") + refute Map.has_key?(updated.join_form_field_required, "first_name") + end + end + + # ---- 2. Validation ---- + + describe "join form settings validation" do + test "only existing member fields or custom field ids are accepted; unknown field names rejected or sanitized" do + {:ok, settings} = Membership.get_settings() + + result = update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "not_a_member_field"], + join_form_field_required: %{"email" => true, "not_a_member_field" => false} + }) + + # Until attributes exist we get NoSuchInput; once implemented we expect validation error + assert {:error, _} = result + end + + test "config without email is rejected or email is auto-added and required" do + {:ok, settings} = Membership.get_settings() + + result = update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["first_name", "last_name"], + join_form_field_required: %{"first_name" => true, "last_name" => false} + }) + + # Either rejected or, when loaded, email must be present and required + case result do + {:error, _} -> + :ok + {:ok, updated} -> + assert "email" in updated.join_form_field_ids + assert updated.join_form_field_required["email"] == true + end + end + + test "required false for email is ignored or forced to true when saved" do + {:ok, settings} = Membership.get_settings() + + {:ok, updated} = update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name"], + join_form_field_required: %{"email" => false, "first_name" => false} + }) + + assert updated.join_form_field_required["email"] == true + end + + test "required flag for field not in join_form_field_ids is rejected or dropped" do + {:ok, settings} = Membership.get_settings() + + result = update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email"], + join_form_field_required: %{"email" => true, "first_name" => true} + }) + + case result do + {:error, _} -> + :ok + {:ok, updated} -> + refute Map.has_key?(updated.join_form_field_required, "first_name") + end + end + end + + # ---- 3. Allowlist for join form ---- + + describe "join form allowlist" do + test "allowlist returns configured fields with required/optional when join form enabled" do + {:ok, settings} = Membership.get_settings() + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name"], + join_form_field_required: %{"email" => true, "first_name" => false} + }) + + allowlist = Membership.get_join_form_allowlist() + + assert length(allowlist) == 2 + email_entry = Enum.find(allowlist, &( &1.id == "email" )) + first_name_entry = Enum.find(allowlist, &( &1.id == "first_name" )) + assert email_entry.required == true + assert first_name_entry.required == false + assert email_entry.type == :member_field + assert first_name_entry.type == :member_field + end + + test "allowlist returns empty or defined default when join form disabled" do + {:ok, settings} = Membership.get_settings() + update_join_form_settings(settings, %{ + join_form_enabled: false, + join_form_field_ids: ["email", "first_name"], + join_form_field_required: %{"email" => true, "first_name" => false} + }) + + allowlist = Membership.get_join_form_allowlist() + + assert allowlist == [] + end + + @tag :requires_custom_field + test "allowlist distinguishes member fields and custom field identifiers" do + {:ok, settings} = Membership.get_settings() + actor = SystemActor.get_system_actor() + {:ok, cf} = + Membership.create_custom_field( + %{name: "join_cf_#{System.unique_integer([:positive])}", value_type: :string}, + actor: actor + ) + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", cf.id], + join_form_field_required: %{"email" => true, cf.id => false} + }) + + allowlist = Membership.get_join_form_allowlist() + + email_entry = Enum.find(allowlist, &( &1.id == "email" )) + cf_entry = Enum.find(allowlist, &( &1.id == cf.id )) + assert email_entry.type == :member_field + assert cf_entry.type == :custom_field + end + end + + # ---- 4. Defaults and fallback ---- + + describe "join form defaults and fallback" do + test "when no join settings stored, allowlist returns defined default (e.g. disabled, empty list)" do + allowlist = Membership.get_join_form_allowlist() + # Default: join form disabled → empty allowlist + assert is_list(allowlist) + assert allowlist == [] || Enum.all?(allowlist, &(is_map(&1) and Map.has_key?(&1, :id))) + end + + test "existing settings without join keys load correctly; new join keys get defaults" do + {:ok, settings} = Membership.get_settings() + # Ensure other attributes still load + assert Map.has_key?(settings, :club_name) + # When join keys exist they have sensible defaults + join_enabled = Map.get(settings, :join_form_enabled) + join_ids = Map.get(settings, :join_form_field_ids) + if join_enabled != nil, do: assert(is_boolean(join_enabled)) + if join_ids != nil, do: assert(is_list(join_ids)) + end + end + + # ---- 5. Authorization (backend: settings update requires authorized actor when policy enforced) ---- + # Authorization for the settings page is covered by GlobalSettingsLive and page-permission tests. + # If the domain later requires an actor for update_settings, tests here would pass an actor. + + # ---- 6. Robustness / edge cases ---- + + describe "join form settings robustness" do + test "invalid or unexpected payload structure yields clean error or ignores unknown keys" do + {:ok, settings} = Membership.get_settings() + + result = update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: "not_a_list", + join_form_field_required: %{} + }) + + assert match?({:error, _}, result) or + (match?({:ok, _}, result) && elem(result, 1).join_form_field_ids != "not_a_list") + end + + test "larger but reasonable number of fields saves and loads without error" do + {:ok, settings} = Membership.get_settings() + all_member = Constants.member_fields() |> Enum.map(&to_string/1) + required_map = Map.new(all_member, fn f -> {f, f == "email"} end) + + assert {:ok, updated} = update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: all_member, + join_form_field_required: required_map + }) + + assert length(updated.join_form_field_ids) == length(all_member) + {:ok, reloaded} = Membership.get_settings() + assert length(reloaded.join_form_field_ids) == length(all_member) + end + end +end From fa738aae88bf92eb26f4dfb1f45dd68136687e27 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 10 Mar 2026 14:29:49 +0100 Subject: [PATCH 2/4] feat: add join form settings --- CODE_GUIDELINES.md | 3 +- docs/development-progress-log.md | 9 +- lib/membership/membership.ex | 50 +++ lib/membership/setting.ex | 76 +++- .../changes/normalize_join_form_settings.ex | 60 +++ lib/mv_web/components/core_components.ex | 2 +- lib/mv_web/live/global_settings_live.ex | 365 +++++++++++++++++- priv/gettext/de/LC_MESSAGES/default.po | 67 ++++ priv/gettext/default.pot | 57 +++ priv/gettext/en/LC_MESSAGES/default.po | 67 ++++ ...701_add_join_form_settings_to_settings.exs | 27 ++ test/membership/setting_join_form_test.exs | 117 +++--- 12 files changed, 846 insertions(+), 54 deletions(-) create mode 100644 lib/membership/setting/changes/normalize_join_form_settings.ex create mode 100644 priv/repo/migrations/20260310114701_add_join_form_settings_to_settings.exs diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 66a93a5..ed9f130 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -89,7 +89,8 @@ lib/ │ ├── join_request/ # JoinRequest changes (SetConfirmationToken, ConfirmRequest) │ ├── custom_field.ex # Custom field (definition) resource │ ├── custom_field_value.ex # Custom field value resource -│ ├── setting.ex # Global settings (singleton resource) +│ ├── setting.ex # Global settings (singleton resource; incl. join form config) +│ ├── setting/ # Setting changes (NormalizeJoinFormSettings, etc.) │ ├── group.ex # Group resource │ ├── member_group.ex # MemberGroup join table resource │ └── email.ex # Email custom type diff --git a/docs/development-progress-log.md b/docs/development-progress-log.md index b2a814b..a6297ba 100644 --- a/docs/development-progress-log.md +++ b/docs/development-progress-log.md @@ -809,8 +809,13 @@ end - **PR review follow-ups (Join confirmation):** Join confirmation email uses `Mailer.deliver/1` and returns `{:ok, email}` \| `{:error, reason}`; domain logs delivery errors but still returns `{:ok, request}` so the user sees success. Comment in `submit_join_request/2` clarifies that the raw token is hashed by `JoinRequest.Changes.SetConfirmationToken`. Cleanup task uses `Ash.bulk_destroy` and logs partial errors without halting. Layout uses assigns `app_name` and `locale` (from config/Gettext) instead of hardcoded "Mila" and `lang="de"`. Production `runtime.exs` sets `:mail_from` from ENV (`MAIL_FROM_NAME`, `MAIL_FROM_EMAIL`). Layout reference unified to `"layout.html"`; redundant `put_layout` removed from senders. - Tests: `join_request_test.exs`, `join_request_submit_email_test.exs`, `join_confirm_controller_test.exs` – all pass. -**Subtask 3 – Admin: Join form settings (TDD tests only):** -- Test file: `test/membership/setting_join_form_test.exs` – TDD tests for join form settings (persistence, validation, allowlist, defaults, robustness). Tests are red until Setting gains `join_form_enabled`, `join_form_field_ids`, `join_form_field_required` and `Mv.Membership.get_join_form_allowlist/0` is implemented. No functionality implemented yet. +**Subtask 3 – Admin: Join form settings (done):** +- **Setting resource** (`lib/membership/setting.ex`): 3 new attributes `join_form_enabled` (boolean, default false), `join_form_field_ids` ({:array, :string} – ordered list of member field names or custom field UUIDs), `join_form_field_required` (:map – field ID → boolean). Added to `:create` and `:update` accept lists. Validation rejects field IDs that are neither valid member field names nor UUID format. Migration: `20260310114701_add_join_form_settings_to_settings.exs`. +- **NormalizeJoinFormSettings** (`lib/membership/setting/changes/normalize_join_form_settings.ex`): Change applied on create/update whenever join form attrs are changing. Ensures email is always in `join_form_field_ids`, forces `join_form_field_required["email"] = true`, drops required flags for fields not in `join_form_field_ids`. +- **Domain** (`lib/membership/membership.ex`): `Mv.Membership.get_join_form_allowlist/0` – returns `[]` when join form is disabled, otherwise a list of `%{id, required, type}` maps (type = `:member_field` or `:custom_field` based on ID format). +- **GlobalSettingsLive** (`lib/mv_web/live/global_settings_live.ex`): New "Join Form" / "Beitrittsformular" section between Club Settings and Vereinfacht. Checkbox to enable/disable, table of selected fields (with Required checkbox per field – email always checked/disabled, other fields can be toggled), "Add field" dropdown with all unselected member fields and custom fields, "Save Join Form Settings" button. State is managed locally (not via AshPhoenix.Form); saved on explicit save click. +- **Translations**: 14 new German strings in `priv/gettext/de/LC_MESSAGES/default.po` (Beitrittsformular, Felder im Beitrittsformular, Feld hinzufügen, etc.). +- Tests: All 13 tests in `test/membership/setting_join_form_test.exs` pass; full test suite 1900 tests, 0 failures. ### Test Data Management diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 5e01a6a..3f34903 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -455,6 +455,56 @@ defmodule Mv.Membership do end end + @doc """ + Returns the allowlist of fields configured for the public join form. + + Reads the current settings. When the join form is disabled (or no settings exist), + returns an empty list. When enabled, returns each configured field as a map with: + - `:id` - field identifier string (member field name or custom field UUID) + - `:required` - boolean; email is always true + - `:type` - `:member_field` or `:custom_field` + + This is the server-side allowlist used by the join form submit action (Subtask 4) + to enforce which fields are accepted from user input. + + ## Returns + + - `[%{id: String.t(), required: boolean(), type: :member_field | :custom_field}]` + - `[]` when join form is disabled or settings are missing + + ## Examples + + iex> Mv.Membership.get_join_form_allowlist() + [%{id: "email", required: true, type: :member_field}, + %{id: "first_name", required: false, type: :member_field}] + + """ + def get_join_form_allowlist do + case get_settings() do + {:ok, settings} -> + if settings.join_form_enabled do + build_join_form_allowlist(settings) + else + [] + end + + {:error, _} -> + [] + end + end + + defp build_join_form_allowlist(settings) do + field_ids = settings.join_form_field_ids || [] + required_config = settings.join_form_field_required || %{} + member_field_names = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + + Enum.map(field_ids, fn id -> + type = if id in member_field_names, do: :member_field, else: :custom_field + required = Map.get(required_config, id, false) + %{id: id, required: required, type: type} + end) + end + defp expired?(nil), do: true defp expired?(expires_at), do: DateTime.compare(expires_at, DateTime.utc_now()) == :lt end diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index 894725f..adf05b9 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -15,6 +15,12 @@ defmodule Mv.Membership.Setting do (e.g., `%{"first_name" => true, "last_name" => true}`). Email is always required; other fields default to optional. - `include_joining_cycle` - Whether to include the joining cycle in membership fee generation (default: true) - `default_membership_fee_type_id` - Default membership fee type for new members (optional) + - `join_form_enabled` - Whether the public /join page is active (default: false) + - `join_form_field_ids` - Ordered list of field IDs shown on the join form. Each entry is + either a member field name string (e.g. "email") or a custom field UUID. Email is always + included and always required; normalization enforces this automatically. + - `join_form_field_required` - Map of field ID => required boolean for the join form. + Email is always forced to true. ## Singleton Pattern This resource uses a singleton pattern - there should only be one settings record. @@ -86,8 +92,13 @@ defmodule Mv.Membership.Setting do :oidc_client_secret, :oidc_admin_group_name, :oidc_groups_claim, - :oidc_only + :oidc_only, + :join_form_enabled, + :join_form_field_ids, + :join_form_field_required ] + + change Mv.Membership.Setting.Changes.NormalizeJoinFormSettings end update :update do @@ -110,8 +121,13 @@ defmodule Mv.Membership.Setting do :oidc_client_secret, :oidc_admin_group_name, :oidc_groups_claim, - :oidc_only + :oidc_only, + :join_form_enabled, + :join_form_field_ids, + :join_form_field_required ] + + change Mv.Membership.Setting.Changes.NormalizeJoinFormSettings end update :update_member_field_visibility do @@ -232,6 +248,39 @@ defmodule Mv.Membership.Setting do end, on: [:create, :update] + # Validate join_form_field_ids: each entry must be a known member field name + # or a UUID-format string (custom field ID). Normalization (NormalizeJoinFormSettings + # change) runs before validations, so email is already present when this runs. + validate fn changeset, _context -> + field_ids = Ash.Changeset.get_attribute(changeset, :join_form_field_ids) + + if is_list(field_ids) and field_ids != [] do + valid_member_fields = + Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + + uuid_pattern = + ~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i + + invalid_ids = + Enum.reject(field_ids, fn id -> + is_binary(id) and + (id in valid_member_fields or Regex.match?(uuid_pattern, id)) + end) + + if Enum.empty?(invalid_ids) do + :ok + else + {:error, + field: :join_form_field_ids, + message: + "Invalid field identifiers: #{inspect(invalid_ids)}. Use member field names or custom field UUIDs."} + end + else + :ok + end + end, + on: [:create, :update] + # Validate default_membership_fee_type_id exists if set validate fn changeset, context -> fee_type_id = @@ -382,6 +431,29 @@ defmodule Mv.Membership.Setting do description "When true and OIDC is configured, sign-in shows only OIDC (password login hidden)" end + # Join form (Beitrittsformular) settings + attribute :join_form_enabled, :boolean do + allow_nil? false + default false + public? true + + description "When true, the public /join page is active and new members can submit a request." + end + + attribute :join_form_field_ids, {:array, :string} do + allow_nil? true + public? true + + description "Ordered list of field IDs shown on the join form. Each entry is a member field name (e.g. 'email') or a custom field UUID. Email is always present after normalization." + end + + attribute :join_form_field_required, :map do + allow_nil? true + public? true + + description "Map of field ID => required boolean for the join form. Email is always true after normalization." + end + timestamps() end diff --git a/lib/membership/setting/changes/normalize_join_form_settings.ex b/lib/membership/setting/changes/normalize_join_form_settings.ex new file mode 100644 index 0000000..d21434a --- /dev/null +++ b/lib/membership/setting/changes/normalize_join_form_settings.ex @@ -0,0 +1,60 @@ +defmodule Mv.Membership.Setting.Changes.NormalizeJoinFormSettings do + @moduledoc """ + Ash change that normalizes join form field settings before persist. + + Applied on create and update actions whenever join form attributes are present. + + Rules enforced: + - Email is always added to join_form_field_ids if not already present. + - Email is always marked as required (true) in join_form_field_required. + - Keys in join_form_field_required that are not in join_form_field_ids are dropped. + + Only runs when join_form_field_ids is being changed; if only + join_form_field_required changes, normalization still uses the current + (possibly changed) field_ids to strip orphaned required flags. + """ + use Ash.Resource.Change + + def change(changeset, _opts, _context) do + changing_ids? = Ash.Changeset.changing_attribute?(changeset, :join_form_field_ids) + changing_required? = Ash.Changeset.changing_attribute?(changeset, :join_form_field_required) + + if changing_ids? or changing_required? do + normalize(changeset) + else + changeset + end + end + + defp normalize(changeset) do + field_ids = Ash.Changeset.get_attribute(changeset, :join_form_field_ids) + required_config = Ash.Changeset.get_attribute(changeset, :join_form_field_required) + + field_ids = normalize_field_ids(field_ids) + required_config = normalize_required(field_ids, required_config) + + changeset + |> Ash.Changeset.force_change_attribute(:join_form_field_ids, field_ids) + |> Ash.Changeset.force_change_attribute(:join_form_field_required, required_config) + end + + defp normalize_field_ids(nil), do: ["email"] + + defp normalize_field_ids(ids) when is_list(ids) do + if "email" in ids do + ids + else + ["email" | ids] + end + end + + defp normalize_field_ids(_), do: ["email"] + + defp normalize_required(field_ids, required_config) do + base = if is_map(required_config), do: required_config, else: %{} + + base + |> Map.filter(fn {key, _} -> key in field_ids end) + |> Map.put("email", true) + end +end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index bb5529e..8a8ff0d 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -990,7 +990,7 @@ defmodule MvWeb.CoreComponents do /> - {gettext("Actions")} + {gettext("Actions")} diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index 58eed2a..651afc0 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -4,16 +4,24 @@ defmodule MvWeb.GlobalSettingsLive do ## Features - Edit the association/club name + - Configure the public join form (Beitrittsformular) - Manage custom fields - Real-time form validation - Success/error feedback ## Settings - `club_name` - The name of the association/club (required) + - `join_form_enabled` - Whether the public /join page is active + - `join_form_field_ids` - Ordered list of field IDs shown on the join form + - `join_form_field_required` - Map of field ID => required boolean ## Events - - `validate` - Real-time form validation - - `save` - Save settings changes + - `validate` / `save` - Club settings form + - `toggle_join_form_enabled` - Enable/disable the join form + - `add_join_form_field` / `remove_join_form_field` - Manage join form fields + - `toggle_join_form_field_required` - Toggle required flag per field + - `toggle_add_field_dropdown` / `hide_add_field_dropdown` - Dropdown visibility + - Join form changes (enable/disable, add/remove fields, required toggles) are persisted immediately ## Note Settings is a singleton resource - there is only one settings record. @@ -31,6 +39,7 @@ defmodule MvWeb.GlobalSettingsLive do alias Mv.Membership alias Mv.Membership.Member, as: MemberResource alias MvWeb.Helpers.MemberHelpers + alias MvWeb.Translations.MemberFields on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded} @@ -42,6 +51,9 @@ defmodule MvWeb.GlobalSettingsLive do locale = session["locale"] || Application.get_env(:mv, :default_locale, "de") Gettext.put_locale(MvWeb.Gettext, locale) + actor = MvWeb.LiveHelpers.current_actor(socket) + custom_fields = load_custom_fields(actor) + socket = socket |> assign(:page_title, gettext("Settings")) @@ -65,6 +77,7 @@ defmodule MvWeb.GlobalSettingsLive do |> assign(:oidc_only_env_set, Mv.Config.oidc_only_env_set?()) |> assign(:oidc_configured, Mv.Config.oidc_configured?()) |> assign(:oidc_client_secret_set, present?(settings.oidc_client_secret)) + |> assign_join_form_state(settings, custom_fields) |> assign_form() {:ok, socket} @@ -103,6 +116,144 @@ defmodule MvWeb.GlobalSettingsLive do + <%!-- Join Form Section (Beitrittsformular) --%> + <.form_section title={gettext("Join Form")}> +

+ {gettext("Configure the public join form that allows new members to submit a join request.")} +

+ + <%!-- Enable/disable --%> +
+ + +
+ + <%!-- Board approval (future feature) --%> +
+ + +
+ +
+ <%!-- Field list header + Add button (left-aligned) --%> +

{gettext("Fields on the join form")}

+
+ <.button + type="button" + variant="primary" + phx-click="toggle_add_field_dropdown" + disabled={Enum.empty?(@available_join_form_member_fields) and Enum.empty?(@available_join_form_custom_fields)} + aria-haspopup="listbox" + aria-expanded={to_string(@show_add_field_dropdown)} + > + <.icon name="hero-plus" class="size-4" /> + {gettext("Add field")} + + + <%!-- Available fields dropdown (sections: Personal data, Custom fields) --%> +
+
+
+ {gettext("Personal data")} +
+
+ {field.label} +
+
+
+
+ {gettext("Individual fields")} +
+
+ {field.label} +
+
+
+
+ + <%!-- Empty state --%> +

+ {gettext("No fields selected. Add at least the email field.")} +

+ + <%!-- Fields table (compact width) --%> +
+ <.table + id="join-form-fields-table" + rows={@join_form_fields} + row_id={fn field -> "join-field-#{field.id}" end} + > + <:col :let={field} label={gettext("Field")} class="min-w-[14rem]"> + {field.label} + + <:col :let={field} label={gettext("Required")} class="w-24 max-w-[9.375rem] text-center"> + + + <:action :let={field}> + <.tooltip content={gettext("Remove")} position="left"> + <.button + type="button" + variant="danger" + size="sm" + disabled={not field.can_remove} + class={if(not field.can_remove, do: "opacity-50 cursor-not-allowed", else: "")} + phx-click="remove_join_form_field" + phx-value-field_id={field.id} + aria-label={gettext("Remove field %{label}", label: field.label)} + > + <.icon name="hero-trash" class="size-4" /> + + + + +
+
+ <%!-- Vereinfacht Integration Section --%> <.form_section title={gettext("Vereinfacht Integration")}> <%= if @vereinfacht_env_configured do %> @@ -426,6 +577,126 @@ defmodule MvWeb.GlobalSettingsLive do end end + # ---- Join form event handlers ---- + + @impl true + def handle_event("toggle_join_form_enabled", _params, socket) do + socket = assign(socket, :join_form_enabled, not socket.assigns.join_form_enabled) + {:noreply, persist_join_form_settings(socket)} + end + + @impl true + def handle_event("toggle_add_field_dropdown", _params, socket) do + {:noreply, + assign(socket, :show_add_field_dropdown, not socket.assigns.show_add_field_dropdown)} + end + + @impl true + def handle_event("hide_add_field_dropdown", _params, socket) do + {:noreply, assign(socket, :show_add_field_dropdown, false)} + end + + @impl true + def handle_event("add_join_form_field", %{"field_id" => field_id}, socket) do + member_avail = socket.assigns.available_join_form_member_fields + custom_avail = socket.assigns.available_join_form_custom_fields + current = socket.assigns.join_form_fields + + field_to_add = + Enum.find(member_avail, &(&1.id == field_id)) || + Enum.find(custom_avail, &(&1.id == field_id)) + + socket = + if field_to_add do + full_field = %{ + id: field_to_add.id, + label: field_to_add.label, + type: field_to_add.type, + required: false, + can_remove: field_to_add.id != "email", + can_toggle_required: field_to_add.id != "email" + } + + new_fields = current ++ [full_field] + new_member = Enum.reject(member_avail, &(&1.id == field_id)) + new_custom = Enum.reject(custom_avail, &(&1.id == field_id)) + + socket + |> assign(:join_form_fields, new_fields) + |> assign(:available_join_form_member_fields, new_member) + |> assign(:available_join_form_custom_fields, new_custom) + |> assign(:show_add_field_dropdown, false) + else + socket + end + + {:noreply, persist_join_form_settings(socket)} + end + + @impl true + def handle_event("remove_join_form_field", %{"field_id" => field_id}, socket) do + if field_id == "email" do + {:noreply, socket} + else + current = socket.assigns.join_form_fields + custom_fields = socket.assigns.join_form_custom_fields + new_fields = Enum.reject(current, &(&1.id == field_id)) + new_ids = Enum.map(new_fields, & &1.id) + %{member_fields: new_member, custom_fields: new_custom} = + build_available_join_form_fields(new_ids, custom_fields) + + socket = + socket + |> assign(:join_form_fields, new_fields) + |> assign(:available_join_form_member_fields, new_member) + |> assign(:available_join_form_custom_fields, new_custom) + |> persist_join_form_settings() + + {:noreply, socket} + end + end + + @impl true + def handle_event("toggle_join_form_field_required", %{"field_id" => "email"}, socket) do + {:noreply, socket} + end + + @impl true + def handle_event("toggle_join_form_field_required", %{"field_id" => field_id}, socket) do + new_fields = + Enum.map(socket.assigns.join_form_fields, &toggle_required_if_matches(&1, field_id)) + + socket = assign(socket, :join_form_fields, new_fields) |> persist_join_form_settings() + {:noreply, socket} + end + + defp persist_join_form_settings(socket) do + settings = socket.assigns.settings + field_ids = Enum.map(socket.assigns.join_form_fields, & &1.id) + + required_map = + socket.assigns.join_form_fields + |> Map.new(fn field -> {field.id, field.required} end) + + attrs = %{ + join_form_enabled: socket.assigns.join_form_enabled, + join_form_field_ids: field_ids, + join_form_field_required: required_map + } + + case Membership.update_settings(settings, attrs) do + {:ok, updated_settings} -> + custom_fields = socket.assigns.join_form_custom_fields + + socket + |> assign(:settings, updated_settings) + |> assign_join_form_state(updated_settings, custom_fields) + + {:error, _error} -> + put_flash(socket, :error, gettext("Could not save join form settings.")) + end + end + @vereinfacht_param_keys ~w[vereinfacht_api_url vereinfacht_api_key vereinfacht_club_id vereinfacht_app_url] defp vereinfacht_params?(params) when is_map(params) do @@ -709,4 +980,94 @@ defmodule MvWeb.GlobalSettingsLive do """ end + + # ---- Join form helper functions ---- + + defp assign_join_form_state(socket, settings, custom_fields) do + enabled = settings.join_form_enabled || false + raw_ids = settings.join_form_field_ids || [] + field_ids = if "email" in raw_ids, do: raw_ids, else: ["email" | raw_ids] + required_config = settings.join_form_field_required || %{} + + join_form_fields = build_join_form_fields(field_ids, required_config, custom_fields) + %{member_fields: member_avail, custom_fields: custom_avail} = + build_available_join_form_fields(field_ids, custom_fields) + + socket + |> assign(:join_form_enabled, enabled) + |> assign(:join_form_fields, join_form_fields) + |> assign(:available_join_form_member_fields, member_avail) + |> assign(:available_join_form_custom_fields, custom_avail) + |> assign(:show_add_field_dropdown, false) + |> assign(:join_form_custom_fields, custom_fields) + end + + defp build_join_form_fields(field_ids, required_config, custom_fields) do + Enum.map(field_ids, fn id -> + label = join_form_field_label(id, custom_fields) + required = if id == "email", do: true, else: Map.get(required_config, id, false) + type = if id in member_field_id_strings(), do: :member_field, else: :custom_field + + %{ + id: id, + label: label, + required: required, + can_remove: id != "email", + can_toggle_required: id != "email", + type: type + } + end) + end + + defp build_available_join_form_fields(selected_ids, custom_fields) do + member_fields = + Mv.Constants.member_fields() + |> Enum.reject(fn field -> Atom.to_string(field) in selected_ids end) + |> Enum.map(fn field -> + %{id: Atom.to_string(field), label: MemberFields.label(field), type: :member_field} + end) + + custom_field_entries = + custom_fields + |> Enum.reject(fn cf -> cf.id in selected_ids end) + |> Enum.map(fn cf -> + %{id: cf.id, label: cf.name, type: :custom_field} + end) + |> Enum.sort_by(& &1.label) + + %{member_fields: member_fields, custom_fields: custom_field_entries} + end + + defp join_form_field_label(id, custom_fields) do + if id in member_field_id_strings() do + MemberFields.label(String.to_existing_atom(id)) + else + case Enum.find(custom_fields, &(&1.id == id)) do + nil -> id + cf -> cf.name + end + end + end + + defp toggle_required_if_matches(%{id: id} = field, id), + do: Map.put(field, :required, not field.required) + + defp toggle_required_if_matches(field, _field_id), do: field + + defp member_field_id_strings do + Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + end + + defp load_custom_fields(nil), do: [] + + defp load_custom_fields(actor) do + case Ash.read(Mv.Membership.CustomField, + actor: actor, + domain: Mv.Membership, + authorize?: true + ) do + {:ok, fields} -> fields + {:error, _} -> [] + end + end end diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 7015d9c..01c49f6 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -357,6 +357,7 @@ msgstr "Passwort-Authentifizierung" #: lib/mv_web/live/custom_field_live/form_component.ex #: lib/mv_web/live/custom_field_live/index_component.ex +#: lib/mv_web/live/global_settings_live.ex #: lib/mv_web/live/member_field_live/form_component.ex #: lib/mv_web/live/member_field_live/index_component.ex #, elixir-autogen, elixir-format @@ -2149,6 +2150,7 @@ msgstr "Mitglied ist nicht in dieser Gruppe." msgid "No email" msgstr "Keine E-Mail" +#: lib/mv_web/live/global_settings_live.ex #: lib/mv_web/live/group_live/show.ex #, elixir-autogen, elixir-format msgid "Remove" @@ -3324,3 +3326,68 @@ msgstr "Unvollständig" #, elixir-autogen, elixir-format, fuzzy msgid "These fields are necessary for MILA to handle member identification and payment calculations in the future. Thus you cannot delete these fields but hide them in the member overview." msgstr "Diese Datenfelder sind für MILA notwendig, um Mitglieder zu identifizieren und zukünftig Beitragszahlungen zu berechnen. Aus diesem Grund können sie nicht gelöscht, aber in der Übersicht ausgeblendet werden." + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Add field" +msgstr "Feld hinzufügen" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Available fields" +msgstr "Verfügbare Felder" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Configure the public join form that allows new members to submit a join request." +msgstr "Konfiguriere das öffentliche Beitrittsformular, über das neue Mitglieder einen Beitrittsantrag stellen können." + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Could not save join form settings." +msgstr "Beitrittsformular-Einstellungen konnten nicht gespeichert werden." + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Field" +msgstr "Feld" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Fields on the join form" +msgstr "Felder im Beitrittsformular" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Join Form" +msgstr "Beitrittsformular" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Join form enabled" +msgstr "Beitrittsformular aktiv" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "No fields selected. Add at least the email field." +msgstr "Keine Felder ausgewählt. Füge mindestens das E-Mail-Feld hinzu." + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Remove field %{label}" +msgstr "Feld %{label} entfernen" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Board approval required (in development)" +msgstr "Bestätigung durch Vorstand erforderlich (in Entwicklung)" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Personal data" +msgstr "Persönliche Daten" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Individual fields" +msgstr "Individuelle Felder" diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 14db165..b5ab449 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -358,6 +358,7 @@ msgstr "" #: lib/mv_web/live/custom_field_live/form_component.ex #: lib/mv_web/live/custom_field_live/index_component.ex +#: lib/mv_web/live/global_settings_live.ex #: lib/mv_web/live/member_field_live/form_component.ex #: lib/mv_web/live/member_field_live/index_component.ex #, elixir-autogen, elixir-format @@ -2150,6 +2151,7 @@ msgstr "" msgid "No email" msgstr "" +#: lib/mv_web/live/global_settings_live.ex #: lib/mv_web/live/group_live/show.ex #, elixir-autogen, elixir-format msgid "Remove" @@ -3324,3 +3326,58 @@ msgstr "" #, elixir-autogen, elixir-format msgid "These fields are necessary for MILA to handle member identification and payment calculations in the future. Thus you cannot delete these fields but hide them in the member overview." msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Add field" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Available fields" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Configure the public join form that allows new members to submit a join request." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Could not save join form settings." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Field" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Fields on the join form" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Join Form" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Join form enabled" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "No fields selected. Add at least the email field." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Remove field %{label}" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Board approval required (in development)" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 7dc5068..5556d10 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -358,6 +358,7 @@ msgstr "" #: lib/mv_web/live/custom_field_live/form_component.ex #: lib/mv_web/live/custom_field_live/index_component.ex +#: lib/mv_web/live/global_settings_live.ex #: lib/mv_web/live/member_field_live/form_component.ex #: lib/mv_web/live/member_field_live/index_component.ex #, elixir-autogen, elixir-format @@ -2150,6 +2151,7 @@ msgstr "" msgid "No email" msgstr "" +#: lib/mv_web/live/global_settings_live.ex #: lib/mv_web/live/group_live/show.ex #, elixir-autogen, elixir-format msgid "Remove" @@ -3324,3 +3326,68 @@ msgstr "" #, elixir-autogen, elixir-format, fuzzy msgid "These fields are necessary for MILA to handle member identification and payment calculations in the future. Thus you cannot delete these fields but hide them in the member overview." msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Add field" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Available fields" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Configure the public join form that allows new members to submit a join request." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Could not save join form settings." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Field" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Fields on the join form" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Join Form" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Join form enabled" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "No fields selected. Add at least the email field." +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Remove field %{label}" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Board approval required (in development)" +msgstr "Board approval required (in development)" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Personal data" +msgstr "Personal data" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Individual fields" +msgstr "Individual fields" diff --git a/priv/repo/migrations/20260310114701_add_join_form_settings_to_settings.exs b/priv/repo/migrations/20260310114701_add_join_form_settings_to_settings.exs new file mode 100644 index 0000000..225c05e --- /dev/null +++ b/priv/repo/migrations/20260310114701_add_join_form_settings_to_settings.exs @@ -0,0 +1,27 @@ +defmodule Mv.Repo.Migrations.AddJoinFormSettingsToSettings do + @moduledoc """ + Adds join form configuration columns to the settings table. + + - join_form_enabled: whether the public /join page is active + - join_form_field_ids: ordered list of field IDs shown on the join form (JSONB array) + - join_form_field_required: map of field ID => required boolean (JSONB) + """ + + use Ecto.Migration + + def up do + alter table(:settings) do + add :join_form_enabled, :boolean, default: false, null: false + add :join_form_field_ids, {:array, :string} + add :join_form_field_required, :map + end + end + + def down do + alter table(:settings) do + remove :join_form_enabled + remove :join_form_field_ids + remove :join_form_field_required + end + end +end diff --git a/test/membership/setting_join_form_test.exs b/test/membership/setting_join_form_test.exs index 9b15ca4..bcafe9f 100644 --- a/test/membership/setting_join_form_test.exs +++ b/test/membership/setting_join_form_test.exs @@ -26,8 +26,17 @@ defmodule Mv.Membership.SettingJoinFormTest do on_exit(fn -> {:ok, s} = Membership.get_settings() attrs = %{} - attrs = if saved_enabled != nil, do: Map.put(attrs, :join_form_enabled, saved_enabled), else: attrs - attrs = if saved_ids != nil, do: Map.put(attrs, :join_form_field_ids, saved_ids || []), else: attrs + + attrs = + if saved_enabled != nil, + do: Map.put(attrs, :join_form_enabled, saved_enabled), + else: attrs + + attrs = + if saved_ids != nil, + do: Map.put(attrs, :join_form_field_ids, saved_ids || []), + else: attrs + attrs = if saved_required != nil, do: Map.put(attrs, :join_form_field_required, saved_required || %{}), @@ -57,6 +66,7 @@ defmodule Mv.Membership.SettingJoinFormTest do describe "join form settings persistence and loading" do test "save and load join_form_enabled plus field selection and required flags returns same config" do {:ok, settings} = Membership.get_settings() + attrs = %{ join_form_enabled: true, join_form_field_ids: ["email", "first_name"], @@ -78,17 +88,20 @@ defmodule Mv.Membership.SettingJoinFormTest do test "repeated save with changed field list overwrites config without leftovers" do {:ok, settings} = Membership.get_settings() - assert {:ok, _} = update_join_form_settings(settings, %{ - join_form_enabled: true, - join_form_field_ids: ["email", "first_name"], - join_form_field_required: %{"email" => true, "first_name" => false} - }) - assert {:ok, updated} = update_join_form_settings(settings, %{ - join_form_enabled: true, - join_form_field_ids: ["email", "last_name"], - join_form_field_required: %{"email" => true, "last_name" => false} - }) + assert {:ok, _} = + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name"], + join_form_field_required: %{"email" => true, "first_name" => false} + }) + + assert {:ok, updated} = + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "last_name"], + join_form_field_required: %{"email" => true, "last_name" => false} + }) assert updated.join_form_field_ids == ["email", "last_name"] assert Map.has_key?(updated.join_form_field_required, "last_name") @@ -102,11 +115,12 @@ defmodule Mv.Membership.SettingJoinFormTest do test "only existing member fields or custom field ids are accepted; unknown field names rejected or sanitized" do {:ok, settings} = Membership.get_settings() - result = update_join_form_settings(settings, %{ - join_form_enabled: true, - join_form_field_ids: ["email", "not_a_member_field"], - join_form_field_required: %{"email" => true, "not_a_member_field" => false} - }) + result = + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "not_a_member_field"], + join_form_field_required: %{"email" => true, "not_a_member_field" => false} + }) # Until attributes exist we get NoSuchInput; once implemented we expect validation error assert {:error, _} = result @@ -115,16 +129,18 @@ defmodule Mv.Membership.SettingJoinFormTest do test "config without email is rejected or email is auto-added and required" do {:ok, settings} = Membership.get_settings() - result = update_join_form_settings(settings, %{ - join_form_enabled: true, - join_form_field_ids: ["first_name", "last_name"], - join_form_field_required: %{"first_name" => true, "last_name" => false} - }) + result = + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["first_name", "last_name"], + join_form_field_required: %{"first_name" => true, "last_name" => false} + }) # Either rejected or, when loaded, email must be present and required case result do {:error, _} -> :ok + {:ok, updated} -> assert "email" in updated.join_form_field_ids assert updated.join_form_field_required["email"] == true @@ -134,11 +150,12 @@ defmodule Mv.Membership.SettingJoinFormTest do test "required false for email is ignored or forced to true when saved" do {:ok, settings} = Membership.get_settings() - {:ok, updated} = update_join_form_settings(settings, %{ - join_form_enabled: true, - join_form_field_ids: ["email", "first_name"], - join_form_field_required: %{"email" => false, "first_name" => false} - }) + {:ok, updated} = + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email", "first_name"], + join_form_field_required: %{"email" => false, "first_name" => false} + }) assert updated.join_form_field_required["email"] == true end @@ -146,15 +163,17 @@ defmodule Mv.Membership.SettingJoinFormTest do test "required flag for field not in join_form_field_ids is rejected or dropped" do {:ok, settings} = Membership.get_settings() - result = update_join_form_settings(settings, %{ - join_form_enabled: true, - join_form_field_ids: ["email"], - join_form_field_required: %{"email" => true, "first_name" => true} - }) + result = + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: ["email"], + join_form_field_required: %{"email" => true, "first_name" => true} + }) case result do {:error, _} -> :ok + {:ok, updated} -> refute Map.has_key?(updated.join_form_field_required, "first_name") end @@ -166,6 +185,7 @@ defmodule Mv.Membership.SettingJoinFormTest do describe "join form allowlist" do test "allowlist returns configured fields with required/optional when join form enabled" do {:ok, settings} = Membership.get_settings() + update_join_form_settings(settings, %{ join_form_enabled: true, join_form_field_ids: ["email", "first_name"], @@ -175,8 +195,8 @@ defmodule Mv.Membership.SettingJoinFormTest do allowlist = Membership.get_join_form_allowlist() assert length(allowlist) == 2 - email_entry = Enum.find(allowlist, &( &1.id == "email" )) - first_name_entry = Enum.find(allowlist, &( &1.id == "first_name" )) + email_entry = Enum.find(allowlist, &(&1.id == "email")) + first_name_entry = Enum.find(allowlist, &(&1.id == "first_name")) assert email_entry.required == true assert first_name_entry.required == false assert email_entry.type == :member_field @@ -185,6 +205,7 @@ defmodule Mv.Membership.SettingJoinFormTest do test "allowlist returns empty or defined default when join form disabled" do {:ok, settings} = Membership.get_settings() + update_join_form_settings(settings, %{ join_form_enabled: false, join_form_field_ids: ["email", "first_name"], @@ -200,11 +221,13 @@ defmodule Mv.Membership.SettingJoinFormTest do test "allowlist distinguishes member fields and custom field identifiers" do {:ok, settings} = Membership.get_settings() actor = SystemActor.get_system_actor() + {:ok, cf} = Membership.create_custom_field( %{name: "join_cf_#{System.unique_integer([:positive])}", value_type: :string}, actor: actor ) + update_join_form_settings(settings, %{ join_form_enabled: true, join_form_field_ids: ["email", cf.id], @@ -213,8 +236,8 @@ defmodule Mv.Membership.SettingJoinFormTest do allowlist = Membership.get_join_form_allowlist() - email_entry = Enum.find(allowlist, &( &1.id == "email" )) - cf_entry = Enum.find(allowlist, &( &1.id == cf.id )) + email_entry = Enum.find(allowlist, &(&1.id == "email")) + cf_entry = Enum.find(allowlist, &(&1.id == cf.id)) assert email_entry.type == :member_field assert cf_entry.type == :custom_field end @@ -252,11 +275,12 @@ defmodule Mv.Membership.SettingJoinFormTest do test "invalid or unexpected payload structure yields clean error or ignores unknown keys" do {:ok, settings} = Membership.get_settings() - result = update_join_form_settings(settings, %{ - join_form_enabled: true, - join_form_field_ids: "not_a_list", - join_form_field_required: %{} - }) + result = + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: "not_a_list", + join_form_field_required: %{} + }) assert match?({:error, _}, result) or (match?({:ok, _}, result) && elem(result, 1).join_form_field_ids != "not_a_list") @@ -267,11 +291,12 @@ defmodule Mv.Membership.SettingJoinFormTest do all_member = Constants.member_fields() |> Enum.map(&to_string/1) required_map = Map.new(all_member, fn f -> {f, f == "email"} end) - assert {:ok, updated} = update_join_form_settings(settings, %{ - join_form_enabled: true, - join_form_field_ids: all_member, - join_form_field_required: required_map - }) + assert {:ok, updated} = + update_join_form_settings(settings, %{ + join_form_enabled: true, + join_form_field_ids: all_member, + join_form_field_required: required_map + }) assert length(updated.join_form_field_ids) == length(all_member) {:ok, reloaded} = Membership.get_settings() From 05e2a298febf5396ec53a490876fdb7730b7e1dc Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 10 Mar 2026 15:40:28 +0100 Subject: [PATCH 3/4] feat: add accessible drag&drop table component --- assets/css/app.css | 31 +++++ assets/js/app.js | 136 +++++++++++++++++++++ assets/vendor/sortable.js | 2 + lib/mv_web/components/core_components.ex | 118 ++++++++++++++++++ lib/mv_web/live/global_settings_live.ex | 54 +++++++- priv/gettext/de/LC_MESSAGES/default.po | 18 ++- priv/gettext/default.pot | 20 +++ priv/gettext/en/LC_MESSAGES/default.po | 18 ++- test/membership/setting_join_form_test.exs | 4 +- 9 files changed, 386 insertions(+), 15 deletions(-) create mode 100644 assets/vendor/sortable.js diff --git a/assets/css/app.css b/assets/css/app.css index 4b28fb7..28ea24b 100644 --- a/assets/css/app.css +++ b/assets/css/app.css @@ -656,3 +656,34 @@ } /* This file is for your main application CSS */ + +/* ============================================ + SortableList: drag-and-drop table rows + ============================================ */ + +/* Ghost row: placeholder showing where the dragged item will be dropped. + Background fills the gap; text invisible so layout matches original row. */ +.sortable-ghost { + background-color: var(--color-base-300) !important; + opacity: 0.5; +} +.sortable-ghost td { + border-color: transparent !important; +} + +/* Chosen row: the row being actively dragged (follows the cursor). */ +.sortable-chosen { + background-color: var(--color-base-200); + box-shadow: 0 4px 16px -2px oklch(0 0 0 / 0.18); + cursor: grabbing !important; +} + +/* Drag handle button: only grab cursor, no hover effect for mouse users. + Keyboard outline is handled via JS outline style. */ +[data-sortable-handle] button { + cursor: grab; +} +[data-sortable-handle] button:hover { + background-color: transparent !important; + color: inherit; +} diff --git a/assets/js/app.js b/assets/js/app.js index b7d1a45..3c4e0f9 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -21,6 +21,7 @@ import "phoenix_html" import {Socket} from "phoenix" import {LiveSocket} from "phoenix_live_view" import topbar from "../vendor/topbar" +import Sortable from "../vendor/sortable" let csrfToken = document.querySelector("meta[name='csrf-token']").getAttribute("content") @@ -120,6 +121,141 @@ Hooks.TabListKeydown = { } } +// SortableList hook: Accessible reorderable table/list. +// Mouse drag: SortableJS (smooth animation, ghost row, items push apart). +// Keyboard: Space = grab/drop, Arrow up/down = move, Escape = cancel, matching the Salesforce a11y pattern. +// Container must have data-reorder-event and data-list-id. +// Each row (tr) must have data-row-index; locked rows have data-locked="true". +// Pushes event with { from_index, to_index } (both integers) on reorder. +Hooks.SortableList = { + mounted() { + this.reorderEvent = this.el.dataset.reorderEvent + this.listId = this.el.dataset.listId + // Keyboard state: store grabbed row id so it survives LiveView re-renders + this.grabbedRowId = null + + this.announcementEl = this.listId ? document.getElementById(this.listId + "-announcement") : null + const announce = (msg) => { + if (!this.announcementEl) return + // Clear then re-set to force screen reader re-read + this.announcementEl.textContent = "" + setTimeout(() => { if (this.announcementEl) this.announcementEl.textContent = msg }, 50) + } + + const tbody = this.el.querySelector("tbody") + if (!tbody) return + + this.getRows = () => Array.from(tbody.querySelectorAll("tr")) + this.getRowIndex = (tr) => { + const idx = tr.getAttribute("data-row-index") + return idx != null ? parseInt(idx, 10) : -1 + } + this.isLocked = (tr) => tr.getAttribute("data-locked") === "true" + + // SortableJS for mouse drag-and-drop with animation + this.sortable = new Sortable(tbody, { + animation: 150, + handle: "[data-sortable-handle]", + // Disable sorting for locked rows (first row = email) + filter: "[data-locked='true']", + preventOnFilter: true, + // Ghost (placeholder showing where the item will land) + ghostClass: "sortable-ghost", + // The item being dragged + chosenClass: "sortable-chosen", + // Cursor while dragging + dragClass: "sortable-drag", + // Don't trigger on handle area clicks (only actual drag) + delay: 0, + onEnd: (e) => { + if (e.oldIndex === e.newIndex) return + this.pushEvent(this.reorderEvent, { from_index: e.oldIndex, to_index: e.newIndex }) + announce(`Dropped. Position ${e.newIndex + 1} of ${this.getRows().length}.`) + // LiveView will reconcile the DOM order after re-render + } + }) + + // Keyboard handler (Salesforce a11y pattern: Space=grab/drop, Arrows=move, Escape=cancel) + this.handleKeyDown = (e) => { + // Don't intercept Space on interactive elements (checkboxes, buttons, inputs) + const tag = e.target.tagName + if (tag === "INPUT" || tag === "BUTTON" || tag === "SELECT" || tag === "TEXTAREA") return + + const tr = e.target.closest("tr") + if (!tr || this.isLocked(tr)) return + const rows = this.getRows() + const idx = this.getRowIndex(tr) + if (idx < 0) return + const total = rows.length + + if (e.key === " ") { + e.preventDefault() + const rowId = tr.id + if (this.grabbedRowId === rowId) { + // Drop + this.grabbedRowId = null + tr.style.outline = "" + announce(`Dropped. Position ${idx + 1} of ${total}.`) + } else { + // Grab + this.grabbedRowId = rowId + tr.style.outline = "2px solid var(--color-primary)" + tr.style.outlineOffset = "-2px" + announce(`Grabbed. Position ${idx + 1} of ${total}. Use arrow keys to move, Space to drop, Escape to cancel.`) + } + return + } + + if (e.key === "Escape") { + if (this.grabbedRowId != null) { + e.preventDefault() + const grabbedTr = document.getElementById(this.grabbedRowId) + if (grabbedTr) { grabbedTr.style.outline = ""; grabbedTr.style.outlineOffset = "" } + this.grabbedRowId = null + announce("Reorder cancelled.") + } + return + } + + if (this.grabbedRowId == null) return + + if (e.key === "ArrowUp" && idx > 0) { + e.preventDefault() + this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx - 1 }) + announce(`Position ${idx} of ${total}.`) + } else if (e.key === "ArrowDown" && idx < total - 1) { + e.preventDefault() + this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx + 1 }) + announce(`Position ${idx + 2} of ${total}.`) + } + } + + this.el.addEventListener("keydown", this.handleKeyDown, true) + }, + + updated() { + // Re-apply keyboard outline and restore focus after LiveView re-render. + // LiveView DOM patching loses focus; without explicit re-focus the next keypress + // goes to document.body (Space scrolls the page instead of triggering our handler). + if (this.grabbedRowId) { + const tr = document.getElementById(this.grabbedRowId) + if (tr) { + tr.style.outline = "2px solid var(--color-primary)" + tr.style.outlineOffset = "-2px" + tr.focus() + } else { + // Row no longer exists (removed while grabbed), clear state + this.grabbedRowId = null + } + } + }, + + destroyed() { + if (this.sortable) this.sortable.destroy() + this.el.removeEventListener("keydown", this.handleKeyDown, true) + } +} + // SidebarState hook: Manages sidebar expanded/collapsed state Hooks.SidebarState = { mounted() { diff --git a/assets/vendor/sortable.js b/assets/vendor/sortable.js new file mode 100644 index 0000000..95423a6 --- /dev/null +++ b/assets/vendor/sortable.js @@ -0,0 +1,2 @@ +/*! Sortable 1.15.6 - MIT | git://github.com/SortableJS/Sortable.git */ +!function(t,e){"object"==typeof exports&&"undefined"!=typeof module?module.exports=e():"function"==typeof define&&define.amd?define(e):(t=t||self).Sortable=e()}(this,function(){"use strict";function e(e,t){var n,o=Object.keys(e);return Object.getOwnPropertySymbols&&(n=Object.getOwnPropertySymbols(e),t&&(n=n.filter(function(t){return Object.getOwnPropertyDescriptor(e,t).enumerable})),o.push.apply(o,n)),o}function I(o){for(var t=1;tt.length)&&(e=t.length);for(var n=0,o=new Array(e);n"===e[0]&&(e=e.substring(1)),t))try{if(t.matches)return t.matches(e);if(t.msMatchesSelector)return t.msMatchesSelector(e);if(t.webkitMatchesSelector)return t.webkitMatchesSelector(e)}catch(t){return}}function g(t){return t.host&&t!==document&&t.host.nodeType?t.host:t.parentNode}function P(t,e,n,o){if(t){n=n||document;do{if(null!=e&&(">"!==e[0]||t.parentNode===n)&&f(t,e)||o&&t===n)return t}while(t!==n&&(t=g(t)))}return null}var m,v=/\s+/g;function k(t,e,n){var o;t&&e&&(t.classList?t.classList[n?"add":"remove"](e):(o=(" "+t.className+" ").replace(v," ").replace(" "+e+" "," "),t.className=(o+(n?" "+e:"")).replace(v," ")))}function R(t,e,n){var o=t&&t.style;if(o){if(void 0===n)return document.defaultView&&document.defaultView.getComputedStyle?n=document.defaultView.getComputedStyle(t,""):t.currentStyle&&(n=t.currentStyle),void 0===e?n:n[e];o[e=!(e in o||-1!==e.indexOf("webkit"))?"-webkit-"+e:e]=n+("string"==typeof n?"":"px")}}function b(t,e){var n="";if("string"==typeof t)n=t;else do{var o=R(t,"transform")}while(o&&"none"!==o&&(n=o+" "+n),!e&&(t=t.parentNode));var i=window.DOMMatrix||window.WebKitCSSMatrix||window.CSSMatrix||window.MSCSSMatrix;return i&&new i(n)}function D(t,e,n){if(t){var o=t.getElementsByTagName(e),i=0,r=o.length;if(n)for(;i=n.left-e&&i<=n.right+e,e=r>=n.top-e&&r<=n.bottom+e;return o&&e?a=t:void 0}}),a);if(e){var n,o={};for(n in t)t.hasOwnProperty(n)&&(o[n]=t[n]);o.target=o.rootEl=e,o.preventDefault=void 0,o.stopPropagation=void 0,e[K]._onDragOver(o)}}var i,r,a}function Ft(t){Z&&Z.parentNode[K]._isOutsideThisEl(t.target)}function jt(t,e){if(!t||!t.nodeType||1!==t.nodeType)throw"Sortable: `el` must be an HTMLElement, not ".concat({}.toString.call(t));this.el=t,this.options=e=a({},e),t[K]=this;var n,o,i={group:null,sort:!0,disabled:!1,store:null,handle:null,draggable:/^[uo]l$/i.test(t.nodeName)?">li":">*",swapThreshold:1,invertSwap:!1,invertedSwapThreshold:null,removeCloneOnHide:!0,direction:function(){return kt(t,this.options)},ghostClass:"sortable-ghost",chosenClass:"sortable-chosen",dragClass:"sortable-drag",ignore:"a, img",filter:null,preventOnFilter:!0,animation:0,easing:null,setData:function(t,e){t.setData("Text",e.textContent)},dropBubble:!1,dragoverBubble:!1,dataIdAttr:"data-id",delay:0,delayOnTouchOnly:!1,touchStartThreshold:(Number.parseInt?Number:window).parseInt(window.devicePixelRatio,10)||1,forceFallback:!1,fallbackClass:"sortable-fallback",fallbackOnBody:!1,fallbackTolerance:0,fallbackOffset:{x:0,y:0},supportPointer:!1!==jt.supportPointer&&"PointerEvent"in window&&(!u||c),emptyInsertThreshold:5};for(n in z.initializePlugins(this,t,i),i)n in e||(e[n]=i[n]);for(o in Rt(e),this)"_"===o.charAt(0)&&"function"==typeof this[o]&&(this[o]=this[o].bind(this));this.nativeDraggable=!e.forceFallback&&It,this.nativeDraggable&&(this.options.touchStartThreshold=1),e.supportPointer?h(t,"pointerdown",this._onTapStart):(h(t,"mousedown",this._onTapStart),h(t,"touchstart",this._onTapStart)),this.nativeDraggable&&(h(t,"dragover",this),h(t,"dragenter",this)),St.push(this.el),e.store&&e.store.get&&this.sort(e.store.get(this)||[]),a(this,A())}function Ht(t,e,n,o,i,r,a,l){var s,c,u=t[K],d=u.options.onMove;return!window.CustomEvent||y||w?(s=document.createEvent("Event")).initEvent("move",!0,!0):s=new CustomEvent("move",{bubbles:!0,cancelable:!0}),s.to=e,s.from=t,s.dragged=n,s.draggedRect=o,s.related=i||e,s.relatedRect=r||X(e),s.willInsertAfter=l,s.originalEvent=a,t.dispatchEvent(s),c=d?d.call(u,s,a):c}function Lt(t){t.draggable=!1}function Kt(){xt=!1}function Wt(t){return setTimeout(t,0)}function zt(t){return clearTimeout(t)}jt.prototype={constructor:jt,_isOutsideThisEl:function(t){this.el.contains(t)||t===this.el||(vt=null)},_getDirection:function(t,e){return"function"==typeof this.options.direction?this.options.direction.call(this,t,e,Z):this.options.direction},_onTapStart:function(e){if(e.cancelable){var n=this,o=this.el,t=this.options,i=t.preventOnFilter,r=e.type,a=e.touches&&e.touches[0]||e.pointerType&&"touch"===e.pointerType&&e,l=(a||e).target,s=e.target.shadowRoot&&(e.path&&e.path[0]||e.composedPath&&e.composedPath()[0])||l,c=t.filter;if(!function(t){Ot.length=0;var e=t.getElementsByTagName("input"),n=e.length;for(;n--;){var o=e[n];o.checked&&Ot.push(o)}}(o),!Z&&!(/mousedown|pointerdown/.test(r)&&0!==e.button||t.disabled)&&!s.isContentEditable&&(this.nativeDraggable||!u||!l||"SELECT"!==l.tagName.toUpperCase())&&!((l=P(l,t.draggable,o,!1))&&l.animated||et===l)){if(it=j(l),at=j(l,t.draggable),"function"==typeof c){if(c.call(this,e,l,this))return V({sortable:n,rootEl:s,name:"filter",targetEl:l,toEl:o,fromEl:o}),U("filter",n,{evt:e}),void(i&&e.preventDefault())}else if(c=c&&c.split(",").some(function(t){if(t=P(s,t.trim(),o,!1))return V({sortable:n,rootEl:t,name:"filter",targetEl:l,fromEl:o,toEl:o}),U("filter",n,{evt:e}),!0}))return void(i&&e.preventDefault());t.handle&&!P(s,t.handle,o,!1)||this._prepareDragStart(e,a,l)}}},_prepareDragStart:function(t,e,n){var o,i=this,r=i.el,a=i.options,l=r.ownerDocument;n&&!Z&&n.parentNode===r&&(o=X(n),J=r,$=(Z=n).parentNode,tt=Z.nextSibling,et=n,st=a.group,ut={target:jt.dragged=Z,clientX:(e||t).clientX,clientY:(e||t).clientY},ft=ut.clientX-o.left,gt=ut.clientY-o.top,this._lastX=(e||t).clientX,this._lastY=(e||t).clientY,Z.style["will-change"]="all",o=function(){U("delayEnded",i,{evt:t}),jt.eventCanceled?i._onDrop():(i._disableDelayedDragEvents(),!s&&i.nativeDraggable&&(Z.draggable=!0),i._triggerDragStart(t,e),V({sortable:i,name:"choose",originalEvent:t}),k(Z,a.chosenClass,!0))},a.ignore.split(",").forEach(function(t){D(Z,t.trim(),Lt)}),h(l,"dragover",Bt),h(l,"mousemove",Bt),h(l,"touchmove",Bt),a.supportPointer?(h(l,"pointerup",i._onDrop),this.nativeDraggable||h(l,"pointercancel",i._onDrop)):(h(l,"mouseup",i._onDrop),h(l,"touchend",i._onDrop),h(l,"touchcancel",i._onDrop)),s&&this.nativeDraggable&&(this.options.touchStartThreshold=4,Z.draggable=!0),U("delayStart",this,{evt:t}),!a.delay||a.delayOnTouchOnly&&!e||this.nativeDraggable&&(w||y)?o():jt.eventCanceled?this._onDrop():(a.supportPointer?(h(l,"pointerup",i._disableDelayedDrag),h(l,"pointercancel",i._disableDelayedDrag)):(h(l,"mouseup",i._disableDelayedDrag),h(l,"touchend",i._disableDelayedDrag),h(l,"touchcancel",i._disableDelayedDrag)),h(l,"mousemove",i._delayedDragTouchMoveHandler),h(l,"touchmove",i._delayedDragTouchMoveHandler),a.supportPointer&&h(l,"pointermove",i._delayedDragTouchMoveHandler),i._dragStartTimer=setTimeout(o,a.delay)))},_delayedDragTouchMoveHandler:function(t){t=t.touches?t.touches[0]:t;Math.max(Math.abs(t.clientX-this._lastX),Math.abs(t.clientY-this._lastY))>=Math.floor(this.options.touchStartThreshold/(this.nativeDraggable&&window.devicePixelRatio||1))&&this._disableDelayedDrag()},_disableDelayedDrag:function(){Z&&Lt(Z),clearTimeout(this._dragStartTimer),this._disableDelayedDragEvents()},_disableDelayedDragEvents:function(){var t=this.el.ownerDocument;p(t,"mouseup",this._disableDelayedDrag),p(t,"touchend",this._disableDelayedDrag),p(t,"touchcancel",this._disableDelayedDrag),p(t,"pointerup",this._disableDelayedDrag),p(t,"pointercancel",this._disableDelayedDrag),p(t,"mousemove",this._delayedDragTouchMoveHandler),p(t,"touchmove",this._delayedDragTouchMoveHandler),p(t,"pointermove",this._delayedDragTouchMoveHandler)},_triggerDragStart:function(t,e){e=e||"touch"==t.pointerType&&t,!this.nativeDraggable||e?this.options.supportPointer?h(document,"pointermove",this._onTouchMove):h(document,e?"touchmove":"mousemove",this._onTouchMove):(h(Z,"dragend",this),h(J,"dragstart",this._onDragStart));try{document.selection?Wt(function(){document.selection.empty()}):window.getSelection().removeAllRanges()}catch(t){}},_dragStarted:function(t,e){var n;Dt=!1,J&&Z?(U("dragStarted",this,{evt:e}),this.nativeDraggable&&h(document,"dragover",Ft),n=this.options,t||k(Z,n.dragClass,!1),k(Z,n.ghostClass,!0),jt.active=this,t&&this._appendGhost(),V({sortable:this,name:"start",originalEvent:e})):this._nulling()},_emulateDragOver:function(){if(dt){this._lastX=dt.clientX,this._lastY=dt.clientY,Xt();for(var t=document.elementFromPoint(dt.clientX,dt.clientY),e=t;t&&t.shadowRoot&&(t=t.shadowRoot.elementFromPoint(dt.clientX,dt.clientY))!==e;)e=t;if(Z.parentNode[K]._isOutsideThisEl(t),e)do{if(e[K])if(e[K]._onDragOver({clientX:dt.clientX,clientY:dt.clientY,target:t,rootEl:e})&&!this.options.dragoverBubble)break}while(e=g(t=e));Yt()}},_onTouchMove:function(t){if(ut){var e=this.options,n=e.fallbackTolerance,o=e.fallbackOffset,i=t.touches?t.touches[0]:t,r=Q&&b(Q,!0),a=Q&&r&&r.a,l=Q&&r&&r.d,e=At&&wt&&E(wt),a=(i.clientX-ut.clientX+o.x)/(a||1)+(e?e[0]-Tt[0]:0)/(a||1),l=(i.clientY-ut.clientY+o.y)/(l||1)+(e?e[1]-Tt[1]:0)/(l||1);if(!jt.active&&!Dt){if(n&&Math.max(Math.abs(i.clientX-this._lastX),Math.abs(i.clientY-this._lastY))E.right+10||S.clientY>x.bottom&&S.clientX>x.left:S.clientY>E.bottom+10||S.clientX>x.right&&S.clientY>x.top)||m.animated)){if(m&&(t=n,e=r,C=X(B((_=this).el,0,_.options,!0)),_=L(_.el,_.options,Q),e?t.clientX<_.left-10||t.clientY "join-field-\#{f.id}" end} + locked_ids={["join-field-email"]} + reorder_event="reorder_join_form_field" + > + <:col :let={field} label={gettext("Field")} class="min-w-[14rem]">{field.label} + <:col :let={field} label={gettext("Required")}>... + <:action :let={field}><.button>Remove + + """ + attr :id, :string, required: true + attr :rows, :list, required: true + attr :row_id, :any, required: true + attr :locked_ids, :list, default: [] + attr :reorder_event, :string, required: true + attr :row_item, :any, default: &Function.identity/1 + + slot :col, required: true do + attr :label, :string, required: true + attr :class, :string + end + + slot :action + + def sortable_table(assigns) do + assigns = assign(assigns, :locked_set, MapSet.new(assigns.locked_ids)) + + ~H""" +
+ + + + + + + + + + + + + + + + +
{col[:label]}{gettext("Actions")}
+ + + Enum.join(" ")} + > + {render_slot(col, @row_item.(row))} + + <%= for action <- @action do %> + {render_slot(action, @row_item.(row))} + <% end %> +
+
+ """ + end + @doc """ Renders a data list. diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index 651afc0..3c75fa8 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -160,7 +160,10 @@ defmodule MvWeb.GlobalSettingsLive do type="button" variant="primary" phx-click="toggle_add_field_dropdown" - disabled={Enum.empty?(@available_join_form_member_fields) and Enum.empty?(@available_join_form_custom_fields)} + disabled={ + Enum.empty?(@available_join_form_member_fields) and + Enum.empty?(@available_join_form_custom_fields) + } aria-haspopup="listbox" aria-expanded={to_string(@show_add_field_dropdown)} > @@ -190,7 +193,15 @@ defmodule MvWeb.GlobalSettingsLive do {field.label} -
+
{gettext("Individual fields")}
@@ -213,12 +224,13 @@ defmodule MvWeb.GlobalSettingsLive do {gettext("No fields selected. Add at least the email field.")}

- <%!-- Fields table (compact width) --%> + <%!-- Fields table (compact width, reorderable) --%>
- <.table + <.sortable_table id="join-form-fields-table" rows={@join_form_fields} row_id={fn field -> "join-field-#{field.id}" end} + reorder_event="reorder_join_form_field" > <:col :let={field} label={gettext("Field")} class="min-w-[14rem]"> {field.label} @@ -250,7 +262,10 @@ defmodule MvWeb.GlobalSettingsLive do - + +

+ {gettext("The order of rows determines the field order in the join form.")} +

@@ -642,6 +657,7 @@ defmodule MvWeb.GlobalSettingsLive do custom_fields = socket.assigns.join_form_custom_fields new_fields = Enum.reject(current, &(&1.id == field_id)) new_ids = Enum.map(new_fields, & &1.id) + %{member_fields: new_member, custom_fields: new_custom} = build_available_join_form_fields(new_ids, custom_fields) @@ -670,6 +686,27 @@ defmodule MvWeb.GlobalSettingsLive do {:noreply, socket} end + @impl true + def handle_event( + "reorder_join_form_field", + %{"from_index" => from_idx, "to_index" => to_idx}, + socket + ) + when is_integer(from_idx) and is_integer(to_idx) do + fields = socket.assigns.join_form_fields + new_fields = reorder_list(fields, from_idx, to_idx) + + socket = + socket + |> assign(:join_form_fields, new_fields) + |> persist_join_form_settings() + + {:noreply, socket} + end + + # Ignore malformed reorder events (e.g. nil indices from aborted drags) + def handle_event("reorder_join_form_field", _params, socket), do: {:noreply, socket} + defp persist_join_form_settings(socket) do settings = socket.assigns.settings field_ids = Enum.map(socket.assigns.join_form_fields, & &1.id) @@ -990,6 +1027,7 @@ defmodule MvWeb.GlobalSettingsLive do required_config = settings.join_form_field_required || %{} join_form_fields = build_join_form_fields(field_ids, required_config, custom_fields) + %{member_fields: member_avail, custom_fields: custom_avail} = build_available_join_form_fields(field_ids, custom_fields) @@ -1054,6 +1092,12 @@ defmodule MvWeb.GlobalSettingsLive do defp toggle_required_if_matches(field, _field_id), do: field + defp reorder_list(list, from_index, to_index) do + item = Enum.at(list, from_index) + rest = List.delete_at(list, from_index) + List.insert_at(rest, to_index, item) + end + defp member_field_id_strings do Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) end diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 01c49f6..90bedc5 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -3384,10 +3384,20 @@ msgstr "Bestätigung durch Vorstand erforderlich (in Entwicklung)" #: lib/mv_web/live/global_settings_live.ex #, elixir-autogen, elixir-format -msgid "Personal data" -msgstr "Persönliche Daten" +msgid "Individual fields" +msgstr "Individuelle Felder" #: lib/mv_web/live/global_settings_live.ex #, elixir-autogen, elixir-format -msgid "Individual fields" -msgstr "Individuelle Felder" +msgid "Personal data" +msgstr "Persönliche Daten" + +#: lib/mv_web/components/core_components.ex +#, elixir-autogen, elixir-format +msgid "Reorder" +msgstr "Umordnen" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "The order of rows determines the field order in the join form." +msgstr "Die Reihenfolge der Zeilen bestimmt die Reihenfolge der Felder im Beitrittsformular." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index b5ab449..70bf233 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -3381,3 +3381,23 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Board approval required (in development)" msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Individual fields" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "Personal data" +msgstr "" + +#: lib/mv_web/components/core_components.ex +#, elixir-autogen, elixir-format +msgid "Reorder" +msgstr "" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "The order of rows determines the field order in the join form." +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 5556d10..02160f9 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -3384,10 +3384,20 @@ msgstr "Board approval required (in development)" #: lib/mv_web/live/global_settings_live.ex #, elixir-autogen, elixir-format -msgid "Personal data" -msgstr "Personal data" +msgid "Individual fields" +msgstr "Individual fields" #: lib/mv_web/live/global_settings_live.ex #, elixir-autogen, elixir-format -msgid "Individual fields" -msgstr "Individual fields" +msgid "Personal data" +msgstr "Personal data" + +#: lib/mv_web/components/core_components.ex +#, elixir-autogen, elixir-format +msgid "Reorder" +msgstr "Reorder" + +#: lib/mv_web/live/global_settings_live.ex +#, elixir-autogen, elixir-format +msgid "The order of rows determines the field order in the join form." +msgstr "The order of rows determines the field order in the join form." diff --git a/test/membership/setting_join_form_test.exs b/test/membership/setting_join_form_test.exs index bcafe9f..26b5f33 100644 --- a/test/membership/setting_join_form_test.exs +++ b/test/membership/setting_join_form_test.exs @@ -13,9 +13,9 @@ defmodule Mv.Membership.SettingJoinFormTest do """ use Mv.DataCase, async: false - alias Mv.Membership - alias Mv.Helpers.SystemActor alias Mv.Constants + alias Mv.Helpers.SystemActor + alias Mv.Membership setup do {:ok, settings} = Membership.get_settings() From 21812542ad121039779faaf5665a395cedf1c97a Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 10 Mar 2026 16:47:38 +0100 Subject: [PATCH 4/4] refactor: address review comments for join request settings --- CODE_GUIDELINES.md | 6 ++++++ assets/js/app.js | 19 +++++++++++++------ lib/membership/setting.ex | 13 ++++++------- .../changes/normalize_join_form_settings.ex | 2 +- lib/mv_web/components/core_components.ex | 2 +- test/membership/setting_join_form_test.exs | 2 ++ 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index ed9f130..47f589d 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -1140,6 +1140,12 @@ let liveSocket = new LiveSocket("/live", Socket, { }) ``` +**Vendor assets (third-party JS):** + +Some JavaScript libraries are committed as vendored files in `assets/vendor/` (e.g. `topbar`, `sortable.js`) when they are not available as npm packages or we need a specific build. Document their origin and how to update them: + +- **Sortable.js** (`assets/vendor/sortable.js`): From [SortableJS](https://github.com/SortableJS/Sortable), version noted in the file header (e.g. `/*! Sortable 1.15.6 - MIT ... */`). To update: download the desired release from the repo and replace the file; keep the header comment for traceability. + ### 3.8 Code Quality: Credo **Static Code Analysis:** diff --git a/assets/js/app.js b/assets/js/app.js index 3c4e0f9..ee423eb 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -219,14 +219,21 @@ Hooks.SortableList = { if (this.grabbedRowId == null) return + // Do not move into a locked row (e.g. email always first) if (e.key === "ArrowUp" && idx > 0) { - e.preventDefault() - this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx - 1 }) - announce(`Position ${idx} of ${total}.`) + const targetRow = rows[idx - 1] + if (!this.isLocked(targetRow)) { + e.preventDefault() + this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx - 1 }) + announce(`Position ${idx} of ${total}.`) + } } else if (e.key === "ArrowDown" && idx < total - 1) { - e.preventDefault() - this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx + 1 }) - announce(`Position ${idx + 2} of ${total}.`) + const targetRow = rows[idx + 1] + if (!this.isLocked(targetRow)) { + e.preventDefault() + this.pushEvent(this.reorderEvent, { from_index: idx, to_index: idx + 1 }) + announce(`Position ${idx + 2} of ${total}.`) + } } } diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index adf05b9..bc2b1e7 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -60,6 +60,10 @@ defmodule Mv.Membership.Setting do domain: Mv.Membership, data_layer: AshPostgres.DataLayer + # Used in join_form_field_ids validation (compile-time to avoid recompiling regex and list on every validation) + @uuid_pattern ~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i + @valid_join_form_member_fields Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + postgres do table "settings" repo Mv.Repo @@ -255,16 +259,10 @@ defmodule Mv.Membership.Setting do field_ids = Ash.Changeset.get_attribute(changeset, :join_form_field_ids) if is_list(field_ids) and field_ids != [] do - valid_member_fields = - Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) - - uuid_pattern = - ~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i - invalid_ids = Enum.reject(field_ids, fn id -> is_binary(id) and - (id in valid_member_fields or Regex.match?(uuid_pattern, id)) + (id in @valid_join_form_member_fields or Regex.match?(@uuid_pattern, id)) end) if Enum.empty?(invalid_ids) do @@ -442,6 +440,7 @@ defmodule Mv.Membership.Setting do attribute :join_form_field_ids, {:array, :string} do allow_nil? true + default [] public? true description "Ordered list of field IDs shown on the join form. Each entry is a member field name (e.g. 'email') or a custom field UUID. Email is always present after normalization." diff --git a/lib/membership/setting/changes/normalize_join_form_settings.ex b/lib/membership/setting/changes/normalize_join_form_settings.ex index d21434a..217ceb2 100644 --- a/lib/membership/setting/changes/normalize_join_form_settings.ex +++ b/lib/membership/setting/changes/normalize_join_form_settings.ex @@ -54,7 +54,7 @@ defmodule Mv.Membership.Setting.Changes.NormalizeJoinFormSettings do base = if is_map(required_config), do: required_config, else: %{} base - |> Map.filter(fn {key, _} -> key in field_ids end) + |> Map.take(field_ids) |> Map.put("email", true) end end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index a2ac7f9..11a60ef 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -990,7 +990,7 @@ defmodule MvWeb.CoreComponents do /> - {gettext("Actions")} + {gettext("Actions")} diff --git a/test/membership/setting_join_form_test.exs b/test/membership/setting_join_form_test.exs index 26b5f33..a9cf599 100644 --- a/test/membership/setting_join_form_test.exs +++ b/test/membership/setting_join_form_test.exs @@ -11,6 +11,8 @@ defmodule Mv.Membership.SettingJoinFormTest do - `Mv.Membership.get_join_form_allowlist/0` is implemented and returns the allowlist for the public join form (subtask 4). """ + # Settings is a singleton; tests mutate shared DB state. We use async: false and on_exit to restore + # original values because Ecto Sandbox transaction rollback does not apply to this singleton pattern. use Mv.DataCase, async: false alias Mv.Constants