Merge pull request 'Cycle Management & Member Integration closes #279' (#294) from feature/279_cycle_management into main

Reviewed-on: #294

docs/membership-fee-architecture.md

@@ -282,8 +282,14 @@ lib/
 **Implementation Pattern:**
 
 - Use Ash change module to validate
-- Use after_action hook to trigger regeneration
+- Use after_action hook to trigger regeneration synchronously
-- Use transaction to ensure atomicity
+- Regeneration runs in the same transaction as the member update to ensure atomicity
+- CycleGenerator uses advisory locks and transactions internally to prevent race conditions
+
+**Validation Behavior:**
+
+- Fail-closed: If membership fee types cannot be loaded during validation, the change is rejected with a validation error
+- Nil assignment prevention: Attempts to set membership_fee_type_id to nil are rejected when a current type exists
 
 ---
 
@@ -417,7 +423,7 @@ lib/
 **AC-TC-3:** On allowed change: future unpaid cycles regenerated  
 **AC-TC-4:** On allowed change: paid/suspended cycles unchanged  
 **AC-TC-5:** On allowed change: amount updated to new type's amount  
-**AC-TC-6:** Change is atomic (transaction)
+**AC-TC-6:** Change is atomic (transaction) - ✅ Implemented: Regeneration runs synchronously in the same transaction as the member update
 
 ### Settings
 

lib/membership/member.ex

@@ -112,10 +112,17 @@ defmodule Mv.Membership.Member do
       # but in test environment it runs synchronously for DB sandbox compatibility
       change after_action(fn _changeset, member, _context ->
                if member.membership_fee_type_id && member.join_date do
-                 generate_fn = fn ->
+                 if Application.get_env(:mv, :sql_sandbox, false) do
-                   case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id) do
+                   # Run synchronously in test environment for DB sandbox compatibility
-                     {:ok, _cycles} ->
+                   # Use skip_lock?: true to avoid nested transactions (after_action runs within action transaction)
-                       :ok
+                   # Return notifications to Ash so they are sent after commit
+                   case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(
+                          member.id,
+                          today: Date.utc_today(),
+                          skip_lock?: true
+                        ) do
+                     {:ok, _cycles, notifications} ->
+                       {:ok, member, notifications}
 
                      {:error, reason} ->
                        require Logger
@@ -123,19 +130,34 @@ defmodule Mv.Membership.Member do
                        Logger.warning(
                          "Failed to generate cycles for member #{member.id}: #{inspect(reason)}"
                        )
-                   end
-                 end
 
-                 if Application.get_env(:mv, :sql_sandbox, false) do
+                       {:ok, member}
-                   # Run synchronously in test environment for DB sandbox compatibility
+                   end
-                   generate_fn.()
                  else
                    # Run asynchronously in other environments
-                   Task.start(generate_fn)
+                   # Send notifications explicitly since they cannot be returned via after_action
-                 end
+                   Task.start(fn ->
-               end
+                     case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id) do
+                       {:ok, _cycles, notifications} ->
+                         # Send notifications manually for async case
+                         if Enum.any?(notifications) do
+                           Ash.Notifier.notify(notifications)
+                         end
 
-               {:ok, member}
+                       {:error, reason} ->
+                         require Logger
+
+                         Logger.warning(
+                           "Failed to generate cycles for member #{member.id}: #{inspect(reason)}"
+                         )
+                     end
+                   end)
+
+                   {:ok, member}
+                 end
+               else
+                 {:ok, member}
+               end
              end)
     end
 
@@ -178,44 +200,44 @@ defmodule Mv.Membership.Member do
         where [changing(:user)]
       end
 
+      # Validate that membership fee type changes only allow same-interval types
+      change Mv.MembershipFees.Changes.ValidateSameInterval do
+        where [changing(:membership_fee_type_id)]
+      end
+
       # Auto-calculate membership_fee_start_date when membership_fee_type_id is set
       # and membership_fee_start_date is not already set
       change Mv.MembershipFees.Changes.SetMembershipFeeStartDate do
         where [changing(:membership_fee_type_id)]
       end
 
-      # Trigger cycle generation when membership_fee_type_id changes
+      # Trigger cycle regeneration when membership_fee_type_id changes
-      # Note: Cycle generation runs asynchronously to not block the action,
+      # This deletes future unpaid cycles and regenerates them with the new type/amount
-      # but in test environment it runs synchronously for DB sandbox compatibility
+      # Note: Cycle regeneration runs synchronously in the same transaction to ensure atomicity
+      # CycleGenerator uses advisory locks and transactions internally to prevent race conditions
+      # Notifications are returned to Ash and sent automatically after commit
       change after_action(fn changeset, member, _context ->
                fee_type_changed =
                  Ash.Changeset.changing_attribute?(changeset, :membership_fee_type_id)
 
                if fee_type_changed && member.membership_fee_type_id && member.join_date do
-                 generate_fn = fn ->
+                 case regenerate_cycles_on_type_change(member) do
-                   case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id) do
+                   {:ok, notifications} ->
-                     {:ok, _cycles} ->
+                     # Return notifications to Ash - they will be sent automatically after commit
-                       :ok
+                     {:ok, member, notifications}
 
-                     {:error, reason} ->
+                   {:error, reason} ->
-                       require Logger
+                     require Logger
 
-                       Logger.warning(
+                     Logger.warning(
-                         "Failed to generate cycles for member #{member.id}: #{inspect(reason)}"
+                       "Failed to regenerate cycles for member #{member.id}: #{inspect(reason)}"
-                       )
+                     )
-                   end
+
-                 end
+                     {:ok, member}
-
-                 if Application.get_env(:mv, :sql_sandbox, false) do
-                   # Run synchronously in test environment for DB sandbox compatibility
-                   generate_fn.()
-                 else
-                   # Run asynchronously in other environments
-                   Task.start(generate_fn)
                  end
+               else
+                 {:ok, member}
                end
-
-               {:ok, member}
              end)
     end
 
@@ -501,6 +523,50 @@ defmodule Mv.Membership.Member do
     has_many :membership_fee_cycles, Mv.MembershipFees.MembershipFeeCycle
   end
 
+  calculations do
+    calculate :current_cycle_status, :atom do
+      description "Status of the current cycle (the one that is active today)"
+      # Automatically load cycles with all attributes and membership_fee_type
+      load membership_fee_cycles: [:cycle_start, :status, membership_fee_type: [:interval]]
+
+      calculation fn [member], _context ->
+        case get_current_cycle(member) do
+          nil -> [nil]
+          cycle -> [cycle.status]
+        end
+      end
+
+      constraints one_of: [:unpaid, :paid, :suspended]
+    end
+
+    calculate :last_cycle_status, :atom do
+      description "Status of the last completed cycle (the most recent cycle that has ended)"
+      # Automatically load cycles with all attributes and membership_fee_type
+      load membership_fee_cycles: [:cycle_start, :status, membership_fee_type: [:interval]]
+
+      calculation fn [member], _context ->
+        case get_last_completed_cycle(member) do
+          nil -> [nil]
+          cycle -> [cycle.status]
+        end
+      end
+
+      constraints one_of: [:unpaid, :paid, :suspended]
+    end
+
+    calculate :overdue_count, :integer do
+      description "Count of unpaid cycles that have already ended (cycle_end < today)"
+      # Automatically load cycles with all attributes and membership_fee_type
+      load membership_fee_cycles: [:cycle_start, :status, membership_fee_type: [:interval]]
+
+      calculation fn [member], _context ->
+        overdue = get_overdue_cycles(member)
+        count = if is_list(overdue), do: length(overdue), else: 0
+        [count]
+      end
+    end
+  end
+
   # Define identities for upsert operations
   identities do
     identity :unique_email, [:email]
@@ -547,6 +613,261 @@ defmodule Mv.Membership.Member do
 
   def show_in_overview?(_), do: true
 
