diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 7f30833..8aa3dd7 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -607,24 +607,27 @@ defmodule Mv.Membership.Member do cycles = Map.get(member, :membership_fee_cycles) if is_list(cycles) and cycles != [] do - Enum.find(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(cycle.cycle_start, today) in [:lt, :eq] and - Date.compare(today, cycle_end) in [:lt, :eq] - - _ -> - false - end - end) + 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 def get_last_completed_cycle(member) do today = Date.utc_today() @@ -634,32 +637,42 @@ defmodule Mv.Membership.Member do if is_list(cycles) and cycles != [] do cycles - |> Enum.filter(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 must have ended (cycle_end < today) - Date.compare(today, cycle_end) == :gt - - _ -> - false - end - end) - |> Enum.sort_by( - fn cycle -> - interval = Map.get(cycle, :membership_fee_type).interval - Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) - end, - {:desc, Date} - ) + |> 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 def get_overdue_cycles(member) do today = Date.utc_today() @@ -668,30 +681,31 @@ defmodule Mv.Membership.Member do cycles = Map.get(member, :membership_fee_cycles) if is_list(cycles) and cycles != [] 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) + 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 defp regenerate_cycles_on_type_change(member) do - alias Mv.MembershipFees.MembershipFeeCycle - alias Mv.MembershipFees.CycleGenerator - alias Mv.MembershipFees.CalendarCycles - require Ash.Query today = Date.utc_today() @@ -699,61 +713,74 @@ defmodule Mv.Membership.Member do # Find all unpaid cycles for this member # We need to check cycle_end for each cycle using its own interval all_unpaid_cycles_query = - MembershipFeeCycle + 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} -> - # Filter cycles that haven't ended yet (cycle_end >= today) - # These are the "future" cycles that should be regenerated - # Use each cycle's own interval to calculate cycle_end - cycles_to_delete = - Enum.filter(all_unpaid_cycles, fn cycle -> - case cycle.membership_fee_type do - %{interval: interval} -> - cycle_end = CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) - Date.compare(today, cycle_end) in [:lt, :eq] - - _ -> - false - end - end) - - # Delete future unpaid cycles - if Enum.empty?(cycles_to_delete) do - # No cycles to delete, just regenerate - case CycleGenerator.generate_cycles_for_member(member.id) do - {:ok, _cycles} -> :ok - {:error, reason} -> {:error, reason} - end - else - delete_results = - Enum.map(cycles_to_delete, fn cycle -> - Ash.destroy(cycle) - end) - - # Check if any deletions failed - if Enum.any?(delete_results, &match?({:error, _}, &1)) do - {:error, :deletion_failed} - else - # Regenerate cycles with new type/amount - # CycleGenerator uses its own transaction with advisory lock - # It will reload the member, so it will see the deleted cycles are gone - # and the new membership_fee_type_id - case CycleGenerator.generate_cycles_for_member(member.id) do - {:ok, _cycles} -> :ok - {:error, reason} -> {:error, reason} - end - end - end + cycles_to_delete = filter_future_cycles(all_unpaid_cycles, today) + delete_and_regenerate_cycles(cycles_to_delete, member.id) {: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 + defp delete_and_regenerate_cycles(cycles_to_delete, member_id) do + if Enum.empty?(cycles_to_delete) do + # No cycles to delete, just regenerate + regenerate_cycles(member_id) + else + case delete_cycles(cycles_to_delete) do + :ok -> regenerate_cycles(member_id) + {: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 + # CycleGenerator uses its own transaction with advisory lock + defp regenerate_cycles(member_id) do + case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member_id) do + {:ok, _cycles} -> :ok + {: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 index 7bfbeee..213efb8 100644 --- a/lib/membership_fees/changes/validate_same_interval.ex +++ b/lib/membership_fees/changes/validate_same_interval.ex @@ -37,29 +37,35 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do current_type_id = get_current_type_id(changeset) new_type_id = get_new_type_id(changeset) - # If no current type, allow any change (first assignment) - if is_nil(current_type_id) do - changeset - else - # If new type is nil, that's allowed (removing type) - if is_nil(new_type_id) do + cond do + # If no current type, allow any change (first assignment) + is_nil(current_type_id) -> changeset - else - # Both types exist - validate intervals match - 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} -> - # If we can't load the types, allow the change (fail open) - # The database constraint will catch invalid foreign keys - changeset + # If new type is nil, that's allowed (removing type) + is_nil(new_type_id) -> + 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 - end + + {:error, _reason} -> + # If we can't load the types, allow the change (fail open) + # The database constraint will catch invalid foreign keys + changeset end end diff --git a/test/membership_fees/changes/validate_same_interval_test.exs b/test/membership_fees/changes/validate_same_interval_test.exs index af777fa..46d1da6 100644 --- a/test/membership_fees/changes/validate_same_interval_test.exs +++ b/test/membership_fees/changes/validate_same_interval_test.exs @@ -215,8 +215,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do defp extract_error_message(%Ash.Error.Invalid{errors: errors}) do errors |> Enum.filter(&(&1.field == :membership_fee_type_id)) - |> Enum.map(& &1.message) - |> Enum.join(" ") + |> Enum.map_join(" ", & &1.message) end end end