refactor: improve cycle generation code quality and documentation

- Remove Process.sleep calls from integration tests (tests run synchronously in SQL sandbox)
- Improve error handling: membership_fee_type_not_found now returns changeset error instead of just logging
- Clarify partial_failure documentation: successful_cycles are not persisted on rollback
- Update documentation: joined_at → join_date, left_at → exit_date
- Document PostgreSQL advisory locks per member (not whole table lock)
- Document gap handling: explicitly deleted cycles are not recreated

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:**

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"
 

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

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

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)