formatting and refactor member fields constant

lib/membership/member.ex

@@ -42,6 +42,10 @@ defmodule Mv.Membership.Member do
   @member_search_limit 10
   @default_similarity_threshold 0.2
 
+  # Use constants from Mv.Constants for member fields
+  # This ensures consistency across the codebase
+  @member_fields Mv.Constants.member_fields()
+
   postgres do
     table "members"
     repo Mv.Repo
@@ -58,21 +62,7 @@ defmodule Mv.Membership.Member do
       # user_id is NOT in accept list to prevent direct foreign key manipulation
       argument :user, :map, allow_nil?: true
 
-      accept [
-        :first_name,
-        :last_name,
-        :email,
-        :birth_date,
-        :paid,
-        :phone_number,
-        :join_date,
-        :exit_date,
-        :notes,
-        :city,
-        :street,
-        :house_number,
-        :postal_code
-      ]
+      accept @member_fields
 
       change manage_relationship(:custom_field_values, type: :create)
 
@@ -105,21 +95,7 @@ defmodule Mv.Membership.Member do
       # user_id is NOT in accept list to prevent direct foreign key manipulation
       argument :user, :map, allow_nil?: true
 
-      accept [
-        :first_name,
-        :last_name,
-        :email,
-        :birth_date,
-        :paid,
-        :phone_number,
-        :join_date,
-        :exit_date,
-        :notes,
-        :city,
-        :street,
-        :house_number,
-        :postal_code
-      ]
+      accept @member_fields
 
       change manage_relationship(:custom_field_values, on_match: :update, on_no_match: :create)
 

lib/membership/setting.ex

@@ -114,26 +114,26 @@ defmodule Mv.Membership.Setting do
     # Validate that member_field_visibility map contains only boolean values
     # This allows dynamic fields without hardcoding specific field names
     validate fn changeset, _context ->
-      visibility = Ash.Changeset.get_attribute(changeset, :member_field_visibility)
+               visibility = Ash.Changeset.get_attribute(changeset, :member_field_visibility)
 
-      if visibility && is_map(visibility) do
-        invalid_entries =
-          Enum.filter(visibility, fn {_key, value} ->
-            not is_boolean(value)
-          end)
+               if visibility && is_map(visibility) do
+                 invalid_entries =
+                   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"}
-        end
-      else
-        :ok
-      end
-    end,
-    on: [:create, :update]
+                 if Enum.empty?(invalid_entries) do
+                   :ok
+                 else
+                   {:error,
+                    field: :member_field_visibility,
+                    message: "All values in member_field_visibility must be booleans"}
+                 end
+               else
+                 :ok
+               end
+             end,
+             on: [:create, :update]
   end
 
   attributes do
@@ -151,7 +151,8 @@ defmodule Mv.Membership.Setting do
     attribute :member_field_visibility, :map,
       allow_nil?: true,
       public?: true,
-      description: "Configuration for member field visibility in overview (JSONB map). Keys are member field names (atoms), values are booleans."
+      description:
+        "Configuration for member field visibility in overview (JSONB map). Keys are member field names (atoms), values are booleans."
 
     timestamps()
   end

lib/mv/constants.ex

@@ -3,7 +3,21 @@ defmodule Mv.Constants do
   Module for defining constants and atoms.
   """
 
-  @member_fields [:first_name, :last_name, :email, :birth_date, :paid, :phone_number, :join_date, :exit_date, :notes, :city, :street, :house_number, :postal_code]
+  @member_fields [
+    :first_name,
+    :last_name,
+    :email,
+    :birth_date,
+    :paid,
+    :phone_number,
+    :join_date,
+    :exit_date,
+    :notes,
+    :city,
+    :street,
+    :house_number,
+    :postal_code
+  ]
 
   def member_fields, do: @member_fields
 end

lib/mv_web/live/member_live/index.ex

@@ -29,11 +29,18 @@ defmodule MvWeb.MemberLive.Index do
   require Ash.Query
   import Ash.Expr
 
+  alias Mv.Membership
   alias MvWeb.MemberLive.Index.Formatter
 
   # Prefix used in sort field names for custom fields (e.g., "custom_field_<id>")
   @custom_field_prefix "custom_field_"
 
+  # Member fields that are loaded for the overview
+  # Uses constants from Mv.Constants to ensure consistency
+  # Note: :id is always included for member identification
+  # All member fields are loaded, but visibility is controlled via settings
+  @overview_fields [:id | Mv.Constants.member_fields()]
+
   @doc """
   Initializes the LiveView state.
 