+  # Helper functions for cycle status calculations
+  #
+  # These functions expect membership_fee_cycles to be loaded with membership_fee_type
+  # preloaded. The calculations explicitly load this relationship, but if called
+  # directly, ensure membership_fee_type is loaded or the functions will return
+  # nil/[] when membership_fee_type is missing.
+
+  @doc false
+  @spec get_current_cycle(Member.t()) :: MembershipFeeCycle.t() | nil
+  def get_current_cycle(member) do
+    today = Date.utc_today()
+
+    # Check if cycles are already loaded
+    cycles = Map.get(member, :membership_fee_cycles)
+
+    if is_list(cycles) and cycles != [] do
+      Enum.find(cycles, &current_cycle?(&1, today))
+    else
+      nil
+    end
+  end
+
+  # Checks if a cycle is the current cycle (active today)
+  defp current_cycle?(cycle, today) do
+    case Map.get(cycle, :membership_fee_type) do
+      %{interval: interval} ->
+        cycle_end =
+          Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval)
+
+        Date.compare(cycle.cycle_start, today) in [:lt, :eq] and
+          Date.compare(today, cycle_end) in [:lt, :eq]
+
+      _ ->
+        false
+    end
+  end
+
+  @doc false
+  @spec get_last_completed_cycle(Member.t()) :: MembershipFeeCycle.t() | nil
+  def get_last_completed_cycle(member) do
+    today = Date.utc_today()
+
+    # Check if cycles are already loaded
+    cycles = Map.get(member, :membership_fee_cycles)
+
+    if is_list(cycles) and cycles != [] do
+      cycles
+      |> filter_completed_cycles(today)
+      |> sort_cycles_by_end_date()
+      |> List.first()
+    else
+      nil
+    end
+  end
+
+  # Filters cycles that have ended (cycle_end < today)
+  defp filter_completed_cycles(cycles, today) do
+    Enum.filter(cycles, fn cycle ->
+      case Map.get(cycle, :membership_fee_type) do
+        %{interval: interval} ->
+          cycle_end =
+            Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval)
+
+          Date.compare(today, cycle_end) == :gt
+
+        _ ->
+          false
+      end
+    end)
+  end
+
+  # Sorts cycles by end date in descending order
+  defp sort_cycles_by_end_date(cycles) do
+    Enum.sort_by(
+      cycles,
+      fn cycle ->
+        interval = Map.get(cycle, :membership_fee_type).interval
+        Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval)
+      end,
+      {:desc, Date}
+    )
+  end
+
+  @doc false
+  @spec get_overdue_cycles(Member.t()) :: [MembershipFeeCycle.t()]
+  def get_overdue_cycles(member) do
+    today = Date.utc_today()
+
+    # Check if cycles are already loaded
+    cycles = Map.get(member, :membership_fee_cycles)
+
+    if is_list(cycles) and cycles != [] do
+      filter_overdue_cycles(cycles, today)
+    else
+      []
+    end
+  end
+
+  # Filters cycles that are unpaid and have ended (cycle_end < today)
+  defp filter_overdue_cycles(cycles, today) do
+    Enum.filter(cycles, fn cycle ->
+      case Map.get(cycle, :membership_fee_type) do
+        %{interval: interval} ->
+          cycle_end =
+            Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval)
+
+          cycle.status == :unpaid and Date.compare(today, cycle_end) == :gt
+
+        _ ->
+          false
+      end
+    end)
+  end
+
+  # Regenerates cycles when membership fee type changes
+  # Deletes future unpaid cycles and regenerates them with the new type/amount
+  # Uses advisory lock to prevent concurrent modifications
+  # Returns {:ok, notifications} or {:error, reason} where notifications are collected
+  # to be sent after transaction commits
+  @doc false
+  def regenerate_cycles_on_type_change(member) do
+    today = Date.utc_today()
+    lock_key = :erlang.phash2(member.id)
+
+    # Use advisory lock to prevent concurrent deletion and regeneration
+    # This ensures atomicity when multiple updates happen simultaneously
+    if Mv.Repo.in_transaction?() do
+      regenerate_cycles_in_transaction(member, today, lock_key)
+    else
+      regenerate_cycles_new_transaction(member, today, lock_key)
+    end
+  end
+
+  # Already in transaction: use advisory lock directly
+  # Returns {:ok, notifications} - notifications should be returned to after_action hook
+  defp regenerate_cycles_in_transaction(member, today, lock_key) do
+    Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
+    do_regenerate_cycles_on_type_change(member, today, skip_lock?: true)
+  end
+
+  # Not in transaction: start new transaction with advisory lock
+  # Returns {:ok, notifications} - notifications should be sent by caller (e.g., via after_action)
+  defp regenerate_cycles_new_transaction(member, today, lock_key) do
+    Mv.Repo.transaction(fn ->
+      Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
+
+      case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) do
+        {:ok, notifications} ->
+          # Return notifications - they will be sent by the caller
+          notifications
+
+        {:error, reason} ->
+          Mv.Repo.rollback(reason)
+      end
+    end)
+    |> case do
+      {:ok, notifications} -> {:ok, notifications}
+      {:error, reason} -> {:error, reason}
+    end
+  end
+
+  # Performs the actual cycle deletion and regeneration
+  # Returns {:ok, notifications} or {:error, reason}
+  # notifications are collected to be sent after transaction commits
+  defp do_regenerate_cycles_on_type_change(member, today, opts) do
+    require Ash.Query
+
+    skip_lock? = Keyword.get(opts, :skip_lock?, false)
+
+    # Find all unpaid cycles for this member
+    # We need to check cycle_end for each cycle using its own interval
+    all_unpaid_cycles_query =
+      Mv.MembershipFees.MembershipFeeCycle
+      |> Ash.Query.filter(member_id == ^member.id)
+      |> Ash.Query.filter(status == :unpaid)
+      |> Ash.Query.load([:membership_fee_type])
+
+    case Ash.read(all_unpaid_cycles_query) do
+      {:ok, all_unpaid_cycles} ->
+        cycles_to_delete = filter_future_cycles(all_unpaid_cycles, today)
+        delete_and_regenerate_cycles(cycles_to_delete, member.id, today, skip_lock?: skip_lock?)
+
+      {:error, reason} ->
+        {:error, reason}
+    end
+  end
+
+  # Filters cycles that haven't ended yet (cycle_end >= today)
+  # These are the "future" cycles that should be regenerated
+  defp filter_future_cycles(all_unpaid_cycles, today) do
+    Enum.filter(all_unpaid_cycles, fn cycle ->
+      case cycle.membership_fee_type do
+        %{interval: interval} ->
+          cycle_end =
+            Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval)
+
+          Date.compare(today, cycle_end) in [:lt, :eq]
+
+        _ ->
+          false
+      end
+    end)
+  end
+
+  # Deletes future cycles and regenerates them with the new type/amount
+  # Passes today to ensure consistent date across deletion and regeneration
+  # Returns {:ok, notifications} or {:error, reason}
+  defp delete_and_regenerate_cycles(cycles_to_delete, member_id, today, opts) do
+    skip_lock? = Keyword.get(opts, :skip_lock?, false)
+
+    if Enum.empty?(cycles_to_delete) do
+      # No cycles to delete, just regenerate
+      regenerate_cycles(member_id, today, skip_lock?: skip_lock?)
+    else
+      case delete_cycles(cycles_to_delete) do
+        :ok -> regenerate_cycles(member_id, today, skip_lock?: skip_lock?)
+        {:error, reason} -> {:error, reason}
+      end
+    end
+  end
+
+  # Deletes cycles and returns :ok if all succeeded, {:error, reason} otherwise
+  defp delete_cycles(cycles_to_delete) do
+    delete_results =
+      Enum.map(cycles_to_delete, fn cycle ->
+        Ash.destroy(cycle)
+      end)
+
+    if Enum.any?(delete_results, &match?({:error, _}, &1)) do
+      {:error, :deletion_failed}
+    else
+      :ok
+    end
+  end
+
+  # Regenerates cycles with new type/amount
+  # Passes today to ensure consistent date across deletion and regeneration
+  # skip_lock?: true means advisory lock is already set by caller
+  # Returns {:ok, notifications} - notifications should be returned to after_action hook
+  defp regenerate_cycles(member_id, today, opts) do
+    skip_lock? = Keyword.get(opts, :skip_lock?, false)
+
+    case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(
+           member_id,
+           today: today,
+           skip_lock?: skip_lock?
+         ) do
+      {:ok, _cycles, notifications} when is_list(notifications) ->
+        {:ok, notifications}
+
+      {:error, reason} ->
+        {:error, reason}
+    end
+  end
+
   # Normalizes visibility config map keys from strings to atoms.
   # JSONB in PostgreSQL converts atom keys to string keys when storing.
   defp normalize_visibility_config(config) when is_map(config) do

lib/membership_fees/changes/validate_same_interval.ex

@@ -0,0 +1,148 @@
+defmodule Mv.MembershipFees.Changes.ValidateSameInterval do
+  @moduledoc """
+  Validates that membership fee type changes only allow same-interval types.
+
+  Prevents changing from yearly to monthly, etc. (MVP constraint).
+
+  ## Usage
+
+  In a Member action:
+
+      update :update_member do
+        # ...
+        change Mv.MembershipFees.Changes.ValidateSameInterval
+      end
+
+  The change module only executes when `membership_fee_type_id` is being changed.
+  If the new type has a different interval than the current type, a validation error is returned.
+  """
+  use Ash.Resource.Change
+
+  @impl true
+  def change(changeset, _opts, _context) do
+    if changing_membership_fee_type?(changeset) do
+      validate_interval_match(changeset)
+    else
+      changeset
+    end
+  end
+
+  # Check if membership_fee_type_id is being changed
+  defp changing_membership_fee_type?(changeset) do
+    Ash.Changeset.changing_attribute?(changeset, :membership_fee_type_id)
+  end
+
+  # Validate that the new type has the same interval as the current type
+  defp validate_interval_match(changeset) do
+    current_type_id = get_current_type_id(changeset)
+    new_type_id = get_new_type_id(changeset)
+
+    cond do
+      # If no current type, allow any change (first assignment)
+      is_nil(current_type_id) ->
+        changeset
+
+      # If new type is nil, reject the change (membership_fee_type_id is required)
+      is_nil(new_type_id) ->
+        add_nil_type_error(changeset)
+
+      # Both types exist - validate intervals match
+      true ->
+        validate_intervals_match(changeset, current_type_id, new_type_id)
+    end
+  end
+
+  # Validates that intervals match when both types exist
+  defp validate_intervals_match(changeset, current_type_id, new_type_id) do
+    case get_intervals(current_type_id, new_type_id) do
+      {:ok, current_interval, new_interval} ->
+        if current_interval == new_interval do
+          changeset
+        else
+          add_interval_mismatch_error(changeset, current_interval, new_interval)
+        end
+
+      {:error, reason} ->
+        # Fail closed: If we can't load the types, reject the change
+        # This prevents inconsistent data states
+        add_type_validation_error(changeset, reason)
+    end
+  end
+
+  # Get current type ID from changeset data
+  defp get_current_type_id(changeset) do
+    case changeset.data do
+      %{membership_fee_type_id: type_id} -> type_id
+      _ -> nil
+    end
+  end
+
+  # Get new type ID from changeset
+  defp get_new_type_id(changeset) do
+    case Ash.Changeset.fetch_change(changeset, :membership_fee_type_id) do
+      {:ok, type_id} -> type_id
+      :error -> nil
+    end
+  end
+
+  # Get intervals for both types
+  defp get_intervals(current_type_id, new_type_id) do
+    alias Mv.MembershipFees.MembershipFeeType
+
+    case {Ash.get(MembershipFeeType, current_type_id), Ash.get(MembershipFeeType, new_type_id)} do
+      {{:ok, current_type}, {:ok, new_type}} ->
+        {:ok, current_type.interval, new_type.interval}
+
+      _ ->
+        {:error, :type_not_found}
+    end
+  end
+
+  # Add validation error for interval mismatch
+  defp add_interval_mismatch_error(changeset, current_interval, new_interval) do
+    current_interval_name = format_interval(current_interval)
+    new_interval_name = format_interval(new_interval)
+
+    message =
+      "Cannot change membership fee type: current type uses #{current_interval_name} interval, " <>
+        "new type uses #{new_interval_name} interval. Only same-interval changes are allowed."
+
+    Ash.Changeset.add_error(
+      changeset,
+      field: :membership_fee_type_id,
+      message: message
+    )
+  end
+
+  # Add validation error when types cannot be loaded
+  defp add_type_validation_error(changeset, _reason) do
+    message =
+      "Could not validate membership fee type intervals. " <>
+        "The current or new membership fee type no longer exists. " <>
+        "This may indicate a data consistency issue."
+
+    Ash.Changeset.add_error(
+      changeset,
+      field: :membership_fee_type_id,
+      message: message
+    )
+  end
+
+  # Add validation error when trying to set membership_fee_type_id to nil
+  defp add_nil_type_error(changeset) do
+    message = "Cannot remove membership fee type. A membership fee type is required."
+
+    Ash.Changeset.add_error(
+      changeset,
+      field: :membership_fee_type_id,
+      message: message
+    )
+  end
+
+  # Format interval atom to human-readable string
+  defp format_interval(:monthly), do: "monthly"
+  defp format_interval(:quarterly), do: "quarterly"
+  defp format_interval(:half_yearly), do: "half-yearly"
+  defp format_interval(:yearly), do: "yearly"
+  defp format_interval(interval), do: to_string(interval)
+end

