From b09cdf7f3ad859c491d8875a1bb211210c895e40 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 3 Jul 2026 11:19:59 +0200 Subject: [PATCH] feat(member): resolve custom-field filters and sorting in PostgreSQL Push the boolean and date custom-field predicates and custom-field sorting down to JSONB expressions so the overview stops classifying custom-field values in memory. A GIN index on custom_field_values.value keeps the boolean membership predicates index-served. --- lib/membership/custom_field_value.ex | 17 ++ .../live/member_live/index/date_filter.ex | 48 ++++ .../live/member_live/index/overview_query.ex | 137 +++++++++- ...dd_custom_field_values_value_gin_index.exs | 21 ++ .../custom_field_values/20260630170638.json | 148 +++++++++++ ...index_boolean_custom_field_filter_test.exs | 83 ++++++ .../index_date_custom_field_filter_test.exs | 91 +++++++ .../index_filter_parity_property_test.exs | 245 ++++++++++++++++++ .../index_overview_query_sort_test.exs | 98 +++++++ 9 files changed, 883 insertions(+), 5 deletions(-) create mode 100644 priv/repo/migrations/20260630170637_add_custom_field_values_value_gin_index.exs create mode 100644 priv/resource_snapshots/repo/custom_field_values/20260630170638.json create mode 100644 test/mv_web/member_live/index_boolean_custom_field_filter_test.exs create mode 100644 test/mv_web/member_live/index_date_custom_field_filter_test.exs create mode 100644 test/mv_web/member_live/index_filter_parity_property_test.exs create mode 100644 test/mv_web/member_live/index_overview_query_sort_test.exs diff --git a/lib/membership/custom_field_value.ex b/lib/membership/custom_field_value.ex index 623455d6..91404aad 100644 --- a/lib/membership/custom_field_value.ex +++ b/lib/membership/custom_field_value.ex @@ -48,6 +48,13 @@ defmodule Mv.Membership.CustomFieldValue do table "custom_field_values" repo Mv.Repo + custom_indexes do + # GIN index on the JSONB value to serve boolean custom-field containment + # predicates (`value @> '{"value": }'`) used by the member overview. + # Default jsonb_ops supports the `@>` operator we rely on. + index ["value"], name: "custom_field_values_value_gin_index", using: "gin" + end + references do reference :member, on_delete: :delete reference :custom_field, on_delete: :delete @@ -133,6 +140,16 @@ defmodule Mv.Membership.CustomFieldValue do calculations do calculate :value_to_string, :string, expr(value[:value] <> "") + + # Typed sort keys for DB-side ordering of the member overview by a custom + # field. The stored JSONB value (`{"type": ..., "value": ...}`) is cast per + # the field's value type so integers sort numerically and dates + # chronologically. Empty strings collapse to NULL so they sort last, matching + # the previous in-memory `CustomFieldSort` behaviour. + calculate :sort_text, :string, expr(fragment("nullif(?->>'value','')", value)) + calculate :sort_numeric, :decimal, expr(fragment("nullif(?->>'value','')::numeric", value)) + calculate :sort_date, :date, expr(fragment("nullif(?->>'value','')::date", value)) + calculate :sort_boolean, :boolean, expr(fragment("nullif(?->>'value','')::boolean", value)) end # Ensure a member can only have one custom field value per custom field diff --git a/lib/mv_web/live/member_live/index/date_filter.ex b/lib/mv_web/live/member_live/index/date_filter.ex index 14d1aef0..b24c2f49 100644 --- a/lib/mv_web/live/member_live/index/date_filter.ex +++ b/lib/mv_web/live/member_live/index/date_filter.ex @@ -271,6 +271,54 @@ defmodule MvWeb.MemberLive.Index.DateFilter do end end + @doc """ + DB-side equivalent of the custom-date portion of `apply_in_memory/3`. + + For each active custom-date filter, adds a predicate keeping members that have + a stored date value for the field within the inclusive `[from, to]` bounds. + The value is read from the JSONB union (`value->>'type' = 'date'`, then + `(value->>'value')::date`). Members without a date value row for the field are + excluded, mirroring the in-memory behaviour exactly. + """ + @spec apply_custom_date_ash_filter(Ash.Query.t(), map(), [map()]) :: Ash.Query.t() + def apply_custom_date_ash_filter(%Ash.Query{} = query, filters, date_custom_fields) + when is_map(filters) and is_list(date_custom_fields) do + filters + |> active_custom_date_filters(date_custom_fields) + |> Enum.reduce(query, fn {id, bounds}, q -> apply_one_custom_date_filter(q, id, bounds) end) + end + + defp apply_one_custom_date_filter(query, id, %{from: from, to: to}) do + case Ecto.UUID.cast(id) do + {:ok, uuid} -> filter_custom_date(query, uuid, from, to) + _ -> query + end + end + + defp filter_custom_date(query, uuid, from, to) do + Ash.Query.filter( + query, + expr( + exists( + custom_field_values, + custom_field_id == ^uuid and + fragment("?->>'type' = 'date'", value) and + ^custom_date_bounds(from, to) + ) + ) + ) + end + + defp custom_date_bounds(nil, to), do: expr(fragment("(?->>'value')::date <= ?", value, ^to)) + defp custom_date_bounds(from, nil), do: expr(fragment("(?->>'value')::date >= ?", value, ^from)) + + defp custom_date_bounds(from, to) do + expr( + fragment("(?->>'value')::date >= ?", value, ^from) and + fragment("(?->>'value')::date <= ?", value, ^to) + ) + end + @doc """ Returns the UUID string keys of `filters` that name an active (at-least-one- bound-set) custom date field. The UUID must appear in `date_custom_fields` diff --git a/lib/mv_web/live/member_live/index/overview_query.ex b/lib/mv_web/live/member_live/index/overview_query.ex index 93882d4d..bf4ef85c 100644 --- a/lib/mv_web/live/member_live/index/overview_query.ex +++ b/lib/mv_web/live/member_live/index/overview_query.ex @@ -18,6 +18,7 @@ defmodule MvWeb.MemberLive.Index.OverviewQuery do alias MvWeb.MemberLive.Index.DateFilter require Ash.Query + require Ash.Sort @type opts :: %{optional(atom()) => term()} @@ -40,13 +41,18 @@ defmodule MvWeb.MemberLive.Index.OverviewQuery do |> apply_search(opts[:search]) |> apply_group_filters(opts[:group_filters], opts[:groups]) |> apply_fee_type_filters(opts[:fee_type_filters], opts[:fee_types]) + |> apply_boolean_custom_field_filters( + opts[:boolean_custom_field_filters], + opts[:boolean_custom_fields] + ) |> apply_date_filters(opts[:date_filters]) + |> apply_custom_date_filters(opts[:date_filters], opts[:date_custom_fields]) |> apply_cycle_status_filter( opts[:cycle_status_filter], opts[:show_current_cycle], today(opts) ) - |> apply_sort(opts[:sort_field], opts[:sort_order]) + |> apply_sort(opts[:sort_field], opts[:sort_order], opts[:custom_fields] || []) end defp today(opts), do: opts[:today] || Date.utc_today() @@ -135,6 +141,48 @@ defmodule MvWeb.MemberLive.Index.OverviewQuery do end) end + # --------------------------------------------------------------------------- + # Boolean custom-field filters (AND across selected fields) + # + # Mirrors the previous in-memory behaviour: a member matches a `{field => bool}` + # filter only if it has a stored value row for that field whose boolean value + # equals `bool`. A member with no value row is excluded. Uses a JSONB + # containment predicate (`value @> '{"value": }'`), which the GIN index on + # `custom_field_values.value` can serve. + # --------------------------------------------------------------------------- + + defp apply_boolean_custom_field_filters(query, filters, _boolean_fields) + when filters in [nil, %{}], + do: query + + defp apply_boolean_custom_field_filters(query, filters, boolean_fields) do + valid_ids = valid_id_set(boolean_fields) + + filters + |> Enum.filter(fn {id_str, _} -> MapSet.member?(valid_ids, id_str) end) + |> Enum.reduce(query, fn {id_str, bool}, q -> + case Ecto.UUID.cast(id_str) do + {:ok, uuid} -> apply_one_boolean_filter(q, uuid, bool) + _ -> q + end + end) + end + + defp apply_one_boolean_filter(query, uuid, bool) when is_boolean(bool) do + Ash.Query.filter( + query, + expr( + exists( + custom_field_values, + custom_field_id == ^uuid and + fragment("? @> jsonb_build_object('value', ?::boolean)", value, ^bool) + ) + ) + ) + end + + defp apply_one_boolean_filter(query, _uuid, _bool), do: query + # --------------------------------------------------------------------------- # Cycle-status filter (paid/unpaid, current or last-completed cycle) # @@ -193,20 +241,99 @@ defmodule MvWeb.MemberLive.Index.OverviewQuery do defp apply_date_filters(query, filters) when is_map(filters), do: DateFilter.apply_ash_filter(query, filters) + defp apply_custom_date_filters(query, filters, custom_fields) + when is_map(filters) and is_list(custom_fields), + do: DateFilter.apply_custom_date_ash_filter(query, filters, custom_fields) + + defp apply_custom_date_filters(query, _filters, _custom_fields), do: query + # --------------------------------------------------------------------------- # Sort (always append the unique id tie-breaker for keyset stability) # --------------------------------------------------------------------------- - defp apply_sort(query, nil, _order), do: Ash.Query.sort(query, id: :asc) - defp apply_sort(query, _field, nil), do: Ash.Query.sort(query, id: :asc) + defp apply_sort(query, nil, _order, _custom_fields), do: Ash.Query.sort(query, id: :asc) + defp apply_sort(query, _field, nil, _custom_fields), do: Ash.Query.sort(query, id: :asc) - defp apply_sort(query, field, order) do + defp apply_sort(query, field, order, custom_fields) do case sort_key(field) do - nil -> Ash.Query.sort(query, id: :asc) + nil -> apply_custom_field_sort(query, field, order, custom_fields) key -> Ash.Query.sort(query, [{key, order}, {:id, :asc}]) end end + # Sorts by a custom field's JSONB value DB-side. The value is read from the + # first matching `custom_field_values` row for the field, cast per the field's + # value type so integers sort numerically and dates chronologically. Empty + # strings and members without a value row sort last in both directions + # (NULLS LAST), mirroring the previous in-memory `CustomFieldSort` behaviour. + defp apply_custom_field_sort(query, field, order, custom_fields) do + with id_str when is_binary(id_str) <- custom_field_id(field), + %{} = cf <- Enum.find(custom_fields, &(to_string(&1.id) == id_str)) do + nils_order = if order == :desc, do: :desc_nils_last, else: :asc_nils_last + + Ash.Query.sort(query, [ + {custom_field_sort_calc(cf.value_type, cf.id), nils_order}, + {:id, :asc} + ]) + else + _ -> Ash.Query.sort(query, id: :asc) + end + end + + defp custom_field_id(field) when is_atom(field), do: custom_field_id(Atom.to_string(field)) + + defp custom_field_id(field) when is_binary(field) do + case String.split(field, Mv.Constants.custom_field_prefix()) do + ["", id_str] -> id_str + _ -> nil + end + end + + defp custom_field_id(_), do: nil + + # Maps the field's value type to the first matching custom-field value's typed + # sort key (scoped to the field), backed by the `sort_*` calculations on + # `CustomFieldValue` (a raw fragment is not allowed as an aggregate `field:`). + defp custom_field_sort_calc(:integer, id), + do: + Ash.Sort.expr_sort( + first(custom_field_values, + field: :sort_numeric, + query: [filter: expr(custom_field_id == ^id)] + ), + :decimal + ) + + defp custom_field_sort_calc(:date, id), + do: + Ash.Sort.expr_sort( + first(custom_field_values, + field: :sort_date, + query: [filter: expr(custom_field_id == ^id)] + ), + :date + ) + + defp custom_field_sort_calc(:boolean, id), + do: + Ash.Sort.expr_sort( + first(custom_field_values, + field: :sort_boolean, + query: [filter: expr(custom_field_id == ^id)] + ), + :boolean + ) + + defp custom_field_sort_calc(_type, id), + do: + Ash.Sort.expr_sort( + first(custom_field_values, + field: :sort_text, + query: [filter: expr(custom_field_id == ^id)] + ), + :string + ) + # Resolves a sort field (atom or string) to a DB sort key, or nil if it is a # computed field handled elsewhere / not sortable. defp sort_key(field) when field in [:membership_fee_type, "membership_fee_type"], diff --git a/priv/repo/migrations/20260630170637_add_custom_field_values_value_gin_index.exs b/priv/repo/migrations/20260630170637_add_custom_field_values_value_gin_index.exs new file mode 100644 index 00000000..00ba06c5 --- /dev/null +++ b/priv/repo/migrations/20260630170637_add_custom_field_values_value_gin_index.exs @@ -0,0 +1,21 @@ +defmodule Mv.Repo.Migrations.AddCustomFieldValuesValueGinIndex do + @moduledoc """ + Adds a GIN index on `custom_field_values.value` (JSONB) to serve the boolean + custom-field containment predicates used by the member overview. + """ + + use Ecto.Migration + + def up do + create index(:custom_field_values, ["value"], + name: "custom_field_values_value_gin_index", + using: "gin" + ) + end + + def down do + drop_if_exists index(:custom_field_values, ["value"], + name: "custom_field_values_value_gin_index" + ) + end +end diff --git a/priv/resource_snapshots/repo/custom_field_values/20260630170638.json b/priv/resource_snapshots/repo/custom_field_values/20260630170638.json new file mode 100644 index 00000000..654d4231 --- /dev/null +++ b/priv/resource_snapshots/repo/custom_field_values/20260630170638.json @@ -0,0 +1,148 @@ +{ + "attributes": [ + { + "allow_nil?": false, + "default": "fragment(\"gen_random_uuid()\")", + "generated?": false, + "precision": null, + "primary_key?": true, + "references": null, + "scale": null, + "size": null, + "source": "id", + "type": "uuid" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": null, + "scale": null, + "size": null, + "source": "value", + "type": "map" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": { + "deferrable": false, + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "index?": false, + "match_type": null, + "match_with": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "custom_field_values_member_id_fkey", + "on_delete": "delete", + "on_update": null, + "primary_key?": true, + "schema": "public", + "table": "members" + }, + "scale": null, + "size": null, + "source": "member_id", + "type": "uuid" + }, + { + "allow_nil?": true, + "default": "nil", + "generated?": false, + "precision": null, + "primary_key?": false, + "references": { + "deferrable": false, + "destination_attribute": "id", + "destination_attribute_default": null, + "destination_attribute_generated": null, + "index?": false, + "match_type": null, + "match_with": null, + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "name": "custom_field_values_custom_field_id_fkey", + "on_delete": "delete", + "on_update": null, + "primary_key?": true, + "schema": "public", + "table": "custom_fields" + }, + "scale": null, + "size": null, + "source": "custom_field_id", + "type": "uuid" + } + ], + "base_filter": null, + "check_constraints": [], + "create_table_options": null, + "custom_indexes": [ + { + "all_tenants?": false, + "concurrently": false, + "error_fields": [ + "value" + ], + "fields": [ + { + "type": "string", + "value": "value" + } + ], + "include": null, + "message": null, + "name": "custom_field_values_value_gin_index", + "nulls_distinct": true, + "prefix": null, + "table": null, + "unique": false, + "using": "gin", + "where": null + } + ], + "custom_statements": [], + "has_create_action": true, + "hash": "6B19604F0AA26F503094E6AF5086FDAE0D054096B24099376EFF4BBCFF7E3CB5", + "identities": [ + { + "all_tenants?": false, + "base_filter": null, + "index_name": "custom_field_values_unique_custom_field_per_member_index", + "keys": [ + { + "type": "atom", + "value": "member_id" + }, + { + "type": "atom", + "value": "custom_field_id" + } + ], + "name": "unique_custom_field_per_member", + "nils_distinct?": true, + "where": null + } + ], + "multitenancy": { + "attribute": null, + "global": null, + "strategy": null + }, + "repo": "Elixir.Mv.Repo", + "schema": null, + "table": "custom_field_values" +} \ No newline at end of file diff --git a/test/mv_web/member_live/index_boolean_custom_field_filter_test.exs b/test/mv_web/member_live/index_boolean_custom_field_filter_test.exs new file mode 100644 index 00000000..46eaf64b --- /dev/null +++ b/test/mv_web/member_live/index_boolean_custom_field_filter_test.exs @@ -0,0 +1,83 @@ +defmodule MvWeb.MemberLive.IndexBooleanCustomFieldFilterTest do + @moduledoc """ + §1.15 — DB-backed boolean custom-field filter (in/not_in) must match the + previous in-memory behaviour exactly, including a member with no stored value + row for the field. + """ + use Mv.DataCase, async: false + + import Mv.Fixtures, only: [member_fixture_with_actor: 2] + + alias Mv.Membership.CustomField + alias Mv.Membership.CustomFieldValue + alias Mv.Membership.Member + alias MvWeb.MemberLive.Index + alias MvWeb.MemberLive.Index.OverviewQuery + + setup do + actor = Mv.Helpers.SystemActor.get_system_actor() + + {:ok, field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "newsletter_#{System.unique_integer([:positive])}", + value_type: :boolean, + show_in_overview: true + }) + |> Ash.create(actor: actor) + + yes = member_fixture_with_actor(%{}, actor) + no = member_fixture_with_actor(%{}, actor) + none = member_fixture_with_actor(%{}, actor) + + set_bool(yes, field, true, actor) + set_bool(no, field, false, actor) + + %{actor: actor, field: field, yes: yes, no: no, none: none} + end + + defp set_bool(member, field, bool, actor) do + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: field.id, + value: %{"_union_type" => "boolean", "_union_value" => bool} + }) + |> Ash.create!(actor: actor) + end + + defp db_ids(filters, field, actor) do + OverviewQuery.build(%{ + boolean_custom_field_filters: filters, + boolean_custom_fields: [field] + }) + |> Ash.read!(actor: actor) + |> MapSet.new(& &1.id) + end + + defp oracle_ids(filters, field, actor) do + members = + Member + |> Ash.Query.load(custom_field_values: [:custom_field]) + |> Ash.read!(actor: actor) + + Index.apply_boolean_custom_field_filters(members, filters, [field]) + |> MapSet.new(& &1.id) + end + + test "filter true matches only members with stored true value", ctx do + filters = %{to_string(ctx.field.id) => true} + assert db_ids(filters, ctx.field, ctx.actor) == oracle_ids(filters, ctx.field, ctx.actor) + assert ctx.yes.id in db_ids(filters, ctx.field, ctx.actor) + refute ctx.no.id in db_ids(filters, ctx.field, ctx.actor) + refute ctx.none.id in db_ids(filters, ctx.field, ctx.actor) + end + + test "filter false matches only members with stored false value (not missing rows)", ctx do + filters = %{to_string(ctx.field.id) => false} + assert db_ids(filters, ctx.field, ctx.actor) == oracle_ids(filters, ctx.field, ctx.actor) + assert ctx.no.id in db_ids(filters, ctx.field, ctx.actor) + refute ctx.yes.id in db_ids(filters, ctx.field, ctx.actor) + refute ctx.none.id in db_ids(filters, ctx.field, ctx.actor) + end +end diff --git a/test/mv_web/member_live/index_date_custom_field_filter_test.exs b/test/mv_web/member_live/index_date_custom_field_filter_test.exs new file mode 100644 index 00000000..3e8ad7df --- /dev/null +++ b/test/mv_web/member_live/index_date_custom_field_filter_test.exs @@ -0,0 +1,91 @@ +defmodule MvWeb.MemberLive.IndexDateCustomFieldFilterTest do + @moduledoc """ + §1.16 — DB-backed date custom-field range filter must match the previous + in-memory behaviour exactly, including inclusive boundaries and members with + no stored value row (excluded). + """ + use Mv.DataCase, async: false + + import Mv.Fixtures, only: [member_fixture_with_actor: 2] + + alias Mv.Membership.CustomField + alias Mv.Membership.CustomFieldValue + alias Mv.Membership.Member + alias MvWeb.MemberLive.Index.DateFilter + alias MvWeb.MemberLive.Index.OverviewQuery + + setup do + actor = Mv.Helpers.SystemActor.get_system_actor() + + {:ok, field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "birthday_#{System.unique_integer([:positive])}", + value_type: :date, + show_in_overview: true + }) + |> Ash.create(actor: actor) + + on_lower = member_with_date(field, ~D[2024-01-01], actor) + inside = member_with_date(field, ~D[2024-06-15], actor) + on_upper = member_with_date(field, ~D[2024-12-31], actor) + below = member_with_date(field, ~D[2023-12-31], actor) + above = member_with_date(field, ~D[2025-01-01], actor) + none = member_fixture_with_actor(%{}, actor) + + %{ + actor: actor, + field: field, + on_lower: on_lower, + inside: inside, + on_upper: on_upper, + below: below, + above: above, + none: none + } + end + + defp member_with_date(field, date, actor) do + member = member_fixture_with_actor(%{}, actor) + + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: field.id, + value: %{"_union_type" => "date", "_union_value" => date} + }) + |> Ash.create!(actor: actor) + + member + end + + defp filters(field), do: %{to_string(field.id) => %{from: ~D[2024-01-01], to: ~D[2024-12-31]}} + + defp db_ids(field, actor) do + OverviewQuery.build(%{date_filters: filters(field), date_custom_fields: [field]}) + |> Ash.read!(actor: actor) + |> MapSet.new(& &1.id) + end + + defp oracle_ids(field, actor) do + members = + Member + |> Ash.Query.load(custom_field_values: [:custom_field]) + |> Ash.read!(actor: actor) + + DateFilter.apply_in_memory(members, filters(field), [field]) + |> MapSet.new(& &1.id) + end + + test "inclusive range matches in-memory oracle, boundaries included", ctx do + db = db_ids(ctx.field, ctx.actor) + assert db == oracle_ids(ctx.field, ctx.actor) + + assert ctx.on_lower.id in db + assert ctx.inside.id in db + assert ctx.on_upper.id in db + refute ctx.below.id in db + refute ctx.above.id in db + refute ctx.none.id in db + end +end diff --git a/test/mv_web/member_live/index_filter_parity_property_test.exs b/test/mv_web/member_live/index_filter_parity_property_test.exs new file mode 100644 index 00000000..8cd17ce8 --- /dev/null +++ b/test/mv_web/member_live/index_filter_parity_property_test.exs @@ -0,0 +1,245 @@ +defmodule MvWeb.MemberLive.IndexFilterParityPropertyTest do + @moduledoc """ + §2.1 — Filter result-set parity. + + For generated member populations × filter-parameter combinations, the set of + member ids returned by the DB-backed `:overview` query equals the set returned + by the previous in-memory implementation (the reference oracle). + + The unchanged DB filters (group, fee-type) are applied identically in both + arms; the property isolates the filters that moved from memory to the DB + (boolean custom fields, date custom fields, paid/unpaid cycle status) by + applying them DB-side in one arm and in-memory in the other. + """ + use Mv.DataCase, async: false + use ExUnitProperties + + import Mv.Fixtures, only: [create_fee_type: 2, member_fixture_with_actor: 2, create_cycle: 4] + + alias Mv.Membership.CustomField + alias Mv.Membership.CustomFieldValue + alias Mv.Membership.Member + alias MvWeb.MemberLive.Index + alias MvWeb.MemberLive.Index.DateFilter + alias MvWeb.MemberLive.Index.MembershipFeeStatus + alias MvWeb.MemberLive.Index.OverviewQuery + + @today ~D[2024-07-15] + @cycle_starts [~D[2023-01-01], ~D[2024-01-01], ~D[2025-01-01]] + @date_values [~D[2023-06-01], ~D[2024-06-15], ~D[2025-06-01]] + + setup do + actor = Mv.Helpers.SystemActor.get_system_actor() + ft1 = create_fee_type(%{interval: :yearly}, actor) + ft2 = create_fee_type(%{interval: :yearly}, actor) + + {:ok, bool_cf} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "flag_#{System.unique_integer([:positive])}", + value_type: :boolean, + show_in_overview: true + }) + |> Ash.create(actor: actor) + + {:ok, date_cf} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "dt_#{System.unique_integer([:positive])}", + value_type: :date, + show_in_overview: true + }) + |> Ash.create(actor: actor) + + %{actor: actor, ft1: ft1, ft2: ft2, bool_cf: bool_cf, date_cf: date_cf} + end + + # ----- generators --------------------------------------------------------- + + defp status_gen, do: StreamData.member_of([:unpaid, :paid, :suspended]) + + defp member_spec_gen do + StreamData.fixed_map(%{ + fee_type: StreamData.member_of([:ft1, :ft2]), + cycles: + StreamData.list_of( + StreamData.tuple({StreamData.member_of(@cycle_starts), status_gen()}), + max_length: 3 + ), + bool: StreamData.member_of([:none, true, false]), + date: StreamData.member_of([:none | @date_values]), + in_group: StreamData.boolean() + }) + end + + defp filter_params_gen do + StreamData.fixed_map(%{ + cycle_status: StreamData.member_of([nil, :paid, :unpaid]), + show_current: StreamData.boolean(), + bool: StreamData.member_of([nil, true, false]), + date_range: StreamData.member_of([nil, {~D[2024-01-01], ~D[2024-12-31]}]), + group: StreamData.member_of([nil, :in, :not_in]), + fee_type: StreamData.member_of([nil, :in, :not_in]) + }) + end + + # ----- helpers ------------------------------------------------------------ + + defp clear_members(actor) do + Member |> Ash.read!(actor: actor) |> Enum.each(&Ash.destroy!(&1, actor: actor)) + end + + defp create_population(specs, ctx) do + Enum.map(specs, fn spec -> + ft = if spec.fee_type == :ft1, do: ctx.ft1, else: ctx.ft2 + member = member_fixture_with_actor(%{membership_fee_type_id: ft.id}, ctx.actor) + + spec.cycles + |> Enum.uniq_by(fn {start, _} -> start end) + |> Enum.each(fn {start, status} -> + create_cycle(member, ft, %{cycle_start: start, status: status}, ctx.actor) + end) + + maybe_set_value(member, ctx.bool_cf, spec.bool, "boolean", ctx.actor) + maybe_set_value(member, ctx.date_cf, spec.date, "date", ctx.actor) + + if spec.in_group, do: add_to_group(member, ctx, ctx.actor) + {member.id, spec} + end) + end + + defp maybe_set_value(_member, _cf, :none, _type, _actor), do: :ok + + defp maybe_set_value(member, cf, value, type, actor) do + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: cf.id, + value: %{"_union_type" => type, "_union_value" => value} + }) + |> Ash.create!(actor: actor) + end + + defp add_to_group(member, ctx, actor) do + Mv.Membership.create_member_group(%{member_id: member.id, group_id: ctx.group.id}, + actor: actor + ) + end + + defp base_opts(params, ctx) do + %{} + |> put_group(params.group, ctx) + |> put_fee_type(params.fee_type, ctx) + end + + defp put_group(opts, nil, _ctx), do: opts + + defp put_group(opts, dir, ctx), + do: Map.merge(opts, %{group_filters: %{to_string(ctx.group.id) => dir}, groups: [ctx.group]}) + + defp put_fee_type(opts, nil, _ctx), do: opts + + defp put_fee_type(opts, dir, ctx), + do: + Map.merge(opts, %{fee_type_filters: %{to_string(ctx.ft1.id) => dir}, fee_types: [ctx.ft1]}) + + defp moved_opts(params, ctx) do + %{today: @today} + |> put_cycle(params.cycle_status, params.show_current) + |> put_bool(params.bool, ctx) + |> put_date(params.date_range, ctx) + end + + defp put_cycle(opts, nil, _show), do: opts + + defp put_cycle(opts, status, show), + do: Map.merge(opts, %{cycle_status_filter: status, show_current_cycle: show}) + + defp put_bool(opts, nil, _ctx), do: opts + + defp put_bool(opts, bool, ctx), + do: + Map.merge(opts, %{ + boolean_custom_field_filters: %{to_string(ctx.bool_cf.id) => bool}, + boolean_custom_fields: [ctx.bool_cf] + }) + + defp put_date(opts, nil, _ctx), do: opts + + defp put_date(opts, {from, to}, ctx), + do: + Map.merge(opts, %{ + date_filters: %{to_string(ctx.date_cf.id) => %{from: from, to: to}}, + date_custom_fields: [ctx.date_cf] + }) + + defp db_ids(params, ctx) do + base_opts(params, ctx) + |> Map.merge(moved_opts(params, ctx)) + |> OverviewQuery.build() + |> Ash.read!(actor: ctx.actor) + |> MapSet.new(& &1.id) + end + + defp oracle_ids(params, ctx) do + base_members = + base_opts(params, ctx) + |> OverviewQuery.build() + |> Ash.Query.load([ + :membership_fee_type, + {:custom_field_values, [:custom_field]}, + {:membership_fee_cycles, [:membership_fee_type]} + ]) + |> Ash.read!(actor: ctx.actor) + + base_members + |> apply_oracle_bool(params, ctx) + |> apply_oracle_date(params, ctx) + |> apply_oracle_cycle(params) + |> MapSet.new(& &1.id) + end + + defp apply_oracle_bool(members, %{bool: nil}, _ctx), do: members + + defp apply_oracle_bool(members, %{bool: bool}, ctx) do + Index.apply_boolean_custom_field_filters( + members, + %{to_string(ctx.bool_cf.id) => bool}, + [ctx.bool_cf] + ) + end + + defp apply_oracle_date(members, %{date_range: nil}, _ctx), do: members + + defp apply_oracle_date(members, %{date_range: {from, to}}, ctx) do + DateFilter.apply_in_memory( + members, + %{to_string(ctx.date_cf.id) => %{from: from, to: to}}, + [ctx.date_cf] + ) + end + + defp apply_oracle_cycle(members, %{cycle_status: nil}), do: members + + defp apply_oracle_cycle(members, %{cycle_status: status, show_current: show}) do + Enum.filter(members, fn m -> + MembershipFeeStatus.get_cycle_status_for_member(m, show, @today) == status + end) + end + + property "DB-backed filter result set equals the in-memory oracle", ctx do + check all( + specs <- StreamData.list_of(member_spec_gen(), min_length: 1, max_length: 5), + params <- filter_params_gen(), + max_runs: 30 + ) do + clear_members(ctx.actor) + group = Mv.Fixtures.group_fixture(%{name: "G#{System.unique_integer([:positive])}"}) + ctx = Map.put(ctx, :group, group) + + _population = create_population(specs, ctx) + + assert db_ids(params, ctx) == oracle_ids(params, ctx) + end + end +end diff --git a/test/mv_web/member_live/index_overview_query_sort_test.exs b/test/mv_web/member_live/index_overview_query_sort_test.exs new file mode 100644 index 00000000..ddf0320b --- /dev/null +++ b/test/mv_web/member_live/index_overview_query_sort_test.exs @@ -0,0 +1,98 @@ +defmodule MvWeb.MemberLive.IndexOverviewQuerySortTest do + @moduledoc """ + §2.3 — Sort determinism for custom-field sorts pushed to the DB. + + The keyset property test covers standard fields; these examples pin the + type-aware custom-field ordering (numeric vs lexical, chronological dates) and + the NULLS-LAST behaviour for missing/empty values in both directions, which + the previous in-memory `CustomFieldSort` guaranteed. + """ + use Mv.DataCase, async: false + + import Mv.Fixtures, only: [member_fixture_with_actor: 2] + + alias Mv.Membership.CustomField + alias Mv.Membership.CustomFieldValue + alias MvWeb.MemberLive.Index.OverviewQuery + + setup do + %{actor: Mv.Helpers.SystemActor.get_system_actor()} + end + + defp clear_members(actor) do + Mv.Membership.Member |> Ash.read!(actor: actor) |> Enum.each(&Ash.destroy!(&1, actor: actor)) + end + + defp create_field(value_type, actor) do + {:ok, field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "sort_#{value_type}_#{System.unique_integer([:positive])}", + value_type: value_type, + show_in_overview: true + }) + |> Ash.create(actor: actor) + + field + end + + defp set_value(member, field, union_type, union_value, actor) do + {:ok, _} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: field.id, + value: %{"_union_type" => union_type, "_union_value" => union_value} + }) + |> Ash.create(actor: actor) + end + + defp sorted_ids(field, order, actor) do + OverviewQuery.build(%{ + sort_field: "custom_field_#{field.id}", + sort_order: order, + custom_fields: [field] + }) + |> Ash.read!(actor: actor, page: [limit: 100]) + |> then(& &1.results) + |> Enum.map(& &1.id) + end + + test "integer custom field sorts numerically, missing value last in both directions", %{ + actor: actor + } do + clear_members(actor) + field = create_field(:integer, actor) + a = member_fixture_with_actor(%{first_name: "A"}, actor) + b = member_fixture_with_actor(%{first_name: "B"}, actor) + none = member_fixture_with_actor(%{first_name: "N"}, actor) + # 9 vs 10: lexical order would place "10" before "9"; numeric must not. + set_value(a, field, "integer", 10, actor) + set_value(b, field, "integer", 9, actor) + + assert sorted_ids(field, :asc, actor) == [b.id, a.id, none.id] + assert sorted_ids(field, :desc, actor) == [a.id, b.id, none.id] + end + + test "date custom field sorts chronologically", %{actor: actor} do + clear_members(actor) + field = create_field(:date, actor) + older = member_fixture_with_actor(%{first_name: "Older"}, actor) + newer = member_fixture_with_actor(%{first_name: "Newer"}, actor) + set_value(older, field, "date", "1981-01-29", actor) + set_value(newer, field, "date", "1986-07-02", actor) + + assert sorted_ids(field, :asc, actor) == [older.id, newer.id] + end + + test "empty string value sorts last like a missing value", %{actor: actor} do + clear_members(actor) + field = create_field(:string, actor) + filled = member_fixture_with_actor(%{first_name: "Filled"}, actor) + empty = member_fixture_with_actor(%{first_name: "Empty"}, actor) + set_value(filled, field, "string", "AAA", actor) + set_value(empty, field, "string", "", actor) + + assert sorted_ids(field, :asc, actor) == [filled.id, empty.id] + end +end