Refactor column visibility logic

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.
 

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

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

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

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

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)