lib/membership_fees/membership_fee_cycle.ex

@@ -51,6 +51,36 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do
       primary? true
       accept [:status, :notes]
     end
+
+    update :mark_as_paid do
+      description "Mark cycle as paid"
+      require_atomic? false
+      accept [:notes]
+
+      change fn changeset, _context ->
+        Ash.Changeset.force_change_attribute(changeset, :status, :paid)
+      end
+    end
+
+    update :mark_as_suspended do
+      description "Mark cycle as suspended"
+      require_atomic? false
+      accept [:notes]
+
+      change fn changeset, _context ->
+        Ash.Changeset.force_change_attribute(changeset, :status, :suspended)
+      end
+    end
+
+    update :mark_as_unpaid do
+      description "Mark cycle as unpaid (for error correction)"
+      require_atomic? false
+      accept [:notes]
+
+      change fn changeset, _context ->
+        Ash.Changeset.force_change_attribute(changeset, :status, :unpaid)
+      end
+    end
   end
 
   attributes do

lib/mv/membership_fees/cycle_generator.ex

@@ -38,16 +38,17 @@ defmodule Mv.MembershipFees.CycleGenerator do
 
   """
 
-  alias Mv.MembershipFees.CalendarCycles
-  alias Mv.MembershipFees.MembershipFeeCycle
-  alias Mv.MembershipFees.Changes.SetMembershipFeeStartDate
   alias Mv.Membership.Member
+  alias Mv.MembershipFees.CalendarCycles
+  alias Mv.MembershipFees.Changes.SetMembershipFeeStartDate
+  alias Mv.MembershipFees.MembershipFeeCycle
   alias Mv.Repo
 
   require Ash.Query
   require Logger
 
-  @type generate_result :: {:ok, [MembershipFeeCycle.t()]} | {:error, term()}
+  @type generate_result ::
+          {:ok, [MembershipFeeCycle.t()], [Ash.Notifier.Notification.t()]} | {:error, term()}
 
   @doc """
   Generates membership fee cycles for a single member.
@@ -62,14 +63,14 @@ defmodule Mv.MembershipFees.CycleGenerator do
 
   ## Returns
 
-    - `{:ok, cycles}` - List of newly created cycles
+    - `{:ok, cycles, notifications}` - List of newly created cycles and notifications
     - `{:error, reason}` - Error with reason
 
   ## Examples
 
-      {:ok, cycles} = CycleGenerator.generate_cycles_for_member(member)
+      {:ok, cycles, notifications} = CycleGenerator.generate_cycles_for_member(member)
-      {:ok, cycles} = CycleGenerator.generate_cycles_for_member(member_id)
+      {:ok, cycles, notifications} = CycleGenerator.generate_cycles_for_member(member_id)
-      {:ok, cycles} = CycleGenerator.generate_cycles_for_member(member, today: ~D[2024-12-31])
+      {:ok, cycles, notifications} = CycleGenerator.generate_cycles_for_member(member, today: ~D[2024-12-31])
 
   """
   @spec generate_cycles_for_member(Member.t() | String.t(), keyword()) :: generate_result()
@@ -84,12 +85,40 @@ defmodule Mv.MembershipFees.CycleGenerator do
 
   def generate_cycles_for_member(%Member{} = member, opts) do
     today = Keyword.get(opts, :today, Date.utc_today())
+    skip_lock? = Keyword.get(opts, :skip_lock?, false)
 
-    # Use advisory lock to prevent concurrent generation
+    do_generate_cycles_with_lock(member, today, skip_lock?)
-    # Notifications are handled inside with_advisory_lock after transaction commits
+  end
-    with_advisory_lock(member.id, fn ->
+
-      do_generate_cycles(member, today)
+  # Generate cycles with lock handling
+  # Returns {:ok, cycles, notifications} - notifications are never sent here,
+  # they should be returned to the caller (e.g., via after_action hook)
+  defp do_generate_cycles_with_lock(member, today, true = _skip_lock?) do
+    # Lock already set by caller (e.g., regenerate_cycles_on_type_change)
+    # Just generate cycles without additional locking
+    do_generate_cycles(member, today)
+  end
+
+  defp do_generate_cycles_with_lock(member, today, false) do
+    lock_key = :erlang.phash2(member.id)
+
+    Repo.transaction(fn ->
+      Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
+
+      case do_generate_cycles(member, today) do
+        {:ok, cycles, notifications} ->
+          # Return cycles and notifications - do NOT send notifications here
+          # They will be sent by the caller (e.g., via after_action hook)
+          {cycles, notifications}
+
+        {:error, reason} ->
+          Repo.rollback(reason)
+      end
     end)
+    |> case do
+      {:ok, {cycles, notifications}} -> {:ok, cycles, notifications}
+      {:error, reason} -> {:error, reason}
+    end
   end
 
   @doc """
@@ -150,7 +179,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
   defp process_batch(batch, today) do
     batch
     |> Task.async_stream(fn member ->
-      {member.id, generate_cycles_for_member(member, today: today)}
+      process_member_cycle_generation(member, today)
     end)
     |> Enum.map(fn
       {:ok, result} ->
@@ -163,8 +192,29 @@ defmodule Mv.MembershipFees.CycleGenerator do
     end)
   end
 
+  # Process cycle generation for a single member in batch job
+  # Returns {member_id, result} tuple where result is {:ok, cycles, notifications} or {:error, reason}
+  defp process_member_cycle_generation(member, today) do
+    case generate_cycles_for_member(member, today: today) do
+      {:ok, _cycles, notifications} = ok ->
+        send_notifications_for_batch_job(notifications)
+        {member.id, ok}
+
+      {:error, _reason} = err ->
+        {member.id, err}
+    end
+  end
+
+  # Send notifications for batch job
+  # This is a top-level job, so we need to send notifications explicitly
+  defp send_notifications_for_batch_job(notifications) do
+    if Enum.any?(notifications) do
+      Ash.Notifier.notify(notifications)
+    end
+  end
+
   defp build_results_summary(results) do
-    success_count = Enum.count(results, fn {_id, result} -> match?({:ok, _}, result) end)
+    success_count = Enum.count(results, fn {_id, result} -> match?({:ok, _, _}, result) end)
     failed_count = Enum.count(results, fn {_id, result} -> match?({:error, _}, result) end)
 
     %{success: success_count, failed: failed_count, total: length(results)}
@@ -184,43 +234,6 @@ defmodule Mv.MembershipFees.CycleGenerator do
     end
   end
 
-  defp with_advisory_lock(member_id, fun) do
-    # Convert UUID to integer for advisory lock (use hash)
-    lock_key = :erlang.phash2(member_id)
-
-    result =
-      Repo.transaction(fn ->
-        # Acquire advisory lock for this transaction
-        Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key])
-
-        case fun.() do
-          {:ok, result, notifications} when is_list(notifications) ->
-            # Return result and notifications separately
-            {result, notifications}
-
-          {:ok, result} ->
-            # Handle case where no notifications were returned (backward compatibility)
-            {result, []}
-
-          {:error, reason} ->
-            Repo.rollback(reason)
-        end
-      end)
-
-    # Extract result and notifications, send notifications after transaction
-    case result do
-      {:ok, {cycles, notifications}} ->
-        if Enum.any?(notifications) do
-          Ash.Notifier.notify(notifications)
-        end
-
-        {:ok, cycles}
-
-      {:error, reason} ->
-        {:error, reason}
-    end
-  end
-
   defp do_generate_cycles(member, today) do
     # Reload member with relationships to ensure fresh data
     case load_member(member.id) do
