diff --git a/docs/membership-fee-architecture.md b/docs/membership-fee-architecture.md index d6b5ee2..7c8da24 100644 --- a/docs/membership-fee-architecture.md +++ b/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 transaction to ensure atomicity +- Use after_action hook to trigger regeneration synchronously +- 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 diff --git a/lib/membership/member.ex b/lib/membership/member.ex index ae32abd..787b1d1 100644 --- a/lib/membership/member.ex +++ b/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 -> - case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id) do - {:ok, _cycles} -> - :ok + if Application.get_env(:mv, :sql_sandbox, false) do + # Run synchronously in test environment for DB sandbox compatibility + # Use skip_lock?: true to avoid nested transactions (after_action runs within action transaction) + # 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 - # Run synchronously in test environment for DB sandbox compatibility - generate_fn.() + {:ok, member} + end else # Run asynchronously in other environments - Task.start(generate_fn) - end - end + # Send notifications explicitly since they cannot be returned via after_action + Task.start(fn -> + 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 - # Note: Cycle generation runs asynchronously to not block the action, - # but in test environment it runs synchronously for DB sandbox compatibility + # Trigger cycle regeneration when membership_fee_type_id changes + # This deletes future unpaid cycles and regenerates them with the new type/amount + # 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 Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id) do - {:ok, _cycles} -> - :ok + case regenerate_cycles_on_type_change(member) do + {:ok, notifications} -> + # Return notifications to Ash - they will be sent automatically after commit + {:ok, member, notifications} - {:error, reason} -> - require Logger + {:error, reason} -> + require Logger - Logger.warning( - "Failed to generate cycles for member #{member.id}: #{inspect(reason)}" - ) - end - end - - 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) + Logger.warning( + "Failed to regenerate cycles for member #{member.id}: #{inspect(reason)}" + ) + + {:ok, member} 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, ¤t_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 diff --git a/lib/membership_fees/changes/validate_same_interval.ex b/lib/membership_fees/changes/validate_same_interval.ex new file mode 100644 index 0000000..8c1efb4 --- /dev/null +++ b/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 diff --git a/lib/membership_fees/membership_fee_cycle.ex b/lib/membership_fees/membership_fee_cycle.ex index 4c47623..b437ead 100644 --- a/lib/membership_fees/membership_fee_cycle.ex +++ b/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 diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 0727a62..feb7b53 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/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} = 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) + {:ok, cycles, notifications} = CycleGenerator.generate_cycles_for_member(member_id) + {: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 - # Notifications are handled inside with_advisory_lock after transaction commits - with_advisory_lock(member.id, fn -> - do_generate_cycles(member, today) + do_generate_cycles_with_lock(member, today, skip_lock?) + end + + # 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} - {:error, reason} -> {:error, {cycle_start, reason}} + {:ok, cycle, notifications} when is_list(notifications) -> + {: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)}") diff --git a/test/membership/member_cycle_calculations_test.exs b/test/membership/member_cycle_calculations_test.exs new file mode 100644 index 0000000..5a9e501 --- /dev/null +++ b/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 diff --git a/test/membership/member_type_change_integration_test.exs b/test/membership/member_type_change_integration_test.exs new file mode 100644 index 0000000..f2dd0e0 --- /dev/null +++ b/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 diff --git a/test/membership_fees/changes/validate_same_interval_test.exs b/test/membership_fees/changes/validate_same_interval_test.exs new file mode 100644 index 0000000..0f4501c --- /dev/null +++ b/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 diff --git a/test/membership_fees/member_cycle_integration_test.exs b/test/membership_fees/member_cycle_integration_test.exs index 7cbfbff..5d1cf28 100644 --- a/test/membership_fees/member_cycle_integration_test.exs +++ b/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) diff --git a/test/membership_fees/membership_fee_cycle_test.exs b/test/membership_fees/membership_fee_cycle_test.exs index ca59e26..14bdf4b 100644 --- a/test/membership_fees/membership_fee_cycle_test.exs +++ b/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 - # Create a member for testing - {:ok, member} = - Ash.create(Member, %{ - first_name: "Test", - last_name: "Member", - email: "test.member.#{System.unique_integer([:positive])}@example.com" - }) + # 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 + } - # Create a fee type for testing - {:ok, fee_type} = - Ash.create(MembershipFeeType, %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("100.00"), - interval: :monthly - }) + attrs = Map.merge(default_attrs, attrs) - %{member: member, fee_type: fee_type} + MembershipFeeType + |> Ash.Changeset.for_create(:create, attrs) + |> Ash.create!() end - describe "create MembershipFeeCycle" do - test "can create cycle with valid attributes", %{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 - } + # 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" + } - assert {:ok, %MembershipFeeCycle{} = cycle} = Ash.create(MembershipFeeCycle, 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 + attrs = Map.merge(default_attrs, attrs) - test "can create cycle with notes", %{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, - notes: "First payment cycle" - } + Member + |> Ash.Changeset.for_create(:create_member, attrs) + |> Ash.create!() + end - assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs) - assert cycle.notes == "First payment cycle" - 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 + } - test "requires cycle_start", %{member: member, fee_type: fee_type} do - attrs = %{ - amount: Decimal.new("100.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id - } + attrs = Map.merge(default_attrs, attrs) - assert {:error, error} = Ash.create(MembershipFeeCycle, attrs) - assert error_on_field?(error, :cycle_start) - end + MembershipFeeCycle + |> Ash.Changeset.for_create(:create, attrs) + |> Ash.create!() + end - test "requires amount", %{member: member, fee_type: fee_type} do - attrs = %{ - cycle_start: ~D[2025-01-01], - member_id: member.id, - membership_fee_type_id: fee_type.id - } + describe "status defaults" do + test "status defaults to :unpaid when creating a cycle" do + fee_type = create_fee_type(%{interval: :yearly}) + member = create_member(%{membership_fee_type_id: fee_type.id}) - assert {:error, error} = Ash.create(MembershipFeeCycle, attrs) - assert error_on_field?(error, :amount) - end + cycle = + MembershipFeeCycle + |> 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 - attrs = %{ - cycle_start: ~D[2025-01-01], - amount: Decimal.new("100.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :unpaid - } + describe "mark_as_paid" do + test "sets status to :paid" 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: :unpaid}) - assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs) - assert cycle.status == :unpaid + assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_paid) + assert updated.status == :paid end - test "validates status enum values - paid", %{member: member, fee_type: fee_type} do - attrs = %{ - cycle_start: ~D[2025-02-01], - amount: Decimal.new("100.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :paid - } + test "can set notes when marking as paid" 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: :unpaid}) - assert {:ok, cycle} = Ash.create(MembershipFeeCycle, attrs) - assert cycle.status == :paid + assert {:ok, updated} = + 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 - attrs = %{ - cycle_start: ~D[2025-03-01], - amount: Decimal.new("100.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id, - status: :suspended - } + test "can change from suspended to paid" 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, cycle} = Ash.create(MembershipFeeCycle, attrs) - assert cycle.status == :suspended - 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")) + assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_paid) + assert updated.status == :paid end end - describe "uniqueness constraint" do - test "cannot create duplicate cycle for same member and cycle_start", %{ - 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 - } + describe "mark_as_suspended" do + test "sets status to :suspended" 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: :unpaid}) - assert {:ok, _cycle1} = Ash.create(MembershipFeeCycle, attrs) - assert {:error, error} = Ash.create(MembershipFeeCycle, attrs) - - # Should fail due to uniqueness constraint - assert is_struct(error, Ash.Error.Invalid) + assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_suspended) + assert updated.status == :suspended end - test "can create cycles for same member with different cycle_start", %{ - member: member, - fee_type: fee_type - } do - attrs1 = %{ - cycle_start: ~D[2025-01-01], - amount: Decimal.new("100.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id - } + test "can set notes when marking as suspended" 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: :unpaid}) - attrs2 = %{ - cycle_start: ~D[2025-02-01], - amount: Decimal.new("100.00"), - member_id: member.id, - membership_fee_type_id: fee_type.id - } + assert {:ok, updated} = + Ash.update(cycle, %{notes: "Waived due to special circumstances"}, + action: :mark_as_suspended + ) - assert {:ok, _cycle1} = Ash.create(MembershipFeeCycle, attrs1) - assert {:ok, _cycle2} = Ash.create(MembershipFeeCycle, attrs2) + assert updated.status == :suspended + 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 - {:ok, member1} = - Ash.create(Member, %{ - first_name: "Member", - last_name: "One", - email: "member.one.#{System.unique_integer([:positive])}@example.com" - }) + test "can change from paid to suspended" 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}) - {:ok, member2} = - Ash.create(Member, %{ - 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) + assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_suspended) + assert updated.status == :suspended end end - # Helper to check if an error occurred on a specific field - defp error_on_field?(%Ash.Error.Invalid{} = error, field) do - Enum.any?(error.errors, fn e -> - case e do - %{field: ^field} -> true - %{fields: fields} when is_list(fields) -> field in fields - _ -> false - end - end) + describe "mark_as_unpaid" do + test "sets status 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: :paid}) + + assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_unpaid) + assert updated.status == :unpaid + 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 diff --git a/test/mv/membership_fees/cycle_generator_edge_cases_test.exs b/test/mv/membership_fees/cycle_generator_edge_cases_test.exs index adca77a..85eb406 100644 --- a/test/mv/membership_fees/cycle_generator_edge_cases_test.exs +++ b/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) diff --git a/test/mv/membership_fees/cycle_generator_test.exs b/test/mv/membership_fees/cycle_generator_test.exs index 06dd59e..e6988da 100644 --- a/test/mv/membership_fees/cycle_generator_test.exs +++ b/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, _}, result2) + assert match?({:ok, _, _}, result1) + assert match?({:ok, _, _}, result2) # One should have created cycles, the other should have empty list (idempotent) - {:ok, cycles1} = result1 - {:ok, cycles2} = result2 + {:ok, cycles1, _} = result1 + {:ok, cycles2, _} = result2 # Combined should not have duplicates all_cycles = cycles1 ++ cycles2