diff --git a/docs/feature-roadmap.md b/docs/feature-roadmap.md index 9a6517d..2313fd7 100644 --- a/docs/feature-roadmap.md +++ b/docs/feature-roadmap.md @@ -94,15 +94,18 @@ - ✅ CustomFieldValue type management - ✅ Dynamic custom field value assignment to members - ✅ Union type storage (JSONB) +- ✅ Default field visibility configuration + +**Closed Issues:** +- [#194](https://git.local-it.org/local-it/mitgliederverwaltung/issues/194) - Custom Fields: Harden implementation (S) +- [#197](https://git.local-it.org/local-it/mitgliederverwaltung/issues/197) - Custom Fields: Add option to show custom fields in member overview (M) **Open Issues:** -- [#194](https://git.local-it.org/local-it/mitgliederverwaltung/issues/194) - Custom Fields: Harden implementation (S) [0/3 tasks] - [#157](https://git.local-it.org/local-it/mitgliederverwaltung/issues/157) - Concept how custom fields are handled (M, High priority) [0/4 tasks] - [#161](https://git.local-it.org/local-it/mitgliederverwaltung/issues/161) - Don't show birthday field for default configurations (S, Low priority) - [#153](https://git.local-it.org/local-it/mitgliederverwaltung/issues/153) - Sorting functionalities for custom fields (M, Low priority) **Missing Features:** -- ❌ Default field visibility configuration - ❌ Field groups/categories - ❌ Conditional fields (show field X if field Y = value) - ❌ Field validation rules (min/max, regex patterns) diff --git a/lib/mv_web/live/custom_field_live/form.ex b/lib/mv_web/live/custom_field_live/form.ex index ab8f104..99317a9 100644 --- a/lib/mv_web/live/custom_field_live/form.ex +++ b/lib/mv_web/live/custom_field_live/form.ex @@ -18,6 +18,7 @@ defmodule MvWeb.CustomFieldLive.Form do - description - Human-readable explanation - immutable - If true, values cannot be changed after creation (default: false) - required - If true, all members must have this custom field (default: false) + - show_in_overview - If true, this custom field will be displayed in the member overview table (default: true) ## Value Type Selection - `:string` - Text data (unlimited length) @@ -60,6 +61,7 @@ defmodule MvWeb.CustomFieldLive.Form do <.input field={@form[:description]} type="text" label={gettext("Description")} /> <.input field={@form[:immutable]} type="checkbox" label={gettext("Immutable")} /> <.input field={@form[:required]} type="checkbox" label={gettext("Required")} /> + <.input field={@form[:show_in_overview]} type="checkbox" label={gettext("Show in overview")} /> <.button phx-disable-with={gettext("Saving...")} variant="primary"> {gettext("Save Custom field")} diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index c933133..4a134f2 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -26,6 +26,11 @@ defmodule MvWeb.MemberLive.Index do """ use MvWeb, :live_view + alias MvWeb.MemberLive.Index.Formatter + + # Prefix used in sort field names for custom fields (e.g., "custom_field_") + @custom_field_prefix "custom_field_" + @doc """ Initializes the LiveView state. @@ -34,6 +39,19 @@ defmodule MvWeb.MemberLive.Index do """ @impl true def mount(_params, _session, socket) do + # Load custom fields that should be shown in overview + require Ash.Query + import Ash.Expr + + # Note: Using Ash.read! (bang version) - errors will be handled by Phoenix LiveView + # and result in a 500 error page. This is appropriate for LiveViews where errors + # should be visible to the user rather than silently failing. + custom_fields_visible = + Mv.Membership.CustomField + |> Ash.Query.filter(expr(show_in_overview == true)) + |> Ash.Query.sort(name: :asc) + |> Ash.read!() + socket = socket |> assign(:page_title, gettext("Members")) @@ -41,6 +59,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(:custom_fields_visible, custom_fields_visible) # We call handle params to use the query from the URL {:ok, socket} @@ -60,6 +79,8 @@ defmodule MvWeb.MemberLive.Index do """ @impl true def handle_event("delete", %{"id" => id}, socket) do + # Note: Using bang versions (!) - errors will be handled by Phoenix LiveView + # This ensures users see error messages if deletion fails (e.g., permission denied) member = Ash.get!(Mv.Membership.Member, id) Ash.destroy!(member) @@ -108,7 +129,14 @@ defmodule MvWeb.MemberLive.Index do """ @impl true def handle_info({:sort, field_str}, socket) do - field = String.to_existing_atom(field_str) + # Handle both atom and string field names (for custom fields) + field = + try do + String.to_existing_atom(field_str) + rescue + ArgumentError -> field_str + end + {new_field, new_order} = determine_new_sort(field, socket) socket @@ -158,10 +186,37 @@ defmodule MvWeb.MemberLive.Index do |> maybe_update_search(params) |> maybe_update_sort(params) |> load_members(params["query"]) + |> prepare_dynamic_cols() {:noreply, socket} end + # Prepares dynamic column definitions for custom fields that should be shown in the overview. + # + # Creates a list of column definitions, each containing: + # - `:custom_field` - The CustomField resource + # - `:render` - A function that formats the custom field value for a given member + # + # Returns the socket with `:dynamic_cols` assigned. + defp prepare_dynamic_cols(socket) do + dynamic_cols = + Enum.map(socket.assigns.custom_fields_visible, fn custom_field -> + %{ + custom_field: custom_field, + render: fn member -> + case get_custom_field_value(member, custom_field) do + nil -> "" + cfv -> + formatted = Formatter.format_custom_field_value(cfv.value, custom_field) + if formatted == "", do: "", else: formatted + end + end + } + end) + + assign(socket, :dynamic_cols, dynamic_cols) + end + # ------------------------------------------------------------- # FUNCTIONS # ------------------------------------------------------------- @@ -177,8 +232,8 @@ defmodule MvWeb.MemberLive.Index do # Updates both the active and old SortHeader components defp update_sort_components(socket, old_field, new_field, new_order) do - active_id = :"sort_#{new_field}" - old_id = :"sort_#{old_field}" + active_id = to_sort_id(new_field) + old_id = to_sort_id(old_field) # Update the new SortHeader send_update(MvWeb.Components.SortHeaderComponent, @@ -197,11 +252,32 @@ defmodule MvWeb.MemberLive.Index do socket end + # Converts a field (atom or string) to a sort component ID atom + # Handles both existing atoms and strings that need to be converted + defp to_sort_id(field) when is_binary(field) do + try do + String.to_existing_atom("sort_#{field}") + rescue + ArgumentError -> :"sort_#{field}" + end + end + + defp to_sort_id(field) when is_atom(field) do + :"sort_#{field}" + end + # Builds sort URL and pushes navigation patch defp push_sort_url(socket, field, order) do + field_str = + if is_atom(field) do + Atom.to_string(field) + else + field + end + query_params = %{ "query" => socket.assigns.query, - "sort_field" => Atom.to_string(field), + "sort_field" => field_str, "sort_order" => Atom.to_string(order) } @@ -214,7 +290,25 @@ defmodule MvWeb.MemberLive.Index do )} end - # Load members eg based on a query for sorting + # Loads members from the database with custom field values and applies search/sort filters. + # + # Process: + # 1. Builds base query with selected fields + # 2. Loads custom field values for visible custom fields + # 3. Applies search filter if provided + # 4. Applies sorting (database-level for regular fields, in-memory for custom fields) + # 5. Filters custom field values to only visible ones (reduces memory usage) + # + # Performance Considerations: + # - In-memory sorting: Custom field sorting is done in memory after loading. + # This is suitable for small to medium datasets (<1000 members). + # For larger datasets, consider implementing database-level sorting or pagination. + # - Memory filtering: Custom field values are filtered after loading to reduce + # memory usage, but all members are still loaded into memory. + # - No pagination: All matching members are loaded at once. For large result sets, + # consider implementing pagination (see Issue #165). + # + # Returns the socket with `:members` assigned. defp load_members(socket, search_query) do query = Mv.Membership.Member @@ -232,16 +326,61 @@ defmodule MvWeb.MemberLive.Index do :join_date ]) + # Load custom field values for visible custom fields + custom_field_ids = Enum.map(socket.assigns.custom_fields_visible, & &1.id) + query = load_custom_field_values(query, custom_field_ids) + # Apply the search filter first query = apply_search_filter(query, search_query) # Apply sorting based on current socket state - query = maybe_sort(query, socket.assigns.sort_field, socket.assigns.sort_order) + # For custom fields, we sort after loading + {query, sort_after_load} = maybe_sort(query, socket.assigns.sort_field, socket.assigns.sort_order, socket.assigns.custom_fields_visible) + # Note: Using Ash.read! - errors will be handled by Phoenix LiveView + # This is appropriate for data loading in LiveViews members = Ash.read!(query) + + # Filter custom field values to only visible ones (reduces memory usage) + # Performance: This iterates through all members and their custom_field_values. + # For large datasets (>1000 members), this could be optimized by filtering + # at the database level, but requires more complex Ash queries. + custom_field_ids = MapSet.new(Enum.map(socket.assigns.custom_fields_visible, & &1.id)) + members = Enum.map(members, fn member -> + # Only filter if custom_field_values is loaded (is a list, not Ash.NotLoaded) + if is_list(member.custom_field_values) do + filtered_values = Enum.filter(member.custom_field_values, fn cfv -> + cfv.custom_field_id in custom_field_ids + end) + %{member | custom_field_values: filtered_values} + else + member + end + end) + + # Sort in memory if needed (for custom fields) + members = if sort_after_load do + sort_members_in_memory(members, socket.assigns.sort_field, socket.assigns.sort_order, socket.assigns.custom_fields_visible) + else + members + end + assign(socket, :members, members) end + # Load custom field values for the given custom field IDs + defp load_custom_field_values(query, []) do + query + end + + defp load_custom_field_values(query, custom_field_ids) when length(custom_field_ids) > 0 do + # Load all custom field values with their custom_field relationship + # Note: We filter to visible custom fields after loading to reduce memory usage + # Ash loads relationships efficiently with JOINs, but we only keep visible ones + query + |> Ash.Query.load(custom_field_values: [custom_field: [:id, :name, :value_type]]) + end + # ------------------------------------------------------------- # Helper Functions # ------------------------------------------------------------- @@ -264,15 +403,24 @@ defmodule MvWeb.MemberLive.Index do defp toggle_order(nil), do: :asc # Function to sort the column if needed - defp maybe_sort(query, nil, _), do: query + # Returns {query, sort_after_load} where sort_after_load is true if we need to sort in memory + defp maybe_sort(query, nil, _, _), do: {query, false} - defp maybe_sort(query, field, :asc) when not is_nil(field), - do: Ash.Query.sort(query, [{field, :asc}]) + defp maybe_sort(query, field, order, _custom_fields) when not is_nil(field) do + if custom_field_sort?(field) do + # Custom fields need to be sorted in memory after loading + {query, true} + else + # Only sort by atom fields (regular member fields) in database + if is_atom(field) do + {Ash.Query.sort(query, [{field, order}]), false} + else + {query, false} + end + end + end - defp maybe_sort(query, field, :desc) when not is_nil(field), - do: Ash.Query.sort(query, [{field, :desc}]) - - defp maybe_sort(query, _, _), do: query + defp maybe_sort(query, _, _, _), do: {query, false} # Validate that a field is sortable defp valid_sort_field?(field) when is_atom(field) do @@ -288,12 +436,156 @@ defmodule MvWeb.MemberLive.Index do :join_date ] - field in valid_fields + field in valid_fields or custom_field_sort?(field) + end + + defp valid_sort_field?(field) when is_binary(field) do + custom_field_sort?(field) end defp valid_sort_field?(_), do: false - # Function to maybe update the sort + # Check if field is a custom field sort field (format: custom_field_) + defp custom_field_sort?(field) when is_atom(field) do + field_str = Atom.to_string(field) + String.starts_with?(field_str, @custom_field_prefix) + end + + defp custom_field_sort?(field) when is_binary(field) do + String.starts_with?(field, @custom_field_prefix) + end + + defp custom_field_sort?(_), do: false + + # Extracts the custom field ID from a sort field name. + # + # Sort fields for custom fields use the format: "custom_field_" + # This function extracts the ID part. + # + # Examples: + # extract_custom_field_id("custom_field_123") -> "123" + # extract_custom_field_id(:custom_field_123) -> "123" + # extract_custom_field_id("first_name") -> nil + defp extract_custom_field_id(field) when is_atom(field) do + field_str = Atom.to_string(field) + extract_custom_field_id(field_str) + end + + defp extract_custom_field_id(field) when is_binary(field) do + case String.split(field, @custom_field_prefix) do + ["", id_str] -> id_str + _ -> nil + end + end + + defp extract_custom_field_id(_), do: nil + + # Sorts members in memory by a custom field value. + # + # Process: + # 1. Extracts custom field ID from sort field name + # 2. Finds the corresponding CustomField resource + # 3. Splits members into those with values and those without + # 4. Sorts members with values by the extracted value + # 5. Combines: sorted values first, then NULL/empty values at the end + # + # Performance Note: + # This function sorts in memory, which is suitable for small to medium datasets (<1000 members). + # For larger datasets, consider implementing database-level sorting or pagination. + # + # Parameters: + # - `members` - List of Member resources to sort + # - `field` - Sort field name (format: "custom_field_" or atom) + # - `order` - Sort order (`:asc` or `:desc`) + # - `custom_fields` - List of visible CustomField resources + # + # Returns the sorted list of members. + defp sort_members_in_memory(members, field, order, custom_fields) do + custom_field_id_str = extract_custom_field_id(field) + + case custom_field_id_str do + nil -> + members + + id_str -> + # Find the custom field by matching the ID string + custom_field = + Enum.find(custom_fields, fn cf -> + to_string(cf.id) == id_str + end) + + case custom_field do + nil -> + members + + cf -> + # Split members into those with values and those without (NULL/empty) + {members_with_values, members_without_values} = + Enum.split_with(members, fn member -> + case get_custom_field_value(member, cf) do + nil -> false + cfv -> + extracted = extract_sort_value(cfv.value, cf.value_type) + not is_empty_value(extracted, cf.value_type) + end + end) + + # Sort members with values + sorted_with_values = Enum.sort_by(members_with_values, fn member -> + cfv = get_custom_field_value(member, cf) + extracted = extract_sort_value(cfv.value, cf.value_type) + normalize_sort_value(extracted, order) + end) + + # For DESC, reverse only the members with values + sorted_with_values = if order == :desc do + Enum.reverse(sorted_with_values) + else + sorted_with_values + end + + # Combine: sorted values first, then NULL/empty values at the end + sorted_with_values ++ members_without_values + end + end + end + + # Extracts a sortable value from a custom field value based on its type. + # + # Handles different value formats: + # - `%Ash.Union{}` - Extracts value and type from union + # - Direct values - Returns as-is for primitive types + # + # Returns the extracted value suitable for sorting. + defp extract_sort_value(%Ash.Union{value: value, type: type}, _expected_type) do + extract_sort_value(value, type) + end + + defp extract_sort_value(value, :string) when is_binary(value), do: value + defp extract_sort_value(value, :integer) when is_integer(value), do: value + defp extract_sort_value(value, :boolean) when is_boolean(value), do: value + defp extract_sort_value(%Date{} = date, :date), do: date + defp extract_sort_value(value, :email) when is_binary(value), do: value + defp extract_sort_value(value, _type), do: to_string(value) + + # Check if a value is considered empty (NULL or empty string) + defp is_empty_value(value, :string) when is_binary(value) do + String.trim(value) == "" + end + defp is_empty_value(value, :email) when is_binary(value) do + String.trim(value) == "" + end + defp is_empty_value(_value, _type), do: false + + # Normalize sort value for DESC order + # For DESC, we sort ascending first, then reverse the list + # This function is kept for consistency but doesn't need to invert values + defp normalize_sort_value(value, _order), do: value + + + # Updates sort field and order from URL parameters if present. + # + # Validates the sort field and order, falling back to defaults if invalid. defp maybe_update_sort(socket, %{"sort_field" => sf, "sort_order" => so}) do field = determine_field(socket.assigns.sort_field, sf) order = determine_order(socket.assigns.sort_order, so) @@ -305,33 +597,50 @@ defmodule MvWeb.MemberLive.Index do defp maybe_update_sort(socket, _), do: socket - defp determine_field(default, sf) do - case sf do - "" -> - default + # Determine sort field from URL parameter, validating against allowed fields + defp determine_field(default, ""), do: default + defp determine_field(default, nil), do: default - nil -> - default - - sf when is_binary(sf) -> - sf - |> String.to_existing_atom() - |> handle_atom_conversion(default) - - sf when is_atom(sf) -> - handle_atom_conversion(sf, default) - - _ -> - default + # Determines the valid sort field from a URL parameter. + # + # Validates the field against allowed sort fields (regular member fields or custom fields). + # Falls back to default if the field is invalid. + # + # Parameters: + # - `default` - Default field to use if validation fails + # - `sf` - Sort field from URL (can be atom, string, nil, or empty string) + # + # Returns a valid sort field (atom or string for custom fields). + defp determine_field(default, sf) when is_binary(sf) do + # Check if it's a custom field sort (starts with "custom_field_") + if custom_field_sort?(sf) do + if valid_sort_field?(sf), do: sf, else: default + else + # Try to convert to atom for regular fields + try do + atom = String.to_existing_atom(sf) + if valid_sort_field?(atom), do: atom, else: default + rescue + ArgumentError -> default + end end end - defp handle_atom_conversion(val, default) when is_atom(val) do - if valid_sort_field?(val), do: val, else: default + defp determine_field(default, sf) when is_atom(sf) do + if valid_sort_field?(sf), do: sf, else: default end - defp handle_atom_conversion(_, default), do: default + defp determine_field(default, _), do: default + # Determines the valid sort order from a URL parameter. + # + # Validates that the order is either "asc" or "desc", falling back to default if invalid. + # + # Parameters: + # - `default` - Default order to use if validation fails + # - `so` - Sort order from URL (string, atom, nil, or empty string) + # + # Returns `:asc` or `:desc`. defp determine_order(default, so) do case so do "" -> default @@ -350,4 +659,32 @@ defmodule MvWeb.MemberLive.Index do # Keep the previous search query if no new one is provided socket end + + # ------------------------------------------------------------- + # Helper Functions for Custom Field Values + # ------------------------------------------------------------- + + # Retrieves the custom field value for a specific member and custom field. + # + # Searches through the member's `custom_field_values` relationship to find + # the value matching the given custom field. + # + # Returns: + # - `%CustomFieldValue{}` if found + # - `nil` if not found or if member has no custom field values + # + # Examples: + # get_custom_field_value(member, custom_field) -> %CustomFieldValue{...} + # get_custom_field_value(member, non_existent_field) -> nil + def get_custom_field_value(member, custom_field) do + case member.custom_field_values do + nil -> nil + values when is_list(values) -> + Enum.find(values, fn cfv -> + cfv.custom_field_id == custom_field.id or + (cfv.custom_field && cfv.custom_field.id == custom_field.id) + end) + _ -> nil + end + end end diff --git a/lib/mv_web/live/member_live/index.html.heex b/lib/mv_web/live/member_live/index.html.heex index cb2ccd8..67fa804 100644 --- a/lib/mv_web/live/member_live/index.html.heex +++ b/lib/mv_web/live/member_live/index.html.heex @@ -19,6 +19,9 @@ id="members" rows={@members} row_click={fn member -> JS.navigate(~p"/members/#{member}") end} + dynamic_cols={@dynamic_cols} + sort_field={@sort_field} + sort_order={@sort_order} > @@ -185,7 +188,6 @@ > {member.join_date} - <:action :let={member}>
<.link navigate={~p"/members/#{member}"}>{gettext("Show")} diff --git a/lib/mv_web/live/member_live/index/formatter.ex b/lib/mv_web/live/member_live/index/formatter.ex new file mode 100644 index 0000000..d97966c --- /dev/null +++ b/lib/mv_web/live/member_live/index/formatter.ex @@ -0,0 +1,78 @@ +defmodule MvWeb.MemberLive.Index.Formatter do + @moduledoc """ + Formats custom field values for display in the member overview table. + + Handles different value types (string, integer, boolean, date, email) and + formats them appropriately for display in the UI. + """ + use Gettext, backend: MvWeb.Gettext + + @doc """ + Formats a custom field value for display. + + Handles different input formats: + - `nil` - Returns empty string + - `%Ash.Union{}` - Extracts value and type from union type + - Map (JSONB format) - Extracts type and value from map keys + - Direct value - Uses custom_field.value_type to determine format + + ## Examples + + iex> format_custom_field_value(nil, %CustomField{value_type: :string}) + "" + + iex> format_custom_field_value("test", %CustomField{value_type: :string}) + "test" + + iex> format_custom_field_value(true, %CustomField{value_type: :boolean}) + "Yes" + """ + def format_custom_field_value(nil, _custom_field), do: "" + + def format_custom_field_value(%Ash.Union{value: value, type: type}, custom_field) do + format_value_by_type(value, type, custom_field) + end + + def format_custom_field_value(value, custom_field) when is_map(value) do + # Handle map format from JSONB + type = Map.get(value, "type") || Map.get(value, "_union_type") + val = Map.get(value, "value") || Map.get(value, "_union_value") + format_value_by_type(val, type, custom_field) + end + + def format_custom_field_value(value, custom_field) do + format_value_by_type(value, custom_field.value_type, custom_field) + end + + # Format value based on type + defp format_value_by_type(value, :string, _) when is_binary(value) do + # Return empty string if value is empty, otherwise return the value + if String.trim(value) == "", do: "", else: value + end + + defp format_value_by_type(value, :string, _), do: to_string(value) + + defp format_value_by_type(value, :integer, _), do: to_string(value) + + defp format_value_by_type(value, :email, _) when is_binary(value) do + # Return empty string if value is empty + if String.trim(value) == "", do: "", else: value + end + + defp format_value_by_type(value, :email, _), do: to_string(value) + + defp format_value_by_type(value, :boolean, _) when value == true, do: gettext("Yes") + defp format_value_by_type(value, :boolean, _) when value == false, do: gettext("No") + defp format_value_by_type(value, :boolean, _), do: to_string(value) + + defp format_value_by_type(%Date{} = date, :date, _), do: Date.to_string(date) + + defp format_value_by_type(value, :date, _) when is_binary(value) do + case Date.from_iso8601(value) do + {:ok, date} -> Date.to_string(date) + _ -> value + end + end + + defp format_value_by_type(value, _type, _), do: to_string(value) +end