@@ -353,6 +366,9 @@ defmodule Mv.MembershipFees.CycleGenerator do
   end
 
   defp create_cycles(cycle_starts, member_id, fee_type_id, amount) do
+    # Always use return_notifications?: true to collect notifications
+    # Notifications will be returned to the caller, who is responsible for
+    # sending them (e.g., via after_action hook returning {:ok, result, notifications})
     results =
       Enum.map(cycle_starts, fn cycle_start ->
         attrs = %{
@@ -363,10 +379,15 @@ defmodule Mv.MembershipFees.CycleGenerator do
           status: :unpaid
         }
 
-        # Return notifications to avoid warnings when creating within a transaction
         case Ash.create(MembershipFeeCycle, attrs, return_notifications?: true) do
-          {:ok, cycle, notifications} -> {:ok, cycle, notifications}
+          {:ok, cycle, notifications} when is_list(notifications) ->
-          {:error, reason} -> {:error, {cycle_start, reason}}
+            {:ok, cycle, notifications}
+
+          {:ok, cycle} ->
+            {:ok, cycle, []}
+
+          {:error, reason} ->
+            {:error, {cycle_start, reason}}
         end
       end)
 
@@ -377,7 +398,6 @@ defmodule Mv.MembershipFees.CycleGenerator do
 
     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)}")

test/membership/member_cycle_calculations_test.exs

@@ -0,0 +1,360 @@
+defmodule Mv.Membership.MemberCycleCalculationsTest do
+  @moduledoc """
+  Tests for Member cycle status calculations.
+  """
+  use Mv.DataCase, async: true
+
+  alias Mv.Membership.Member
+  alias Mv.MembershipFees.MembershipFeeType
+  alias Mv.MembershipFees.MembershipFeeCycle
+  alias Mv.MembershipFees.CalendarCycles
+
+  # Helper to create a membership fee type
+  defp create_fee_type(attrs) 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!()
+  end
+
+  # Helper to create a member
+  defp create_member(attrs) do
+    default_attrs = %{
+      first_name: "Test",
+      last_name: "Member",
+      email: "test.member.#{System.unique_integer([:positive])}@example.com"
+    }
+
+    attrs = Map.merge(default_attrs, attrs)
+
+    Member
+    |> Ash.Changeset.for_create(:create_member, attrs)
+    |> Ash.create!()
+  end
+
+  # Helper to create a cycle
+  defp create_cycle(member, fee_type, attrs) 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!()
+  end
+
+  describe "current_cycle_status" do
+    test "returns status of current cycle for member with active cycle" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      # Create a cycle that is active today (2024-01-01 to 2024-12-31)
+      # Assuming today is in 2024
+      today = Date.utc_today()
+      cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: cycle_start,
+        status: :paid
+      })
+
+      member = Ash.load!(member, :current_cycle_status)
+      assert member.current_cycle_status == :paid
+    end
+
+    test "returns nil for member without current cycle" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      # Create a cycle in the past (not current)
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2020-01-01],
+        status: :paid
+      })
+
+      member = Ash.load!(member, :current_cycle_status)
+      assert member.current_cycle_status == nil
+    end
+
+    test "returns nil for member without cycles" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      member = Ash.load!(member, :current_cycle_status)
+      assert member.current_cycle_status == nil
+    end
+
+    test "returns status of current cycle for monthly interval" do
+      fee_type = create_fee_type(%{interval: :monthly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      # Create a cycle that is active today (current month)
+      today = Date.utc_today()
+      cycle_start = CalendarCycles.calculate_cycle_start(today, :monthly)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: cycle_start,
+        status: :unpaid
+      })
+
+      member = Ash.load!(member, :current_cycle_status)
+      assert member.current_cycle_status == :unpaid
+    end
+  end
+
+  describe "last_cycle_status" do
+    test "returns status of last completed cycle" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      # Create cycles: 2022 (completed), 2023 (completed), 2024 (current)
+      today = Date.utc_today()
+
+      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
+      })
+
+      # Current cycle
+      cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: cycle_start,
+        status: :paid
+      })
+
+      member = Ash.load!(member, :last_cycle_status)
+      # Should return status of 2023 (last completed)
+      assert member.last_cycle_status == :unpaid
+    end
+
+    test "returns nil for member without completed cycles" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      # Only create current cycle (not completed yet)
+      today = Date.utc_today()
+      cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: cycle_start,
+        status: :paid
+      })
+
+      member = Ash.load!(member, :last_cycle_status)
+      assert member.last_cycle_status == nil
+    end
+
+    test "returns nil for member without cycles" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      member = Ash.load!(member, :last_cycle_status)
+      assert member.last_cycle_status == nil
+    end
+
+    test "returns status of last completed cycle for monthly interval" do
+      fee_type = create_fee_type(%{interval: :monthly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      today = Date.utc_today()
+      # Create cycles: last month (completed), current month (not completed)
+      last_month_start = Date.add(today, -32) |> CalendarCycles.calculate_cycle_start(:monthly)
+      current_month_start = CalendarCycles.calculate_cycle_start(today, :monthly)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: last_month_start,
+        status: :paid
+      })
+
+      create_cycle(member, fee_type, %{
+        cycle_start: current_month_start,
+        status: :unpaid
+      })
+
+      member = Ash.load!(member, :last_cycle_status)
+      # Should return status of last month (last completed)
+      assert member.last_cycle_status == :paid
+    end
+  end
+
+  describe "overdue_count" do
+    test "counts only unpaid cycles that have ended" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      today = Date.utc_today()
+
+      # Create cycles:
+      # 2022: unpaid, ended (overdue)
+      # 2023: paid, ended (not overdue)
+      # 2024: unpaid, current (not overdue)
+      # 2025: unpaid, future (not overdue)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2022-01-01],
+        status: :unpaid
+      })
+
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2023-01-01],
+        status: :paid
+      })
+
+      # Current cycle
+      cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: cycle_start,
+        status: :unpaid
+      })
+
+      # Future cycle (if we're not at the end of the year)
+      next_year = today.year + 1
+
+      if today.month < 12 or today.day < 31 do
+        next_year_start = Date.new!(next_year, 1, 1)
+
+        create_cycle(member, fee_type, %{
+          cycle_start: next_year_start,
+          status: :unpaid
+        })
+      end
+
+      member = Ash.load!(member, :overdue_count)
+      # Should only count 2022 (unpaid and ended)
+      assert member.overdue_count == 1
+    end
+
+    test "returns 0 when no overdue cycles" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      # Create only paid cycles
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2022-01-01],
+        status: :paid
+      })
+
+      member = Ash.load!(member, :overdue_count)
+      assert member.overdue_count == 0
+    end
+
+    test "returns 0 for member without cycles" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      member = Ash.load!(member, :overdue_count)
+      assert member.overdue_count == 0
+    end
+
+    test "counts overdue cycles for monthly interval" do
+      fee_type = create_fee_type(%{interval: :monthly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      today = Date.utc_today()
+
+      # Create cycles: two months ago (unpaid, ended), last month (paid, ended), current month (unpaid, not ended)
+      two_months_ago_start =
+        Date.add(today, -65) |> CalendarCycles.calculate_cycle_start(:monthly)
+
+      last_month_start = Date.add(today, -32) |> CalendarCycles.calculate_cycle_start(:monthly)
+      current_month_start = CalendarCycles.calculate_cycle_start(today, :monthly)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: two_months_ago_start,
+        status: :unpaid
+      })
+
+      create_cycle(member, fee_type, %{
+        cycle_start: last_month_start,
+        status: :paid
+      })
+
+      create_cycle(member, fee_type, %{
+        cycle_start: current_month_start,
+        status: :unpaid
+      })
+
+      member = Ash.load!(member, :overdue_count)
+      # Should only count two_months_ago (unpaid and ended)
+      assert member.overdue_count == 1
+    end
+
+    test "counts multiple overdue cycles" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      # Create multiple unpaid, ended cycles
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2020-01-01],
+        status: :unpaid
+      })
+
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2021-01-01],
+        status: :unpaid
+      })
+
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2022-01-01],
+        status: :unpaid
+      })
+
+      member = Ash.load!(member, :overdue_count)
+      assert member.overdue_count == 3
+    end
+  end
+
+  describe "calculations with multiple cycles" do
+    test "all calculations work correctly with multiple cycles" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      today = Date.utc_today()
+
+      # Create cycles: 2022 (unpaid, ended), 2023 (paid, ended), 2024 (unpaid, current)
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2022-01-01],
+        status: :unpaid
+      })
+
+      create_cycle(member, fee_type, %{
+        cycle_start: ~D[2023-01-01],
+        status: :paid
+      })
+
+      cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      create_cycle(member, fee_type, %{
+        cycle_start: cycle_start,
+        status: :unpaid
+      })
+
+      member =
+        Ash.load!(member, [:current_cycle_status, :last_cycle_status, :overdue_count])
+
+      assert member.current_cycle_status == :unpaid
+      assert member.last_cycle_status == :paid
+      assert member.overdue_count == 1
+    end
+  end
+end

test/membership/member_type_change_integration_test.exs

