From 9b9e7ec99557b6c0275326c3753719e80c149859 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 5 Feb 2026 15:03:25 +0100 Subject: [PATCH] fix: sorting and filter for export --- lib/mv/membership/member_export.ex | 344 +++++++ lib/mv/membership/member_export_sort.ex | 44 + lib/mv/membership/members_csv.ex | 117 +-- lib/mv_web/components/core_components.ex | 8 +- .../controllers/member_export_controller.ex | 220 ++++- .../field_visibility_dropdown_component.ex | 29 +- lib/mv_web/live/member_live/index.ex | 853 ++++++------------ lib/mv_web/live/member_live/index.html.heex | 1 + .../member_live/index/field_visibility.ex | 110 ++- lib/mv_web/translations/member_fields.ex | 1 + 10 files changed, 1013 insertions(+), 714 deletions(-) create mode 100644 lib/mv/membership/member_export.ex create mode 100644 lib/mv/membership/member_export_sort.ex diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex new file mode 100644 index 0000000..5f771cc --- /dev/null +++ b/lib/mv/membership/member_export.ex @@ -0,0 +1,344 @@ +defmodule Mv.Membership.MemberExport do + @moduledoc """ + Builds member list and column specs for CSV export. + + Used by `MvWeb.MemberExportController`. Does not perform translations; + the controller applies headers (e.g. via `MemberFields.label` / gettext) + and sends the download. + """ + + require Ash.Query + import Ash.Expr + + alias Mv.Membership.CustomField + alias Mv.Membership.Member + alias Mv.Membership.MemberExportSort + alias MvWeb.MemberLive.Index.MembershipFeeStatus + + @member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++ + ["membership_fee_status", "payment_status"] + @computed_export_fields ["membership_fee_status", "payment_status"] + @custom_field_prefix Mv.Constants.custom_field_prefix() + @domain_member_field_strings Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) + + @doc """ + Fetches members and column specs for export. + + - `actor` - Ash actor (e.g. current user) + - `parsed` - Map from controller's parse_and_validate (selected_ids, member_fields, etc.) + + Returns `{:ok, members, column_specs}` or `{:error, :forbidden}`. + Column specs have `:kind`, `:key`, and for custom fields `:custom_field`; + the controller adds `:header` and optional computed columns to members before CSV export. + """ + @spec fetch(struct(), map()) :: + {:ok, [struct()], [map()]} | {:error, :forbidden} + def fetch(actor, parsed) do + custom_field_ids_union = + (parsed.custom_field_ids ++ Map.keys(parsed.boolean_filters || %{})) |> Enum.uniq() + + with {:ok, custom_fields_by_id} <- load_custom_fields_by_id(custom_field_ids_union, actor), + {:ok, members} <- load_members(actor, parsed, custom_fields_by_id) do + column_specs = build_column_specs(parsed, custom_fields_by_id) + {:ok, members, column_specs} + end + end + + defp load_custom_fields_by_id([], _actor), do: {:ok, %{}} + + defp load_custom_fields_by_id(custom_field_ids, actor) do + query = + CustomField + |> Ash.Query.filter(expr(id in ^custom_field_ids)) + |> Ash.Query.select([:id, :name, :value_type]) + + case Ash.read(query, actor: actor) do + {:ok, custom_fields} -> + by_id = + Enum.reduce(custom_field_ids, %{}, fn id, acc -> + case Enum.find(custom_fields, fn cf -> to_string(cf.id) == to_string(id) end) do + nil -> acc + cf -> Map.put(acc, id, cf) + end + end) + + {:ok, by_id} + + {:error, %Ash.Error.Forbidden{}} -> + {:error, :forbidden} + end + end + + defp build_column_specs(parsed, custom_fields_by_id) do + member_specs = + Enum.map(parsed.member_fields, fn f -> + if f in parsed.selectable_member_fields do + %{kind: :member_field, key: f} + else + %{kind: :computed, key: String.to_existing_atom(f)} + end + end) + + custom_specs = + parsed.custom_field_ids + |> Enum.map(fn id -> Map.get(custom_fields_by_id, id) end) + |> Enum.reject(&is_nil/1) + |> Enum.map(fn cf -> %{kind: :custom_field, key: cf.id, custom_field: cf} end) + + member_specs ++ custom_specs + end + + defp load_members(actor, parsed, custom_fields_by_id) do + select_fields = + [:id] ++ Enum.map(parsed.selectable_member_fields, &String.to_existing_atom/1) + + custom_field_ids_union = parsed.custom_field_ids ++ Map.keys(parsed.boolean_filters || %{}) + + need_cycles = + parsed.show_current_cycle or parsed.cycle_status_filter != nil or + parsed.computed_fields != [] + + query = + Member + |> Ash.Query.new() + |> Ash.Query.select(select_fields) + |> load_custom_field_values_query(custom_field_ids_union) + |> maybe_load_cycles(need_cycles, parsed.show_current_cycle) + + query = + if parsed.selected_ids != [] do + Ash.Query.filter(query, expr(id in ^parsed.selected_ids)) + else + query + |> apply_search(parsed.query) + |> then(fn q -> + {q, _sort_after_load} = maybe_sort(q, parsed.sort_field, parsed.sort_order) + q + end) + end + + case Ash.read(query, actor: actor) do + {:ok, members} -> + members = + if parsed.selected_ids == [] do + members + |> apply_cycle_status_filter(parsed.cycle_status_filter, parsed.show_current_cycle) + |> MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( + parsed.boolean_filters || %{}, + Map.values(custom_fields_by_id) + ) + else + members + end + + members = + if parsed.selected_ids == [] and sort_after_load?(parsed.sort_field) do + sort_members_by_custom_field( + members, + parsed.sort_field, + parsed.sort_order, + Map.values(custom_fields_by_id) + ) + else + members + end + + {:ok, members} + + {:error, %Ash.Error.Forbidden{}} -> + {:error, :forbidden} + end + end + + defp load_custom_field_values_query(query, []), do: query + + defp load_custom_field_values_query(query, custom_field_ids) do + cfv_query = + Mv.Membership.CustomFieldValue + |> Ash.Query.filter(expr(custom_field_id in ^custom_field_ids)) + |> Ash.Query.load(custom_field: [:id, :name, :value_type]) + + Ash.Query.load(query, custom_field_values: cfv_query) + end + + defp apply_search(query, nil), do: query + defp apply_search(query, ""), do: query + + defp apply_search(query, q) when is_binary(q) do + if String.trim(q) != "" do + Member.fuzzy_search(query, %{query: q}) + else + query + end + end + + defp maybe_sort(query, nil, _order), do: {query, false} + defp maybe_sort(query, _field, nil), do: {query, false} + + defp maybe_sort(query, field, order) when is_binary(field) do + if custom_field_sort?(field) do + {query, true} + else + field_atom = String.to_existing_atom(field) + + if field_atom in (Mv.Constants.member_fields() -- [:notes]) do + {Ash.Query.sort(query, [{field_atom, String.to_existing_atom(order)}]), false} + else + {query, false} + end + end + rescue + ArgumentError -> {query, false} + end + + defp sort_after_load?(field) when is_binary(field), + do: String.starts_with?(field, @custom_field_prefix) + + defp sort_after_load?(_), do: false + + defp sort_members_by_custom_field(members, _field, _order, _custom_fields) when members == [], + do: [] + + defp sort_members_by_custom_field(members, field, order, custom_fields) when is_binary(field) do + id_str = String.trim_leading(field, @custom_field_prefix) + custom_field = Enum.find(custom_fields, fn cf -> to_string(cf.id) == id_str end) + if is_nil(custom_field), do: members + + key_fn = fn member -> + cfv = find_cfv(member, custom_field) + raw = if cfv, do: cfv.value, else: nil + MemberExportSort.custom_field_sort_key(custom_field.value_type, raw) + end + + members + |> Enum.map(fn m -> {m, key_fn.(m)} end) + |> Enum.sort(fn {_, ka}, {_, kb} -> MemberExportSort.key_lt(ka, kb, order) end) + |> Enum.map(fn {m, _} -> m end) + end + + defp find_cfv(member, custom_field) do + (member.custom_field_values || []) + |> Enum.find(fn cfv -> + to_string(cfv.custom_field_id) == to_string(custom_field.id) or + (Map.get(cfv, :custom_field) && + to_string(cfv.custom_field.id) == to_string(custom_field.id)) + end) + end + + defp custom_field_sort?(field), do: String.starts_with?(field, @custom_field_prefix) + + defp maybe_load_cycles(query, false, _show_current), do: query + + defp maybe_load_cycles(query, true, show_current) do + MembershipFeeStatus.load_cycles_for_members(query, show_current) + end + + defp apply_cycle_status_filter(members, nil, _show_current), do: members + + defp apply_cycle_status_filter(members, status, show_current) when status in [:paid, :unpaid] do + MembershipFeeStatus.filter_members_by_cycle_status(members, status, show_current) + end + + defp apply_cycle_status_filter(members, _status, _show_current), do: members + + # Called by controller to build parsed map from raw params (kept here so controller stays thin) + @doc """ + Parses and validates export params (from JSON payload). + + Returns a map with :selected_ids, :member_fields, :selectable_member_fields, + :computed_fields, :custom_field_ids, :query, :sort_field, :sort_order, + :show_current_cycle, :cycle_status_filter, :boolean_filters. + """ + @spec parse_params(map()) :: map() + def parse_params(params) do + member_fields = filter_allowed_member_fields(extract_list(params, "member_fields")) + {selectable_member_fields, computed_fields} = split_member_fields(member_fields) + + %{ + selected_ids: filter_valid_uuids(extract_list(params, "selected_ids")), + member_fields: member_fields, + selectable_member_fields: selectable_member_fields, + computed_fields: computed_fields, + custom_field_ids: filter_valid_uuids(extract_list(params, "custom_field_ids")), + query: extract_string(params, "query"), + sort_field: extract_string(params, "sort_field"), + sort_order: extract_sort_order(params), + show_current_cycle: extract_boolean(params, "show_current_cycle"), + cycle_status_filter: extract_cycle_status_filter(params), + boolean_filters: extract_boolean_filters(params) + } + end + + defp split_member_fields(member_fields) do + selectable = Enum.filter(member_fields, fn f -> f in @domain_member_field_strings end) + computed = Enum.filter(member_fields, fn f -> f in @computed_export_fields end) + {selectable, computed} + end + + defp extract_boolean(params, key) do + case Map.get(params, key) do + true -> true + "true" -> true + _ -> false + end + end + + defp extract_cycle_status_filter(params) do + case Map.get(params, "cycle_status_filter") do + "paid" -> :paid + "unpaid" -> :unpaid + _ -> nil + end + end + + defp extract_boolean_filters(params) do + case Map.get(params, "boolean_filters") do + map when is_map(map) -> + map + |> Enum.filter(fn {k, v} -> is_binary(k) and is_boolean(v) end) + |> Enum.filter(fn {k, _} -> match?({:ok, _}, Ecto.UUID.cast(k)) end) + |> Enum.into(%{}) + + _ -> + %{} + end + end + + defp extract_list(params, key) do + case Map.get(params, key) do + list when is_list(list) -> list + _ -> [] + end + end + + defp extract_string(params, key) do + case Map.get(params, key) do + s when is_binary(s) -> s + _ -> nil + end + end + + defp extract_sort_order(params) do + case Map.get(params, "sort_order") do + "asc" -> "asc" + "desc" -> "desc" + _ -> nil + end + end + + defp filter_allowed_member_fields(field_list) do + allowlist = MapSet.new(@member_fields_allowlist) + + field_list + |> Enum.filter(fn field -> is_binary(field) and MapSet.member?(allowlist, field) end) + |> Enum.uniq() + end + + defp filter_valid_uuids(id_list) when is_list(id_list) do + id_list + |> Enum.filter(fn id -> + is_binary(id) and match?({:ok, _}, Ecto.UUID.cast(id)) + end) + |> Enum.uniq() + end +end diff --git a/lib/mv/membership/member_export_sort.ex b/lib/mv/membership/member_export_sort.ex new file mode 100644 index 0000000..324fb75 --- /dev/null +++ b/lib/mv/membership/member_export_sort.ex @@ -0,0 +1,44 @@ +defmodule Mv.Membership.MemberExportSort do + @moduledoc """ + Type-stable sort keys for CSV export custom-field sorting. + + Used only by `MvWeb.MemberExportController` when sorting members by a custom field + after load. Nil values sort last in ascending order and first in descending order. + String and email comparison is case-insensitive. + """ + @doc """ + Returns a comparable sort key for (value_type, value). + + - Nil: rank 1 so that in asc order nil sorts last, in desc nil sorts first. + - date: chronological (ISO8601 string). + - boolean: false < true (0 < 1). + - integer: numerical order. + - string / email: case-insensitive (downcased). + + Handles Ash.Union in value; value_type is the custom field's value_type atom. + """ + @spec custom_field_sort_key(:string | :integer | :boolean | :date | :email, term()) :: + {0 | 1, term()} + def custom_field_sort_key(_value_type, nil), do: {1, nil} + + def custom_field_sort_key(value_type, %Ash.Union{value: value, type: _type}) do + custom_field_sort_key(value_type, value) + end + + def custom_field_sort_key(:date, %Date{} = d), do: {0, Date.to_iso8601(d)} + def custom_field_sort_key(:boolean, true), do: {0, 1} + def custom_field_sort_key(:boolean, false), do: {0, 0} + def custom_field_sort_key(:integer, v) when is_integer(v), do: {0, v} + def custom_field_sort_key(:string, v) when is_binary(v), do: {0, String.downcase(v)} + def custom_field_sort_key(:email, v) when is_binary(v), do: {0, String.downcase(v)} + def custom_field_sort_key(_value_type, v), do: {0, to_string(v)} + + @doc """ + Returns true if key_a should sort before key_b for the given order. + + "asc" -> nil last; "desc" -> nil first. No reverse of list needed. + """ + @spec key_lt({0 | 1, term()}, {0 | 1, term()}, String.t()) :: boolean() + def key_lt(key_a, key_b, "asc"), do: key_a < key_b + def key_lt(key_a, key_b, "desc"), do: key_b < key_a +end diff --git a/lib/mv/membership/members_csv.ex b/lib/mv/membership/members_csv.ex index 6eab399..a0fd463 100644 --- a/lib/mv/membership/members_csv.ex +++ b/lib/mv/membership/members_csv.ex @@ -2,9 +2,11 @@ defmodule Mv.Membership.MembersCSV do @moduledoc """ Exports members to CSV (RFC 4180) as iodata. - Uses NimbleCSV.RFC4180 for encoding. Member fields are formatted as strings; - custom field values use the same formatting logic as the member overview (neutral formatter). - Column order for custom fields follows the key order of the `custom_fields_by_id` map. + Uses a column-based API: `export(members, columns)` where each column has + `header` (display string, e.g. from Web layer), `kind` (:member_field | :custom_field | :computed), + and `key` (member attribute name, custom_field id, or computed key). Custom field columns + include a `custom_field` struct for value formatting. Domain code does not use Gettext; + headers and computed values come from the caller (e.g. controller). """ alias Mv.Membership.CustomFieldValueFormatter alias NimbleCSV.RFC4180 @@ -12,57 +14,82 @@ defmodule Mv.Membership.MembersCSV do @doc """ Exports a list of members to CSV iodata. - - `members` - List of member structs (with optional `custom_field_values` loaded) - - `member_fields` - List of member field names (strings, e.g. `["first_name", "email"]`) - - `custom_fields_by_id` - Map of custom_field_id => %CustomField{}. Key order defines column order. + - `members` - List of member structs or maps (with optional `custom_field_values` loaded) + - `columns` - List of column specs: `%{header: String.t(), kind: :member_field | :custom_field | :computed, key: term()}` + For `:custom_field`, also pass `custom_field: %CustomField{}`. Header is used as-is (localized by caller). Returns iodata suitable for `IO.iodata_to_binary/1` or sending as response body. + RFC 4180 escaping and formula-injection safe_cell are applied. """ - @spec export( - [struct()], - [String.t()], - %{optional(String.t() | Ecto.UUID.t()) => struct()} - ) :: iodata() - def export(members, member_fields, custom_fields_by_id) when is_list(members) do - custom_entries = custom_field_entries(custom_fields_by_id) - header = build_header(member_fields, custom_entries) - rows = Enum.map(members, &build_row(&1, member_fields, custom_entries)) + @spec export([struct() | map()], [map()]) :: iodata() + def export(members, columns) when is_list(members) do + header = build_header(columns) + rows = Enum.map(members, fn member -> build_row(member, columns) end) RFC4180.dump_to_iodata([header | rows]) end - defp custom_field_entries(by_id) when is_map(by_id) do - Enum.map(by_id, fn {id, cf} -> {to_string(id), cf} end) + defp build_header(columns) do + columns + |> Enum.map(fn col -> col.header end) + |> Enum.map(&safe_cell/1) end - defp build_header(member_fields, custom_entries) do - member_headers = member_fields - custom_headers = Enum.map(custom_entries, fn {_id, cf} -> cf.name end) - member_headers ++ custom_headers + defp build_row(member, columns) do + columns + |> Enum.map(fn col -> cell_value(member, col) end) + |> Enum.map(&safe_cell/1) end - defp build_row(member, member_fields, custom_entries) do - member_cells = Enum.map(member_fields, &format_member_field(member, &1)) - - custom_cells = - Enum.map(custom_entries, fn {id, cf} -> format_custom_field(member, id, cf) end) - - member_cells ++ custom_cells - end - - defp format_member_field(member, field_name) do - key = member_field_key(field_name) - value = Map.get(member, key) + defp cell_value(member, %{kind: :member_field, key: key}) do + key_atom = key_to_atom(key) + value = Map.get(member, key_atom) format_member_value(value) end - defp member_field_key(field_name) when is_binary(field_name) do + defp cell_value(member, %{kind: :custom_field, key: id, custom_field: cf}) do + cfv = get_cfv_by_id(member, id) + + if cfv, + do: CustomFieldValueFormatter.format_custom_field_value(cfv.value, cf), + else: "" + end + + defp cell_value(member, %{kind: :computed, key: key}) do + value = Map.get(member, key_to_atom(key)) + if is_binary(value), do: value, else: "" + end + + defp key_to_atom(k) when is_atom(k), do: k + + defp key_to_atom(k) when is_binary(k) do try do - String.to_existing_atom(field_name) + String.to_existing_atom(k) rescue - ArgumentError -> field_name + ArgumentError -> k end end + defp get_cfv_by_id(member, id) do + values = + case Map.get(member, :custom_field_values) do + v when is_list(v) -> v + _ -> [] + end + + id_str = to_string(id) + + Enum.find(values, fn cfv -> + to_string(cfv.custom_field_id) == id_str or + (Map.get(cfv, :custom_field) && to_string(cfv.custom_field.id) == id_str) + end) + end + + @doc false + @spec safe_cell(String.t()) :: String.t() + def safe_cell(s) when is_binary(s) do + if String.starts_with?(s, ["=", "+", "-", "@", "\t"]), do: "'" <> s, else: s + end + defp format_member_value(nil), do: "" defp format_member_value(true), do: "true" defp format_member_value(false), do: "false" @@ -70,22 +97,4 @@ defmodule Mv.Membership.MembersCSV do defp format_member_value(%DateTime{} = dt), do: DateTime.to_iso8601(dt) defp format_member_value(%NaiveDateTime{} = dt), do: NaiveDateTime.to_iso8601(dt) defp format_member_value(value), do: to_string(value) - - defp format_custom_field(member, custom_field_id, custom_field) do - cfv = find_custom_field_value(member, custom_field_id) - - if cfv, - do: CustomFieldValueFormatter.format_custom_field_value(cfv.value, custom_field), - else: "" - end - - defp find_custom_field_value(member, custom_field_id) do - values = Map.get(member, :custom_field_values) || [] - id_str = to_string(custom_field_id) - - Enum.find(values, fn cfv -> - to_string(cfv.custom_field_id) == id_str or - (Map.get(cfv, :custom_field) && to_string(cfv.custom_field.id) == id_str) - end) - end end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 45bcae0..e74020c 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -178,7 +178,8 @@ defmodule MvWeb.CoreComponents do aria-haspopup="menu" aria-expanded={@open} aria-controls={@id} - class="btn" + aria-label={@button_label} + class="btn focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-base-content/20" phx-click="toggle_dropdown" phx-target={@phx_target} data-testid="dropdown-button" @@ -232,11 +233,12 @@ defmodule MvWeb.CoreComponents do