From d71d5881cf45212312ae308298ff552c9f049d94 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 4 Mar 2026 20:12:45 +0100 Subject: [PATCH 1/2] CSV export: apply cycle_status_filter and boolean_filters when exporting all --- lib/mv/membership/member_export.ex | 27 ++++++++++++ .../controllers/member_export_controller.ex | 44 +++++++++++++++++-- .../member_export_controller_test.exs | 43 ++++++++++++++++++ 3 files changed, 110 insertions(+), 4 deletions(-) diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex index 635c832..1c004f3 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -378,6 +378,33 @@ defmodule Mv.Membership.MemberExport do end end + @doc """ + Applies export filters (cycle status and boolean custom field filters) when exporting "all" (no selected_ids). + + Used by the CSV export controller so that "Export (all)" with active filters exports only the filtered members, + matching PDF export behavior. + + - `members` - Loaded members (must have cycle data loaded when cycle_status_filter is used). + - `opts` - Map with `:selected_ids`, `:cycle_status_filter`, `:show_current_cycle`, `:boolean_filters`. + - `custom_fields_by_id` - Map of custom field id => custom field struct (for boolean filter resolution). + + When `opts.selected_ids` is not empty, returns `members` unchanged. Otherwise applies + cycle status filter and boolean custom field filters. + """ + @spec apply_export_filters([struct()], map(), map()) :: [struct()] + def apply_export_filters(members, opts, custom_fields_by_id) do + if opts[:selected_ids] != [] do + members + else + members + |> apply_cycle_status_filter(opts[:cycle_status_filter], opts[:show_current_cycle]) + |> Index.apply_boolean_custom_field_filters( + Map.get(opts, :boolean_filters, %{}), + Map.values(custom_fields_by_id) + ) + end + end + defp extract_list(params, key) do case Map.get(params, key) do list when is_list(list) -> list diff --git a/lib/mv_web/controllers/member_export_controller.ex b/lib/mv_web/controllers/member_export_controller.ex index a1730ee..2c299a3 100644 --- a/lib/mv_web/controllers/member_export_controller.ex +++ b/lib/mv_web/controllers/member_export_controller.ex @@ -13,6 +13,7 @@ defmodule MvWeb.MemberExportController do alias Mv.Authorization.Actor alias Mv.Membership.CustomField alias Mv.Membership.Member + alias Mv.Membership.MemberExport alias Mv.Membership.MembersCSV alias MvWeb.MemberLive.Index.MembershipFeeStatus alias MvWeb.Translations.MemberFields @@ -76,10 +77,34 @@ defmodule MvWeb.MemberExportController do 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") + 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 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) and match?({:ok, _}, Ecto.UUID.cast(k)) + end) + |> Enum.into(%{}) + + _ -> + %{} + end + end + defp split_member_fields(member_fields) do domain_fields = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) selectable = Enum.filter(member_fields, fn f -> f in domain_fields end) @@ -156,7 +181,10 @@ defmodule MvWeb.MemberExportController do parsed |> ensure_sort_custom_field_loaded() - with {:ok, custom_fields_by_id} <- load_custom_fields_by_id(parsed.custom_field_ids, actor), + 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_for_export(actor, parsed, custom_fields_by_id) do columns = build_columns(conn, parsed, custom_fields_by_id) csv_iodata = MembersCSV.export(members, columns) @@ -232,8 +260,12 @@ defmodule MvWeb.MemberExportController do defp load_members_for_export(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 || %{})) |> Enum.uniq() + need_cycles = - parsed.computed_fields != [] and "membership_fee_status" in parsed.computed_fields + (parsed.computed_fields != [] and "membership_fee_status" in parsed.computed_fields) or + parsed.cycle_status_filter != nil need_groups = "groups" in parsed.member_fields @@ -245,7 +277,7 @@ defmodule MvWeb.MemberExportController do Member |> Ash.Query.new() |> Ash.Query.select(select_fields) - |> load_custom_field_values_query(parsed.custom_field_ids) + |> load_custom_field_values_query(custom_field_ids_union) |> maybe_load_cycles(need_cycles, parsed.show_current_cycle) |> maybe_load_groups(need_groups) |> maybe_load_membership_fee_type(need_membership_fee_type) @@ -276,6 +308,10 @@ defmodule MvWeb.MemberExportController do members end + # When exporting "all" (no selected_ids), apply same filters as PDF: cycle status and boolean custom fields + members = + MemberExport.apply_export_filters(members, parsed, custom_fields_by_id) + # Calculate membership_fee_status for computed fields members = add_computed_fields(members, parsed.computed_fields, parsed.show_current_cycle) diff --git a/test/mv_web/controllers/member_export_controller_test.exs b/test/mv_web/controllers/member_export_controller_test.exs index 4192a27..9d9d197 100644 --- a/test/mv_web/controllers/member_export_controller_test.exs +++ b/test/mv_web/controllers/member_export_controller_test.exs @@ -564,6 +564,49 @@ defmodule MvWeb.MemberExportControllerTest do assert phone_idx < membership_idx assert membership_idx < active_idx end + + test "exports only filtered members when selected_ids empty and boolean_filters set (Export all)", + %{ + conn: conn, + boolean_field: boolean_field, + member_with_boolean: member_with_boolean, + member_with_string: member_with_string, + member_with_integer: member_with_integer, + member_without_value: member_without_value + } do + # Simulate "filter + Export (all)": no selection, but boolean filter "Active Member = true" + payload = %{ + "selected_ids" => [], + "member_fields" => ["first_name", "last_name"], + "custom_field_ids" => [boolean_field.id], + "query" => nil, + "sort_field" => nil, + "sort_order" => nil, + "boolean_filters" => %{to_string(boolean_field.id) => true} + } + + conn = get(conn, "/members") + csrf_token = csrf_token_from_conn(conn) + + conn = + post(conn, "/members/export.csv", %{ + "payload" => Jason.encode!(payload), + "_csrf_token" => csrf_token + }) + + assert conn.status == 200 + body = response(conn, 200) + lines = export_lines(body) + + # Header + data rows: only members matching the boolean filter (Active Member = true) + assert length(lines) >= 2 + assert body =~ "Boolean" + assert body =~ member_with_boolean.last_name + # Other test members (no value or different value for that custom field) must not appear + refute body =~ member_with_string.last_name + refute body =~ member_with_integer.last_name + refute body =~ member_without_value.last_name + end end describe "POST /members/export.pdf" do From fc7b0351230a945c8e0a7a9869dfbcd3e5f43972 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 4 Mar 2026 20:50:52 +0100 Subject: [PATCH 2/2] CSV export: robust apply_export_filters, single custom_field_ids_union, string boolean_filters, more tests --- lib/mv/membership/member_export.ex | 15 +- .../controllers/member_export_controller.ex | 43 ++++-- .../member_export_controller_test.exs | 136 ++++++++++++++++++ 3 files changed, 176 insertions(+), 18 deletions(-) diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex index 1c004f3..16341c4 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -388,20 +388,25 @@ defmodule Mv.Membership.MemberExport do - `opts` - Map with `:selected_ids`, `:cycle_status_filter`, `:show_current_cycle`, `:boolean_filters`. - `custom_fields_by_id` - Map of custom field id => custom field struct (for boolean filter resolution). - When `opts.selected_ids` is not empty, returns `members` unchanged. Otherwise applies - cycle status filter and boolean custom field filters. + When `opts.selected_ids` is not empty, returns `members` unchanged (selected_ids + override filters). Otherwise applies cycle status filter and boolean custom field filters. + + Uses `Map.get(opts, :selected_ids, [])` so that `nil` or a missing key is treated as + "export all" and filters are applied. """ @spec apply_export_filters([struct()], map(), map()) :: [struct()] def apply_export_filters(members, opts, custom_fields_by_id) do - if opts[:selected_ids] != [] do - members - else + selected_ids = Map.get(opts, :selected_ids, []) + + if Enum.empty?(selected_ids) do members |> apply_cycle_status_filter(opts[:cycle_status_filter], opts[:show_current_cycle]) |> Index.apply_boolean_custom_field_filters( Map.get(opts, :boolean_filters, %{}), Map.values(custom_fields_by_id) ) + else + members end end diff --git a/lib/mv_web/controllers/member_export_controller.ex b/lib/mv_web/controllers/member_export_controller.ex index 2c299a3..9b08f5d 100644 --- a/lib/mv_web/controllers/member_export_controller.ex +++ b/lib/mv_web/controllers/member_export_controller.ex @@ -66,6 +66,9 @@ defmodule MvWeb.MemberExportController do defp parse_and_validate(params) do member_fields = filter_allowed_member_fields(extract_list(params, "member_fields")) {selectable_member_fields, computed_fields} = split_member_fields(member_fields) + custom_field_ids = filter_valid_uuids(extract_list(params, "custom_field_ids")) + boolean_filters = extract_boolean_filters(params) + custom_field_ids_union = (custom_field_ids ++ Map.keys(boolean_filters)) |> Enum.uniq() %{ selected_ids: filter_valid_uuids(extract_list(params, "selected_ids")), @@ -73,16 +76,19 @@ defmodule MvWeb.MemberExportController do selectable_member_fields: selectable_member_fields, computed_fields: computed_fields ++ filter_existing_atoms(extract_list(params, "computed_fields")), - custom_field_ids: filter_valid_uuids(extract_list(params, "custom_field_ids")), + custom_field_ids: custom_field_ids, + custom_field_ids_union: custom_field_ids_union, 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) + boolean_filters: boolean_filters } end + # Only paid and unpaid are supported for list/export filter. :suspended exists in the + # domain (e.g. membership fee status display) but is not used as a filter in the member index. defp extract_cycle_status_filter(params) do case Map.get(params, "cycle_status_filter") do "paid" -> :paid @@ -91,13 +97,15 @@ defmodule MvWeb.MemberExportController do end end + # Normalizes values so that "true"/"false" from query/form encoding are accepted as well as JSON booleans. 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) and match?({:ok, _}, Ecto.UUID.cast(k)) + is_binary(k) and match?({:ok, _}, Ecto.UUID.cast(k)) and boolean_value?(v) end) + |> Enum.map(fn {k, v} -> {k, normalize_boolean_value(v)} end) |> Enum.into(%{}) _ -> @@ -105,6 +113,14 @@ defmodule MvWeb.MemberExportController do end end + defp boolean_value?(v) when is_boolean(v), do: true + defp boolean_value?(v) when v in ["true", "false"], do: true + defp boolean_value?(_), do: false + + defp normalize_boolean_value(v) when is_boolean(v), do: v + defp normalize_boolean_value("true"), do: true + defp normalize_boolean_value("false"), do: false + defp split_member_fields(member_fields) do domain_fields = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1) selectable = Enum.filter(member_fields, fn f -> f in domain_fields end) @@ -181,10 +197,8 @@ defmodule MvWeb.MemberExportController do parsed |> ensure_sort_custom_field_loaded() - 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), + with {:ok, custom_fields_by_id} <- + load_custom_fields_by_id(parsed.custom_field_ids_union, actor), {:ok, members} <- load_members_for_export(actor, parsed, custom_fields_by_id) do columns = build_columns(conn, parsed, custom_fields_by_id) csv_iodata = MembersCSV.export(members, columns) @@ -202,13 +216,19 @@ defmodule MvWeb.MemberExportController do end end - defp ensure_sort_custom_field_loaded(%{custom_field_ids: ids, sort_field: sort_field} = parsed) do + defp ensure_sort_custom_field_loaded( + %{custom_field_ids: ids, custom_field_ids_union: union, sort_field: sort_field} = parsed + ) do case extract_sort_custom_field_id(sort_field) do nil -> parsed id -> - %{parsed | custom_field_ids: Enum.uniq([id | ids])} + %{ + parsed + | custom_field_ids: Enum.uniq([id | ids]), + custom_field_ids_union: Enum.uniq([id | union]) + } end end @@ -260,9 +280,6 @@ defmodule MvWeb.MemberExportController do defp load_members_for_export(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 || %{})) |> Enum.uniq() - need_cycles = (parsed.computed_fields != [] and "membership_fee_status" in parsed.computed_fields) or parsed.cycle_status_filter != nil @@ -277,7 +294,7 @@ defmodule MvWeb.MemberExportController do Member |> Ash.Query.new() |> Ash.Query.select(select_fields) - |> load_custom_field_values_query(custom_field_ids_union) + |> load_custom_field_values_query(parsed.custom_field_ids_union) |> maybe_load_cycles(need_cycles, parsed.show_current_cycle) |> maybe_load_groups(need_groups) |> maybe_load_membership_fee_type(need_membership_fee_type) diff --git a/test/mv_web/controllers/member_export_controller_test.exs b/test/mv_web/controllers/member_export_controller_test.exs index 9d9d197..78bc5f9 100644 --- a/test/mv_web/controllers/member_export_controller_test.exs +++ b/test/mv_web/controllers/member_export_controller_test.exs @@ -119,6 +119,78 @@ defmodule MvWeb.MemberExportControllerTest do assert body =~ "Carol" end + test "selected_ids override filters: only selected members exported when filters also set", %{ + conn: conn, + member1: m1, + member2: m2, + member3: _m3 + } do + # When selected_ids is set, cycle_status_filter and boolean_filters must not reduce the set: + # only the selected members are exported. + payload = %{ + "selected_ids" => [m1.id, m2.id], + "member_fields" => ["first_name", "last_name", "email"], + "custom_field_ids" => [], + "query" => nil, + "sort_field" => nil, + "sort_order" => nil, + "cycle_status_filter" => "paid", + "boolean_filters" => %{} + } + + conn = get(conn, "/members") + csrf_token = csrf_token_from_conn(conn) + + conn = + post(conn, "/members/export.csv", %{ + "payload" => Jason.encode!(payload), + "_csrf_token" => csrf_token + }) + + assert conn.status == 200 + body = response(conn, 200) + lines = export_lines(body) + + assert length(lines) == 3 + assert body =~ "Alice" + assert body =~ "Bob" + refute body =~ "Carol" + end + + test "cycle_status_filter applied when export all returns CSV", %{ + conn: conn, + member1: _m1, + member2: _m2, + member3: _m3 + } do + payload = %{ + "selected_ids" => [], + "member_fields" => ["first_name", "email"], + "custom_field_ids" => [], + "query" => nil, + "sort_field" => nil, + "sort_order" => nil, + "cycle_status_filter" => "paid", + "show_current_cycle" => true + } + + conn = get(conn, "/members") + csrf_token = csrf_token_from_conn(conn) + + conn = + post(conn, "/members/export.csv", %{ + "payload" => Jason.encode!(payload), + "_csrf_token" => csrf_token + }) + + assert conn.status == 200 + assert get_resp_header(conn, "content-type") |> List.first() =~ "text/csv" + body = response(conn, 200) + lines = export_lines(body) + assert lines != [] + assert hd(lines) =~ "First Name" + end + test "filters out unknown member fields from export", %{conn: conn, member1: m1} do payload = %{ "selected_ids" => [m1.id], @@ -607,6 +679,70 @@ defmodule MvWeb.MemberExportControllerTest do refute body =~ member_with_integer.last_name refute body =~ member_without_value.last_name end + + test "boolean_filters accept string true/false from query encoding", %{ + conn: conn, + boolean_field: boolean_field, + member_with_boolean: member_with_boolean + } do + payload = %{ + "selected_ids" => [], + "member_fields" => ["first_name", "last_name"], + "custom_field_ids" => [boolean_field.id], + "query" => nil, + "sort_field" => nil, + "sort_order" => nil, + "boolean_filters" => %{to_string(boolean_field.id) => "true"} + } + + conn = get(conn, "/members") + csrf_token = csrf_token_from_conn(conn) + + conn = + post(conn, "/members/export.csv", %{ + "payload" => Jason.encode!(payload), + "_csrf_token" => csrf_token + }) + + assert conn.status == 200 + body = response(conn, 200) + assert body =~ member_with_boolean.last_name + end + + test "combination cycle_status_filter and boolean_filters applied when export all", %{ + conn: conn, + boolean_field: boolean_field, + member_with_boolean: _member_with_boolean + } do + # Both filters are applied (AND). Export returns 200 and valid CSV. + payload = %{ + "selected_ids" => [], + "member_fields" => ["first_name", "last_name"], + "custom_field_ids" => [boolean_field.id], + "query" => nil, + "sort_field" => nil, + "sort_order" => nil, + "cycle_status_filter" => "paid", + "show_current_cycle" => true, + "boolean_filters" => %{to_string(boolean_field.id) => true} + } + + conn = get(conn, "/members") + csrf_token = csrf_token_from_conn(conn) + + conn = + post(conn, "/members/export.csv", %{ + "payload" => Jason.encode!(payload), + "_csrf_token" => csrf_token + }) + + assert conn.status == 200 + assert get_resp_header(conn, "content-type") |> List.first() =~ "text/csv" + body = response(conn, 200) + lines = export_lines(body) + assert lines != [] + assert hd(lines) =~ "First Name" + end end describe "POST /members/export.pdf" do