diff --git a/docs/membership-fee-architecture.md b/docs/membership-fee-architecture.md index c601b79..d6b5ee2 100644 --- a/docs/membership-fee-architecture.md +++ b/docs/membership-fee-architecture.md @@ -153,8 +153,8 @@ lib/ **Existing Fields Used:** -- `joined_at` - For calculating membership fee start -- `left_at` - For limiting cycle generation +- `join_date` - For calculating membership fee start +- `exit_date` - For limiting cycle generation - These fields must remain member fields and should not be replaced by custom fields in the future ### Settings Integration @@ -186,8 +186,9 @@ lib/ - Calculate which cycles should exist for a member - Generate missing cycles -- Respect membership_fee_start_date and left_at boundaries +- Respect membership_fee_start_date and exit_date boundaries - Skip existing cycles (idempotent) +- Use PostgreSQL advisory locks per member to prevent race conditions **Triggers:** @@ -199,17 +200,20 @@ lib/ **Algorithm Steps:** 1. Retrieve member with membership fee type and dates -2. Determine first cycle start (based on membership_fee_start_date) -3. Calculate all cycle starts from first to today (or left_at) -4. Query existing cycles for member -5. Generate missing cycles with current membership fee type's amount -6. Insert new cycles (batch operation) +2. Determine generation start point: + - If NO cycles exist: Start from `membership_fee_start_date` (or calculated from `join_date`) + - If cycles exist: Start from the cycle AFTER the last existing one +3. Generate all cycle starts from the determined start point to today (or `exit_date`) +4. Create new cycles with current membership fee type's amount +5. Use PostgreSQL advisory locks per member to prevent race conditions **Edge Case Handling:** -- If membership_fee_start_date is NULL: Calculate from joined_at + global setting -- If left_at is set: Stop generation at left_at +- If membership_fee_start_date is NULL: Calculate from join_date + global setting +- If exit_date is set: Stop generation at exit_date - If membership fee type changes: Handled separately by regeneration logic +- **Gap Handling:** If cycles were explicitly deleted (gaps exist), they are NOT recreated. + The generator always continues from the cycle AFTER the last existing cycle, regardless of gaps. ### Calendar Cycle Calculations @@ -381,7 +385,7 @@ lib/ **AC-M-1:** Member has membership_fee_type_id field (NOT NULL with default) **AC-M-2:** Member has membership_fee_start_date field (nullable) **AC-M-3:** New members get default membership fee type from global setting -**AC-M-4:** membership_fee_start_date auto-set based on joined_at and global setting +**AC-M-4:** membership_fee_start_date auto-set based on join_date and global setting **AC-M-5:** Admin can manually override membership_fee_start_date **AC-M-6:** Cannot change to membership fee type with different interval (MVP) @@ -391,7 +395,7 @@ lib/ **AC-CG-2:** Cycles generated when member created (via change hook) **AC-CG-3:** Scheduled job generates missing cycles daily **AC-CG-4:** Generation respects membership_fee_start_date -**AC-CG-5:** Generation stops at left_at if member exited +**AC-CG-5:** Generation stops at exit_date if member exited **AC-CG-6:** Generation is idempotent (skips existing cycles) **AC-CG-7:** Cycles align to calendar boundaries (1st of month/quarter/half/year) **AC-CG-8:** Amount comes from membership_fee_type at generation time @@ -472,8 +476,9 @@ lib/ - Correct cycle_start calculation for all interval types - Correct cycle count from start to end date - Respects membership_fee_start_date boundary -- Respects left_at boundary +- Respects exit_date boundary - Skips existing cycles (idempotent) +- Does not fill gaps when cycles were deleted - Handles edge dates (year boundaries, leap years) **Calendar Cycles Tests:** diff --git a/docs/membership-fee-overview.md b/docs/membership-fee-overview.md index 229b73b..bd47faa 100644 --- a/docs/membership-fee-overview.md +++ b/docs/membership-fee-overview.md @@ -120,7 +120,7 @@ This document provides a comprehensive overview of the Membership Fees system. I ``` - membership_fee_type_id (FK → membership_fee_types.id, NOT NULL, default from settings) - membership_fee_start_date (Date, nullable) - When to start generating membership fees -- left_at (Date, nullable) - Exit date (existing) +- exit_date (Date, nullable) - Exit date (existing) ``` **Logic for membership_fee_start_date:** @@ -167,16 +167,17 @@ value: UUID (Required) - Default membership fee type for new members **Algorithm:** -Lock the whole cycle table for the duration of the algorithm +Use PostgreSQL advisory locks per member to prevent race conditions 1. Get `member.membership_fee_start_date` and member's membership fee type -2. Generate cycles until today (or `left_at` if present): - - If no cycle exists: - - Generate all cycles from `membership_fee_start_date` - - else: - - Generate all cycles from last existing cycle - - use the interval to generate the cycles -3. Set `amount` to current membership fee type's amount +2. Determine generation start point: + - If NO cycles exist: Start from `membership_fee_start_date` + - If cycles exist: Start from the cycle AFTER the last existing one +3. Generate cycles until today (or `exit_date` if present): + - Use the interval to generate the cycles + - **Note:** If cycles were explicitly deleted (gaps exist), they are NOT recreated. + The generator always continues from the cycle AFTER the last existing cycle. +4. Set `amount` to current membership fee type's amount **Example (Yearly):** @@ -246,7 +247,7 @@ suspended → unpaid **Logic:** -- Cycles only generated until `member.left_at` +- Cycles only generated until `member.exit_date` - Existing cycles remain visible - Unpaid exit cycle can be marked as "suspended" diff --git a/lib/membership_fees/changes/set_membership_fee_start_date.ex b/lib/membership_fees/changes/set_membership_fee_start_date.ex index c274781..a2e1ad0 100644 --- a/lib/membership_fees/changes/set_membership_fee_start_date.ex +++ b/lib/membership_fees/changes/set_membership_fee_start_date.ex @@ -64,13 +64,26 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do start_date = calculate_start_date(join_date, interval, include_joining_cycle) Ash.Changeset.force_change_attribute(changeset, :membership_fee_start_date, start_date) else - {:error, reason} -> - # Log warning for debugging purposes, but don't fail the action - # Missing join_date or membership_fee_type_id is expected for partial creates - unless reason in [:join_date_not_set, :membership_fee_type_not_set] do - Logger.warning("Could not auto-set membership_fee_start_date: #{inspect(reason)}") - end + {:error, :join_date_not_set} -> + # Missing join_date is expected for partial creates + changeset + {:error, :membership_fee_type_not_set} -> + # Missing membership_fee_type_id is expected for partial creates + changeset + + {:error, :membership_fee_type_not_found} -> + # This is a data integrity error - membership_fee_type_id references non-existent type + # Return changeset error to fail the action + Ash.Changeset.add_error( + changeset, + field: :membership_fee_type_id, + message: "not found" + ) + + {:error, reason} -> + # Log warning for other unexpected errors + Logger.warning("Could not auto-set membership_fee_start_date: #{inspect(reason)}") changeset end end diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 2162b9e..0727a62 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -371,19 +371,20 @@ defmodule Mv.MembershipFees.CycleGenerator do end) {successes, errors} = Enum.split_with(results, &match?({:ok, _, _}, &1)) - successful_cycles = Enum.map(successes, fn {:ok, cycle, _notifications} -> cycle end) all_notifications = Enum.flat_map(successes, fn {:ok, _cycle, notifications} -> notifications end) if Enum.empty?(errors) do + successful_cycles = Enum.map(successes, fn {:ok, cycle, _notifications} -> cycle end) # Return cycles and notifications to be sent after transaction commits {:ok, successful_cycles, all_notifications} else Logger.warning("Some cycles failed to create: #{inspect(errors)}") - # Return partial failure with both successful and failed cycles - # This allows callers to decide how to handle partial failures - {:error, {:partial_failure, successful_cycles, errors}} + # Return partial failure with errors + # Note: When this error occurs, the transaction will be rolled back, + # so no cycles were actually persisted in the database + {:error, {:partial_failure, errors}} end end end diff --git a/test/membership_fees/member_cycle_integration_test.exs b/test/membership_fees/member_cycle_integration_test.exs index acd68a6..7cbfbff 100644 --- a/test/membership_fees/member_cycle_integration_test.exs +++ b/test/membership_fees/member_cycle_integration_test.exs @@ -58,9 +58,6 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do }) |> Ash.create!() - # Wait for async cycle generation - Process.sleep(300) - cycles = get_member_cycles(member.id) # Should have cycles for 2023 and 2024 (and possibly current year) @@ -89,9 +86,6 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do }) |> Ash.create!() - # Wait for potential async cycle generation - Process.sleep(200) - cycles = get_member_cycles(member.id) assert cycles == [] @@ -112,9 +106,6 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do }) |> Ash.create!() - # Wait for potential async cycle generation - Process.sleep(200) - cycles = get_member_cycles(member.id) assert cycles == [] @@ -145,9 +136,6 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: fee_type.id}) |> Ash.update!() - # Wait for async cycle generation - Process.sleep(300) - cycles = get_member_cycles(member.id) # Should have generated cycles @@ -178,9 +166,6 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do members = Enum.map(tasks, &Task.await/1) - # Wait for all async cycle generations - Process.sleep(500) - # Each member should have cycles Enum.each(members, fn member -> cycles = get_member_cycles(member.id) @@ -205,9 +190,6 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do }) |> Ash.create!() - # Wait for async cycle generation - Process.sleep(300) - initial_cycles = get_member_cycles(member.id) initial_count = length(initial_cycles)