From 18fb954f73ca8edf6d0d914bcf7eceb1a4518f58 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:49:50 +0200 Subject: [PATCH 01/10] test(membership-fees): share create_fee_type and create_cycle fixtures Replace the create_fee_type/create_cycle helpers duplicated across 18/8 membership-fee test files with a single shared definition in Mv.Fixtures, reconciling the divergent local signatures (including the reversed argument order) into one superset so behavior is unchanged. --- .../member_cycle_calculations_test.exs | 36 +---- .../member_type_change_integration_test.exs | 35 +---- .../changes/validate_same_interval_test.exs | 18 +-- .../member_cycle_integration_test.exs | 18 +-- .../membership_fee_cycle_test.exs | 34 +---- .../membership_fee_type_integration_test.exs | 17 +-- .../cycle_generator_edge_cases_test.exs | 18 +-- .../membership_fees/cycle_generator_test.exs | 18 +-- .../membership_fee_cycle_policies_test.exs | 56 +++----- test/mv/statistics_test.exs | 26 +--- .../membership_fee_type_live/form_test.exs | 18 +-- .../membership_fee_type_live/index_test.exs | 17 +-- .../form_membership_fee_type_test.exs | 19 +-- .../index/membership_fee_status_test.exs | 42 +----- .../index_membership_fee_status_test.exs | 136 ++++++++++-------- test/mv_web/member_live/index_test.exs | 62 ++------ .../membership_fee_integration_test.exs | 19 +-- .../member_live/show_membership_fees_test.exs | 125 ++++++++-------- test/support/fixtures.ex | 78 ++++++++++ 19 files changed, 279 insertions(+), 513 deletions(-) diff --git a/test/membership/member_cycle_calculations_test.exs b/test/membership/member_cycle_calculations_test.exs index 9be1272..860d784 100644 --- a/test/membership/member_cycle_calculations_test.exs +++ b/test/membership/member_cycle_calculations_test.exs @@ -4,30 +4,15 @@ defmodule Mv.Membership.MemberCycleCalculationsTest do """ use Mv.DataCase, async: true + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] + alias Mv.MembershipFees.CalendarCycles - alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member defp create_member(attrs, actor) do default_attrs = %{ @@ -41,23 +26,6 @@ defmodule Mv.Membership.MemberCycleCalculationsTest do member end - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs, actor) do - default_attrs = %{ - cycle_start: ~D[2024-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - describe "current_cycle_status" do test "returns status of current cycle for member with active cycle", %{actor: actor} do fee_type = create_fee_type(%{interval: :yearly}, actor) diff --git a/test/membership/member_type_change_integration_test.exs b/test/membership/member_type_change_integration_test.exs index 35b3137..a2baf78 100644 --- a/test/membership/member_type_change_integration_test.exs +++ b/test/membership/member_type_change_integration_test.exs @@ -4,9 +4,10 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do """ use Mv.DataCase, async: true + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] + alias Mv.MembershipFees.CalendarCycles alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -15,21 +16,6 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member defp create_member(attrs, actor) do default_attrs = %{ @@ -44,23 +30,6 @@ defmodule Mv.Membership.MemberTypeChangeIntegrationTest do member end - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs, actor) do - default_attrs = %{ - cycle_start: ~D[2024-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - describe "type change cycle regeneration" do test "future unpaid cycles are regenerated with new amount", %{actor: actor} do today = Date.utc_today() diff --git a/test/membership_fees/changes/validate_same_interval_test.exs b/test/membership_fees/changes/validate_same_interval_test.exs index 31c2847..21e0a55 100644 --- a/test/membership_fees/changes/validate_same_interval_test.exs +++ b/test/membership_fees/changes/validate_same_interval_test.exs @@ -4,29 +4,15 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do """ use Mv.DataCase, async: true + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.MembershipFees.Changes.ValidateSameInterval - alias Mv.MembershipFees.MembershipFeeType setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member defp create_member(attrs, actor) do default_attrs = %{ diff --git a/test/membership_fees/member_cycle_integration_test.exs b/test/membership_fees/member_cycle_integration_test.exs index 76f4d08..c6a1486 100644 --- a/test/membership_fees/member_cycle_integration_test.exs +++ b/test/membership_fees/member_cycle_integration_test.exs @@ -4,8 +4,9 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -14,21 +15,6 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to set up settings defp setup_settings(include_joining_cycle, actor) do {:ok, settings} = Mv.Membership.get_settings() diff --git a/test/membership_fees/membership_fee_cycle_test.exs b/test/membership_fees/membership_fee_cycle_test.exs index 8770134..f4f89af 100644 --- a/test/membership_fees/membership_fee_cycle_test.exs +++ b/test/membership_fees/membership_fee_cycle_test.exs @@ -4,29 +4,15 @@ defmodule Mv.MembershipFees.MembershipFeeCycleTest do """ use Mv.DataCase, async: true + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] + alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member defp create_member(attrs, actor) do default_attrs = %{ @@ -40,22 +26,6 @@ defmodule Mv.MembershipFees.MembershipFeeCycleTest do member end - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs, actor) do - default_attrs = %{ - cycle_start: ~D[2024-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - describe "status defaults" do test "status defaults to :unpaid when creating a cycle", %{actor: actor} do fee_type = create_fee_type(%{interval: :yearly}, actor) diff --git a/test/membership_fees/membership_fee_type_integration_test.exs b/test/membership_fees/membership_fee_type_integration_test.exs index f834094..86ef284 100644 --- a/test/membership_fees/membership_fee_type_integration_test.exs +++ b/test/membership_fees/membership_fee_type_integration_test.exs @@ -4,6 +4,8 @@ defmodule Mv.MembershipFees.MembershipFeeTypeIntegrationTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.Membership.Member alias Mv.MembershipFees.MembershipFeeCycle alias Mv.MembershipFees.MembershipFeeType @@ -15,21 +17,6 @@ defmodule Mv.MembershipFees.MembershipFeeTypeIntegrationTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - describe "admin can create membership fee type" do test "creates type with all fields", %{actor: actor} do attrs = %{ diff --git a/test/mv/membership_fees/cycle_generator_edge_cases_test.exs b/test/mv/membership_fees/cycle_generator_edge_cases_test.exs index a9e3316..eb9da26 100644 --- a/test/mv/membership_fees/cycle_generator_edge_cases_test.exs +++ b/test/mv/membership_fees/cycle_generator_edge_cases_test.exs @@ -12,9 +12,10 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.MembershipFees.CycleGenerator alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -23,21 +24,6 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member. Note: If membership_fee_type_id is provided, # cycles will be auto-generated during creation in test environment. defp create_member(attrs, actor) do diff --git a/test/mv/membership_fees/cycle_generator_test.exs b/test/mv/membership_fees/cycle_generator_test.exs index f193903..a715ca9 100644 --- a/test/mv/membership_fees/cycle_generator_test.exs +++ b/test/mv/membership_fees/cycle_generator_test.exs @@ -4,9 +4,10 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 2] + alias Mv.MembershipFees.CycleGenerator alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query @@ -15,21 +16,6 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do %{actor: system_actor} end - # Helper to create a membership fee type - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - # Helper to create a member without triggering cycle generation defp create_member_without_cycles(attrs, actor) do default_attrs = %{ diff --git a/test/mv/membership_fees/membership_fee_cycle_policies_test.exs b/test/mv/membership_fees/membership_fee_cycle_policies_test.exs index cf7e889..e3e37ba 100644 --- a/test/mv/membership_fees/membership_fee_cycle_policies_test.exs +++ b/test/mv/membership_fees/membership_fee_cycle_policies_test.exs @@ -8,6 +8,8 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 1, create_cycle: 3] + alias Mv.Membership alias Mv.MembershipFees @@ -32,41 +34,15 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do member end - defp create_fee_type_fixture do - admin = Mv.Fixtures.user_with_role_fixture("admin") - - {:ok, fee_type} = - MembershipFees.create_membership_fee_type( - %{ - name: "Test Fee #{System.unique_integer([:positive])}", - amount: Decimal.new("10.00"), - interval: :yearly, - description: "Test" - }, - actor: admin - ) - - fee_type + defp fee_type_fixture do + create_fee_type(%{amount: Decimal.new("10.00"), description: "Test"}) end - defp create_cycle_fixture do - admin = Mv.Fixtures.user_with_role_fixture("admin") - member = create_member_fixture() - fee_type = create_fee_type_fixture() - - {:ok, cycle} = - MembershipFees.create_membership_fee_cycle( - %{ - member_id: member.id, - membership_fee_type_id: fee_type.id, - cycle_start: Date.utc_today(), - amount: Decimal.new("10.00"), - status: :unpaid - }, - actor: admin - ) - - cycle + defp cycle_fixture do + create_cycle(create_member_fixture(), fee_type_fixture(), %{ + cycle_start: Date.utc_today(), + amount: Decimal.new("10.00") + }) end describe "own_data permission set" do @@ -74,7 +50,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do user = Mv.Fixtures.user_with_role_fixture("own_data") linked_member = create_member_fixture() other_member = create_member_fixture() - fee_type = create_fee_type_fixture() + fee_type = fee_type_fixture() admin = Mv.Fixtures.user_with_role_fixture("admin") user = @@ -130,7 +106,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do describe "read_only permission set" do setup %{actor: actor} do user = Mv.Fixtures.user_with_role_fixture("read_only") - cycle = create_cycle_fixture() + cycle = cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -156,7 +132,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "cannot create cycle (returns forbidden)", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = create_fee_type_fixture() + fee_type = fee_type_fixture() assert {:error, %Ash.Error.Forbidden{}} = MembershipFees.create_membership_fee_cycle( @@ -180,7 +156,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do describe "normal_user permission set" do setup %{actor: actor} do user = Mv.Fixtures.user_with_role_fixture("normal_user") - cycle = create_cycle_fixture() + cycle = cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -210,7 +186,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "can create cycle", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = create_fee_type_fixture() + fee_type = fee_type_fixture() assert {:ok, created} = MembershipFees.create_membership_fee_cycle( @@ -235,7 +211,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do describe "admin permission set" do setup %{actor: actor} do user = Mv.Fixtures.user_with_role_fixture("admin") - cycle = create_cycle_fixture() + cycle = cycle_fixture() %{actor: actor, user: user, cycle: cycle} end @@ -270,7 +246,7 @@ defmodule Mv.MembershipFees.MembershipFeeCyclePoliciesTest do test "can create cycle", %{user: user, actor: _actor} do member = create_member_fixture() - fee_type = create_fee_type_fixture() + fee_type = fee_type_fixture() assert {:ok, created} = MembershipFees.create_membership_fee_cycle( diff --git a/test/mv/statistics_test.exs b/test/mv/statistics_test.exs index 6b72ffb..e016a10 100644 --- a/test/mv/statistics_test.exs +++ b/test/mv/statistics_test.exs @@ -6,11 +6,11 @@ defmodule Mv.StatisticsTest do require Ash.Query import Ash.Expr + import Mv.Fixtures, only: [create_fee_type: 2] alias Mv.Membership.Member alias Mv.MembershipFees alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType alias Mv.Statistics setup do @@ -18,22 +18,6 @@ defmodule Mv.StatisticsTest do %{actor: actor} end - defp create_fee_type(actor, attrs) do - MembershipFeeType - |> Ash.Changeset.for_create( - :create, - Map.merge( - %{ - name: "Test Fee #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - }, - attrs - ) - ) - |> Ash.create!(actor: actor) - end - describe "first_join_year/1" do test "returns the year of the earliest join_date", %{actor: actor} do Mv.Fixtures.member_fixture(%{join_date: ~D[2019-03-15]}) @@ -131,7 +115,7 @@ defmodule Mv.StatisticsTest do end test "returns totals by status for cycles in that year", %{actor: actor} do - fee_type = create_fee_type(actor, %{amount: Decimal.new("50.00")}) + fee_type = create_fee_type(%{amount: Decimal.new("50.00")}, actor) # Creating members with fee type triggers cycle generation (2020..today). We use 2024 cycles. _member1 = @@ -171,8 +155,8 @@ defmodule Mv.StatisticsTest do test "when fee_type_id is passed in opts, returns only cycles of that fee type", %{ actor: actor } do - fee_type_a = create_fee_type(actor, %{amount: Decimal.new("30.00")}) - fee_type_b = create_fee_type(actor, %{amount: Decimal.new("70.00")}) + fee_type_a = create_fee_type(%{amount: Decimal.new("30.00")}, actor) + fee_type_b = create_fee_type(%{amount: Decimal.new("70.00")}, actor) _m1 = Mv.Fixtures.member_fixture(%{ @@ -207,7 +191,7 @@ defmodule Mv.StatisticsTest do end test "returns sum of amount for all unpaid cycles", %{actor: actor} do - fee_type = create_fee_type(actor, %{amount: Decimal.new("50.00")}) + fee_type = create_fee_type(%{amount: Decimal.new("50.00")}, actor) _member = Mv.Fixtures.member_fixture(%{ diff --git a/test/mv_web/live/membership_fee_type_live/form_test.exs b/test/mv_web/live/membership_fee_type_live/form_test.exs index a836c4d..998fb65 100644 --- a/test/mv_web/live/membership_fee_type_live/form_test.exs +++ b/test/mv_web/live/membership_fee_type_live/form_test.exs @@ -5,6 +5,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 1] alias Mv.MembershipFees.MembershipFeeType @@ -17,23 +18,6 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do %{conn: authenticated_conn, user: user} end - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - # Helper to create a member defp create_member(attrs) do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/live/membership_fee_type_live/index_test.exs b/test/mv_web/live/membership_fee_type_live/index_test.exs index c65341f..8c99d90 100644 --- a/test/mv_web/live/membership_fee_type_live/index_test.exs +++ b/test/mv_web/live/membership_fee_type_live/index_test.exs @@ -5,6 +5,7 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 2] alias Mv.MembershipFees.MembershipFeeType @@ -13,22 +14,6 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do # Use global setup from ConnCase which provides admin user with role # No custom setup needed - # Helper to create a membership fee type - # Uses admin_user to test permissions (UI-/Permissions-nah) - defp create_fee_type(attrs, admin_user) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: admin_user) - end - # Helper to create a member. Requires actor (e.g. admin_user from setup); no fallback so # missing-actor bugs are not masked in tests. defp create_member(attrs, actor) do diff --git a/test/mv_web/member_live/form_membership_fee_type_test.exs b/test/mv_web/member_live/form_membership_fee_type_test.exs index 5382dfc..01812df 100644 --- a/test/mv_web/member_live/form_membership_fee_type_test.exs +++ b/test/mv_web/member_live/form_membership_fee_type_test.exs @@ -5,27 +5,10 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest - - alias Mv.MembershipFees.MembershipFeeType + import Mv.Fixtures, only: [create_fee_type: 2] require Ash.Query - # Helper to create a membership fee type - # Uses admin_user to test permissions (UI-/Permissions-nah) - defp create_fee_type(attrs, admin_user) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: admin_user) - end - # Helper to create a member # Uses admin_user to test permissions (UI-/Permissions-nah) defp create_member(attrs, admin_user) do diff --git a/test/mv_web/member_live/index/membership_fee_status_test.exs b/test/mv_web/member_live/index/membership_fee_status_test.exs index 5d96e68..8412f61 100644 --- a/test/mv_web/member_live/index/membership_fee_status_test.exs +++ b/test/mv_web/member_live/index/membership_fee_status_test.exs @@ -4,30 +4,13 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do """ use Mv.DataCase, async: false + import Mv.Fixtures, only: [create_fee_type: 1, create_cycle: 3] + alias Mv.Membership.Member - alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType alias MvWeb.MemberLive.Index.MembershipFeeStatus require Ash.Query - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - # Helper to create a member defp create_member(attrs) do system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -43,27 +26,6 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do member end - # Helper to create a cycle - # Note: Does not delete existing cycles - tests should manage their own test data - # If cleanup is needed, it should be done in setup or explicitly in the test - defp create_cycle(member, fee_type, attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - cycle_start: ~D[2023-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - describe "load_cycles_for_members/2" do test "efficiently loads cycles for members" do fee_type = create_fee_type(%{interval: :yearly}) diff --git a/test/mv_web/member_live/index_membership_fee_status_test.exs b/test/mv_web/member_live/index_membership_fee_status_test.exs index ce6bee8..2efe055 100644 --- a/test/mv_web/member_live/index_membership_fee_status_test.exs +++ b/test/mv_web/member_live/index_membership_fee_status_test.exs @@ -5,29 +5,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 1, create_cycle: 3] alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - # Helper to create a member defp create_member(attrs) do system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -43,38 +26,16 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do member end - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - # Delete any auto-generated cycles first to avoid conflicts - existing_cycles = - MembershipFeeCycle - |> Ash.Query.filter(member_id == ^member.id) - |> Ash.read!(actor: system_actor) - - Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle, actor: system_actor) end) - - default_attrs = %{ - cycle_start: ~D[2023-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - describe "status column display" do test "shows status column in member list", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) {:ok, _view, html} = live(conn, "/members") @@ -85,8 +46,18 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do test "shows last completed cycle status by default", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2022-01-01], status: :paid}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2022-01-01], + status: :paid, + replace_existing: true + }) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -102,8 +73,17 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do today = Date.utc_today() current_year_start = %{today | month: 1, day: 1} - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) - create_cycle(member, fee_type, %{cycle_start: current_year_start, status: :suspended}) + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) + + create_cycle(member, fee_type, %{ + cycle_start: current_year_start, + status: :suspended, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -120,7 +100,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do test "shows correct color coding for paid status", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -131,7 +116,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do test "shows correct color coding for unpaid status", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -142,7 +132,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do test "shows correct color coding for suspended status", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :suspended}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :suspended, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members") @@ -169,11 +164,21 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do # Member with unpaid last cycle member1 = create_member(%{first_name: "UnpaidMember", membership_fee_type_id: fee_type.id}) - create_cycle(member1, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + create_cycle(member1, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) # Member with paid last cycle member2 = create_member(%{first_name: "PaidMember", membership_fee_type_id: fee_type.id}) - create_cycle(member2, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + + create_cycle(member2, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -205,11 +210,21 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do # Member with unpaid current cycle member1 = create_member(%{first_name: "UnpaidCurrent", membership_fee_type_id: fee_type.id}) - create_cycle(member1, fee_type, %{cycle_start: current_year_start, status: :unpaid}) + + create_cycle(member1, fee_type, %{ + cycle_start: current_year_start, + status: :unpaid, + replace_existing: true + }) # Member with paid current cycle member2 = create_member(%{first_name: "PaidCurrent", membership_fee_type_id: fee_type.id}) - create_cycle(member2, fee_type, %{cycle_start: current_year_start, status: :paid}) + + create_cycle(member2, fee_type, %{ + cycle_start: current_year_start, + status: :paid, + replace_existing: true + }) system_actor = Mv.Helpers.SystemActor.get_system_actor() @@ -243,7 +258,12 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do # Create multiple members with cycles Enum.each(1..5, fn _ -> member = create_member(%{membership_fee_type_id: fee_type.id}) - create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) end) {:ok, _view, html} = live(conn, "/members") diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index 0cde8fd..be61c9c 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -7,49 +7,9 @@ defmodule MvWeb.MemberLive.IndexTest do alias Mv.Membership alias Mv.Membership.CustomField alias Mv.Membership.CustomFieldValue - alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType alias MvWeb.MemberLive.Index, as: MemberIndex - # Helper to create a membership fee type (shared across all tests) - defp create_fee_type(attrs, actor) do - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end - - # Helper to create a cycle (shared across all tests) - defp create_cycle(member, fee_type, attrs, actor) do - # Delete any auto-generated cycles first to avoid conflicts - existing_cycles = - MembershipFeeCycle - |> Ash.Query.filter(member_id == ^member.id) - |> Ash.read!(actor: actor) - - Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle, actor: actor) end) - - default_attrs = %{ - cycle_start: ~D[2023-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: actor) - end + import Mv.Fixtures, only: [create_fee_type: 2, create_cycle: 4] describe "desktop layout: scroll container and sticky table header" do @describetag :ui @@ -1110,7 +1070,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: last_year_start, status: :paid}, + %{cycle_start: last_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -1127,7 +1087,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: last_year_start, status: :unpaid}, + %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, system_actor ) @@ -1157,7 +1117,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: last_year_start, status: :paid}, + %{cycle_start: last_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -1174,7 +1134,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: last_year_start, status: :unpaid}, + %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, system_actor ) @@ -1204,7 +1164,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: current_year_start, status: :paid}, + %{cycle_start: current_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -1221,7 +1181,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: current_year_start, status: :unpaid}, + %{cycle_start: current_year_start, status: :unpaid, replace_existing: true}, system_actor ) @@ -1251,7 +1211,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( paid_member, fee_type, - %{cycle_start: current_year_start, status: :paid}, + %{cycle_start: current_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -1268,7 +1228,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( unpaid_member, fee_type, - %{cycle_start: current_year_start, status: :unpaid}, + %{cycle_start: current_year_start, status: :unpaid, replace_existing: true}, system_actor ) @@ -2196,7 +2156,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( member_paid_true, fee_type, - %{cycle_start: last_year_start, status: :paid}, + %{cycle_start: last_year_start, status: :paid, replace_existing: true}, system_actor ) @@ -2224,7 +2184,7 @@ defmodule MvWeb.MemberLive.IndexTest do create_cycle( member_unpaid_true, fee_type, - %{cycle_start: last_year_start, status: :unpaid}, + %{cycle_start: last_year_start, status: :unpaid, replace_existing: true}, system_actor ) diff --git a/test/mv_web/member_live/membership_fee_integration_test.exs b/test/mv_web/member_live/membership_fee_integration_test.exs index ac60220..16f1064 100644 --- a/test/mv_web/member_live/membership_fee_integration_test.exs +++ b/test/mv_web/member_live/membership_fee_integration_test.exs @@ -5,29 +5,12 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 1] alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - # Helper to create a member defp create_member(attrs) do system_actor = Mv.Helpers.SystemActor.get_system_actor() diff --git a/test/mv_web/member_live/show_membership_fees_test.exs b/test/mv_web/member_live/show_membership_fees_test.exs index 394d743..9c1b73f 100644 --- a/test/mv_web/member_live/show_membership_fees_test.exs +++ b/test/mv_web/member_live/show_membership_fees_test.exs @@ -5,56 +5,12 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest + import Mv.Fixtures, only: [create_fee_type: 1, create_cycle: 3] alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.MembershipFeeType require Ash.Query - # Helper to create a membership fee type - defp create_fee_type(attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - default_attrs = %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("50.00"), - interval: :yearly - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeType - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - - # Helper to create a cycle - defp create_cycle(member, fee_type, attrs) do - system_actor = Mv.Helpers.SystemActor.get_system_actor() - - # Delete any auto-generated cycles first to avoid conflicts - existing_cycles = - MembershipFeeCycle - |> Ash.Query.filter(member_id == ^member.id) - |> Ash.read!(actor: system_actor) - - Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle, actor: system_actor) end) - - default_attrs = %{ - cycle_start: ~D[2023-01-01], - amount: Decimal.new("50.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } - - attrs = Map.merge(default_attrs, attrs) - - MembershipFeeCycle - |> Ash.Changeset.for_create(:create, attrs) - |> Ash.create!(actor: system_actor) - end - describe "cycle-regeneration control tooltip (§3.5 icon/tooltip audit)" do test "the regenerate_cycles control carries a tooltip and accessible label", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) @@ -76,8 +32,19 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - _cycle1 = create_cycle(member, fee_type, %{cycle_start: ~D[2022-01-01], status: :paid}) - _cycle2 = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + _cycle1 = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2022-01-01], + status: :paid, + replace_existing: true + }) + + _cycle2 = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -101,7 +68,8 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do create_cycle(member, fee_type, %{ cycle_start: ~D[2023-01-01], amount: Decimal.new("60.00"), - status: :paid + status: :paid, + replace_existing: true }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -158,7 +126,12 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -189,7 +162,12 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -220,7 +198,12 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :paid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -316,7 +299,13 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do } do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - _cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + _cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -335,7 +324,13 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do } do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") @@ -361,7 +356,13 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do # This test verifies that Ash.destroy(cycle, actor: read_only_user) returns Forbidden. fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + cycle = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) assert {:error, %Ash.Error.Forbidden{}} = Ash.destroy(cycle, domain: Mv.MembershipFees, actor: read_only_user) @@ -389,8 +390,20 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do fee_type = create_fee_type(%{interval: :yearly}) member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id}) - _c1 = create_cycle(member, fee_type, %{cycle_start: ~D[2022-01-01], status: :paid}) - _c2 = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid}) + + _c1 = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2022-01-01], + status: :paid, + replace_existing: true + }) + + _c2 = + create_cycle(member, fee_type, %{ + cycle_start: ~D[2023-01-01], + status: :unpaid, + replace_existing: true + }) {:ok, view, _html} = live(conn, "/members/#{member.id}") diff --git a/test/support/fixtures.ex b/test/support/fixtures.ex index 73bf12a..e022600 100644 --- a/test/support/fixtures.ex +++ b/test/support/fixtures.ex @@ -335,4 +335,82 @@ defmodule Mv.Fixtures do {:ok, request} = Membership.confirm_join_request(token, actor: nil) request end + + @doc """ + Creates a membership fee type with default or custom attributes. + + Defaults: a unique `name`, `amount` 50.00, `interval` :yearly. + + ## Parameters + - `attrs` - Map of attributes to override defaults (e.g. `%{interval: :monthly}`). + - `actor` - the authorization actor; defaults to the system actor when omitted/nil. + + ## Returns + - MembershipFeeType struct. + + ## Examples + + iex> create_fee_type(%{interval: :monthly}) + %Mv.MembershipFees.MembershipFeeType{interval: :monthly, ...} + + iex> create_fee_type(%{amount: Decimal.new("10.00")}, admin) + %Mv.MembershipFees.MembershipFeeType{...} + """ + def create_fee_type(attrs \\ %{}, actor \\ nil) do + actor = actor || SystemActor.get_system_actor() + + default_attrs = %{ + name: "Test Fee Type #{System.unique_integer([:positive])}", + amount: Decimal.new("50.00"), + interval: :yearly + } + + Mv.MembershipFees.MembershipFeeType + |> Ash.Changeset.for_create(:create, Map.merge(default_attrs, attrs)) + |> Ash.create!(actor: actor) + end + + @doc """ + Creates a membership fee cycle for the given member and fee type. + + Defaults: `cycle_start` ~D[2024-01-01], `amount` 50.00, `status` :unpaid, + with `member_id`/`membership_fee_type_id` derived from the passed structs. + + ## Parameters + - `member` - the Member struct the cycle belongs to. + - `fee_type` - the MembershipFeeType struct the cycle references. + - `attrs` - Map overriding the cycle defaults (e.g. `%{cycle_start: ~D[2023-01-01], status: :paid}`). + A reserved `:replace_existing` key (truthy) deletes any pre-existing cycles for the member + before creating the new one (used where auto-generated cycles would otherwise conflict); + it is stripped from the attrs and never passed to the create action. Defaults to absent/false. + - `actor` - the authorization actor; defaults to the system actor when omitted/nil. + + ## Returns + - MembershipFeeCycle struct. + """ + def create_cycle(member, fee_type, attrs \\ %{}, actor \\ nil) do + actor = actor || SystemActor.get_system_actor() + {replace_existing, attrs} = Map.pop(attrs, :replace_existing, false) + + if replace_existing do + require Ash.Query + + Mv.MembershipFees.MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member.id) + |> Ash.read!(actor: actor) + |> Enum.each(&Ash.destroy!(&1, actor: actor)) + end + + default_attrs = %{ + cycle_start: ~D[2024-01-01], + amount: Decimal.new("50.00"), + member_id: member.id, + membership_fee_type_id: fee_type.id, + status: :unpaid + } + + Mv.MembershipFees.MembershipFeeCycle + |> Ash.Changeset.for_create(:create, Map.merge(default_attrs, attrs)) + |> Ash.create!(actor: actor) + end end From 3bd55fbfec75f394271f393643b3447ec4f16b24 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:50:24 +0200 Subject: [PATCH 02/10] test(seeds): drop the dead per-process seeds-run guard --- test/seeds_test.exs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/test/seeds_test.exs b/test/seeds_test.exs index c729dbb..2721d13 100644 --- a/test/seeds_test.exs +++ b/test/seeds_test.exs @@ -24,21 +24,11 @@ defmodule Mv.SeedsTest do require Ash.Query - # Module attribute to track if seeds have been run - # This allows us to run seeds once per test module while respecting sandbox isolation - @seeds_run_key :seeds_test_run - setup do system_actor = Mv.Helpers.SystemActor.get_system_actor() - # Run seeds once per test process (due to sandbox isolation, each test runs in its own process) - # Process.get/put ensures seeds are only executed once per test process, not once per test - # Note: With async: false and sandbox isolation, this effectively runs seeds once per test, - # but the guard prevents multiple executions within the same test process if setup is called multiple times - unless Process.get(@seeds_run_key) do - Code.eval_file("priv/repo/seeds.exs") - Process.put(@seeds_run_key, true) - end + # Each test runs in its own sandbox-owned process, so seeds are loaded once per test. + Code.eval_file("priv/repo/seeds.exs") %{actor: system_actor} end From ccd1f81e3ec50ce1fc07e132e9c316a0b85b5f87 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:50:57 +0200 Subject: [PATCH 03/10] test(member-live): assert rendered behavior instead of socket internals in the index view Replace :sys.get_state assertions on the LiveView socket with assertions on rendered output, so the tests pin user-visible behavior rather than internal state; the few sites with no observable equivalent are kept and annotated. --- test/mv_web/member_live/index_test.exs | 316 +++++++++++++------------ 1 file changed, 169 insertions(+), 147 deletions(-) diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index be61c9c..cd166cc 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -1,4 +1,8 @@ defmodule MvWeb.MemberLive.IndexTest do + # async: false on purpose: the @slow "boolean filter performance with 150 members" + # test asserts a wall-clock budget (duration < 1000ms). Running this module in + # parallel with others adds CPU contention that inflates that measurement and makes + # the timing assertion flaky, so this module stays synchronous. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query @@ -297,10 +301,14 @@ defmodule MvWeb.MemberLive.IndexTest do send(view.pid, {:search_changed, "Friedrich"}) - state = :sys.get_state(view.pid) - - assert state.socket.assigns.query == "Friedrich" - assert is_list(state.socket.assigns.members) + # Rationale: this exercises the handle_info(:search_changed) callback in isolation. + # The search box value is owned by SearchBarComponent (assign_new), and scope is + # recomputed on handle_params rather than this handle_info, so the updated :query + # and :members assigns have no faithful rendered proxy here. The assigns are + # asserted on internal state to preserve the original coverage of the callback. + assigns = :sys.get_state(view.pid).socket.assigns + assert assigns.query == "Friedrich" + assert is_list(assigns.members) end @tag :ui @@ -393,6 +401,38 @@ defmodule MvWeb.MemberLive.IndexTest do |> LazyHTML.query(~s([data-testid="bulk-actions-scope-badge"])) end + # Opens the bulk-actions dropdown and returns the mailto link's BCC payload + # (everything after "mailto:?bcc="). This is the observable carrier of the + # recipient set / recipient_count, replacing direct socket-assign inspection. + defp mailto_bcc(view) do + view |> element(~s([data-testid="bulk-actions-button"])) |> render_click() + + href = + render(view) + |> LazyHTML.from_fragment() + |> LazyHTML.query(~s([data-testid="bulk-actions-mailto"])) + |> LazyHTML.attribute("href") + |> List.first() + + case href do + "mailto:?bcc=" <> bcc -> bcc + other -> other || "" + end + end + + # Opens the member-filter dropdown so its boolean filter controls are rendered. + defp open_member_filter(view) do + view + |> element(~s(button[phx-click="toggle_dropdown"][aria-label="Filter members"])) + |> render_click() + end + + # Returns the rendered HTML of the member-filter dropdown (with it open). + defp member_filter_html(view) do + open_member_filter(view) + render(view) + end + describe "copy_emails feature" do setup do system_actor = SystemActor.get_system_actor() @@ -528,15 +568,13 @@ defmodule MvWeb.MemberLive.IndexTest do # Select two members by sending the select_member event directly render_click(view, "select_member", %{"id" => member1.id}) - render_click(view, "select_member", %{"id" => member2.id}) + html = render_click(view, "select_member", %{"id" => member2.id}) - # Get the socket state to verify the formatted email string - state = :sys.get_state(view.pid) - selected_members = state.socket.assigns.selected_members - - # Verify MapSet is used - assert %MapSet{} = selected_members - assert MapSet.size(selected_members) == 2 + # Both selected members are observably reflected: their row checkboxes are + # checked and the scope badge shows the selection count ("2"). + assert has_element?(view, ~s(input[role="checkbox"][name="#{member1.id}"][checked])) + assert has_element?(view, ~s(input[role="checkbox"][name="#{member2.id}"][checked])) + assert scope_badge(html) |> LazyHTML.text() |> String.trim() == "2" end test "email format is 'First Last ' with comma separator", %{ @@ -969,26 +1007,23 @@ defmodule MvWeb.MemberLive.IndexTest do test "scope is :all when nothing selected and no filter", %{conn: conn} do conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/members") + {:ok, _view, html} = live(conn, "/members") - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.scope == :all + # :all scope renders the muted "all" badge. + assert scope_badge(html) |> LazyHTML.text() |> String.trim() == "all" end test "scope is :filtered when a search term is active", %{conn: conn} do conn = conn_with_oidc_user(conn) - {:ok, view, _html} = live(conn, "/members?query=Scope") + {:ok, _view, html} = live(conn, "/members?query=Scope") - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.scope == :filtered + # An active search narrows the list, so the scope badge reads "filtered". + assert scope_badge(html) |> LazyHTML.text() |> String.trim() == "filtered" end test "scope is :filtered when a non-search filter is active", %{conn: conn} do conn = conn_with_oidc_user(conn) - {:ok, view, html} = live(conn, "/members?cycle_status_filter=paid") - - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.scope == :filtered + {:ok, _view, html} = live(conn, "/members?cycle_status_filter=paid") badge = scope_badge(html) assert badge |> LazyHTML.text() |> String.trim() == "filtered" @@ -1001,10 +1036,13 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") - render_click(view, "select_member", %{"id" => member1.id}) + html = render_click(view, "select_member", %{"id" => member1.id}) - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.scope == :selection + # A selection switches the badge to the emphasized (primary) variant whose + # label is the selected count ("1"), which is the observable proxy for scope == :selection. + badge = scope_badge(html) + assert badge |> LazyHTML.text() |> String.trim() == "1" + assert badge |> LazyHTML.attribute("class") |> List.first() =~ "badge-primary" end test "with no selection, recipient_count and mailto_bcc cover all members", %{ @@ -1013,11 +1051,11 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") - assigns = :sys.get_state(view.pid).socket.assigns + # The mailto link's BCC is the observable carrier of recipient_count/mailto_bcc. + bcc = mailto_bcc(view) # Both seeded members have an email, so the no-selection scope covers both. - assert assigns.recipient_count == 2 - assert assigns.mailto_bcc =~ "scope1%40example.com" - assert assigns.mailto_bcc =~ "scope2%40example.com" + assert bcc =~ "scope1%40example.com" + assert bcc =~ "scope2%40example.com" end test "with a selection, recipient_count and mailto_bcc cover only the selection", %{ @@ -1029,10 +1067,9 @@ defmodule MvWeb.MemberLive.IndexTest do render_click(view, "select_member", %{"id" => member1.id}) - assigns = :sys.get_state(view.pid).socket.assigns - assert assigns.recipient_count == 1 - assert assigns.mailto_bcc =~ "scope1%40example.com" - refute assigns.mailto_bcc =~ "scope2%40example.com" + bcc = mailto_bcc(view) + assert bcc =~ "scope1%40example.com" + refute bcc =~ "scope2%40example.com" end end @@ -1298,6 +1335,9 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") + # Rationale: with no boolean fields and no active filter there is no rendered + # filter control or active-count badge to observe, so the empty initial filter + # map is asserted on internal state directly. state = :sys.get_state(view.pid) assert state.socket.assigns.boolean_custom_field_filters == %{} end @@ -1309,6 +1349,9 @@ defmodule MvWeb.MemberLive.IndexTest do conn = conn_with_oidc_user(conn) {:ok, view, _html} = live(conn, "/members") + # Rationale: the absence of boolean fields means the filter dropdown renders + # no boolean fieldsets; "empty list" has no positive rendered signal, so it is + # asserted on internal state directly. state = :sys.get_state(view.pid) assert state.socket.assigns.boolean_custom_fields == [] end @@ -1319,89 +1362,82 @@ defmodule MvWeb.MemberLive.IndexTest do # Create boolean and non-boolean custom fields boolean_field1 = create_boolean_custom_field(%{name: "Active Member"}) boolean_field2 = create_boolean_custom_field(%{name: "Newsletter Subscription"}) - _string_field = create_string_custom_field(%{name: "Phone Number"}) + string_field = create_string_custom_field(%{name: "Phone Number"}) {:ok, view, _html} = live(conn, "/members") + open_member_filter(view) - state = :sys.get_state(view.pid) - boolean_custom_fields = state.socket.assigns.boolean_custom_fields - - # Should only contain boolean fields - assert length(boolean_custom_fields) == 2 - assert Enum.all?(boolean_custom_fields, &(&1.value_type == :boolean)) - assert Enum.any?(boolean_custom_fields, &(&1.id == boolean_field1.id)) - assert Enum.any?(boolean_custom_fields, &(&1.id == boolean_field2.id)) + # Only the boolean fields render a tri-state filter control; the string field does not. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field1.id}-all"}") + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field2.id}-all"}") + refute has_element?(view, "##{"custom-boolean-filter-#{string_field.id}-all"}") end test "mount sorts boolean custom fields by name ascending", %{conn: conn} do conn = conn_with_oidc_user(conn) # Create boolean fields with specific names to test sorting - _boolean_field_z = create_boolean_custom_field(%{name: "Zebra Field"}) - _boolean_field_a = create_boolean_custom_field(%{name: "Alpha Field"}) - _boolean_field_m = create_boolean_custom_field(%{name: "Middle Field"}) + field_z = create_boolean_custom_field(%{name: "Zebra Field"}) + field_a = create_boolean_custom_field(%{name: "Alpha Field"}) + field_m = create_boolean_custom_field(%{name: "Middle Field"}) {:ok, view, _html} = live(conn, "/members") + html = member_filter_html(view) - state = :sys.get_state(view.pid) - boolean_custom_fields = state.socket.assigns.boolean_custom_fields + # The rendered boolean filter controls appear in name-ascending order. + rendered_order = + html + |> LazyHTML.from_fragment() + |> LazyHTML.query(~s(input[id$="-all"][name^="custom_boolean"])) + |> LazyHTML.attribute("id") + |> Enum.map( + &(&1 + |> String.replace_prefix("custom-boolean-filter-", "") + |> String.replace_suffix("-all", "")) + ) - # Should be sorted by name ascending - names = Enum.map(boolean_custom_fields, & &1.name) - assert names == ["Alpha Field", "Middle Field", "Zebra Field"] + assert rendered_order == [field_a.id, field_m.id, field_z.id] end test "handle_params parses bf_ values correctly", %{conn: conn} do conn = conn_with_oidc_user(conn) boolean_field = create_boolean_custom_field() - # Test true value - {:ok, view1, _html} = - live(conn, "/members?bf_#{boolean_field.id}=true") + # Test true value: the "Yes" radio is checked (the boolean true, not the string "true"). + {:ok, view1, _html} = live(conn, "/members?bf_#{boolean_field.id}=true") + open_member_filter(view1) + assert has_element?(view1, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") + refute has_element?(view1, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]") - state1 = :sys.get_state(view1.pid) - filters1 = state1.socket.assigns.boolean_custom_field_filters - assert filters1[boolean_field.id] == true - refute filters1[boolean_field.id] == "true" - - # Test false value - {:ok, view2, _html} = - live(conn, "/members?bf_#{boolean_field.id}=false") - - state2 = :sys.get_state(view2.pid) - filters2 = state2.socket.assigns.boolean_custom_field_filters - assert filters2[boolean_field.id] == false - refute filters2[boolean_field.id] == "false" + # Test false value: the "No" radio is checked. + {:ok, view2, _html} = live(conn, "/members?bf_#{boolean_field.id}=false") + open_member_filter(view2) + assert has_element?(view2, "##{"custom-boolean-filter-#{boolean_field.id}-false"}[checked]") + refute has_element?(view2, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]") end test "handle_params ignores non-existent custom field IDs", %{conn: conn} do conn = conn_with_oidc_user(conn) fake_id = Ecto.UUID.generate() - {:ok, view, _html} = - live(conn, "/members?bf_#{fake_id}=true") + {:ok, view, _html} = live(conn, "/members?bf_#{fake_id}=true") + open_member_filter(view) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters - - # Filter should not be added for non-existent custom field - refute Map.has_key?(filters, fake_id) - assert filters == %{} + # No filter control exists for a non-existent field, and no active-filter badge appears. + refute has_element?(view, "##{"custom-boolean-filter-#{fake_id}-true"}") + refute has_element?(view, ~s(button[aria-label="Filter members"].btn-active)) end test "handle_params ignores non-boolean custom fields", %{conn: conn} do conn = conn_with_oidc_user(conn) string_field = create_string_custom_field() - {:ok, view, _html} = - live(conn, "/members?bf_#{string_field.id}=true") + {:ok, view, _html} = live(conn, "/members?bf_#{string_field.id}=true") + open_member_filter(view) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters - - # Filter should not be added for non-boolean custom field - refute Map.has_key?(filters, string_field.id) - assert filters == %{} + # A string field is never rendered as a boolean filter, and no filter becomes active. + refute has_element?(view, "##{"custom-boolean-filter-#{string_field.id}-true"}") + refute has_element?(view, ~s(button[aria-label="Filter members"].btn-active)) end test "handle_params ignores invalid filter values", %{conn: conn} do @@ -1412,15 +1448,12 @@ defmodule MvWeb.MemberLive.IndexTest do invalid_values = ["1", "0", "yes", "no", "True", "False", "", "invalid", "null"] for invalid_value <- invalid_values do - {:ok, view, _html} = - live(conn, "/members?bf_#{boolean_field.id}=#{invalid_value}") + {:ok, view, _html} = live(conn, "/members?bf_#{boolean_field.id}=#{invalid_value}") + open_member_filter(view) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters - - # Invalid values should not be added to filters - refute Map.has_key?(filters, boolean_field.id), - "Invalid value '#{invalid_value}' should not be added to filters" + # An invalid value leaves the field's filter at "All" (no filter applied). + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]"), + "Invalid value '#{invalid_value}' should leave the filter at All" end end @@ -1435,12 +1468,16 @@ defmodule MvWeb.MemberLive.IndexTest do "/members?bf_#{boolean_field1.id}=true&bf_#{boolean_field2.id}=false" ) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - assert filters[boolean_field1.id] == true - assert filters[boolean_field2.id] == false - assert map_size(filters) == 2 + # Both filters are reflected: field1 at "Yes", field2 at "No", and the + # active-filter count badge shows 2. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field1.id}-true"}[checked]") + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field2.id}-false"}[checked]") + + assert view + |> element(~s(button[aria-label="Filter members"] .badge), "2") + |> has_element?() end test "build_query_params includes active boolean filters and excludes nil filters", %{ @@ -1514,12 +1551,12 @@ defmodule MvWeb.MemberLive.IndexTest do "/members?cycle_status_filter=paid&bf_#{boolean_field.id}=true" ) - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - # Both filters should be set - assert filters[boolean_field.id] == true - assert state.socket.assigns.cycle_status_filter == :paid + # Both filters are reflected in the rendered controls: the boolean field at + # "Yes" and the payment-status filter at "paid". + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") + assert has_element?(view, "#payment-filter-paid[checked]") # Both should be in URL when triggering search view @@ -1540,9 +1577,8 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{boolean_field.id}=true") - state_before = :sys.get_state(view.pid) - filters_before = state_before.socket.assigns.boolean_custom_field_filters - assert filters_before[boolean_field.id] == true + open_member_filter(view) + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") # Delete the custom field (requires actor with destroy permission) Ash.destroy!(boolean_field, actor: system_actor) @@ -1551,12 +1587,11 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view2, _html} = live(conn, "/members?bf_#{boolean_field.id}=true") - state_after = :sys.get_state(view2.pid) - filters_after = state_after.socket.assigns.boolean_custom_field_filters + open_member_filter(view2) - # Filter should not be present for deleted custom field - refute Map.has_key?(filters_after, boolean_field.id) - assert filters_after == %{} + # The deleted field renders no filter control and no filter is active. + refute has_element?(view2, "##{"custom-boolean-filter-#{boolean_field.id}-true"}") + refute has_element?(view2, ~s(button[aria-label="Filter members"].btn-active)) end test "handle_params handles URL-encoded custom field IDs correctly", %{conn: conn} do @@ -1569,12 +1604,11 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{encoded_id}=true") - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - # Filter should work with URL-encoded ID - # Phoenix should decode it automatically, so we check with original ID - assert filters[boolean_field.id] == true + # Phoenix decodes the param, so the filter applies under the original ID: + # the "Yes" radio for the field is checked. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-true"}[checked]") end test "handle_params ignores malformed prefix (bf_bf_)", %{conn: conn} do @@ -1585,12 +1619,12 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_bf_#{boolean_field.id}=true") - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - # Should not parse as valid filter (UUID validation should fail) - refute Map.has_key?(filters, boolean_field.id) - assert filters == %{} + # The double-prefixed param is not a valid filter: the field stays at "All" + # and no filter is active. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]") + refute has_element?(view, ~s(button[aria-label="Filter members"].btn-active)) end test "handle_params limits number of boolean filters to prevent DoS", %{conn: conn} do @@ -1605,17 +1639,11 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?#{filter_params}") - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters - - # Should limit to maximum 50 filters - assert map_size(filters) <= 50 - # All filters in the result should be valid - Enum.each(filters, fn {id, value} -> - assert value in [true, false] - # Verify the ID corresponds to one of our boolean fields - assert id in Enum.map(boolean_fields, &to_string(&1.id)) - end) + # The active-filter count badge is the observable carrier of the filter count. + # With 60 requested filters, the DoS cap clamps it to exactly 50. + assert view + |> element(~s(button[aria-label="Filter members"] .badge), "50") + |> has_element?() end test "handle_params ignores extremely long custom field IDs", %{conn: conn} do @@ -1628,14 +1656,12 @@ defmodule MvWeb.MemberLive.IndexTest do {:ok, view, _html} = live(conn, "/members?bf_#{fake_long_id}=true") - state = :sys.get_state(view.pid) - filters = state.socket.assigns.boolean_custom_field_filters + open_member_filter(view) - # Should not accept the extremely long ID - refute Map.has_key?(filters, fake_long_id) - # Valid boolean field should still work - refute Map.has_key?(filters, boolean_field.id) - assert filters == %{} + # The over-long ID is rejected: the real field stays at "All" and no filter + # is active. + assert has_element?(view, "##{"custom-boolean-filter-#{boolean_field.id}-all"}[checked]") + refute has_element?(view, ~s(button[aria-label="Filter members"].btn-active)) end # Helper to create a member with a boolean custom field value @@ -2283,24 +2309,20 @@ defmodule MvWeb.MemberLive.IndexTest do # Start with no boolean custom fields {:ok, view, _html} = live(conn, "/members") + open_member_filter(view) - state_before = :sys.get_state(view.pid) - boolean_fields_before = state_before.socket.assigns.boolean_custom_fields - assert boolean_fields_before == [] + # No boolean field control is rendered yet. + refute has_element?(view, ~s(input[name^="custom_boolean"])) # Create a new boolean custom field new_boolean_field = create_boolean_custom_field(%{name: "Newly Added Field"}) - # Navigate again - the new field should appear + # Navigate again - the new field should appear in the filter dropdown. {:ok, view2, _html} = live(conn, "/members") + html_after = member_filter_html(view2) - state_after = :sys.get_state(view2.pid) - boolean_fields_after = state_after.socket.assigns.boolean_custom_fields - - # New boolean field should be present - assert length(boolean_fields_after) == 1 - assert Enum.any?(boolean_fields_after, &(&1.id == new_boolean_field.id)) - assert Enum.any?(boolean_fields_after, &(&1.name == "Newly Added Field")) + assert has_element?(view2, "##{"custom-boolean-filter-#{new_boolean_field.id}-all"}") + assert html_after =~ "Newly Added Field" end @tag :slow From 655fd80524f8da2f3781224d89a46aa5ca12a385 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:51:43 +0200 Subject: [PATCH 04/10] test: wait on observable state instead of blind sleeps Replace the fixed Process.sleep waits in the import, members-PDF and field-visibility tests with event-based / bounded-poll waits on the observable condition, removing a known flakiness vector. --- test/mv/membership/members_pdf_test.exs | 31 ++++++++++++--- test/mv_web/live/import_live_test.exs | 38 ++++++++++++++----- .../index_field_visibility_test.exs | 22 ----------- 3 files changed, 55 insertions(+), 36 deletions(-) diff --git a/test/mv/membership/members_pdf_test.exs b/test/mv/membership/members_pdf_test.exs index 2b83e3b..3ae18cf 100644 --- a/test/mv/membership/members_pdf_test.exs +++ b/test/mv/membership/members_pdf_test.exs @@ -274,17 +274,38 @@ defmodule Mv.Membership.MembersPDFTest do assert {:ok, _pdf_binary} = result - # Wait a bit for cleanup (async cleanup might take a moment) - Process.sleep(100) - - # Count temp directories after - after_count = + count_export_dirs = fn -> temp_base |> File.ls!() |> Enum.count(fn name -> String.starts_with?(name, "mv_pdf_export_") end) + end + + # Poll the observable cleanup condition (temp-dir count returns to the baseline) + # with a bounded deadline instead of a fixed sleep, so the test waits no longer + # than the cleanup actually needs and still fails if cleanup never runs. + after_count = poll_until_cleaned(count_export_dirs, before_count, 100) # Should have same or fewer temp dirs (cleanup should have run) assert after_count <= before_count + 1 end end + + # Bounded poll: returns the export-dir count once it drops back to the baseline + # (cleanup done), or the last observed count when the attempt budget is exhausted + # (so the caller's assertion reports the real state on a genuine cleanup stall). + defp poll_until_cleaned(count_fun, baseline, attempts_left) do + current = count_fun.() + + cond do + current <= baseline -> + current + + attempts_left <= 0 -> + current + + true -> + Process.sleep(10) + poll_until_cleaned(count_fun, baseline, attempts_left - 1) + end + end end diff --git a/test/mv_web/live/import_live_test.exs b/test/mv_web/live/import_live_test.exs index bd5cdec..8837fc6 100644 --- a/test/mv_web/live/import_live_test.exs +++ b/test/mv_web/live/import_live_test.exs @@ -37,7 +37,27 @@ defmodule MvWeb.ImportLiveTest do confirm_import(view) end - defp wait_for_import_completion, do: Process.sleep(1000) + # Waits for the asynchronous chunk-import to finish by polling the rendered + # results panel, instead of a fixed sleep. In the test sandbox the chunks run + # in-process and signal completion via self-messages; each render/2 forces the + # LiveView to drain its mailbox before replying, so the panel appears once all + # chunk messages have been processed. Bounded so a genuine stall still fails. + defp wait_for_import_completion(view) do + wait_for_import_completion(view, 200) + end + + defp wait_for_import_completion(_view, 0) do + flunk("import did not complete: results panel never rendered") + end + + defp wait_for_import_completion(view, attempts_left) do + if has_element?(view, "[data-testid='import-results-panel']") do + :ok + else + Process.sleep(10) + wait_for_import_completion(view, attempts_left - 1) + end + end # ---------- Business logic: Authorization ---------- describe "Authorization" do @@ -67,7 +87,7 @@ defmodule MvWeb.ImportLiveTest do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-summary']") @@ -131,7 +151,7 @@ defmodule MvWeb.ImportLiveTest do } do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content, "invalid_import.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-error-list']") @@ -150,7 +170,7 @@ defmodule MvWeb.ImportLiveTest do for i <- 1..100, do: "Row#{i};Last#{i};;Country#{i};City#{i};Street#{i};12345\n" run_full_import(view, header <> Enum.join(invalid_rows), "large_invalid.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-error-list']") @@ -182,7 +202,7 @@ defmodule MvWeb.ImportLiveTest do |> File.read!() run_full_import(view, csv_content, "bom_import.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") html = render(view) @@ -200,7 +220,7 @@ defmodule MvWeb.ImportLiveTest do |> File.read!() run_full_import(view, csv_content, "empty_lines.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-error-list']") html = render(view) @@ -214,7 +234,7 @@ defmodule MvWeb.ImportLiveTest do } do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content, "unknown_custom.csv") - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") assert has_element?(view, "[data-testid='import-warnings']") @@ -279,7 +299,7 @@ defmodule MvWeb.ImportLiveTest do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-progress-container']") html = render(view) assert html =~ "aria-live" @@ -342,7 +362,7 @@ defmodule MvWeb.ImportLiveTest do } do {:ok, view, _html} = live(conn, ~p"/admin/import") run_full_import(view, csv_content) - wait_for_import_completion() + wait_for_import_completion(view) assert has_element?(view, "[data-testid='import-results-panel']") diff --git a/test/mv_web/member_live/index_field_visibility_test.exs b/test/mv_web/member_live/index_field_visibility_test.exs index 79d078b..007939d 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -157,9 +157,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_click() - # Wait for update - :timer.sleep(100) - # Email should no longer be visible html = render(view) refute html =~ "alice@example.com" @@ -186,9 +183,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='#{custom_field_string}']") |> render_click() - # Wait for update - :timer.sleep(100) - # Custom field should no longer be visible html = render(view) refute html =~ "M001" @@ -213,9 +207,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_all']") |> render_click() - # Wait for update - :timer.sleep(100) - # All fields should be visible html = render(view) assert html =~ "alice@example.com" @@ -237,9 +228,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_none']") |> render_click() - # Wait for update - :timer.sleep(100) - # Only first_name should be visible (it's always shown) html = render(view) # Email and street should be hidden @@ -262,9 +250,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_click() - # Wait for URL update - :timer.sleep(100) - # Check that URL contains fields parameter # Note: In LiveView tests, we check the rendered HTML for the updated state # The actual URL update happens via push_patch @@ -329,8 +314,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_click() - :timer.sleep(100) - html = render(view) refute html =~ "alice@example.com" end @@ -387,8 +370,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do view |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_click() - - :timer.sleep(50) end # Should still work correctly @@ -458,9 +439,6 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do |> element("button[phx-click='select_item'][phx-value-item='email']") |> render_keydown(%{key: "Enter"}) - # Wait for update - :timer.sleep(100) - # Email should no longer be visible html = render(view) refute html =~ "alice@example.com" From cb54c2c46efc6e04244b07fbba6974b0e59cb746 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:52:17 +0200 Subject: [PATCH 05/10] test(member-live): build date-filter property bounds without a reject-filter The bound-pair generator filtered out ~1/4 of generated values, so unlucky seeds hit StreamData's FilterTooNarrowError under full property runs. Construct an at-least-one-bound-set pair directly instead, preserving the exact domain with no rejection. --- .../member_live/date_filter_property_test.exs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/test/mv_web/live/member_live/date_filter_property_test.exs b/test/mv_web/live/member_live/date_filter_property_test.exs index fcce2a3..8844f91 100644 --- a/test/mv_web/live/member_live/date_filter_property_test.exs +++ b/test/mv_web/live/member_live/date_filter_property_test.exs @@ -20,11 +20,12 @@ defmodule MvWeb.MemberLive.Index.DateFilterPropertyTest do # Generators ----------------------------------------------------------- + defp date_gen do + map(integer(-3650..3650), &Date.add(~D[2000-01-01], &1)) + end + defp optional_date_gen do - one_of([ - constant(nil), - map(integer(-3650..3650), &Date.add(~D[2000-01-01], &1)) - ]) + one_of([constant(nil), date_gen()]) end defp exit_date_mode_gen do @@ -57,19 +58,25 @@ defmodule MvWeb.MemberLive.Index.DateFilterPropertyTest do # Property ------------------------------------------------------------- + # Generates a {from, to} pair with at least one bound set. Built by construction + # (pick which bounds are set, then generate the required dates) rather than by + # filtering out the both-nil case, so StreamData never rejects values and cannot + # raise FilterTooNarrowError on unlucky seeds. defp bound_pair_with_at_least_one_set_gen do gen all( - from <- optional_date_gen(), - to <- optional_date_gen(), - from != nil or to != nil + which <- member_of([:from_only, :to_only, :both]), + date_a <- date_gen(), + date_b <- date_gen() ) do - {from, to} + case which do + :from_only -> {date_a, nil} + :to_only -> {nil, date_a} + :both -> {date_a, date_b} + end end end - defp value_date_gen do - map(integer(-3650..3650), &Date.add(~D[2000-01-01], &1)) - end + defp value_date_gen, do: date_gen() # Property ------------------------------------------------------------- From ef94d2ef105f69674d6d4a127b97b3b2c5361790 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:52:51 +0200 Subject: [PATCH 06/10] fix(repo): make member/user foreign keys deferrable to avoid create_member deadlock Concurrent create_member transactions took FK FOR KEY SHARE (MultiXact) locks on shared rows across members/users/membership_fee_types and could form a cross-transaction cycle, producing intermittent PostgreSQL deadlocks (40P01) under load. Making the three foreign keys DEFERRABLE INITIALLY DEFERRED moves the check to commit time and breaks the cycle, without weakening integrity (NOT NULL and ON DELETE RESTRICT are unaffected). --- ...165309_make_member_user_fks_deferrable.exs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs diff --git a/priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs b/priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs new file mode 100644 index 0000000..2fa45fb --- /dev/null +++ b/priv/repo/migrations/20260616165309_make_member_user_fks_deferrable.exs @@ -0,0 +1,32 @@ +defmodule Mv.Repo.Migrations.MakeMemberUserFksDeferrable do + @moduledoc """ + Makes the members/users foreign keys DEFERRABLE INITIALLY DEFERRED. + + Concurrent `create_member` transactions take FK `FOR KEY SHARE` (MultiXact) + locks on these foreign keys at statement time and can form a cross-transaction + lock cycle, producing a PostgreSQL `deadlock_detected` (40P01). Deferring the + FK checks to commit time breaks the cycle. + + Constraint deferrability is not tracked by AshPostgres resource snapshots, so + this is a hand-written migration with raw `execute/2`. Do not regenerate it + via `mix ash_postgres.generate_migrations`. + """ + use Ecto.Migration + + def change do + execute( + "ALTER TABLE users ALTER CONSTRAINT users_member_id_fkey DEFERRABLE INITIALLY DEFERRED", + "ALTER TABLE users ALTER CONSTRAINT users_member_id_fkey NOT DEFERRABLE" + ) + + execute( + "ALTER TABLE users ALTER CONSTRAINT users_role_id_fkey DEFERRABLE INITIALLY DEFERRED", + "ALTER TABLE users ALTER CONSTRAINT users_role_id_fkey NOT DEFERRABLE" + ) + + execute( + "ALTER TABLE members ALTER CONSTRAINT members_membership_fee_type_id_fkey DEFERRABLE INITIALLY DEFERRED", + "ALTER TABLE members ALTER CONSTRAINT members_membership_fee_type_id_fkey NOT DEFERRABLE" + ) + end +end From 5e84c342b7c54eceff25445292f9c3a90fed4e3a Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:53:25 +0200 Subject: [PATCH 07/10] test(repo): assert member/user foreign keys are deferrable --- test/mv/repo/deferrable_fk_test.exs | 37 +++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 test/mv/repo/deferrable_fk_test.exs diff --git a/test/mv/repo/deferrable_fk_test.exs b/test/mv/repo/deferrable_fk_test.exs new file mode 100644 index 0000000..4127082 --- /dev/null +++ b/test/mv/repo/deferrable_fk_test.exs @@ -0,0 +1,37 @@ +defmodule Mv.DeferrableFkTest do + @moduledoc """ + Regression guard for the deferrable-FK migration. + + Asserts the schema property directly (`condeferred = true`) for the three + members/users foreign keys. A multi-connection deadlock reproduction cannot + be made deterministic under the Ecto sandbox (ownership serializes + connections), so the structural assertion is the guard here; see the + migration moduledoc for the full FOR-KEY-SHARE/MultiXact deadlock rationale. + """ + use Mv.DataCase, async: true + + @deferrable_fks ~w( + users_member_id_fkey + users_role_id_fkey + members_membership_fee_type_id_fkey + ) + + test "member/user foreign keys are DEFERRABLE INITIALLY DEFERRED" do + query = """ + SELECT conname, condeferrable, condeferred + FROM pg_constraint + WHERE conname = ANY($1) + """ + + {:ok, %{rows: rows}} = Mv.Repo.query(query, [@deferrable_fks]) + + found = Map.new(rows, fn [name, deferrable, deferred] -> {name, {deferrable, deferred}} end) + + for fk <- @deferrable_fks do + assert Map.has_key?(found, fk), "expected foreign key #{fk} to exist" + + assert found[fk] == {true, true}, + "expected #{fk} to be DEFERRABLE INITIALLY DEFERRED, got #{inspect(found[fk])}" + end + end +end From c0f40a13cee7341fdc8e2865f966f09f9badc90b Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:53:59 +0200 Subject: [PATCH 08/10] test(member-live): keep deadlock-prone member tests synchronous These member/group/custom-field LiveView tests stay async: false. With the foreign keys now deferrable the create_member deadlock no longer forces it, so the rationale is updated: they remain synchronous as a deferred scope decision, and index_groups_url_params/member_filter_component additionally have separate async-isolation issues that must be fixed before they can run in parallel. --- test/mv_web/components/member_filter_component_test.exs | 5 ++++- .../mv_web/member_live/index_custom_fields_display_test.exs | 5 ++++- test/mv_web/member_live/index_field_visibility_test.exs | 5 ++++- test/mv_web/member_live/index_groups_accessibility_test.exs | 5 ++++- test/mv_web/member_live/index_groups_display_test.exs | 5 ++++- test/mv_web/member_live/index_groups_filter_test.exs | 5 ++++- test/mv_web/member_live/index_groups_integration_test.exs | 5 ++++- test/mv_web/member_live/index_groups_performance_test.exs | 5 ++++- test/mv_web/member_live/index_groups_sorting_test.exs | 5 ++++- test/mv_web/member_live/index_groups_url_params_test.exs | 5 ++++- test/mv_web/member_live/show_groups_display_test.exs | 6 ++++-- test/mv_web/user_live/form_test.exs | 5 ++++- 12 files changed, 48 insertions(+), 13 deletions(-) diff --git a/test/mv_web/components/member_filter_component_test.exs b/test/mv_web/components/member_filter_component_test.exs index fcb45f6..58aee87 100644 --- a/test/mv_web/components/member_filter_component_test.exs +++ b/test/mv_web/components/member_filter_component_test.exs @@ -8,7 +8,10 @@ defmodule MvWeb.Components.MemberFilterComponentTest do - Button label and badge logic - Filtering to show only boolean custom fields """ - # async: false to prevent PostgreSQL deadlocks when running LiveView tests against DB + # Kept async: false. The deferrable-FK migration removed the concurrent + # create_member deadlock, but this file additionally showed an async-isolation + # failure under load (filtered members from a parallel test leaking in), so it + # is not trivially async-safe; resolving that is a separate follow-up. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest diff --git a/test/mv_web/member_live/index_custom_fields_display_test.exs b/test/mv_web/member_live/index_custom_fields_display_test.exs index 9ada92b..bcc9913 100644 --- a/test/mv_web/member_live/index_custom_fields_display_test.exs +++ b/test/mv_web/member_live/index_custom_fields_display_test.exs @@ -9,7 +9,10 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsDisplayTest do - Custom field values are correctly formatted for different types - Members without custom field values show empty cell or "-" """ - # async: false to prevent PostgreSQL deadlocks when creating members and custom fields + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/custom-fields suite to async is a possible follow-up + # rather than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_field_visibility_test.exs b/test/mv_web/member_live/index_field_visibility_test.exs index 007939d..b992ddd 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -10,7 +10,10 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do - Integration with member list display - Custom fields visibility """ - # async: false to prevent PostgreSQL deadlocks when creating members and custom fields + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/custom-fields suite to async is a possible follow-up + # rather than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest diff --git a/test/mv_web/member_live/index_groups_accessibility_test.exs b/test/mv_web/member_live/index_groups_accessibility_test.exs index d14cd9f..e54a0ba 100644 --- a/test/mv_web/member_live/index_groups_accessibility_test.exs +++ b/test/mv_web/member_live/index_groups_accessibility_test.exs @@ -8,7 +8,10 @@ defmodule MvWeb.MemberLive.IndexGroupsAccessibilityTest do - Sort header has aria-label for screen reader - Keyboard navigation works (Tab through filter, sort header) """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_display_test.exs b/test/mv_web/member_live/index_groups_display_test.exs index 263ac2a..bc39967 100644 --- a/test/mv_web/member_live/index_groups_display_test.exs +++ b/test/mv_web/member_live/index_groups_display_test.exs @@ -8,7 +8,10 @@ defmodule MvWeb.MemberLive.IndexGroupsDisplayTest do - No badge for members without groups - Badge shows group name correctly """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_filter_test.exs b/test/mv_web/member_live/index_groups_filter_test.exs index d32b17f..e0c7b22 100644 --- a/test/mv_web/member_live/index_groups_filter_test.exs +++ b/test/mv_web/member_live/index_groups_filter_test.exs @@ -6,7 +6,10 @@ defmodule MvWeb.MemberLive.IndexGroupsFilterTest do All / Yes / No (per group). Multiple active group filters combine with AND (member must match all selected group conditions). """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_integration_test.exs b/test/mv_web/member_live/index_groups_integration_test.exs index 86738da..0832faa 100644 --- a/test/mv_web/member_live/index_groups_integration_test.exs +++ b/test/mv_web/member_live/index_groups_integration_test.exs @@ -10,7 +10,10 @@ defmodule MvWeb.MemberLive.IndexGroupsIntegrationTest do - Groups work with existing search (but not testing search integration itself) - Member index search by group name returns members in that group (Issue #375) """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_performance_test.exs b/test/mv_web/member_live/index_groups_performance_test.exs index 761c4eb..6eb3311 100644 --- a/test/mv_web/member_live/index_groups_performance_test.exs +++ b/test/mv_web/member_live/index_groups_performance_test.exs @@ -8,7 +8,10 @@ defmodule MvWeb.MemberLive.IndexGroupsPerformanceTest do - Filter works at database level (not in-memory) - Sort runs in-memory but uses preloaded group data (no extra DB queries) """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_sorting_test.exs b/test/mv_web/member_live/index_groups_sorting_test.exs index 068152c..9427419 100644 --- a/test/mv_web/member_live/index_groups_sorting_test.exs +++ b/test/mv_web/member_live/index_groups_sorting_test.exs @@ -2,7 +2,10 @@ defmodule MvWeb.MemberLive.IndexGroupsSortingTest do @moduledoc """ Tests for sorting by groups in the member overview. """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/groups suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/index_groups_url_params_test.exs b/test/mv_web/member_live/index_groups_url_params_test.exs index 469b010..66bc186 100644 --- a/test/mv_web/member_live/index_groups_url_params_test.exs +++ b/test/mv_web/member_live/index_groups_url_params_test.exs @@ -9,7 +9,10 @@ defmodule MvWeb.MemberLive.IndexGroupsUrlParamsTest do - URL parameters work with other parameters (query, sort_field, etc.) - URL is bookmarkable (filter/sorting persist) """ - # async: false to prevent PostgreSQL deadlocks when creating members and groups + # Kept async: false. The deferrable-FK migration removed the concurrent + # create_member deadlock, but this file additionally showed an async-isolation + # failure under load (filtered members from a parallel test leaking in), so it + # is not trivially async-safe; resolving that is a separate follow-up. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/show_groups_display_test.exs b/test/mv_web/member_live/show_groups_display_test.exs index f8434b3..913cf76 100644 --- a/test/mv_web/member_live/show_groups_display_test.exs +++ b/test/mv_web/member_live/show_groups_display_test.exs @@ -10,8 +10,10 @@ defmodule MvWeb.MemberLive.ShowGroupsDisplayTest do - Accessibility: group links have aria-label for screen readers ## Note on async - async: false to avoid PostgreSQL deadlocks when creating members and groups - in the same test run (same as IndexGroupsDisplayTest). + Kept async: false as a deferred scope decision. The deferrable-FK migration + removed the concurrent-create_member deadlock that previously forced this, so + re-flipping this members/groups suite to async is a possible follow-up rather + than part of the original change. """ use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest diff --git a/test/mv_web/user_live/form_test.exs b/test/mv_web/user_live/form_test.exs index a22c230..63649e4 100644 --- a/test/mv_web/user_live/form_test.exs +++ b/test/mv_web/user_live/form_test.exs @@ -1,5 +1,8 @@ defmodule MvWeb.UserLive.FormTest do - # async: false to prevent PostgreSQL deadlocks when creating members and users + # Kept async: false as a deferred scope decision. The deferrable-FK migration + # removed the concurrent-create_member deadlock that previously forced this, so + # re-flipping this members/users suite to async is a possible follow-up rather + # than part of the original change. use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest From 3d792e8b047fdca7ec18dd07791dadf40651e896 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:54:32 +0200 Subject: [PATCH 09/10] docs(testing): document create_member deadlock fix and async-test-safety --- docs/test-performance-optimization.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/test-performance-optimization.md b/docs/test-performance-optimization.md index b45c9fc..0a243c3 100644 --- a/docs/test-performance-optimization.md +++ b/docs/test-performance-optimization.md @@ -90,6 +90,29 @@ test/ --- +## 5. Concurrent `create_member` deadlock and deferrable FKs + +A class of intermittent failures (PostgreSQL `deadlock_detected`, SQLSTATE `40P01`) was traced to **concurrent `create_member` transactions**, not to any single test. It surfaced as a `MatchError` on `{:ok, member} = ...` in member-heavy LiveView tests (e.g. `FormMemberSelectionTest`) and reproduced only under CPU contention (≈1 in 12 full-fast-suite runs at high `async: true` concurrency; effectively never on an idle machine). + +**Root cause.** `create_member` writes a cascade in one transaction (member row, `custom_field_values`, the `user` link, fee-type defaulting, cycle generation). Concurrent inserts take FK `FOR KEY SHARE` (MultiXact) locks on shared parent rows across `members` / `users` / `membership_fee_types`; under contention these can form a cross-transaction lock cycle that Postgres resolves by aborting one transaction. It is a product-level concurrency property, **not** test-data contention, so it is not fixable by test-state isolation. + +**Fix.** Migration `…_make_member_user_fks_deferrable.exs` makes the three FKs (`users.member_id`, `users.role_id`, `members.membership_fee_type_id`) `DEFERRABLE INITIALLY DEFERRED`, moving the FK check (and its lock) to commit time and breaking the cycle. Verified: **0 deadlocks in 15 full-suite runs under maximum CPU contention**, versus 1/12 before. This does **not** weaken integrity — `NOT NULL` is independent of FK deferral, a real dangling reference still aborts the commit, and `ON DELETE RESTRICT` (e.g. `users.role_id`) stays immediate regardless of deferrability. `Mv.DeferrableFkTest` asserts the constraint state as a regression guard (a deterministic in-process concurrent reproduction is infeasible under the Ecto sandbox, which serializes connections by ownership). + +This deadlock is also a latent **production** risk under concurrent sign-ups; the deferrable-FK fix addresses both. + +### Async-test-safety checklist (members/groups/custom fields) + +Several member-creating test files historically used `async: false` with a "prevent PostgreSQL deadlocks" comment. With the deferrable-FK migration in place those files are deadlock-safe, but before flipping any such file to `async: true`: + +- **Prove isolation under load, not just one green run.** Re-run the file (and the full suite) under varying `--seed` **and** CPU contention; a single green run is not evidence (the deadlock and the isolation flakes below are load-dependent). +- **Watch for separate async-isolation issues beyond the deadlock.** `index_groups_url_params_test.exs` and `member_filter_component_test.exs` showed filtered-member-leak failures (`refute html =~ name`) under concurrency that are independent of the FK deadlock — these need their own per-file isolation fix before they can run async. + +### StreamData generator pitfall + +`FilterTooNarrowError` appeared on unlucky seeds (e.g. 222) in a property test that built a value with a reject-filter (`StreamData.filter` discarding ~1/4 of generated pairs). Under full property-run counts this hits too many consecutive rejections. Fix: **construct the desired value directly** instead of generating-then-filtering (preserves the exact domain, no rejection). Prefer constructive generators over reject-filters in property tests. + +--- + ## References - Testing Standards: `CODE_GUIDELINES.md` §4 From 6d7ece20a8c151e07cb47fe1a686c27c01464ac4 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Jun 2026 17:55:24 +0200 Subject: [PATCH 10/10] docs(changelog): record member-creation deadlock fix under Unreleased --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 293d07c..f1df7ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Column-header tooltips clipped** – Tooltips on the members-overview column headers are no longer clipped by the sticky table header. - **Text selection opens member** – Dragging to select text in a members-overview row (for example to copy an email) no longer opens the member details; a plain click still opens them. - **Sort by custom date** – Sorting the member list or member export by a custom date field now orders rows chronologically instead of like text, so e.g. 29.01.1981 correctly comes before 01.03.1982. +- **Concurrent member creation no longer deadlocks** – Creating members in parallel (e.g. simultaneous sign-ups, or batch operations) could intermittently fail with a PostgreSQL deadlock; the affected foreign keys are now deferrable, so concurrent member creation succeeds reliably. ## [1.2.0] - 2026-05-08