@@ -0,0 +1,453 @@
+defmodule Mv.Membership.MemberTypeChangeIntegrationTest do
+  @moduledoc """
+  Integration tests for membership fee type changes and cycle regeneration.
+  """
+  use Mv.DataCase, async: true
+
+  alias Mv.Membership.Member
+  alias Mv.MembershipFees.MembershipFeeType
+  alias Mv.MembershipFees.MembershipFeeCycle
+  alias Mv.MembershipFees.CalendarCycles
+
+  require Ash.Query
+
+  # Helper to create a membership fee type
+  defp create_fee_type(attrs) 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!()
+  end
+
+  # Helper to create a member
+  defp create_member(attrs) do
+    default_attrs = %{
+      first_name: "Test",
+      last_name: "Member",
+      email: "test.member.#{System.unique_integer([:positive])}@example.com",
+      join_date: ~D[2023-01-15]
+    }
+
+    attrs = Map.merge(default_attrs, attrs)
+
+    Member
+    |> Ash.Changeset.for_create(:create_member, attrs)
+    |> Ash.create!()
+  end
+
+  # Helper to create a cycle
+  defp create_cycle(member, fee_type, attrs) 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!()
+  end
+
+  describe "type change cycle regeneration" do
+    test "future unpaid cycles are regenerated with new amount" do
+      today = Date.utc_today()
+      yearly_type1 = create_fee_type(%{interval: :yearly, amount: Decimal.new("100.00")})
+      yearly_type2 = create_fee_type(%{interval: :yearly, amount: Decimal.new("150.00")})
+
+      # Create member without fee type first to avoid auto-generation
+      member = create_member(%{})
+
+      # Manually assign fee type (this will trigger cycle generation)
+      member =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: yearly_type1.id
+        })
+        |> Ash.update!()
+
+      # Cycle generation runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Create cycles: one in the past (paid), one current (unpaid)
+      # Note: Future cycles are not automatically generated by CycleGenerator,
+      # so we only test with current cycle
+      past_cycle_start = CalendarCycles.calculate_cycle_start(~D[2023-01-01], :yearly)
+      current_cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      # Past cycle (paid) - should remain unchanged
+      # Check if it already exists (from auto-generation), if not create it
+      case MembershipFeeCycle
+           |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^past_cycle_start)
+           |> Ash.read_one() do
+        {:ok, existing_cycle} when not is_nil(existing_cycle) ->
+          # Update to paid
+          existing_cycle
+          |> Ash.Changeset.for_update(:update, %{status: :paid})
+          |> Ash.update!()
+
+        _ ->
+          create_cycle(member, yearly_type1, %{
+            cycle_start: past_cycle_start,
+            status: :paid,
+            amount: Decimal.new("100.00")
+          })
+      end
+
+      # Current cycle (unpaid) - should be regenerated
+      # Delete if exists (from auto-generation), then create with old amount
+      case MembershipFeeCycle
+           |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^current_cycle_start)
+           |> Ash.read_one() do
+        {:ok, existing_cycle} when not is_nil(existing_cycle) ->
+          Ash.destroy!(existing_cycle)
+
+        _ ->
+          :ok
+      end
+
+      _current_cycle =
+        create_cycle(member, yearly_type1, %{
+          cycle_start: current_cycle_start,
+          status: :unpaid,
+          amount: Decimal.new("100.00")
+        })
+
+      # Change membership fee type (same interval, different amount)
+      assert {:ok, _updated_member} =
+               member
+               |> Ash.Changeset.for_update(:update_member, %{
+                 membership_fee_type_id: yearly_type2.id
+               })
+               |> Ash.update()
+
+      # Cycle regeneration runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Verify past cycle is unchanged
+      past_cycle_after =
+        MembershipFeeCycle
+        |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^past_cycle_start)
+        |> Ash.read_one!()
+
+      assert past_cycle_after.status == :paid
+      assert Decimal.equal?(past_cycle_after.amount, Decimal.new("100.00"))
+      assert past_cycle_after.membership_fee_type_id == yearly_type1.id
+
+      # Verify current cycle was deleted and regenerated
+      # Check that cycle with new type exists (regenerated)
+      new_current_cycle =
+        MembershipFeeCycle
+        |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^current_cycle_start)
+        |> Ash.read_one!()
+
+      # Verify it has the new type and amount
+      assert new_current_cycle.membership_fee_type_id == yearly_type2.id
+      assert Decimal.equal?(new_current_cycle.amount, Decimal.new("150.00"))
+      assert new_current_cycle.status == :unpaid
+
+      # Verify old cycle with old type doesn't exist anymore
+      old_current_cycles =
+        MembershipFeeCycle
+        |> Ash.Query.filter(
+          member_id == ^member.id and cycle_start == ^current_cycle_start and
+            membership_fee_type_id == ^yearly_type1.id
+        )
+        |> Ash.read!()
+
+      assert Enum.empty?(old_current_cycles)
+    end
+
+    test "paid cycles remain unchanged" do
+      today = Date.utc_today()
+      yearly_type1 = create_fee_type(%{interval: :yearly, amount: Decimal.new("100.00")})
+      yearly_type2 = create_fee_type(%{interval: :yearly, amount: Decimal.new("150.00")})
+
+      # Create member without fee type first to avoid auto-generation
+      member = create_member(%{})
+
+      # Manually assign fee type (this will trigger cycle generation)
+      member =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: yearly_type1.id
+        })
+        |> Ash.update!()
+
+      # Cycle generation runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Get the current cycle and mark it as paid
+      current_cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      # Find current cycle and mark as paid
+      paid_cycle =
+        MembershipFeeCycle
+        |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^current_cycle_start)
+        |> Ash.read_one!()
+        |> Ash.Changeset.for_update(:mark_as_paid)
+        |> Ash.update!()
+
+      # Change membership fee type
+      assert {:ok, _updated_member} =
+               member
+               |> Ash.Changeset.for_update(:update_member, %{
+                 membership_fee_type_id: yearly_type2.id
+               })
+               |> Ash.update()
+
+      # Cycle regeneration runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Verify paid cycle is unchanged (not deleted and regenerated)
+      {:ok, cycle_after} = Ash.get(MembershipFeeCycle, paid_cycle.id)
+      assert cycle_after.status == :paid
+      assert Decimal.equal?(cycle_after.amount, Decimal.new("100.00"))
+      assert cycle_after.membership_fee_type_id == yearly_type1.id
+    end
+
+    test "suspended cycles remain unchanged" do
+      today = Date.utc_today()
+      yearly_type1 = create_fee_type(%{interval: :yearly, amount: Decimal.new("100.00")})
+      yearly_type2 = create_fee_type(%{interval: :yearly, amount: Decimal.new("150.00")})
+
+      # Create member without fee type first to avoid auto-generation
+      member = create_member(%{})
+
+      # Manually assign fee type (this will trigger cycle generation)
+      member =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: yearly_type1.id
+        })
+        |> Ash.update!()
+
+      # Cycle generation runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Get the current cycle and mark it as suspended
+      current_cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      # Find current cycle and mark as suspended
+      suspended_cycle =
+        MembershipFeeCycle
+        |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^current_cycle_start)
+        |> Ash.read_one!()
+        |> Ash.Changeset.for_update(:mark_as_suspended)
+        |> Ash.update!()
+
+      # Change membership fee type
+      assert {:ok, _updated_member} =
+               member
+               |> Ash.Changeset.for_update(:update_member, %{
+                 membership_fee_type_id: yearly_type2.id
+               })
+               |> Ash.update()
+
+      # Cycle regeneration runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Verify suspended cycle is unchanged (not deleted and regenerated)
+      {:ok, cycle_after} = Ash.get(MembershipFeeCycle, suspended_cycle.id)
+      assert cycle_after.status == :suspended
+      assert Decimal.equal?(cycle_after.amount, Decimal.new("100.00"))
+      assert cycle_after.membership_fee_type_id == yearly_type1.id
+    end
+
+    test "only cycles that haven't ended yet are deleted" do
+      today = Date.utc_today()
+      yearly_type1 = create_fee_type(%{interval: :yearly, amount: Decimal.new("100.00")})
+      yearly_type2 = create_fee_type(%{interval: :yearly, amount: Decimal.new("150.00")})
+
+      # Create member without fee type first to avoid auto-generation
+      member = create_member(%{})
+
+      # Manually assign fee type (this will trigger cycle generation)
+      member =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: yearly_type1.id
+        })
+        |> Ash.update!()
+
+      # Cycle generation runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Create cycles: one in the past (unpaid, ended), one current (unpaid, not ended)
+      past_cycle_start =
+        CalendarCycles.calculate_cycle_start(
+          Date.add(today, -365),
+          :yearly
+        )
+
+      current_cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      # Past cycle (unpaid) - should remain unchanged (cycle_start < today)
+      # Delete existing cycle if it exists (from auto-generation)
+      case MembershipFeeCycle
+           |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^past_cycle_start)
+           |> Ash.read_one() do
+        {:ok, existing_cycle} when not is_nil(existing_cycle) ->
+          Ash.destroy!(existing_cycle)
+
+        _ ->
+          :ok
+      end
+
+      past_cycle =
+        create_cycle(member, yearly_type1, %{
+          cycle_start: past_cycle_start,
+          status: :unpaid,
+          amount: Decimal.new("100.00")
+        })
+
+      # Current cycle (unpaid) - should be regenerated (cycle_start >= today)
+      # Delete existing cycle if it exists (from auto-generation)
+      case MembershipFeeCycle
+           |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^current_cycle_start)
+           |> Ash.read_one() do
+        {:ok, existing_cycle} when not is_nil(existing_cycle) ->
+          Ash.destroy!(existing_cycle)
+
+        _ ->
+          :ok
+      end
+
+      _current_cycle =
+        create_cycle(member, yearly_type1, %{
+          cycle_start: current_cycle_start,
+          status: :unpaid,
+          amount: Decimal.new("100.00")
+        })
+
+      # Change membership fee type
+      assert {:ok, _updated_member} =
+               member
+               |> Ash.Changeset.for_update(:update_member, %{
+                 membership_fee_type_id: yearly_type2.id
+               })
+               |> Ash.update()
+
+      # Cycle regeneration runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Verify past cycle is unchanged
+      {:ok, past_cycle_after} = Ash.get(MembershipFeeCycle, past_cycle.id)
+      assert past_cycle_after.status == :unpaid
+      assert Decimal.equal?(past_cycle_after.amount, Decimal.new("100.00"))
+      assert past_cycle_after.membership_fee_type_id == yearly_type1.id
+
+      # Verify current cycle was regenerated
+      # Check that cycle with new type exists
+      new_current_cycle =
+        MembershipFeeCycle
+        |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^current_cycle_start)
+        |> Ash.read_one!()
+
+      assert new_current_cycle.membership_fee_type_id == yearly_type2.id
+      assert Decimal.equal?(new_current_cycle.amount, Decimal.new("150.00"))
+
+      # Verify old cycle with old type doesn't exist anymore
+      old_current_cycles =
+        MembershipFeeCycle
+        |> Ash.Query.filter(
+          member_id == ^member.id and cycle_start == ^current_cycle_start and
+            membership_fee_type_id == ^yearly_type1.id
+        )
+        |> Ash.read!()
+
+      assert Enum.empty?(old_current_cycles)
+    end
+
+    test "member calculations update after type change" do
+      today = Date.utc_today()
+      yearly_type1 = create_fee_type(%{interval: :yearly, amount: Decimal.new("100.00")})
+      yearly_type2 = create_fee_type(%{interval: :yearly, amount: Decimal.new("150.00")})
+
+      # Create member with join_date = today to avoid past cycles
+      # This ensures no overdue cycles exist
+      member = create_member(%{join_date: today})
+
+      # Manually assign fee type (this will trigger cycle generation)
+      member =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: yearly_type1.id
+        })
+        |> Ash.update!()
+
+      # Cycle generation runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Get current cycle start
+      current_cycle_start = CalendarCycles.calculate_cycle_start(today, :yearly)
+
+      # Ensure only one cycle exists (the current one)
+      # Delete all cycles except the current one
+      existing_cycles =
+        MembershipFeeCycle
+        |> Ash.Query.filter(member_id == ^member.id)
+        |> Ash.read!()
+
+      Enum.each(existing_cycles, fn cycle ->
+        if cycle.cycle_start != current_cycle_start do
+          Ash.destroy!(cycle)
+        end
+      end)
+
+      # Ensure current cycle exists and is unpaid
+      case MembershipFeeCycle
+           |> Ash.Query.filter(member_id == ^member.id and cycle_start == ^current_cycle_start)
+           |> Ash.read_one() do
+        {:ok, existing_cycle} when not is_nil(existing_cycle) ->
+          # Update to unpaid if it's not
+          if existing_cycle.status != :unpaid do
+            existing_cycle
+            |> Ash.Changeset.for_update(:mark_as_unpaid)
+            |> Ash.update!()
+          end
+
+        _ ->
+          # Create if it doesn't exist
+          create_cycle(member, yearly_type1, %{
+            cycle_start: current_cycle_start,
+            status: :unpaid,
+            amount: Decimal.new("100.00")
+          })
+      end
+
+      # Load calculations before change
+      member = Ash.load!(member, [:current_cycle_status, :overdue_count])
+      assert member.current_cycle_status == :unpaid
+      assert member.overdue_count == 0
+
+      # Change membership fee type
+      assert {:ok, updated_member} =
+               member
+               |> Ash.Changeset.for_update(:update_member, %{
+                 membership_fee_type_id: yearly_type2.id
+               })
+               |> Ash.update()
+
+      # Cycle regeneration runs synchronously in the same transaction
+      # No need to wait for async completion
+
+      # Reload member with calculations
+      updated_member = Ash.load!(updated_member, [:current_cycle_status, :overdue_count])
+
+      # Calculations should still work (cycle was regenerated)
+      assert updated_member.current_cycle_status == :unpaid
+      assert updated_member.overdue_count == 0
+    end
+  end
+end

