From d3a346824e03436f576e9dd74218ee0b8347aada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Eppl=C3=A9e?= Date: Tue, 2 Dec 2025 12:16:02 +0100 Subject: [PATCH 1/2] Refactor column visibility logic --- lib/mv_web/live/member_live/index.ex | 41 ++++++---------------------- 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index 6bce495..5bbb16d 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -74,9 +74,7 @@ defmodule MvWeb.MemberLive.Index do |> assign_new(:sort_field, fn -> :first_name end) |> assign_new(:sort_order, fn -> :asc end) |> assign(:selected_members, []) - |> assign(:settings, settings) |> assign(:custom_fields_visible, custom_fields_visible) - |> assign(:member_field_configurations, get_member_field_configurations(settings)) |> assign(:member_fields_visible, get_visible_member_fields(settings)) # We call handle params to use the query from the URL @@ -736,11 +734,10 @@ defmodule MvWeb.MemberLive.Index do end end - # Gets the configuration for all member fields with their show_in_overview values. + # Gets the list of member fields that should be visible in the overview. # - # Reads the visibility configuration from Settings and returns a map with all member fields - # and their show_in_overview values (true or false). Fields not configured in settings - # default to true. + # Reads the visibility configuration from Settings and returns only the fields + # where show_in_overview is true. Fields not configured in settings default to true. # # Performance: This function uses the already-loaded settings to avoid N+1 queries. # Settings should be loaded once in mount/3 and passed to this function. @@ -748,43 +745,23 @@ defmodule MvWeb.MemberLive.Index do # Parameters: # - `settings` - The settings struct loaded from the database # - # Returns a map: %{field_name => show_in_overview} - # - # This can be used for: - # - Rendering the overview (filtering visible fields) - # - UI configuration dropdowns (showing all fields with their current state) - # - Dynamic field management + # Returns a list of atoms representing visible member field names. # # Fields are read from the global Constants module. - @spec get_member_field_configurations(map()) :: %{atom() => boolean()} - defp get_member_field_configurations(settings) do + @spec get_visible_member_fields(map()) :: [atom()] + defp get_visible_member_fields(settings) 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 || %{}) - Enum.reduce(all_fields, %{}, fn field, acc -> - show_in_overview = Map.get(visibility_config, field, true) - Map.put(acc, field, show_in_overview) + # Filter to only return visible fields + Enum.filter(all_fields, fn field -> + Map.get(visibility_config, field, true) end) end - # Gets the list of member fields that should be visible in the overview. - # - # Filters the member field configurations to return only fields with show_in_overview: true. - # - # Parameters: - # - `settings` - The settings struct loaded from the database - # - # Returns a list of atoms representing visible member field names. - @spec get_visible_member_fields(map()) :: [atom()] - defp get_visible_member_fields(settings) do - get_member_field_configurations(settings) - |> Enum.filter(fn {_field, show_in_overview} -> show_in_overview end) - |> Enum.map(fn {field, _show_in_overview} -> field 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. From f25f0216ec6a855c62095a4047dde963abb87c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Eppl=C3=A9e?= Date: Tue, 2 Dec 2025 12:16:02 +0100 Subject: [PATCH 2/2] Refactor column visibility logic --- lib/membership/member.ex | 64 ---------------- lib/membership/membership.ex | 8 +- lib/membership/setting.ex | 75 +++++++------------ lib/mv_web/live/member_live/index.ex | 69 +++-------------- .../member_field_visibility_test.exs | 66 ---------------- .../index_member_fields_display_test.exs | 2 +- 6 files changed, 43 insertions(+), 241 deletions(-) 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 6bce495..c301592 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -74,9 +74,7 @@ defmodule MvWeb.MemberLive.Index do |> assign_new(:sort_field, fn -> :first_name end) |> assign_new(:sort_order, fn -> :asc end) |> assign(:selected_members, []) - |> assign(:settings, settings) |> assign(:custom_fields_visible, custom_fields_visible) - |> assign(:member_field_configurations, get_member_field_configurations(settings)) |> assign(:member_fields_visible, get_visible_member_fields(settings)) # We call handle params to use the query from the URL @@ -736,11 +734,10 @@ defmodule MvWeb.MemberLive.Index do end end - # Gets the configuration for all member fields with their show_in_overview values. + # Gets the list of member fields that should be visible in the overview. # - # Reads the visibility configuration from Settings and returns a map with all member fields - # and their show_in_overview values (true or false). Fields not configured in settings - # default to true. + # Reads the visibility configuration from Settings and returns only the fields + # where show_in_overview is true. Fields not configured in settings default to true. # # Performance: This function uses the already-loaded settings to avoid N+1 queries. # Settings should be loaded once in mount/3 and passed to this function. @@ -748,64 +745,20 @@ defmodule MvWeb.MemberLive.Index do # Parameters: # - `settings` - The settings struct loaded from the database # - # Returns a map: %{field_name => show_in_overview} - # - # This can be used for: - # - Rendering the overview (filtering visible fields) - # - UI configuration dropdowns (showing all fields with their current state) - # - Dynamic field management + # Returns a list of atoms representing visible member field names. # # Fields are read from the global Constants module. - @spec get_member_field_configurations(map()) :: %{atom() => boolean()} - defp get_member_field_configurations(settings) do + @spec get_visible_member_fields(map()) :: [atom()] + defp get_visible_member_fields(settings) 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 || %{} - Enum.reduce(all_fields, %{}, fn field, acc -> - show_in_overview = Map.get(visibility_config, field, true) - Map.put(acc, field, show_in_overview) + # Filter to only return visible fields + Enum.filter(all_fields, fn field -> + Map.get(visibility_config, Atom.to_string(field), true) end) end - - # Gets the list of member fields that should be visible in the overview. - # - # Filters the member field configurations to return only fields with show_in_overview: true. - # - # Parameters: - # - `settings` - The settings struct loaded from the database - # - # Returns a list of atoms representing visible member field names. - @spec get_visible_member_fields(map()) :: [atom()] - defp get_visible_member_fields(settings) do - get_member_field_configurations(settings) - |> Enum.filter(fn {_field, show_in_overview} -> show_in_overview end) - |> Enum.map(fn {field, _show_in_overview} -> field 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)