diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 31a825b..bcd505e 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -410,70 +410,6 @@ defmodule Mv.Membership.Member do identity :unique_email, [:email] end - @doc """ - Checks if a member field should be shown in the overview. - - Reads the visibility configuration from Settings resource. If a field is not - configured in settings, it defaults to `true` (visible). - - ## Parameters - - `field` - Atom representing the member field name (e.g., `:email`, `:street`) - - ## Returns - - `true` if the field should be shown in overview (default) - - `false` if the field is configured as hidden in settings - - ## Examples - - iex> Member.show_in_overview?(:email) - true - - iex> Member.show_in_overview?(:street) - true # or false if configured in settings - - """ - @spec show_in_overview?(atom()) :: boolean() - def show_in_overview?(field) when is_atom(field) do - case Mv.Membership.get_settings() do - {:ok, settings} -> - visibility_config = settings.member_field_visibility || %{} - # Normalize map keys to atoms (JSONB may return string keys) - normalized_config = normalize_visibility_config(visibility_config) - - # Get value from normalized config, default to true - Map.get(normalized_config, field, true) - - {:error, _} -> - # If settings can't be loaded, default to visible - true - end - end - - def show_in_overview?(_), do: true - - # Normalizes visibility config map keys from strings to atoms. - # JSONB in PostgreSQL converts atom keys to string keys when storing. - defp normalize_visibility_config(config) when is_map(config) do - Enum.reduce(config, %{}, fn - {key, value}, acc when is_atom(key) -> - Map.put(acc, key, value) - - {key, value}, acc when is_binary(key) -> - try do - atom_key = String.to_existing_atom(key) - Map.put(acc, atom_key, value) - rescue - ArgumentError -> - acc - end - - _, acc -> - acc - end) - end - - defp normalize_visibility_config(_), do: %{} - @doc """ Performs fuzzy search on members using PostgreSQL trigram similarity. diff --git a/lib/membership/membership.ex b/lib/membership/membership.ex index 516448c..f5a708b 100644 --- a/lib/membership/membership.ex +++ b/lib/membership/membership.ex @@ -134,8 +134,8 @@ defmodule Mv.Membership do ## Parameters - `settings` - The settings record to update - - `visibility_config` - A map of member field names (atoms) to boolean visibility values - (e.g., `%{street: false, house_number: false}`) + - `visibility_config` - A map of member field names (strings) to boolean visibility values + (e.g., `%{"street" => false, "house_number" => false}`) ## Returns @@ -145,9 +145,9 @@ defmodule Mv.Membership do ## Examples iex> {:ok, settings} = Mv.Membership.get_settings() - iex> {:ok, updated} = Mv.Membership.update_member_field_visibility(settings, %{street: false, house_number: false}) + iex> {:ok, updated} = Mv.Membership.update_member_field_visibility(settings, %{"street" => false, "house_number" => false}) iex> updated.member_field_visibility - %{street: false, house_number: false} + %{"street" => false, "house_number" => false} """ def update_member_field_visibility(settings, visibility_config) do diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index 3405a3f..52c0328 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -10,7 +10,7 @@ defmodule Mv.Membership.Setting do ## Attributes - `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`. + (e.g., `%{"street" => false, "house_number" => false}`). Fields not in the map default to `true`. ## Singleton Pattern This resource uses a singleton pattern - there should only be one settings record. @@ -32,7 +32,7 @@ defmodule Mv.Membership.Setting do {:ok, updated} = Mv.Membership.update_settings(settings, %{club_name: "New Name"}) # Update member field visibility - {:ok, updated} = Mv.Membership.update_member_field_visibility(settings, %{street: false, house_number: false}) + {:ok, updated} = Mv.Membership.update_member_field_visibility(settings, %{"street" => false, "house_number" => false}) """ use Ash.Resource, domain: Mv.Membership, @@ -67,43 +67,6 @@ defmodule Mv.Membership.Setting do description "Updates the visibility configuration for member fields in the overview" require_atomic? false accept [:member_field_visibility] - - change fn changeset, _context -> - visibility = Ash.Changeset.get_attribute(changeset, :member_field_visibility) - - if visibility && is_map(visibility) do - valid_fields = Mv.Constants.member_fields() - # Normalize keys to atoms (JSONB may return string keys) - invalid_keys = - Enum.filter(visibility, fn {key, _value} -> - atom_key = - if is_atom(key) do - key - else - try do - String.to_existing_atom(key) - rescue - ArgumentError -> nil - end - end - - atom_key && atom_key not in valid_fields - end) - |> Enum.map(fn {key, _value} -> key end) - - if Enum.empty?(invalid_keys) do - changeset - else - Ash.Changeset.add_error( - changeset, - field: :member_field_visibility, - message: "Invalid member field keys: #{inspect(invalid_keys)}" - ) - end - else - changeset - end - end end end @@ -111,23 +74,39 @@ defmodule Mv.Membership.Setting do validate present(:club_name), on: [:create, :update] validate string_length(:club_name, min: 1), on: [:create, :update] - # Validate that member_field_visibility map contains only boolean values - # This allows dynamic fields without hardcoding specific field names + # Validate member_field_visibility map structure and content validate fn changeset, _context -> visibility = Ash.Changeset.get_attribute(changeset, :member_field_visibility) if visibility && is_map(visibility) do - invalid_entries = + # Validate all values are booleans + invalid_values = Enum.filter(visibility, fn {_key, value} -> not is_boolean(value) end) - if Enum.empty?(invalid_entries) do - :ok - else - {:error, - field: :member_field_visibility, - message: "All values in member_field_visibility must be booleans"} + # Validate all keys are valid member fields + valid_field_strings = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + + invalid_keys = + Enum.filter(visibility, 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_visibility, + message: "All values in member_field_visibility must be booleans"} + + not Enum.empty?(invalid_keys) -> + {:error, + field: :member_field_visibility, + message: "Invalid member field keys: #{inspect(invalid_keys)}"} + + true -> + :ok end else :ok diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index 5bbb16d..c301592 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -753,36 +753,12 @@ defmodule MvWeb.MemberLive.Index do # Get all eligible fields from the global constants all_fields = Mv.Constants.member_fields() - # Normalize visibility config (JSONB may return string keys) - visibility_config = normalize_visibility_config(settings.member_field_visibility || %{}) + # JSONB stores keys as strings + visibility_config = settings.member_field_visibility || %{} # Filter to only return visible fields Enum.filter(all_fields, fn field -> - Map.get(visibility_config, field, true) + Map.get(visibility_config, Atom.to_string(field), true) end) end - - # Normalizes visibility config map keys from strings to atoms. - # JSONB in PostgreSQL converts atom keys to string keys when storing. - # This is a local helper to avoid N+1 queries by reusing the normalization logic. - defp normalize_visibility_config(config) when is_map(config) do - Enum.reduce(config, %{}, fn - {key, value}, acc when is_atom(key) -> - Map.put(acc, key, value) - - {key, value}, acc when is_binary(key) -> - try do - atom_key = String.to_existing_atom(key) - Map.put(acc, atom_key, value) - rescue - ArgumentError -> - acc - end - - _, acc -> - acc - end) - end - - defp normalize_visibility_config(_), do: %{} end diff --git a/test/membership/member_field_visibility_test.exs b/test/membership/member_field_visibility_test.exs index 46bdb74..9963169 100644 --- a/test/membership/member_field_visibility_test.exs +++ b/test/membership/member_field_visibility_test.exs @@ -11,70 +11,4 @@ defmodule Mv.Membership.MemberFieldVisibilityTest do use Mv.DataCase, async: true alias Mv.Membership.Member - - describe "show_in_overview?/1" do - test "returns true for all member fields by default" do - # When no settings exist or member_field_visibility is not configured - # Test with fields from constants - member_fields = Mv.Constants.member_fields() - - Enum.each(member_fields, fn field -> - assert Member.show_in_overview?(field) == true, - "Field #{field} should be visible by default" - end) - end - - test "returns false for fields with show_in_overview: false in settings" do - # Get or create settings - {:ok, settings} = Mv.Membership.get_settings() - - # Use a field that exists in member fields - member_fields = Mv.Constants.member_fields() - field_to_hide = List.first(member_fields) - field_to_show = List.last(member_fields) - - # Update settings to hide a field - {:ok, _updated_settings} = - Mv.Membership.update_settings(settings, %{ - member_field_visibility: %{field_to_hide => false} - }) - - # JSONB may convert atom keys to string keys, so we check via show_in_overview? instead - assert Member.show_in_overview?(field_to_hide) == false - assert Member.show_in_overview?(field_to_show) == true - end - - test "returns true for non-configured fields (default)" do - # Get or create settings - {:ok, settings} = Mv.Membership.get_settings() - - # Use fields that exist in member fields - member_fields = Mv.Constants.member_fields() - fields_to_hide = Enum.take(member_fields, 2) - fields_to_show = Enum.take(member_fields, -2) - - # Update settings to hide some fields - visibility_config = - Enum.reduce(fields_to_hide, %{}, fn field, acc -> - Map.put(acc, field, false) - end) - - {:ok, _updated_settings} = - Mv.Membership.update_settings(settings, %{ - member_field_visibility: visibility_config - }) - - # Hidden fields should be false - Enum.each(fields_to_hide, fn field -> - assert Member.show_in_overview?(field) == false, - "Field #{field} should be hidden" - end) - - # Unconfigured fields should still be true (default) - Enum.each(fields_to_show, fn field -> - assert Member.show_in_overview?(field) == true, - "Field #{field} should be visible by default" - end) - end - end end diff --git a/test/mv_web/member_live/index_member_fields_display_test.exs b/test/mv_web/member_live/index_member_fields_display_test.exs index c4a5b9f..6b4f50c 100644 --- a/test/mv_web/member_live/index_member_fields_display_test.exs +++ b/test/mv_web/member_live/index_member_fields_display_test.exs @@ -51,7 +51,7 @@ defmodule MvWeb.MemberLive.IndexMemberFieldsDisplayTest do {:ok, _} = Mv.Membership.update_settings(settings, %{ - member_field_visibility: Map.new(fields_to_hide, &{&1, false}) + member_field_visibility: Map.new(fields_to_hide, &{Atom.to_string(&1), false}) }) conn = conn_with_oidc_user(conn)