test/membership_fees/changes/validate_same_interval_test.exs

@@ -0,0 +1,227 @@
+defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do
+  @moduledoc """
+  Tests for ValidateSameInterval change module.
+  """
+  use Mv.DataCase, async: true
+
+  alias Mv.Membership.Member
+  alias Mv.MembershipFees.MembershipFeeType
+  alias Mv.MembershipFees.Changes.ValidateSameInterval
+
+  # Helper to create a membership fee type
+  defp create_fee_type(attrs) 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!()
+  end
+
+  # Helper to create a member
+  defp create_member(attrs) do
+    default_attrs = %{
+      first_name: "Test",
+      last_name: "Member",
+      email: "test.member.#{System.unique_integer([:positive])}@example.com"
+    }
+
+    attrs = Map.merge(default_attrs, attrs)
+
+    Member
+    |> Ash.Changeset.for_create(:create_member, attrs)
+    |> Ash.create!()
+  end
+
+  describe "validate_interval_match/1" do
+    test "allows change to type with same interval" do
+      yearly_type1 = create_fee_type(%{interval: :yearly, name: "Yearly Type 1"})
+      yearly_type2 = create_fee_type(%{interval: :yearly, name: "Yearly Type 2"})
+
+      member = create_member(%{membership_fee_type_id: yearly_type1.id})
+
+      changeset =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: yearly_type2.id
+        })
+        |> ValidateSameInterval.change(%{}, %{})
+
+      assert changeset.valid?
+    end
+
+    test "prevents change to type with different interval" do
+      yearly_type = create_fee_type(%{interval: :yearly})
+      monthly_type = create_fee_type(%{interval: :monthly})
+
+      member = create_member(%{membership_fee_type_id: yearly_type.id})
+
+      changeset =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: monthly_type.id
+        })
+        |> ValidateSameInterval.change(%{}, %{})
+
+      refute changeset.valid?
+      assert %{errors: errors} = changeset
+
+      assert Enum.any?(errors, fn error ->
+               error.field == :membership_fee_type_id and
+                 error.message =~ "yearly" and
+                 error.message =~ "monthly"
+             end)
+    end
+
+    test "allows first assignment of membership fee type" do
+      yearly_type = create_fee_type(%{interval: :yearly})
+      # No fee type assigned
+      member = create_member(%{})
+
+      changeset =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: yearly_type.id
+        })
+        |> ValidateSameInterval.change(%{}, %{})
+
+      assert changeset.valid?
+    end
+
+    test "prevents removal of membership fee type" do
+      yearly_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: yearly_type.id})
+
+      changeset =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: nil
+        })
+        |> ValidateSameInterval.change(%{}, %{})
+
+      refute changeset.valid?
+      assert %{errors: errors} = changeset
+
+      assert Enum.any?(errors, fn error ->
+               error.field == :membership_fee_type_id and
+                 error.message =~ "Cannot remove membership fee type"
+             end)
+    end
+
+    test "does nothing when membership_fee_type_id is not changed" do
+      yearly_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: yearly_type.id})
+
+      changeset =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          first_name: "New Name"
+        })
+        |> ValidateSameInterval.change(%{}, %{})
+
+      assert changeset.valid?
+    end
+
+    test "error message is clear and helpful" do
+      yearly_type = create_fee_type(%{interval: :yearly})
+      quarterly_type = create_fee_type(%{interval: :quarterly})
+
+      member = create_member(%{membership_fee_type_id: yearly_type.id})
+
+      changeset =
+        member
+        |> Ash.Changeset.for_update(:update_member, %{
+          membership_fee_type_id: quarterly_type.id
+        })
+        |> ValidateSameInterval.change(%{}, %{})
+
+      error = Enum.find(changeset.errors, &(&1.field == :membership_fee_type_id))
+      assert error.message =~ "yearly"
+      assert error.message =~ "quarterly"
+      assert error.message =~ "same-interval"
+    end
+
+    test "handles all interval types correctly" do
+      intervals = [:monthly, :quarterly, :half_yearly, :yearly]
+
+      for interval1 <- intervals,
+          interval2 <- intervals,
+          interval1 != interval2 do
+        type1 =
+          create_fee_type(%{
+            interval: interval1,
+            name: "Type #{interval1} #{System.unique_integer([:positive])}"
+          })
+
+        type2 =
+          create_fee_type(%{
+            interval: interval2,
+            name: "Type #{interval2} #{System.unique_integer([:positive])}"
+          })
+
+        member = create_member(%{membership_fee_type_id: type1.id})
+
+        changeset =
+          member
+          |> Ash.Changeset.for_update(:update_member, %{
+            membership_fee_type_id: type2.id
+          })
+          |> ValidateSameInterval.change(%{}, %{})
+
+        refute changeset.valid?,
+               "Should prevent change from #{interval1} to #{interval2}"
+      end
+    end
+  end
+
+  describe "integration with update_member action" do
+    test "validation works when updating member via update_member action" do
+      yearly_type = create_fee_type(%{interval: :yearly})
+      monthly_type = create_fee_type(%{interval: :monthly})
+
+      member = create_member(%{membership_fee_type_id: yearly_type.id})
+
+      # Try to update member with different interval type
+      assert {:error, %Ash.Error.Invalid{} = error} =
+               member
+               |> Ash.Changeset.for_update(:update_member, %{
+                 membership_fee_type_id: monthly_type.id
+               })
+               |> Ash.update()
+
+      # Check that error is about interval mismatch
+      error_message = extract_error_message(error)
+      assert error_message =~ "yearly"
+      assert error_message =~ "monthly"
+      assert error_message =~ "same-interval"
+    end
+
+    test "allows update when interval matches" do
+      yearly_type1 = create_fee_type(%{interval: :yearly, name: "Yearly Type 1"})
+      yearly_type2 = create_fee_type(%{interval: :yearly, name: "Yearly Type 2"})
+
+      member = create_member(%{membership_fee_type_id: yearly_type1.id})
+
+      # Update member with same-interval type
+      assert {:ok, updated_member} =
+               member
+               |> Ash.Changeset.for_update(:update_member, %{
+                 membership_fee_type_id: yearly_type2.id
+               })
+               |> Ash.update()
+
+      assert updated_member.membership_fee_type_id == yearly_type2.id
+    end
+
+    defp extract_error_message(%Ash.Error.Invalid{errors: errors}) do
+      errors
+      |> Enum.filter(&(&1.field == :membership_fee_type_id))
+      |> Enum.map_join(" ", & &1.message)
+    end
+  end
+end

