From 2d2865b5a642c1d9a638f4ad8805a7028743b476 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 23 Dec 2025 17:01:21 +0100 Subject: [PATCH] feat: improve validation for custom fields --- lib/membership/member.ex | 188 +++++++++++++++++++++++++++------------ 1 file changed, 129 insertions(+), 59 deletions(-) diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 286bb1a..ae53116 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -39,6 +39,7 @@ defmodule Mv.Membership.Member do require Ash.Query import Ash.Expr + require Logger # Module constants @member_search_limit 10 @@ -73,6 +74,9 @@ defmodule Mv.Membership.Member do create :create_member do primary? true + + # Note: Custom validation function cannot be done atomically (queries DB for required custom fields) + # In Ash 3.0, require_atomic? is not available for create actions, but the validation will still work # Custom field values can be created along with member argument :custom_field_values, {:array, :map} # Allow user to be passed as argument for relationship management @@ -332,66 +336,27 @@ defmodule Mv.Membership.Member do # Validate required custom fields validate fn changeset, _ -> - custom_field_values_arg = Ash.Changeset.get_argument(changeset, :custom_field_values) + provided_values = provided_custom_field_values(changeset) - # If argument is not provided (nil), check existing values from member data (update scenario) - # If argument is provided (empty list or list with values), use those values - provided_values = - if is_nil(custom_field_values_arg) do - # Update scenario: check existing custom field values from member - case Ash.load(changeset.data, :custom_field_values) do - {:ok, %{custom_field_values: existing_values}} -> - Enum.reduce(existing_values, %{}, fn cfv, acc -> - value = if is_map(cfv.value), do: Map.get(cfv.value, :value), else: nil - Map.put(acc, cfv.custom_field_id, value) - end) - - _ -> - %{} - end - else - # Create or update scenario: use provided argument values - Enum.reduce(custom_field_values_arg, %{}, fn cfv, acc -> - custom_field_id = Map.get(cfv, "custom_field_id") - value_map = Map.get(cfv, "value", %{}) - - # Support both "value" and "_union_value" keys, without using || to preserve false values - actual_value = - cond do - Map.has_key?(value_map, "value") -> Map.get(value_map, "value") - Map.has_key?(value_map, "_union_value") -> Map.get(value_map, "_union_value") - true -> nil - end - - Map.put(acc, custom_field_id, actual_value) - end) - end - - # Load all required custom fields - case Mv.Membership.list_custom_fields() do - {:ok, all_custom_fields} -> - required_custom_fields = Enum.filter(all_custom_fields, & &1.required) - - # Check each required custom field - missing_fields = - Enum.filter(required_custom_fields, fn cf -> - value = Map.get(provided_values, cf.id) - not value_present?(value, cf.value_type) - end) + case Mv.Membership.list_required_custom_fields() do + {:ok, required_custom_fields} -> + missing_fields = missing_required_fields(required_custom_fields, provided_values) if Enum.empty?(missing_fields) do :ok else - missing_names = Enum.map_join(missing_fields, ", ", & &1.name) - - {:error, - field: :custom_field_values, - message: "Required custom fields missing: #{missing_names}"} + build_custom_field_validation_error(missing_fields) end - {:error, _} -> - # If we can't load custom fields, skip validation (shouldn't happen in normal operation) - :ok + {: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."} end end end @@ -731,17 +696,122 @@ defmodule Mv.Membership.Member do end end + # Extracts provided custom field values from changeset + # Handles both create (from argument) and update (from existing data) scenarios + defp provided_custom_field_values(changeset) do + custom_field_values_arg = Ash.Changeset.get_argument(changeset, :custom_field_values) + + if is_nil(custom_field_values_arg) do + extract_existing_values(changeset.data) + else + extract_argument_values(custom_field_values_arg) + end + end + + # Extracts custom field values from existing member data (update scenario) + defp extract_existing_values(member_data) do + case Ash.load(member_data, :custom_field_values) do + {:ok, %{custom_field_values: existing_values}} -> + Enum.reduce(existing_values, %{}, &extract_value_from_cfv/2) + + _ -> + %{} + end + end + + # Extracts value from a CustomFieldValue struct + defp extract_value_from_cfv(cfv, acc) do + value = extract_union_value(cfv.value) + Map.put(acc, cfv.custom_field_id, value) + end + + # Extracts value from union type (map or direct value) + defp extract_union_value(value) when is_map(value), do: Map.get(value, :value) + defp extract_union_value(value), do: value + + # Extracts custom field values from provided argument (create/update scenario) + defp extract_argument_values(custom_field_values_arg) do + Enum.reduce(custom_field_values_arg, %{}, &extract_value_from_arg/2) + end + + # Extracts value from argument map + defp extract_value_from_arg(cfv, acc) do + custom_field_id = Map.get(cfv, "custom_field_id") + value_map = Map.get(cfv, "value", %{}) + actual_value = extract_value_from_map(value_map) + Map.put(acc, custom_field_id, actual_value) + end + + # Extracts value from map, supporting both "value" and "_union_value" keys + # Also handles Ash.Union structs (which have atom keys :value and :type) + # Uses cond instead of || to preserve false values + defp extract_value_from_map(value_map) do + cond do + # Handle Ash.Union struct - check if it's a struct with __struct__ == Ash.Union + match?({:ok, Ash.Union}, Map.fetch(value_map, :__struct__)) -> + Map.get(value_map, :value) + + # Handle map with string keys + Map.has_key?(value_map, "value") -> + Map.get(value_map, "value") + + Map.has_key?(value_map, "_union_value") -> + Map.get(value_map, "_union_value") + + # Handle map with atom keys + Map.has_key?(value_map, :value) -> + Map.get(value_map, :value) + + true -> + nil + end + end + + # Finds which required custom fields are missing from provided values + defp missing_required_fields(required_custom_fields, provided_values) do + Enum.filter(required_custom_fields, fn cf -> + value = Map.get(provided_values, cf.id) + not value_present?(value, cf.value_type) + end) + end + + # Builds validation error message for missing required custom fields + defp build_custom_field_validation_error(missing_fields) do + # Sort missing fields alphabetically for consistent error messages + sorted_missing_fields = Enum.sort_by(missing_fields, & &1.name) + missing_names = Enum.map_join(sorted_missing_fields, ", ", & &1.name) + + {:error, + field: :custom_field_values, message: "Required custom fields missing: #{missing_names}"} + end + # Helper function to check if a value is present for a given custom field type # Boolean: false is valid, only nil is invalid # String: nil or empty strings are invalid - # Integer: nil is invalid, 0 is valid - # Date: nil is invalid - # Email: nil is invalid + # Integer: nil or empty strings are invalid, 0 is valid + # Date: nil or empty strings are invalid + # Email: nil or empty strings are invalid defp value_present?(nil, _type), do: false + defp value_present?(value, :boolean), do: not is_nil(value) + defp value_present?(value, :string), do: is_binary(value) and String.trim(value) != "" - defp value_present?(value, :integer), do: not is_nil(value) - defp value_present?(value, :date), do: not is_nil(value) - defp value_present?(value, :email), do: not is_nil(value) + + defp value_present?(value, :integer) when is_integer(value), do: true + + defp value_present?(value, :integer) when is_binary(value), do: String.trim(value) != "" + + defp value_present?(_value, :integer), do: false + + defp value_present?(value, :date) when is_struct(value, Date), do: true + + defp value_present?(value, :date) when is_binary(value), do: String.trim(value) != "" + + defp value_present?(_value, :date), do: false + + defp value_present?(value, :email) when is_binary(value), do: String.trim(value) != "" + + defp value_present?(_value, :email), do: false + defp value_present?(_value, _type), do: false end