@@ -52,6 +59,14 @@ defmodule MvWeb.MemberLive.Index do
       |> Ash.Query.sort(name: :asc)
       |> Ash.read!()
 
+    # Load settings once to avoid N+1 queries
+    settings =
+      case Membership.get_settings() do
+        {:ok, s} -> s
+        # Fallback if settings can't be loaded
+        {:error, _} -> %{member_field_visibility: %{}}
+      end
+
     socket =
       socket
       |> assign(:page_title, gettext("Members"))
@@ -59,9 +74,10 @@ 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())
-      |> assign(:member_fields_visible, get_visible_member_fields())
+      |> 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
     {:ok, socket}
@@ -315,18 +331,7 @@ defmodule MvWeb.MemberLive.Index do
     query =
       Mv.Membership.Member
       |> Ash.Query.new()
-      |> Ash.Query.select([
-        :id,
-        :first_name,
-        :last_name,
-        :email,
-        :street,
-        :house_number,
-        :postal_code,
-        :city,
-        :phone_number,
-        :join_date
-      ])
+      |> Ash.Query.select(@overview_fields)
 
     # Load custom field values for visible custom fields
     custom_field_ids_list = Enum.map(socket.assigns.custom_fields_visible, & &1.id)
@@ -435,18 +440,13 @@ defmodule MvWeb.MemberLive.Index do
   defp maybe_sort(query, _, _, _), do: {query, false}
 
   # Validate that a field is sortable
+  # Uses member fields from constants, but excludes fields that don't make sense to sort
+  # (e.g., :notes is too long, :paid is boolean and not very useful for sorting)
   defp valid_sort_field?(field) when is_atom(field) do
-    valid_fields = [
-      :first_name,
-      :last_name,
-      :email,
-      :street,
-      :house_number,
-      :postal_code,
-      :city,
-      :phone_number,
-      :join_date
-    ]
+    # All member fields are sortable, but we exclude some that don't make sense
+    # :id is not in member_fields, but we don't want to sort by it anyway
+    non_sortable_fields = [:notes, :paid]
+    valid_fields = Mv.Constants.member_fields() -- non_sortable_fields
 
     field in valid_fields or custom_field_sort?(field)
   end
@@ -742,6 +742,12 @@ defmodule MvWeb.MemberLive.Index do
   # and their show_in_overview values (true or false). 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.
+  #
+  # Parameters:
+  #   - `settings` - The settings struct loaded from the database
+  #
   # Returns a map: %{field_name => show_in_overview}
   #
   # This can be used for:
@@ -750,12 +756,16 @@ defmodule MvWeb.MemberLive.Index do
   # - Dynamic field management
   #
   # Fields are read from the global Constants module.
-  defp get_member_field_configurations do
+  @spec get_member_field_configurations(map()) :: %{atom() => boolean()}
+  defp get_member_field_configurations(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 = Mv.Membership.Member.show_in_overview?(field)
+      show_in_overview = Map.get(visibility_config, field, true)
       Map.put(acc, field, show_in_overview)
     end)
   end
@@ -764,10 +774,38 @@ defmodule MvWeb.MemberLive.Index do
   #
   # 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.
-  defp get_visible_member_fields do
-    get_member_field_configurations()
+  @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

lib/mv_web/live/member_live/index.html.heex

@@ -69,7 +69,10 @@
     >
       {member.first_name} {member.last_name}
     </:col>