test/membership_fees/member_cycle_integration_test.exs

@@ -198,7 +198,7 @@ defmodule Mv.MembershipFees.MemberCycleIntegrationTest do
       today = ~D[2025-12-31]
 
       # Manually trigger generation again with fixed "today" date
-      {:ok, _} =
+      {:ok, _, _} =
         Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       final_cycles = get_member_cycles(member.id)

test/membership_fees/membership_fee_cycle_test.exs

@@ -1,6 +1,6 @@
 defmodule Mv.MembershipFees.MembershipFeeCycleTest do
   @moduledoc """
-  Tests for MembershipFeeCycle resource.
+  Tests for MembershipFeeCycle resource, focusing on status management actions.
   """
   use Mv.DataCase, async: true
 
@@ -8,275 +8,200 @@ defmodule Mv.MembershipFees.MembershipFeeCycleTest do
   alias Mv.MembershipFees.MembershipFeeType
   alias Mv.Membership.Member
 
-  setup do
+  # Helper to create a membership fee type
-    # Create a member for testing
+  defp create_fee_type(attrs) do
-    {:ok, member} =
+    default_attrs = %{
-      Ash.create(Member, %{
+      name: "Test Fee Type #{System.unique_integer([:positive])}",
-        first_name: "Test",
+      amount: Decimal.new("50.00"),
-        last_name: "Member",
+      interval: :yearly
-        email: "test.member.#{System.unique_integer([:positive])}@example.com"
+    }
-      })
 
-    # Create a fee type for testing
+    attrs = Map.merge(default_attrs, attrs)
-    {:ok, fee_type} =
-      Ash.create(MembershipFeeType, %{
-        name: "Test Fee Type #{System.unique_integer([:positive])}",
-        amount: Decimal.new("100.00"),
-        interval: :monthly
-      })
 
-    %{member: member, fee_type: fee_type}
+    MembershipFeeType
+    |> Ash.Changeset.for_create(:create, attrs)
+    |> Ash.create!()
   end
 
-  describe "create MembershipFeeCycle" do
+  # Helper to create a member
-    test "can create cycle with valid attributes", %{member: member, fee_type: fee_type} do
+  defp create_member(attrs) do
-      attrs = %{
+    default_attrs = %{
-        cycle_start: ~D[2025-01-01],
+      first_name: "Test",
-        amount: Decimal.new("100.00"),
+      last_name: "Member",
-        member_id: member.id,
+      email: "test.member.#{System.unique_integer([:positive])}@example.com"
-        membership_fee_type_id: fee_type.id
+    }
-      }
 
-      assert {:ok, %MembershipFeeCycle{} = cycle} = Ash.create(MembershipFeeCycle, attrs)
+    attrs = Map.merge(default_attrs, attrs)
-      assert cycle.cycle_start == ~D[2025-01-01]
-      assert Decimal.equal?(cycle.amount, Decimal.new("100.00"))
-      assert cycle.member_id == member.id
-      assert cycle.membership_fee_type_id == fee_type.id
-    end
 
-    test "can create cycle with notes", %{member: member, fee_type: fee_type} do
+    Member
-      attrs = %{
+    |> Ash.Changeset.for_create(:create_member, attrs)
-        cycle_start: ~D[2025-01-01],
+    |> Ash.create!()
-        amount: Decimal.new("100.00"),
+  end
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id,
-        notes: "First payment cycle"
-      }
 
-      assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs)
+  # Helper to create a cycle
-      assert cycle.notes == "First payment cycle"
+  defp create_cycle(member, fee_type, attrs) do
-    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
+    }
 
-    test "requires cycle_start", %{member: member, fee_type: fee_type} do
+    attrs = Map.merge(default_attrs, attrs)
-      attrs = %{
-        amount: Decimal.new("100.00"),
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id
-      }
 
-      assert {:error, error} = Ash.create(MembershipFeeCycle, attrs)
+    MembershipFeeCycle
-      assert error_on_field?(error, :cycle_start)
+    |> Ash.Changeset.for_create(:create, attrs)
-    end
+    |> Ash.create!()
+  end
 
-    test "requires amount", %{member: member, fee_type: fee_type} do
+  describe "status defaults" do
-      attrs = %{
+    test "status defaults to :unpaid when creating a cycle" do
-        cycle_start: ~D[2025-01-01],
+      fee_type = create_fee_type(%{interval: :yearly})
-        member_id: member.id,
+      member = create_member(%{membership_fee_type_id: fee_type.id})
-        membership_fee_type_id: fee_type.id
-      }
 
-      assert {:error, error} = Ash.create(MembershipFeeCycle, attrs)
+      cycle =
-      assert error_on_field?(error, :amount)
+        MembershipFeeCycle
-    end
+        |> Ash.Changeset.for_create(:create, %{
+          cycle_start: ~D[2024-01-01],
+          amount: Decimal.new("50.00"),
+          member_id: member.id,
+          membership_fee_type_id: fee_type.id
+        })
+        |> Ash.create!()
 
-    test "requires member_id", %{fee_type: fee_type} do
-      attrs = %{
-        cycle_start: ~D[2025-01-01],
-        amount: Decimal.new("100.00"),
-        membership_fee_type_id: fee_type.id
-      }
-
-      assert {:error, error} = Ash.create(MembershipFeeCycle, attrs)
-      assert error_on_field?(error, :member_id) or error_on_field?(error, :member)
-    end
-
-    test "requires membership_fee_type_id", %{member: member} do
-      attrs = %{
-        cycle_start: ~D[2025-01-01],
-        amount: Decimal.new("100.00"),
-        member_id: member.id
-      }
-
-      assert {:error, error} = Ash.create(MembershipFeeCycle, attrs)
-
-      assert error_on_field?(error, :membership_fee_type_id) or
-               error_on_field?(error, :membership_fee_type)
-    end
-
-    test "status defaults to :unpaid", %{member: member, fee_type: fee_type} do
-      attrs = %{
-        cycle_start: ~D[2025-01-01],
-        amount: Decimal.new("100.00"),
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id
-      }
-
-      assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs)
       assert cycle.status == :unpaid
     end
+  end
 
-    test "validates status enum values - unpaid", %{member: member, fee_type: fee_type} do
+  describe "mark_as_paid" do
-      attrs = %{
+    test "sets status to :paid" do
-        cycle_start: ~D[2025-01-01],
+      fee_type = create_fee_type(%{interval: :yearly})
-        amount: Decimal.new("100.00"),
+      member = create_member(%{membership_fee_type_id: fee_type.id})
-        member_id: member.id,
+      cycle = create_cycle(member, fee_type, %{status: :unpaid})
-        membership_fee_type_id: fee_type.id,
-        status: :unpaid
-      }
 
-      assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs)
+      assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_paid)
-      assert cycle.status == :unpaid
+      assert updated.status == :paid
     end
 
-    test "validates status enum values - paid", %{member: member, fee_type: fee_type} do
+    test "can set notes when marking as paid" do
-      attrs = %{
+      fee_type = create_fee_type(%{interval: :yearly})
-        cycle_start: ~D[2025-02-01],
+      member = create_member(%{membership_fee_type_id: fee_type.id})
-        amount: Decimal.new("100.00"),
+      cycle = create_cycle(member, fee_type, %{status: :unpaid})
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id,
-        status: :paid
-      }
 
-      assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs)
+      assert {:ok, updated} =
-      assert cycle.status == :paid
+               Ash.update(cycle, %{notes: "Payment received via bank transfer"},
+                 action: :mark_as_paid
+               )
+
+      assert updated.status == :paid
+      assert updated.notes == "Payment received via bank transfer"
     end
 
-    test "validates status enum values - suspended", %{member: member, fee_type: fee_type} do
+    test "can change from suspended to paid" do
-      attrs = %{
+      fee_type = create_fee_type(%{interval: :yearly})
-        cycle_start: ~D[2025-03-01],
+      member = create_member(%{membership_fee_type_id: fee_type.id})
-        amount: Decimal.new("100.00"),
+      cycle = create_cycle(member, fee_type, %{status: :suspended})
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id,
-        status: :suspended
-      }
 
-      assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs)
+      assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_paid)
-      assert cycle.status == :suspended
+      assert updated.status == :paid
-    end
-
-    test "rejects invalid status values", %{member: member, fee_type: fee_type} do
-      attrs = %{
-        cycle_start: ~D[2025-01-01],
-        amount: Decimal.new("100.00"),
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id,
-        status: :cancelled
-      }
-
-      assert {:error, error} = Ash.create(MembershipFeeCycle, attrs)
-      assert error_on_field?(error, :status)
-    end
-
-    test "rejects negative amount", %{member: member, fee_type: fee_type} do
-      attrs = %{
-        cycle_start: ~D[2025-04-01],
-        amount: Decimal.new("-50.00"),
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id
-      }
-
-      assert {:error, error} = Ash.create(MembershipFeeCycle, attrs)
-      assert error_on_field?(error, :amount)
-    end
-
-    test "accepts zero amount", %{member: member, fee_type: fee_type} do
-      attrs = %{
-        cycle_start: ~D[2025-05-01],
-        amount: Decimal.new("0.00"),
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id
-      }
-
-      assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs)
-      assert Decimal.equal?(cycle.amount, Decimal.new("0.00"))
     end
   end
 
