From 128c712dbce4b610f32253283a254c76da9eb34c Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 16 Dec 2025 14:11:15 +0100 Subject: [PATCH] fix: improve get_last_completed_cycle and fix test helpers - Fix get_last_completed_cycle to find most recent completed cycle - Fix create_cycle helpers to delete auto-generated cycles first - Fix Ash.destroy return value handling - Fix form selectors to use specific IDs - Fix URL parameter names for filters - Fix Ash.read_one return value expectations in tests --- lib/membership/member.ex | 3 ++ .../set_default_membership_fee_type.ex | 37 +++++++++++++++++ lib/mv_web/helpers/membership_fee_helpers.ex | 16 +++++--- .../show/membership_fees_component.ex | 14 ++++++- .../helpers/membership_fee_helpers_test.exs | 6 +-- .../membership_fee_type_live/form_test.exs | 26 +++++++----- .../membership_fee_type_live/index_test.exs | 10 ++--- .../form_membership_fee_type_test.exs | 12 +++--- .../index/membership_fee_status_test.exs | 8 ++++ .../index_membership_fee_status_test.exs | 40 ++++++++++++++++++- .../membership_fee_integration_test.exs | 40 +++++++++++++------ .../member_live/show_membership_fees_test.exs | 8 ++++ 12 files changed, 177 insertions(+), 43 deletions(-) create mode 100644 lib/membership/member/changes/set_default_membership_fee_type.ex diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 787b1d1..50ababe 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -102,6 +102,9 @@ defmodule Mv.Membership.Member do where [changing(:user)] end + # Auto-assign default membership fee type if not explicitly set + change Mv.Membership.Member.Changes.SetDefaultMembershipFeeType + # Auto-calculate membership_fee_start_date if not manually set # Requires both join_date and membership_fee_type_id to be present change Mv.MembershipFees.Changes.SetMembershipFeeStartDate diff --git a/lib/membership/member/changes/set_default_membership_fee_type.ex b/lib/membership/member/changes/set_default_membership_fee_type.ex new file mode 100644 index 0000000..060c590 --- /dev/null +++ b/lib/membership/member/changes/set_default_membership_fee_type.ex @@ -0,0 +1,37 @@ +defmodule Mv.Membership.Member.Changes.SetDefaultMembershipFeeType do + @moduledoc """ + Ash change that automatically assigns the default membership fee type to new members + if no membership_fee_type_id is explicitly provided. + + This change reads the default_membership_fee_type_id from global settings and + assigns it to the member if membership_fee_type_id is nil. + """ + use Ash.Resource.Change + + def change(changeset, _opts, _context) do + # Only set default if membership_fee_type_id is not already set + current_type_id = Ash.Changeset.get_attribute(changeset, :membership_fee_type_id) + + if is_nil(current_type_id) do + case Mv.Membership.get_settings() do + {:ok, settings} -> + if settings.default_membership_fee_type_id do + Ash.Changeset.force_change_attribute( + changeset, + :membership_fee_type_id, + settings.default_membership_fee_type_id + ) + else + changeset + end + + {:error, _error} -> + # If settings can't be loaded, continue without default + # This prevents member creation from failing if settings are misconfigured + changeset + end + else + changeset + end + end +end diff --git a/lib/mv_web/helpers/membership_fee_helpers.ex b/lib/mv_web/helpers/membership_fee_helpers.ex index f6b6ec0..93f309b 100644 --- a/lib/mv_web/helpers/membership_fee_helpers.ex +++ b/lib/mv_web/helpers/membership_fee_helpers.ex @@ -126,11 +126,17 @@ defmodule MvWeb.Helpers.MembershipFeeHelpers do fee_type -> cycles = member.membership_fee_cycles || [] - cycles - |> Enum.filter(fn cycle -> - CalendarCycles.last_completed_cycle?(cycle.cycle_start, fee_type.interval, today) - end) - |> List.first() + # Get all completed cycles (cycle_end < today) + completed_cycles = + cycles + |> Enum.filter(fn cycle -> + cycle_end = CalendarCycles.calculate_cycle_end(cycle.cycle_start, fee_type.interval) + Date.compare(today, cycle_end) == :gt + end) + + # Return the most recent completed cycle (highest cycle_start) + completed_cycles + |> Enum.max_by(& &1.cycle_start, Date, fn -> nil end) end end diff --git a/lib/mv_web/live/member_live/show/membership_fees_component.ex b/lib/mv_web/live/member_live/show/membership_fees_component.ex index a4ea5d4..fe0030a 100644 --- a/lib/mv_web/live/member_live/show/membership_fees_component.ex +++ b/lib/mv_web/live/member_live/show/membership_fees_component.ex @@ -442,7 +442,9 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do case Decimal.parse(amount_str) do {amount, _} when is_struct(amount, Decimal) -> - case Ash.update(cycle, :update, %{amount: amount}) do + case cycle + |> Ash.Changeset.for_update(:update, %{amount: amount}) + |> Ash.update() do {:ok, updated_cycle} -> updated_cycles = replace_cycle(socket.assigns.cycles, updated_cycle) @@ -489,6 +491,16 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do |> assign(:deleting_cycle, nil) |> put_flash(:info, gettext("Cycle deleted"))} + {:ok, _destroyed} -> + # Handle case where return_destroyed? is true + updated_cycles = Enum.reject(socket.assigns.cycles, &(&1.id == cycle_id)) + + {:noreply, + socket + |> assign(:cycles, updated_cycles) + |> assign(:deleting_cycle, nil) + |> put_flash(:info, gettext("Cycle deleted"))} + {:error, error} -> {:noreply, socket diff --git a/test/mv_web/helpers/membership_fee_helpers_test.exs b/test/mv_web/helpers/membership_fee_helpers_test.exs index cdb7b43..6d6d35c 100644 --- a/test/mv_web/helpers/membership_fee_helpers_test.exs +++ b/test/mv_web/helpers/membership_fee_helpers_test.exs @@ -31,7 +31,7 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do test "formats yearly cycle range correctly" do cycle_start = ~D[2024-01-01] interval = :yearly - cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, interval) + _cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, interval) result = MembershipFeeHelpers.format_cycle_range(cycle_start, interval) assert result =~ "2024" @@ -42,7 +42,7 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do test "formats quarterly cycle range correctly" do cycle_start = ~D[2024-01-01] interval = :quarterly - cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, interval) + _cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, interval) result = MembershipFeeHelpers.format_cycle_range(cycle_start, interval) assert result =~ "2024" @@ -53,7 +53,7 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do test "formats monthly cycle range correctly" do cycle_start = ~D[2024-03-01] interval = :monthly - cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, interval) + _cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, interval) result = MembershipFeeHelpers.format_cycle_range(cycle_start, interval) assert result =~ "2024" 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 a29da2b..52f2b1b 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 @@ -68,7 +68,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do {:error, {:live_redirect, %{to: to}}} = view - |> form("form", form_data) + |> form("#membership-fee-type-form", form_data) |> render_submit() assert to == "/membership_fee_types" @@ -84,7 +84,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do end test "interval field is editable on create", %{conn: conn} do - {:ok, view, html} = live(conn, "/membership_fee_types/new") + {:ok, _view, html} = live(conn, "/membership_fee_types/new") # Interval field should be editable (not disabled) refute html =~ "disabled" || html =~ "readonly" @@ -95,7 +95,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do test "loads existing type data", %{conn: conn} do fee_type = create_fee_type(%{name: "Existing Type", amount: Decimal.new("60.00")}) - {:ok, view, html} = live(conn, "/membership_fee_types/#{fee_type.id}/edit") + {:ok, _view, html} = live(conn, "/membership_fee_types/#{fee_type.id}/edit") assert html =~ "Existing Type" assert html =~ "60" || html =~ "60,00" @@ -104,7 +104,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do test "interval field is grayed out on edit", %{conn: conn} do fee_type = create_fee_type(%{interval: :yearly}) - {:ok, view, html} = live(conn, "/membership_fee_types/#{fee_type.id}/edit") + {:ok, _view, html} = live(conn, "/membership_fee_types/#{fee_type.id}/edit") # Interval field should be disabled assert html =~ "disabled" || html =~ "readonly" @@ -119,7 +119,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do # Change amount html = view - |> form("form", %{"membership_fee_type[amount]" => "75.00"}) + |> form("#membership-fee-type-form", %{"membership_fee_type[amount]" => "75.00"}) |> render_change() # Should show warning @@ -139,7 +139,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do # Change amount html = view - |> form("form", %{"membership_fee_type[amount]" => "75.00"}) + |> form("#membership-fee-type-form", %{"membership_fee_type[amount]" => "75.00"}) |> render_change() # Should show affected count @@ -153,13 +153,18 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do # Change amount and confirm view - |> form("form", %{"membership_fee_type[amount]" => "75.00"}) + |> form("#membership-fee-type-form", %{"membership_fee_type[amount]" => "75.00"}) |> render_change() view |> element("button[phx-click='confirm_amount_change']") |> render_click() + # Submit the form to actually save the change + view + |> form("#membership-fee-type-form", %{"membership_fee_type[amount]" => "75.00"}) + |> render_submit() + # Amount should be updated updated_type = Ash.read_one!(MembershipFeeType |> Ash.Query.filter(id == ^fee_type.id)) assert updated_type.amount == Decimal.new("75.00") @@ -172,7 +177,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do # Change amount and cancel view - |> form("form", %{"membership_fee_type[amount]" => "75.00"}) + |> form("#membership-fee-type-form", %{"membership_fee_type[amount]" => "75.00"}) |> render_change() view @@ -190,7 +195,10 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do # Submit with invalid data html = view - |> form("form", %{"membership_fee_type[name]" => "", "membership_fee_type[amount]" => ""}) + |> form("#membership-fee-type-form", %{ + "membership_fee_type[name]" => "", + "membership_fee_type[amount]" => "" + }) |> render_submit() # Should show validation errors 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 f423e79..49cd481 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 @@ -57,13 +57,13 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do describe "list display" do test "displays all membership fee types with correct data", %{conn: conn} do - fee_type1 = + _fee_type1 = create_fee_type(%{name: "Regular", amount: Decimal.new("60.00"), interval: :yearly}) - fee_type2 = + _fee_type2 = create_fee_type(%{name: "Reduced", amount: Decimal.new("30.00"), interval: :yearly}) - {:ok, view, html} = live(conn, "/membership_fee_types") + {:ok, _view, html} = live(conn, "/membership_fee_types") assert html =~ "Regular" assert html =~ "Reduced" @@ -80,7 +80,7 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do create_member(%{membership_fee_type_id: fee_type.id}) end) - {:ok, view, html} = live(conn, "/membership_fee_types") + {:ok, _view, html} = live(conn, "/membership_fee_types") assert html =~ "3" || html =~ "Members" || html =~ "Mitglieder" end @@ -115,7 +115,7 @@ defmodule MvWeb.MembershipFeeTypeLive.IndexTest do fee_type = create_fee_type(%{interval: :yearly}) create_member(%{membership_fee_type_id: fee_type.id}) - {:ok, view, html} = live(conn, "/membership_fee_types") + {:ok, _view, html} = live(conn, "/membership_fee_types") # Delete button should be disabled assert html =~ "disabled" || html =~ "cursor-not-allowed" 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 5edcb76..6e9d833 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 @@ -57,7 +57,7 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do describe "membership fee type dropdown" do test "displays in form", %{conn: conn} do - {:ok, view, html} = live(conn, "/members/new") + {:ok, _view, html} = live(conn, "/members/new") # Should show membership fee type dropdown assert html =~ "membership_fee_type_id" || html =~ "Membership Fee Type" || @@ -65,10 +65,10 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do end test "shows available types", %{conn: conn} do - fee_type1 = create_fee_type(%{name: "Type 1", interval: :yearly}) - fee_type2 = create_fee_type(%{name: "Type 2", interval: :yearly}) + _fee_type1 = create_fee_type(%{name: "Type 1", interval: :yearly}) + _fee_type2 = create_fee_type(%{name: "Type 2", interval: :yearly}) - {:ok, view, html} = live(conn, "/members/new") + {:ok, _view, html} = live(conn, "/members/new") assert html =~ "Type 1" assert html =~ "Type 2" @@ -76,11 +76,11 @@ defmodule MvWeb.MemberLive.FormMembershipFeeTypeTest do test "filters to same interval types if member has type", %{conn: conn} do yearly_type = create_fee_type(%{name: "Yearly Type", interval: :yearly}) - monthly_type = create_fee_type(%{name: "Monthly Type", interval: :monthly}) + _monthly_type = create_fee_type(%{name: "Monthly Type", interval: :monthly}) member = create_member(%{membership_fee_type_id: yearly_type.id}) - {:ok, view, html} = live(conn, "/members/#{member.id}/edit") + {:ok, _view, html} = live(conn, "/members/#{member.id}/edit") # Should show yearly type but not monthly assert html =~ "Yearly Type" 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 e6365a2..e10f280 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 @@ -43,6 +43,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do # Helper to create a cycle defp create_cycle(member, fee_type, attrs) do + # Delete any auto-generated cycles first to avoid conflicts + existing_cycles = + MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member.id) + |> Ash.read!() + + Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle) end) + default_attrs = %{ cycle_start: ~D[2023-01-01], amount: Decimal.new("50.00"), 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 6ce55c1..d807b4f 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 @@ -58,6 +58,14 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do # Helper to create a cycle defp create_cycle(member, fee_type, attrs) do + # Delete any auto-generated cycles first to avoid conflicts + existing_cycles = + MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member.id) + |> Ash.read!() + + Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle) end) + default_attrs = %{ cycle_start: ~D[2023-01-01], amount: Decimal.new("50.00"), @@ -178,7 +186,21 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do member2 = create_member(%{membership_fee_type_id: fee_type.id}) create_cycle(member2, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid}) - {:ok, view, _html} = live(conn, "/members?membership_fee_status_filter=unpaid_last") + # Verify cycles exist in database + cycles1 = + MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member1.id) + |> Ash.read!() + + cycles2 = + MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member2.id) + |> Ash.read!() + + assert length(cycles1) > 0 + assert length(cycles2) > 0 + + {:ok, view, _html} = live(conn, "/members?membership_fee_filter=unpaid_last") html = render(view) assert html =~ member1.first_name @@ -199,7 +221,21 @@ defmodule MvWeb.MemberLive.IndexMembershipFeeStatusTest do member2 = create_member(%{membership_fee_type_id: fee_type.id}) create_cycle(member2, fee_type, %{cycle_start: current_year_start, status: :paid}) - {:ok, view, _html} = live(conn, "/members?membership_fee_status_filter=unpaid_current") + # Verify cycles exist in database + cycles1 = + MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member1.id) + |> Ash.read!() + + cycles2 = + MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member2.id) + |> Ash.read!() + + assert length(cycles1) > 0 + assert length(cycles2) > 0 + + {:ok, view, _html} = live(conn, "/members?membership_fee_filter=unpaid_current") html = render(view) assert html =~ member1.first_name 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 3b7f4c8..d07f677 100644 --- a/test/mv_web/member_live/membership_fee_integration_test.exs +++ b/test/mv_web/member_live/membership_fee_integration_test.exs @@ -78,9 +78,14 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do if !Enum.empty?(cycles) do cycle = List.first(cycles) + # Switch to Membership Fees tab + view + |> element("button[phx-click='switch_tab'][phx-value-tab='membership_fees']") + |> render_click() + # Change status view - |> element("button[phx-click='mark_as_paid'][phx-value-cycle-id='#{cycle.id}']") + |> element("button[phx-click='mark_cycle_status'][phx-value-cycle_id='#{cycle.id}']") |> render_click() # Verify status changed @@ -102,7 +107,7 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do {:ok, view, _html} = live(conn, "/members/#{member.id}/edit") view - |> form("form", %{"member[membership_fee_type_id]" => fee_type2.id}) + |> form("#member-form", %{"member[membership_fee_type_id]" => fee_type2.id}) |> render_submit() # Verify cycles regenerated with new amount @@ -124,7 +129,9 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do fee_type = create_fee_type(%{interval: :yearly}) # Update settings - Mv.Membership.Setting + {:ok, settings} = Mv.Membership.get_settings() + + settings |> Ash.Changeset.for_update(:update_membership_fee_settings, %{ default_membership_fee_type_id: fee_type.id }) @@ -141,7 +148,7 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do {:error, {:live_redirect, %{to: _to}}} = view - |> form("form", form_data) + |> form("#member-form", form_data) |> render_submit() # Verify member got default type @@ -170,20 +177,24 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do {:ok, view, _html} = live(conn, "/members/#{member.id}") + # Switch to Membership Fees tab + view + |> element("button[phx-click='switch_tab'][phx-value-tab='membership_fees']") + |> render_click() + # Delete cycle with confirmation view - |> element("button[phx-click='delete_cycle'][phx-value-cycle-id='#{cycle.id}']") + |> element("button[phx-click='delete_cycle'][phx-value-cycle_id='#{cycle.id}']") |> render_click() # Confirm deletion view - |> element("button[phx-click='confirm_delete_cycle'][phx-value-cycle-id='#{cycle.id}']") + |> element("button[phx-click='confirm_delete_cycle'][phx-value-cycle_id='#{cycle.id}']") |> render_click() - # Verify cycle deleted - assert_raise Ash.Error.Query.NotFound, fn -> - Ash.read_one!(MembershipFeeCycle |> Ash.Query.filter(id == ^cycle.id)) - end + # Verify cycle deleted - Ash.read_one returns {:ok, nil} if not found + result = MembershipFeeCycle |> Ash.Query.filter(id == ^cycle.id) |> Ash.read_one() + assert result == {:ok, nil} end test "edit cycle amount → modal → amount updated", %{conn: conn} do @@ -203,14 +214,19 @@ defmodule MvWeb.MemberLive.MembershipFeeIntegrationTest do {:ok, view, _html} = live(conn, "/members/#{member.id}") + # Switch to Membership Fees tab + view + |> element("button[phx-click='switch_tab'][phx-value-tab='membership_fees']") + |> render_click() + # Open edit modal view - |> element("button[phx-click='edit_cycle_amount'][phx-value-cycle-id='#{cycle.id}']") + |> element("button[phx-click='edit_cycle_amount'][phx-value-cycle_id='#{cycle.id}']") |> render_click() # Update amount view - |> form("form", %{"amount" => "75.00"}) + |> form("form[phx-submit='save_cycle_amount']", %{"amount" => "75.00"}) |> render_submit() # Verify amount updated 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 e37fcc9..1f68244 100644 --- a/test/mv_web/member_live/show_membership_fees_test.exs +++ b/test/mv_web/member_live/show_membership_fees_test.exs @@ -58,6 +58,14 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do # Helper to create a cycle defp create_cycle(member, fee_type, attrs) do + # Delete any auto-generated cycles first to avoid conflicts + existing_cycles = + MembershipFeeCycle + |> Ash.Query.filter(member_id == ^member.id) + |> Ash.read!() + + Enum.each(existing_cycles, fn cycle -> Ash.destroy!(cycle) end) + default_attrs = %{ cycle_start: ~D[2023-01-01], amount: Decimal.new("50.00"),