-    <:col :if={:email in @member_fields_visible} :let={member} label={
+    <:col
+      :let={member}
+      :if={:email in @member_fields_visible}
+      label={
         ~H"""
         <.live_component
           module={MvWeb.Components.SortHeaderComponent}
@@ -80,10 +83,14 @@
           sort_order={@sort_order}
         />
         """
-      }>
+      }
+    >
       {member.email}
     </:col>
-    <:col :if={:street in @member_fields_visible} :let={member} label={
+    <:col
+      :let={member}
+      :if={:street in @member_fields_visible}
+      label={
         ~H"""
         <.live_component
           module={MvWeb.Components.SortHeaderComponent}
@@ -94,10 +101,14 @@
           sort_order={@sort_order}
         />
         """
-      }>
+      }
+    >
       {member.street}
     </:col>
-    <:col :if={:house_number in @member_fields_visible} :let={member} label={
+    <:col
+      :let={member}
+      :if={:house_number in @member_fields_visible}
+      label={
         ~H"""
         <.live_component
           module={MvWeb.Components.SortHeaderComponent}
@@ -108,10 +119,14 @@
           sort_order={@sort_order}
         />
         """
-      }>
+      }
+    >
       {member.house_number}
     </:col>
-    <:col :if={:postal_code in @member_fields_visible} :let={member} label={
+    <:col
+      :let={member}
+      :if={:postal_code in @member_fields_visible}
+      label={
         ~H"""
         <.live_component
           module={MvWeb.Components.SortHeaderComponent}
@@ -122,10 +137,14 @@
           sort_order={@sort_order}
         />
         """
-      }>
+      }
+    >
       {member.postal_code}
     </:col>
-    <:col :if={:city in @member_fields_visible} :let={member} label={
+    <:col
+      :let={member}
+      :if={:city in @member_fields_visible}
+      label={
         ~H"""
         <.live_component
           module={MvWeb.Components.SortHeaderComponent}
@@ -136,10 +155,14 @@
           sort_order={@sort_order}
         />
         """
-      }>
+      }
+    >
       {member.city}
     </:col>
-    <:col :if={:phone_number in @member_fields_visible} :let={member} label={
+    <:col
+      :let={member}
+      :if={:phone_number in @member_fields_visible}
+      label={
         ~H"""
         <.live_component
           module={MvWeb.Components.SortHeaderComponent}
@@ -150,10 +173,14 @@
           sort_order={@sort_order}
         />
         """
-      }>
+      }
+    >
       {member.phone_number}
     </:col>
-    <:col :if={:join_date in @member_fields_visible} :let={member} label={
+    <:col
+      :let={member}
+      :if={:join_date in @member_fields_visible}
+      label={
         ~H"""
         <.live_component
           module={MvWeb.Components.SortHeaderComponent}
@@ -164,7 +191,8 @@
           sort_order={@sort_order}
         />
         """
-      }>
+      }
+    >
       {member.join_date}
     </:col>
     <:action :let={member}>

test/mv_web/member_live/index_member_fields_display_test.exs

@@ -36,7 +36,6 @@ defmodule MvWeb.MemberLive.IndexMemberFieldsDisplayTest do
     }
   end
 
-
   test "shows multiple members correctly", %{conn: conn, member1: m1, member2: m2} do
     conn = conn_with_oidc_user(conn)
     {:ok, _view, html} = live(conn, "/members")
@@ -62,14 +61,4 @@ defmodule MvWeb.MemberLive.IndexMemberFieldsDisplayTest do
     assert html =~ m.email
     refute html =~ m.street
   end
-
-  defp get_field_label(:street), do: "Street"
-  defp get_field_label(:house_number), do: "House Number"
-  defp get_field_label(:postal_code), do: "Postal Code"
-  defp get_field_label(:city), do: "City"
-  defp get_field_label(:phone_number), do: "Phone Number"
-  defp get_field_label(:join_date), do: "Join Date"
-  defp get_field_label(:email), do: "Email"
-  defp get_field_label(:first_name), do: "First name"
-  defp get_field_label(:last_name), do: "Last name"
 end