diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index 439eee8..3e2bbd0 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -2849,12 +2849,14 @@ Building accessible applications ensures that all users, including those with di **Required Fields:** +Which member fields are required (asterisk, tooltip, validation) is configured in **Settings** (Memberdata section: edit a member field and set "Required"). The member create/edit form and Member resource validation both read `settings.member_field_required`. Email is always required; other fields default to optional. + ```heex - + <.input field={@form[:first_name]} label={gettext("First Name")} - required + required={@member_field_required_map[:first_name]} aria-required="true" /> ``` diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 7b70e89..3af514e 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -500,48 +500,97 @@ defmodule Mv.Membership.Member do end end - # Validate required custom fields (actor from validation context only; no fallback) + # Validate required custom fields (actor from validation context only; no fallback). + # Only for create_member/update_member; skip for set_vereinfacht_contact_id (internal sync + # only sets vereinfacht_contact_id; custom fields were already validated and saved). validate fn changeset, context -> - provided_values = provided_custom_field_values(changeset) - actor = context.actor + provided_values = provided_custom_field_values(changeset) + actor = context.actor - case Mv.Membership.list_required_custom_fields(actor: actor) do - {:ok, required_custom_fields} -> - missing_fields = missing_required_fields(required_custom_fields, provided_values) + case Mv.Membership.list_required_custom_fields(actor: actor) do + {:ok, required_custom_fields} -> + missing_fields = + missing_required_fields(required_custom_fields, provided_values) - if Enum.empty?(missing_fields) do - :ok - else - build_custom_field_validation_error(missing_fields) - end + if Enum.empty?(missing_fields) do + :ok + else + build_custom_field_validation_error(missing_fields) + end - {:error, %Ash.Error.Forbidden{}} -> - Logger.warning( - "Required custom fields validation: actor not authorized to read CustomField" - ) + {:error, %Ash.Error.Forbidden{}} -> + Logger.warning( + "Required custom fields validation: actor not authorized to read CustomField" + ) - {:error, - field: :custom_field_values, - message: - "You are not authorized to perform this action. Please sign in again or contact support."} + {:error, + field: :custom_field_values, + message: + "You are not authorized to perform this action. Please sign in again or contact support."} - {:error, :missing_actor} -> - Logger.warning("Required custom fields validation: no actor in context") + {:error, :missing_actor} -> + Logger.warning("Required custom fields validation: no actor in context") - {:error, - field: :custom_field_values, - message: - "You are not authorized to perform this action. Please sign in again or contact support."} + {:error, + field: :custom_field_values, + message: + "You are not authorized to perform this action. Please sign in again or contact support."} - {:error, error} -> - Logger.error( - "Failed to load custom fields for validation: #{inspect(error)}. Required field validation cannot be performed." - ) + {:error, error} -> + Logger.error( + "Failed to load custom fields for validation: #{inspect(error)}. Required field validation cannot be performed." + ) - {:error, - field: :custom_field_values, - message: - "Unable to validate required custom fields. Please try again or contact support."} + {:error, + field: :custom_field_values, + message: + "Unable to validate required custom fields. Please try again or contact support."} + end + end, + where: [action_is([:create_member, :update_member])] + + # Validate member fields that are marked as required in settings or by Vereinfacht. + # When settings cannot be loaded, we still enforce email + Vereinfacht-required fields. + validate fn changeset, _context -> + vereinfacht_required? = Mv.Config.vereinfacht_configured?() + + required_fields = + case Mv.Membership.get_settings() do + {:ok, settings} -> + required_config = settings.member_field_required || %{} + normalized = VisibilityConfig.normalize(required_config) + + Enum.filter(Mv.Constants.member_fields(), fn field -> + field == :email || + (vereinfacht_required? && Mv.Constants.vereinfacht_required_field?(field)) || + Map.get(normalized, field, false) + end) + + {:error, reason} -> + Logger.warning( + "Member required-fields validation: could not load settings (#{inspect(reason)}). " <> + "Enforcing only email and Vereinfacht-required fields." + ) + + Enum.filter(Mv.Constants.member_fields(), fn field -> + field == :email || + (vereinfacht_required? && Mv.Constants.vereinfacht_required_field?(field)) + end) + end + + missing = + Enum.filter(required_fields, fn field -> + value = Ash.Changeset.get_attribute(changeset, field) + not member_field_value_present?(field, value) + end) + + if Enum.empty?(missing) do + :ok + else + field = hd(missing) + + {:error, + field: field, message: Gettext.dgettext(MvWeb.Gettext, "default", "can't be blank")} end end end @@ -1420,4 +1469,14 @@ defmodule Mv.Membership.Member do defp value_present?(_value, :email), do: false defp value_present?(_value, _type), do: false + + # Used by member-field-required validation (settings-driven required fields) + defp member_field_value_present?(_field, nil), do: false + + defp member_field_value_present?(_, value) when is_binary(value), + do: String.trim(value) != "" + + defp member_field_value_present?(_, %Date{}), do: true + defp member_field_value_present?(_, value) when is_struct(value, Date), do: true + defp member_field_value_present?(_, _), do: false end diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 74735e4..2583718 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -64,6 +64,8 @@ defmodule Mv.Membership do define :update_single_member_field_visibility, action: :update_single_member_field_visibility + + define :update_single_member_field, action: :update_single_member_field end resource Mv.Membership.Group do @@ -257,6 +259,46 @@ defmodule Mv.Membership do |> Ash.update(domain: __MODULE__) end + @doc """ + Atomically updates visibility and required for a single member field. + + Updates both `member_field_visibility` and `member_field_required` in one + operation. Use this when saving from the member field settings form. + + ## Parameters + + - `settings` - The settings record to update + - `field` - The member field name as a string (e.g., "first_name", "street") + - `show_in_overview` - Boolean value indicating visibility in member overview + - `required` - Boolean value indicating whether the field is required in member forms + + ## Returns + + - `{:ok, updated_settings}` - Successfully updated settings + - `{:error, error}` - Validation or update error + + ## Examples + + iex> {:ok, settings} = Mv.Membership.get_settings() + iex> {:ok, updated} = Mv.Membership.update_single_member_field(settings, field: "first_name", show_in_overview: true, required: true) + iex> updated.member_field_required["first_name"] + true + + """ + def update_single_member_field(settings, + field: field, + show_in_overview: show_in_overview, + required: required + ) do + settings + |> Ash.Changeset.new() + |> Ash.Changeset.set_argument(:field, field) + |> Ash.Changeset.set_argument(:show_in_overview, show_in_overview) + |> Ash.Changeset.set_argument(:required, required) + |> Ash.Changeset.for_update(:update_single_member_field, %{}) + |> Ash.update(domain: __MODULE__) + end + @doc """ Gets a group by its slug. diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index f56daa0..154288b 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -11,6 +11,8 @@ defmodule Mv.Membership.Setting do - `club_name` - The name of the association/club (required, cannot be empty) - `member_field_visibility` - JSONB map storing visibility configuration for member fields (e.g., `%{"street" => false, "house_number" => false}`). Fields not in the map default to `true`. + - `member_field_required` - JSONB map storing which member fields are required in forms + (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) @@ -42,6 +44,9 @@ defmodule Mv.Membership.Setting do # Update member field visibility {:ok, updated} = Mv.Membership.update_member_field_visibility(settings, %{"street" => false, "house_number" => false}) + # Update visibility and required for a single member field (e.g. from settings UI) + {:ok, updated} = Mv.Membership.update_single_member_field(settings, field: "first_name", show_in_overview: true, required: true) + # Update membership fee settings {:ok, updated} = Mv.Membership.update_settings(settings, %{include_joining_cycle: false}) """ @@ -68,6 +73,7 @@ defmodule Mv.Membership.Setting do accept [ :club_name, :member_field_visibility, + :member_field_required, :include_joining_cycle, :default_membership_fee_type_id, :vereinfacht_api_url, @@ -84,6 +90,7 @@ defmodule Mv.Membership.Setting do accept [ :club_name, :member_field_visibility, + :member_field_required, :include_joining_cycle, :default_membership_fee_type_id, :vereinfacht_api_url, @@ -109,6 +116,17 @@ defmodule Mv.Membership.Setting do change Mv.Membership.Setting.Changes.UpdateSingleMemberFieldVisibility end + update :update_single_member_field do + description "Atomically updates visibility and required for a single member field" + require_atomic? false + + argument :field, :string, allow_nil?: false + argument :show_in_overview, :boolean, allow_nil?: false + argument :required, :boolean, allow_nil?: false + + change Mv.Membership.Setting.Changes.UpdateSingleMemberField + end + update :update_membership_fee_settings do description "Updates the membership fee configuration" require_atomic? false @@ -162,6 +180,44 @@ defmodule Mv.Membership.Setting do end, on: [:create, :update] + # Validate member_field_required map structure and content + validate fn changeset, _context -> + required_config = Ash.Changeset.get_attribute(changeset, :member_field_required) + + if required_config && is_map(required_config) do + invalid_values = + Enum.filter(required_config, fn {_key, value} -> + not is_boolean(value) + end) + + valid_field_strings = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + + invalid_keys = + Enum.filter(required_config, fn {key, _value} -> + key not in valid_field_strings + end) + |> Enum.map(fn {key, _value} -> key end) + + cond do + not Enum.empty?(invalid_values) -> + {:error, + field: :member_field_required, + message: "All values in member_field_required must be booleans"} + + not Enum.empty?(invalid_keys) -> + {:error, + field: :member_field_required, + message: "Invalid member field keys: #{inspect(invalid_keys)}"} + + true -> + :ok + end + else + :ok + end + end, + on: [:create, :update] + # Validate default_membership_fee_type_id exists if set validate fn changeset, context -> fee_type_id = @@ -219,6 +275,12 @@ defmodule Mv.Membership.Setting do description: "Configuration for member field visibility in overview (JSONB map). Keys are member field names (atoms), values are booleans." + attribute :member_field_required, :map, + allow_nil?: true, + public?: true, + description: + "Configuration for which member fields are required in forms (JSONB map). Keys are member field names (strings), values are booleans. Email is always required." + # Membership fee settings attribute :include_joining_cycle, :boolean do allow_nil? false diff --git a/lib/membership/setting/changes/update_single_member_field.ex b/lib/membership/setting/changes/update_single_member_field.ex new file mode 100644 index 0000000..e24860c --- /dev/null +++ b/lib/membership/setting/changes/update_single_member_field.ex @@ -0,0 +1,179 @@ +defmodule Mv.Membership.Setting.Changes.UpdateSingleMemberField do + @moduledoc """ + Ash change that atomically updates visibility and required for a single member field. + + Updates both `member_field_visibility` and `member_field_required` JSONB maps + in one SQL UPDATE to avoid lost updates when saving from the settings UI. + + ## Arguments + - `field` - The member field name as a string (e.g., "street", "first_name") + - `show_in_overview` - Boolean value indicating visibility in member overview + - `required` - Boolean value indicating whether the field is required in member forms + + ## Example + settings + |> Ash.Changeset.for_update(:update_single_member_field, %{}, + arguments: %{field: "first_name", show_in_overview: true, required: true} + ) + |> Ash.update(domain: Mv.Membership) + """ + use Ash.Resource.Change + + alias Ash.Error.Invalid + alias Ecto.Adapters.SQL + require Logger + + def change(changeset, _opts, _context) do + with {:ok, field} <- get_and_validate_field(changeset), + {:ok, show_in_overview} <- get_and_validate_boolean(changeset, :show_in_overview), + {:ok, required} <- get_and_validate_boolean(changeset, :required) do + add_after_action(changeset, field, show_in_overview, required) + else + {:error, updated_changeset} -> updated_changeset + end + end + + defp get_and_validate_field(changeset) do + case Ash.Changeset.get_argument(changeset, :field) do + nil -> + {:error, + add_error(changeset, + field: :field, + message: "field argument is required" + )} + + field -> + valid_fields = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + + if field in valid_fields do + {:ok, field} + else + {:error, + add_error( + changeset, + field: :field, + message: "Invalid member field: #{field}" + )} + end + end + end + + defp get_and_validate_boolean(changeset, :show_in_overview = arg_name) do + do_validate_boolean(changeset, arg_name, :show_in_overview) + end + + defp get_and_validate_boolean(changeset, :required = arg_name) do + do_validate_boolean(changeset, arg_name, :member_field_required) + end + + defp do_validate_boolean(changeset, arg_name, error_field) do + case Ash.Changeset.get_argument(changeset, arg_name) do + nil -> + {:error, + add_error( + changeset, + field: error_field, + message: "#{arg_name} argument is required" + )} + + value when is_boolean(value) -> + {:ok, value} + + _ -> + {:error, + add_error( + changeset, + field: error_field, + message: "#{arg_name} must be a boolean" + )} + end + end + + defp add_error(changeset, opts) do + Ash.Changeset.add_error(changeset, opts) + end + + defp add_after_action(changeset, field, show_in_overview, required) do + Ash.Changeset.after_action(changeset, fn _changeset, settings -> + # Update both JSONB columns in one statement + sql = """ + UPDATE settings + SET + member_field_visibility = jsonb_set( + COALESCE(member_field_visibility, '{}'::jsonb), + ARRAY[$1::text], + to_jsonb($2::boolean), + true + ), + member_field_required = jsonb_set( + COALESCE(member_field_required, '{}'::jsonb), + ARRAY[$1::text], + to_jsonb($3::boolean), + true + ), + updated_at = (now() AT TIME ZONE 'utc') + WHERE id = $4 + RETURNING member_field_visibility, member_field_required + """ + + uuid_binary = Ecto.UUID.dump!(settings.id) + + case SQL.query(Mv.Repo, sql, [field, show_in_overview, required, uuid_binary]) do + {:ok, %{rows: [[updated_visibility, updated_required] | _]}} -> + vis = normalize_jsonb_result(updated_visibility) + req = normalize_jsonb_result(updated_required) + + updated_settings = %{ + settings + | member_field_visibility: vis, + member_field_required: req + } + + {:ok, updated_settings} + + {:ok, %{rows: []}} -> + {:error, + Invalid.exception( + field: :member_field_required, + message: "Settings not found" + )} + + {:error, error} -> + Logger.error("Failed to atomically update member field settings: #{inspect(error)}") + + {:error, + Invalid.exception( + field: :member_field_required, + message: "Failed to update member field settings" + )} + end + end) + end + + defp normalize_jsonb_result(updated_jsonb) do + case updated_jsonb do + map when is_map(map) -> + Enum.reduce(map, %{}, fn + {k, v}, acc when is_atom(k) -> Map.put(acc, Atom.to_string(k), v) + {k, v}, acc -> Map.put(acc, k, v) + end) + + binary when is_binary(binary) -> + case Jason.decode(binary) do + {:ok, decoded} when is_map(decoded) -> + decoded + + {:ok, _} -> + %{} + + {:error, reason} -> + Logger.warning("Failed to decode JSONB: #{inspect(reason)}") + %{} + end + + _ -> + Logger.warning("Unexpected JSONB format: #{inspect(updated_jsonb)}") + %{} + end + end +end diff --git a/lib/mv/constants.ex b/lib/mv/constants.ex index 4ef355d..de429e8 100644 --- a/lib/mv/constants.ex +++ b/lib/mv/constants.ex @@ -27,8 +27,26 @@ defmodule Mv.Constants do @email_validator_checks [:html_input, :pow] + # Member fields that are required when Vereinfacht integration is active (contact sync) + @vereinfacht_required_member_fields [:first_name, :last_name, :street, :postal_code, :city] + def member_fields, do: @member_fields + @doc """ + Returns member fields that are always required when Vereinfacht integration is configured. + + Used for validation, member form required indicators, and settings UI (checkbox disabled). + """ + def vereinfacht_required_member_fields, do: @vereinfacht_required_member_fields + + @doc """ + Returns whether the given member field is required by Vereinfacht when integration is active. + """ + def vereinfacht_required_field?(field) when is_atom(field), + do: field in @vereinfacht_required_member_fields + + def vereinfacht_required_field?(_), do: false + @doc """ Returns the prefix used for custom field keys in field visibility maps. diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 40cb800..21e3546 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -448,6 +448,8 @@ defmodule MvWeb.CoreComponents do end def input(%{type: "select"} = assigns) do + assigns = ensure_aria_required_for_input(assigns) + ~H"""