-  describe "uniqueness constraint" do
+  describe "mark_as_suspended" do
-    test "cannot create duplicate cycle for same member and cycle_start", %{
+    test "sets status to :suspended" do
-      member: member,
+      fee_type = create_fee_type(%{interval: :yearly})
-      fee_type: fee_type
+      member = create_member(%{membership_fee_type_id: fee_type.id})
-    } do
+      cycle = create_cycle(member, fee_type, %{status: :unpaid})
-      attrs = %{
-        cycle_start: ~D[2025-01-01],
-        amount: Decimal.new("100.00"),
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id
-      }
 
-      assert {:ok, _cycle1} = Ash.create(MembershipFeeCycle, attrs)
+      assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_suspended)
-      assert {:error, error} = Ash.create(MembershipFeeCycle, attrs)
+      assert updated.status == :suspended
-
-      # Should fail due to uniqueness constraint
-      assert is_struct(error, Ash.Error.Invalid)
     end
 
-    test "can create cycles for same member with different cycle_start", %{
+    test "can set notes when marking as suspended" do
-      member: member,
+      fee_type = create_fee_type(%{interval: :yearly})
-      fee_type: fee_type
+      member = create_member(%{membership_fee_type_id: fee_type.id})
-    } do
+      cycle = create_cycle(member, fee_type, %{status: :unpaid})
-      attrs1 = %{
-        cycle_start: ~D[2025-01-01],
-        amount: Decimal.new("100.00"),
-        member_id: member.id,
-        membership_fee_type_id: fee_type.id
-      }
 
-      attrs2 = %{
+      assert {:ok, updated} =
-        cycle_start: ~D[2025-02-01],
+               Ash.update(cycle, %{notes: "Waived due to special circumstances"},
-        amount: Decimal.new("100.00"),
+                 action: :mark_as_suspended
-        member_id: member.id,
+               )
-        membership_fee_type_id: fee_type.id
-      }
 
-      assert {:ok, _cycle1} = Ash.create(MembershipFeeCycle, attrs1)
+      assert updated.status == :suspended
-      assert {:ok, _cycle2} = Ash.create(MembershipFeeCycle, attrs2)
+      assert updated.notes == "Waived due to special circumstances"
     end
 
-    test "can create cycles for different members with same cycle_start", %{fee_type: fee_type} do
+    test "can change from paid to suspended" do
-      {:ok, member1} =
+      fee_type = create_fee_type(%{interval: :yearly})
-        Ash.create(Member, %{
+      member = create_member(%{membership_fee_type_id: fee_type.id})
-          first_name: "Member",
+      cycle = create_cycle(member, fee_type, %{status: :paid})
-          last_name: "One",
-          email: "member.one.#{System.unique_integer([:positive])}@example.com"
-        })
 
-      {:ok, member2} =
+      assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_suspended)
-        Ash.create(Member, %{
+      assert updated.status == :suspended
-          first_name: "Member",
-          last_name: "Two",
-          email: "member.two.#{System.unique_integer([:positive])}@example.com"
-        })
-
-      attrs1 = %{
-        cycle_start: ~D[2025-01-01],
-        amount: Decimal.new("100.00"),
-        member_id: member1.id,
-        membership_fee_type_id: fee_type.id
-      }
-
-      attrs2 = %{
-        cycle_start: ~D[2025-01-01],
-        amount: Decimal.new("100.00"),
-        member_id: member2.id,
-        membership_fee_type_id: fee_type.id
-      }
-
-      assert {:ok, _cycle1} = Ash.create(MembershipFeeCycle, attrs1)
-      assert {:ok, _cycle2} = Ash.create(MembershipFeeCycle, attrs2)
     end
   end
 
-  # Helper to check if an error occurred on a specific field
+  describe "mark_as_unpaid" do
-  defp error_on_field?(%Ash.Error.Invalid{} = error, field) do
+    test "sets status to :unpaid" do
-    Enum.any?(error.errors, fn e ->
+      fee_type = create_fee_type(%{interval: :yearly})
-      case e do
+      member = create_member(%{membership_fee_type_id: fee_type.id})
-        %{field: ^field} -> true
+      cycle = create_cycle(member, fee_type, %{status: :paid})
-        %{fields: fields} when is_list(fields) -> field in fields
+
-        _ -> false
+      assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_unpaid)
-      end
+      assert updated.status == :unpaid
-    end)
+    end
+
+    test "can set notes when marking as unpaid" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+      cycle = create_cycle(member, fee_type, %{status: :paid})
+
+      assert {:ok, updated} =
+               Ash.update(cycle, %{notes: "Payment was reversed"}, action: :mark_as_unpaid)
+
+      assert updated.status == :unpaid
+      assert updated.notes == "Payment was reversed"
+    end
+
+    test "can change from suspended to unpaid" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+      cycle = create_cycle(member, fee_type, %{status: :suspended})
+
+      assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_unpaid)
+      assert updated.status == :unpaid
+    end
   end
 
-  defp error_on_field?(_, _), do: false
+  describe "status transitions" do
+    test "all status transitions are allowed" do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{membership_fee_type_id: fee_type.id})
+
+      # unpaid -> paid
+      cycle1 = create_cycle(member, fee_type, %{status: :unpaid})
+      assert {:ok, c1} = Ash.update(cycle1, %{}, action: :mark_as_paid)
+      assert c1.status == :paid
+
+      # paid -> suspended
+      assert {:ok, c2} = Ash.update(c1, %{}, action: :mark_as_suspended)
+      assert c2.status == :suspended
+
+      # suspended -> unpaid
+      assert {:ok, c3} = Ash.update(c2, %{}, action: :mark_as_unpaid)
+      assert c3.status == :unpaid
+
+      # unpaid -> suspended
+      assert {:ok, c4} = Ash.update(c3, %{}, action: :mark_as_suspended)
+      assert c4.status == :suspended
+
+      # suspended -> paid
+      assert {:ok, c5} = Ash.update(c4, %{}, action: :mark_as_paid)
+      assert c5.status == :paid
+
+      # paid -> unpaid
+      assert {:ok, c6} = Ash.update(c5, %{}, action: :mark_as_unpaid)
+      assert c6.status == :unpaid
+    end
+  end
 end

test/mv/membership_fees/cycle_generator_edge_cases_test.exs

@@ -84,7 +84,7 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do
       Enum.each(existing_cycles, &Ash.destroy!(&1))
 
       # Generate cycles with fixed "today" date
-      {:ok, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, _, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
     end
 
     member
@@ -128,7 +128,7 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do
         |> Ash.update!()
 
       # Explicitly generate cycles with fixed "today" date
-      {:ok, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, _, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       # Check all cycles
       cycles = get_member_cycles(member.id)
@@ -158,7 +158,7 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do
         |> Ash.update!()
 
       # Explicitly generate cycles with fixed "today" date
-      {:ok, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, _, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       # Check all cycles
       cycles = get_member_cycles(member.id)
@@ -333,7 +333,7 @@ defmodule Mv.MembershipFees.CycleGeneratorEdgeCasesTest do
 
       # Explicitly generate cycles with fixed "today" date
       today = ~D[2024-06-15]
-      {:ok, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, _, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       # Check all cycles
       all_cycles = get_member_cycles(member.id)

test/mv/membership_fees/cycle_generator_test.exs

@@ -78,7 +78,7 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do
 
       # Explicitly generate cycles with fixed "today" date to avoid date dependency
       today = ~D[2024-06-15]
-      {:ok, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, _, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       # Verify cycles were generated
       all_cycles = get_member_cycles(member.id)
@@ -122,7 +122,7 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do
 
       # Generate cycles with specific "today" date
       today = ~D[2024-06-15]
-      {:ok, new_cycles} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, new_cycles, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       # Should generate only 2023 and 2024 (2022 already exists)
       new_cycle_years = Enum.map(new_cycles, & &1.cycle_start.year) |> Enum.sort()
@@ -144,7 +144,7 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do
 
       # Generate cycles with specific "today" date far in the future
       today = ~D[2025-06-15]
-      {:ok, cycles} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, cycles, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       # With exit_date in 2023, should only generate 2022 and 2023 cycles
       cycle_years = Enum.map(cycles, & &1.cycle_start.year) |> Enum.sort()
@@ -168,10 +168,10 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do
       today = ~D[2024-06-15]
 
       # First generation
-      {:ok, _first_cycles} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, _first_cycles, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       # Second generation (should be idempotent)
-      {:ok, second_cycles} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
+      {:ok, second_cycles, _} = CycleGenerator.generate_cycles_for_member(member.id, today: today)
 
       # Second call should return empty list (no new cycles)
       assert second_cycles == []
@@ -411,12 +411,12 @@ defmodule Mv.MembershipFees.CycleGeneratorTest do
       result2 = Task.await(task2)
 
       # Both should succeed
-      assert match?({:ok, _}, result1)
+      assert match?({:ok, _, _}, result1)
-      assert match?({:ok, _}, result2)
+      assert match?({:ok, _, _}, result2)
 
       # One should have created cycles, the other should have empty list (idempotent)
-      {:ok, cycles1} = result1
+      {:ok, cycles1, _} = result1
-      {:ok, cycles2} = result2
+      {:ok, cycles2, _} = result2
 
       # Combined should not have duplicates
       all_cycles = cycles1 ++ cycles2