From ad641ed49e9e92ce44ef12a29614e63272080081 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 4 Mar 2026 20:12:45 +0100 Subject: [PATCH 1/5] 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 01b9ebd74bc531581e91682505f7bb1dede9f9cc Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 4 Mar 2026 20:55:29 +0100 Subject: [PATCH 2/5] Vereinfacht client: email normalization, multi-match warning, Bypass tests, doc note - Normalize email (trim + downcase) before filter lookup - Log warning when API returns multiple contacts for same email - Add Bypass tests for find_contact_by_email (query params, empty/single response parsing) - Document vereinfacht_required_field? as legacy/unused in vereinfacht-api.md - Add bypass dependency (dev+test) for HTTP stubbing --- docs/vereinfacht-api.md | 2 +- lib/mv/vereinfacht/client.ex | 17 ++++-- mix.exs | 1 + mix.lock | 6 +++ test/mv/vereinfacht/client_test.exs | 81 ++++++++++++++++++++++++++++- 5 files changed, 100 insertions(+), 7 deletions(-) diff --git a/docs/vereinfacht-api.md b/docs/vereinfacht-api.md index e50274f..1a9cb37 100644 --- a/docs/vereinfacht-api.md +++ b/docs/vereinfacht-api.md @@ -42,6 +42,6 @@ Additional attributes (firstName, lastName, email, address, zipCode, city, count ## References - **Config:** `Mv.Config` (`vereinfacht_api_url`, `vereinfacht_api_key`, `vereinfacht_club_id`, `vereinfacht_app_url`, `vereinfacht_configured?/0`). -- **Constants:** `Mv.Constants.vereinfacht_required_member_fields/0` (empty), `vereinfacht_required_field?/1`. +- **Constants:** `Mv.Constants.vereinfacht_required_member_fields/0` (empty), `vereinfacht_required_field?/1` (legacy; currently unused in UI or validation). - **Tests:** `test/mv/vereinfacht/`, `test/mv/config_vereinfacht_test.exs`; see `test/mv/vereinfacht/vereinfacht_test_README.md` for scope. - **Roadmap:** Payment/transaction import and deeper integration are tracked in `docs/feature-roadmap.md` and `docs/membership-fee-architecture.md`. diff --git a/lib/mv/vereinfacht/client.ex b/lib/mv/vereinfacht/client.ex index 3ed70b8..e7ca04c 100644 --- a/lib/mv/vereinfacht/client.ex +++ b/lib/mv/vereinfacht/client.ex @@ -183,8 +183,9 @@ defmodule Mv.Vereinfacht.Client do end defp do_find_contact_by_email(email) do + normalized_email = email |> String.trim() |> String.downcase() base = base_url() |> String.trim_trailing("/") |> then(&"#{&1}/finance-contacts") - encoded_email = URI.encode_www_form(email |> String.trim()) + encoded_email = URI.encode_www_form(normalized_email) url = "#{base}?filter[isExternal]=true&filter[email]=#{encoded_email}" case Req.get(url, [headers: headers(api_key())] ++ req_http_options()) do @@ -202,11 +203,19 @@ defmodule Mv.Vereinfacht.Client do end end - defp get_first_contact_id_from_list(%{"data" => [%{"id" => id} | _]}) do - normalize_contact_id(id) + defp get_first_contact_id_from_list(%{"data" => data} = _body) when is_list(data) do + if length(data) > 1 do + Logger.warning( + "Vereinfacht find_contact_by_email: API returned multiple contacts for same email (count: #{length(data)}), using first. Check for duplicate or inconsistent data." + ) + end + + case data do + [%{"id" => id} | _] -> normalize_contact_id(id) + [] -> nil + end end - defp get_first_contact_id_from_list(%{"data" => []}), do: nil defp get_first_contact_id_from_list(_), do: nil defp normalize_contact_id(id) when is_binary(id), do: id diff --git a/mix.exs b/mix.exs index d200150..19c598c 100644 --- a/mix.exs +++ b/mix.exs @@ -75,6 +75,7 @@ defmodule Mv.MixProject do {:bandit, "~> 1.5"}, {:mix_audit, "~> 2.1", only: [:dev, :test], runtime: false}, {:sobelow, "~> 0.14", only: [:dev, :test], runtime: false}, + {:bypass, "~> 2.1", only: [:dev, :test]}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, {:picosat_elixir, "~> 0.1"}, {:ecto_commons, "~> 0.3"}, diff --git a/mix.lock b/mix.lock index 113aea8..d9ab997 100644 --- a/mix.lock +++ b/mix.lock @@ -10,11 +10,15 @@ "bandit": {:hex, :bandit, "1.10.3", "1e5d168fa79ec8de2860d1b4d878d97d4fbbe2fdbe7b0a7d9315a4359d1d4bb9", [:mix], [{:hpax, "~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.18", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.0", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "99a52d909c48db65ca598e1962797659e3c0f1d06e825a50c3d75b74a5e2db18"}, "bcrypt_elixir": {:hex, :bcrypt_elixir, "3.3.2", "d50091e3c9492d73e17fc1e1619a9b09d6a5ef99160eb4d736926fd475a16ca3", [:make, :mix], [{:comeonin, "~> 5.3", [hex: :comeonin, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "471be5151874ae7931911057d1467d908955f93554f7a6cd1b7d804cac8cef53"}, "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, + "bypass": {:hex, :bypass, "2.1.0", "909782781bf8e20ee86a9cabde36b259d44af8b9f38756173e8f5e2e1fabb9b1", [:mix], [{:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.0", [hex: :plug_cowboy, repo: "hexpm", optional: false]}, {:ranch, "~> 1.3", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "d9b5df8fa5b7a6efa08384e9bbecfe4ce61c77d28a4282f79e02f1ef78d96b80"}, "castore": {:hex, :castore, "1.0.17", "4f9770d2d45fbd91dcf6bd404cf64e7e58fed04fadda0923dc32acca0badffa2", [:mix], [], "hexpm", "12d24b9d80b910dd3953e165636d68f147a31db945d2dcb9365e441f8b5351e5"}, "cc_precompiler": {:hex, :cc_precompiler, "0.1.11", "8c844d0b9fb98a3edea067f94f616b3f6b29b959b6b3bf25fee94ffe34364768", [:mix], [{:elixir_make, "~> 0.7", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "3427232caf0835f94680e5bcf082408a70b48ad68a5f5c0b02a3bea9f3a075b9"}, "cinder": {:hex, :cinder, "0.12.1", "02ae4988e025fb32c37e4e7f2e491586b952918c0dd99d856da13271cd680e16", [:mix], [{:ash, "~> 3.0", [hex: :ash, repo: "hexpm", optional: false]}, {:ash_phoenix, "~> 2.3", [hex: :ash_phoenix, repo: "hexpm", optional: false]}, {:gettext, "~> 1.0.0", [hex: :gettext, repo: "hexpm", optional: false]}, {:phoenix_live_view, "~> 1.0", [hex: :phoenix_live_view, repo: "hexpm", optional: false]}, {:spark, "~> 2.0", [hex: :spark, repo: "hexpm", optional: false]}], "hexpm", "a48b5677c1f57619d9d7564fb2bd7928f93750a2e8c0b1b145852a30ecf2aa20"}, "circular_buffer": {:hex, :circular_buffer, "1.0.0", "25c004da0cba7bd8bc1bdabded4f9a902d095e20600fd15faf1f2ffbaea18a07", [:mix], [], "hexpm", "c829ec31c13c7bafd1f546677263dff5bfb006e929f25635878ac3cfba8749e5"}, "comeonin": {:hex, :comeonin, "5.5.1", "5113e5f3800799787de08a6e0db307133850e635d34e9fab23c70b6501669510", [:mix], [], "hexpm", "65aac8f19938145377cee73973f192c5645873dcf550a8a6b18187d17c13ccdb"}, + "cowboy": {:hex, :cowboy, "2.14.2", "4008be1df6ade45e4f2a4e9e2d22b36d0b5aba4e20b0a0d7049e28d124e34847", [:make, :rebar3], [{:cowlib, ">= 2.16.0 and < 3.0.0", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, ">= 1.8.0 and < 3.0.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "569081da046e7b41b5df36aa359be71a0c8874e5b9cff6f747073fc57baf1ab9"}, + "cowboy_telemetry": {:hex, :cowboy_telemetry, "0.4.0", "f239f68b588efa7707abce16a84d0d2acf3a0f50571f8bb7f56a15865aae820c", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7d98bac1ee4565d31b62d59f8823dfd8356a169e7fcbb83831b8a5397404c9de"}, + "cowlib": {:hex, :cowlib, "2.16.0", "54592074ebbbb92ee4746c8a8846e5605052f29309d3a873468d76cdf932076f", [:make, :rebar3], [], "hexpm", "7f478d80d66b747344f0ea7708c187645cfcc08b11aa424632f78e25bf05db51"}, "credo": {:hex, :credo, "1.7.16", "a9f1389d13d19c631cb123c77a813dbf16449a2aebf602f590defa08953309d4", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "d0562af33756b21f248f066a9119e3890722031b6d199f22e3cf95550e4f1579"}, "crux": {:hex, :crux, "0.1.2", "4441c9e3a34f1e340954ce96b9ad5a2de13ceb4f97b3f910211227bb92e2ca90", [:mix], [{:picosat_elixir, "~> 0.2", [hex: :picosat_elixir, repo: "hexpm", optional: true]}, {:simple_sat, ">= 0.1.1 and < 1.0.0-0", [hex: :simple_sat, repo: "hexpm", optional: true]}, {:stream_data, "~> 1.0", [hex: :stream_data, repo: "hexpm", optional: true]}], "hexpm", "563ea3748ebfba9cc078e6d198a1d6a06015a8fae503f0b721363139f0ddb350"}, "db_connection": {:hex, :db_connection, "2.9.0", "a6a97c5c958a2d7091a58a9be40caf41ab496b0701d21e1d1abff3fa27a7f371", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "17d502eacaf61829db98facf6f20808ed33da6ccf495354a41e64fe42f9c509c"}, @@ -65,8 +69,10 @@ "phoenix_view": {:hex, :phoenix_view, "2.0.4", "b45c9d9cf15b3a1af5fb555c674b525391b6a1fe975f040fb4d913397b31abf4", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}], "hexpm", "4e992022ce14f31fe57335db27a28154afcc94e9983266835bb3040243eb620b"}, "picosat_elixir": {:hex, :picosat_elixir, "0.2.3", "bf326d0f179fbb3b706bb2c15fbc367dacfa2517157d090fdfc32edae004c597", [:make, :mix], [{:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "f76c9db2dec9d2561ffaa9be35f65403d53e984e8cd99c832383b7ab78c16c66"}, "plug": {:hex, :plug, "1.19.1", "09bac17ae7a001a68ae393658aa23c7e38782be5c5c00c80be82901262c394c0", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "560a0017a8f6d5d30146916862aaf9300b7280063651dd7e532b8be168511e62"}, + "plug_cowboy": {:hex, :plug_cowboy, "2.8.0", "07789e9c03539ee51bb14a07839cc95aa96999fd8846ebfd28c97f0b50c7b612", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:cowboy_telemetry, "~> 0.3", [hex: :cowboy_telemetry, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "9cbfaaf17463334ca31aed38ea7e08a68ee37cabc077b1e9be6d2fb68e0171d0"}, "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"}, "postgrex": {:hex, :postgrex, "0.22.0", "fb027b58b6eab1f6de5396a2abcdaaeb168f9ed4eccbb594e6ac393b02078cbd", [:mix], [{:db_connection, "~> 2.9", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "a68c4261e299597909e03e6f8ff5a13876f5caadaddd0d23af0d0a61afcc5d84"}, + "ranch": {:hex, :ranch, "1.8.1", "208169e65292ac5d333d6cdbad49388c1ae198136e4697ae2f474697140f201c", [:make, :rebar3], [], "hexpm", "aed58910f4e21deea992a67bf51632b6d60114895eb03bb392bb733064594dd0"}, "reactor": {:hex, :reactor, "1.0.0", "024bd13df910bcb8c01cebed4f10bd778269a141a1c8a234e4f67796ac4883cf", [:mix], [{:igniter, "~> 0.4", [hex: :igniter, repo: "hexpm", optional: true]}, {:iterex, "~> 0.1", [hex: :iterex, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:libgraph, "~> 0.16", [hex: :libgraph, repo: "hexpm", optional: false]}, {:spark, ">= 2.3.3 and < 3.0.0-0", [hex: :spark, repo: "hexpm", optional: false]}, {:splode, "~> 0.2", [hex: :splode, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.2", [hex: :telemetry, repo: "hexpm", optional: false]}, {:yaml_elixir, "~> 2.11", [hex: :yaml_elixir, repo: "hexpm", optional: false]}, {:ymlr, "~> 5.0", [hex: :ymlr, repo: "hexpm", optional: false]}], "hexpm", "ae8eb507fffc517f5aa5947db9d2ede2db8bae63b66c94ccb5a2027d30f830a0"}, "req": {:hex, :req, "0.5.17", "0096ddd5b0ed6f576a03dde4b158a0c727215b15d2795e59e0916c6971066ede", [:mix], [{:brotli, "~> 0.3.1", [hex: :brotli, repo: "hexpm", optional: true]}, {:ezstd, "~> 1.0", [hex: :ezstd, repo: "hexpm", optional: true]}, {:finch, "~> 0.17", [hex: :finch, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mime, "~> 2.0.6 or ~> 2.1", [hex: :mime, repo: "hexpm", optional: false]}, {:nimble_csv, "~> 1.0", [hex: :nimble_csv, repo: "hexpm", optional: true]}, {:plug, "~> 1.0", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "0b8bc6ffdfebbc07968e59d3ff96d52f2202d0536f10fef4dc11dc02a2a43e39"}, "rewrite": {:hex, :rewrite, "1.2.0", "80220eb14010e175b67c939397e1a8cdaa2c32db6e2e0a9d5e23e45c0414ce21", [:mix], [{:glob_ex, "~> 0.1", [hex: :glob_ex, repo: "hexpm", optional: false]}, {:sourceror, "~> 1.0", [hex: :sourceror, repo: "hexpm", optional: false]}, {:text_diff, "~> 0.1", [hex: :text_diff, repo: "hexpm", optional: false]}], "hexpm", "a1cd702bbb9d51613ab21091f04a386d750fc6f4516b81900df082d78b2d8c50"}, diff --git a/test/mv/vereinfacht/client_test.exs b/test/mv/vereinfacht/client_test.exs index d326879..dda1e81 100644 --- a/test/mv/vereinfacht/client_test.exs +++ b/test/mv/vereinfacht/client_test.exs @@ -2,8 +2,8 @@ defmodule Mv.Vereinfacht.ClientTest do @moduledoc """ Tests for Mv.Vereinfacht.Client. - Only tests the "not configured" path; no real HTTP calls. Config reads from - ENV first, then from Settings (DB), so we use DataCase so get_settings() is available. + "Not configured" path: no HTTP. When configured we use Bypass to stub the API + and assert on request (query params) and response parsing. """ use Mv.DataCase, async: false @@ -35,6 +35,77 @@ defmodule Mv.Vereinfacht.ClientTest do assert Client.find_contact_by_email("kayley.becker@example.com") == {:error, :not_configured} end + + @tag :bypass + test "sends filter[isExternal]=true and filter[email]= and returns :not_found when data is empty" do + bypass = Bypass.open() + base = "http://127.0.0.1:#{bypass.port}" + set_vereinfacht_env(base) + + Bypass.expect_once(bypass, "GET", "/finance-contacts", fn conn -> + qs = conn.query_string || "" + + assert qs =~ "filter[isExternal]=true", + "expected query to contain filter[isExternal]=true, got: #{inspect(qs)}" + + assert qs =~ "filter[email]=", + "expected query to contain filter[email]=..., got: #{inspect(qs)}" + + # Email should be encoded (e.g. @ as %40) + assert qs =~ "filter[email]=test%40example.com", + "expected filter[email] to be URL-encoded (downcased), got: #{inspect(qs)}" + + body = Jason.encode!(%{"jsonapi" => %{"version" => "1.0"}, "data" => []}) + + conn + |> Plug.Conn.put_resp_content_type("application/vnd.api+json") + |> Plug.Conn.send_resp(200, body) + end) + + assert Client.find_contact_by_email(" Test@Example.com ") == {:error, :not_found} + end + + @tag :bypass + test "returns {:ok, id} when API returns one contact (string id)" do + bypass = Bypass.open() + base = "http://127.0.0.1:#{bypass.port}" + set_vereinfacht_env(base) + + Bypass.expect_once(bypass, "GET", "/finance-contacts", fn conn -> + body = + Jason.encode!(%{ + "jsonapi" => %{"version" => "1.0"}, + "data" => [%{"type" => "finance-contacts", "id" => "123", "attributes" => %{}}] + }) + + conn + |> Plug.Conn.put_resp_content_type("application/vnd.api+json") + |> Plug.Conn.send_resp(200, body) + end) + + assert Client.find_contact_by_email("user@example.com") == {:ok, "123"} + end + + @tag :bypass + test "returns {:ok, id} when API returns one contact (integer id)" do + bypass = Bypass.open() + base = "http://127.0.0.1:#{bypass.port}" + set_vereinfacht_env(base) + + Bypass.expect_once(bypass, "GET", "/finance-contacts", fn conn -> + body = + Jason.encode!(%{ + "jsonapi" => %{"version" => "1.0"}, + "data" => [%{"type" => "finance-contacts", "id" => 456, "attributes" => %{}}] + }) + + conn + |> Plug.Conn.put_resp_content_type("application/vnd.api+json") + |> Plug.Conn.send_resp(200, body) + end) + + assert Client.find_contact_by_email("other@example.com") == {:ok, "456"} + end end defp build_member_struct do @@ -49,6 +120,12 @@ defmodule Mv.Vereinfacht.ClientTest do } end + defp set_vereinfacht_env(base_url) do + System.put_env("VEREINFACHT_API_URL", base_url) + System.put_env("VEREINFACHT_API_KEY", "test-key") + System.put_env("VEREINFACHT_CLUB_ID", "2") + end + defp clear_vereinfacht_env do System.delete_env("VEREINFACHT_API_URL") System.delete_env("VEREINFACHT_API_KEY") From a2dfdccef342354c1e765738a5eec72a7b949089 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 4 Mar 2026 20:50:52 +0100 Subject: [PATCH 3/5] 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 From d71d5881cf45212312ae308298ff552c9f049d94 Mon Sep 17 00:00:00 2001 From: Moritz Date: Wed, 4 Mar 2026 20:12:45 +0100 Subject: [PATCH 4/5] 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 5/5] 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