feat(membership-fees): denormalize cycle_end so fee status filters run in the database
Store the cycle's end date alongside its start so the payment-status filter can be a plain date comparison in SQL instead of loading every cycle and classifying it in memory. The value is fixed when the cycle is created (the fee type's interval is immutable and the update action never changes the start date), so the denormalized column can never drift from the computed end and needs no write-time reconciliation.
This commit is contained in:
parent
a629bfb617
commit
b745b13ca5
7 changed files with 448 additions and 4 deletions
53
lib/membership_fees/cycle_end_backfill.ex
Normal file
53
lib/membership_fees/cycle_end_backfill.ex
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
defmodule Mv.MembershipFees.CycleEndBackfill do
|
||||
@moduledoc """
|
||||
One-time backfill for the denormalized `cycle_end` column on
|
||||
`membership_fee_cycles`.
|
||||
|
||||
For every cycle whose `cycle_end` is still `NULL`, sets it to
|
||||
`CalendarCycles.calculate_cycle_end(cycle_start, interval)` using the cycle's
|
||||
own fee-type interval. Idempotent: rows that already have a `cycle_end` are
|
||||
left untouched, so it is safe to re-run.
|
||||
|
||||
Invoked from the `cycle_end` migration. Kept as a module function (rather than
|
||||
inline migration SQL) so the exact calendar arithmetic is reused from
|
||||
`CalendarCycles` and can be tested directly.
|
||||
"""
|
||||
|
||||
import Ecto.Query
|
||||
|
||||
alias Mv.MembershipFees.CalendarCycles
|
||||
alias Mv.Repo
|
||||
|
||||
@doc """
|
||||
Backfills `cycle_end` for all cycles missing it. Returns `{:ok, updated_count}`.
|
||||
"""
|
||||
@spec run() :: {:ok, non_neg_integer()}
|
||||
def run do
|
||||
rows =
|
||||
from(c in "membership_fee_cycles",
|
||||
join: t in "membership_fee_types",
|
||||
on: c.membership_fee_type_id == t.id,
|
||||
where: is_nil(c.cycle_end),
|
||||
select: {c.id, c.cycle_start, t.interval}
|
||||
)
|
||||
|> Repo.all()
|
||||
|
||||
count =
|
||||
Enum.reduce(rows, 0, fn {id, cycle_start, interval}, acc ->
|
||||
cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, normalize_interval(interval))
|
||||
|
||||
from(c in "membership_fee_cycles", where: c.id == ^id)
|
||||
|> Repo.update_all(set: [cycle_end: cycle_end])
|
||||
|
||||
acc + 1
|
||||
end)
|
||||
|
||||
{:ok, count}
|
||||
end
|
||||
|
||||
# The raw query returns the enum column as a string.
|
||||
defp normalize_interval(interval) when is_atom(interval), do: interval
|
||||
|
||||
defp normalize_interval(interval) when is_binary(interval),
|
||||
do: String.to_existing_atom(interval)
|
||||
end
|
||||
|
|
@ -13,7 +13,9 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do
|
|||
- `notes` - Optional notes for this cycle
|
||||
|
||||
## Design Decisions
|
||||
- **No cycle_end field**: Calculated from cycle_start + interval (from fee type)
|
||||
- **Denormalized cycle_end**: Set at create from cycle_start + interval (fee type).
|
||||
Because the fee type interval and the cycle_start are immutable after create,
|
||||
cycle_end never drifts, so it can be filtered/sorted in SQL.
|
||||
- **Amount stored per cycle**: Preserves historical amounts when fee type changes
|
||||
- **Calendar-aligned cycles**: All cycles start on calendar boundaries
|
||||
|
||||
|
|
@ -46,6 +48,14 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do
|
|||
create :create do
|
||||
primary? true
|
||||
accept [:cycle_start, :amount, :status, :notes, :member_id, :membership_fee_type_id]
|
||||
|
||||
argument :interval, :atom do
|
||||
allow_nil? true
|
||||
|
||||
description "Optional pre-resolved fee-type interval; when given, SetCycleEnd skips the fee-type lookup"
|
||||
end
|
||||
|
||||
change Mv.MembershipFees.MembershipFeeCycle.Changes.SetCycleEnd
|
||||
end
|
||||
|
||||
update :update do
|
||||
|
|
@ -106,6 +116,13 @@ defmodule Mv.MembershipFees.MembershipFeeCycle do
|
|||
description "Start date of the billing cycle"
|
||||
end
|
||||
|
||||
attribute :cycle_end, :date do
|
||||
allow_nil? false
|
||||
public? true
|
||||
|
||||
description "Denormalized end date of the cycle (cycle_start + interval), set at create"
|
||||
end
|
||||
|
||||
attribute :amount, :decimal do
|
||||
allow_nil? false
|
||||
public? true
|
||||
|
|
|
|||
|
|
@ -0,0 +1,68 @@
|
|||
defmodule Mv.MembershipFees.MembershipFeeCycle.Changes.SetCycleEnd do
|
||||
@moduledoc """
|
||||
Ash change that denormalizes `cycle_end` on a membership fee cycle at create time.
|
||||
|
||||
`cycle_end` is computed as `CalendarCycles.calculate_cycle_end(cycle_start, interval)`,
|
||||
where `interval` is read from the cycle's `membership_fee_type`. Because a fee type's
|
||||
`interval` is immutable and the cycle's `cycle_start` is never updated, the stored
|
||||
`cycle_end` is fixed at create time and never drifts.
|
||||
|
||||
When the caller already knows the interval (e.g. the bulk cycle generator, which
|
||||
resolves the fee type once for a whole batch), it may pass it via the `:interval`
|
||||
create argument; the change then uses that value and issues no fee-type query,
|
||||
avoiding an N+1 lookup across a batch of cycles.
|
||||
|
||||
Otherwise the fee-type interval is read with `authorize?: false`: it is an internal
|
||||
lookup of immutable reference data needed to populate a derived column, not a
|
||||
user-facing read, and cycle creation is itself a systemic operation. Reading it under
|
||||
the caller's actor is unreliable here (the actor is not always present in the change
|
||||
context, e.g. when a changeset is built with `for_create/3` and the actor is only
|
||||
supplied to `Ash.create/2`).
|
||||
|
||||
When `cycle_start` or `membership_fee_type_id` are missing the changeset is returned
|
||||
unchanged so the normal `allow_nil?` validations surface the error.
|
||||
"""
|
||||
use Ash.Resource.Change
|
||||
|
||||
alias Mv.MembershipFees.CalendarCycles
|
||||
alias Mv.MembershipFees.MembershipFeeType
|
||||
|
||||
@impl true
|
||||
def change(changeset, _opts, _context) do
|
||||
with {:ok, cycle_start} <- fetch(changeset, :cycle_start),
|
||||
{:ok, fee_type_id} <- fetch(changeset, :membership_fee_type_id),
|
||||
{:ok, interval} <- resolve_interval(changeset, fee_type_id) do
|
||||
cycle_end = CalendarCycles.calculate_cycle_end(cycle_start, interval)
|
||||
Ash.Changeset.force_change_attribute(changeset, :cycle_end, cycle_end)
|
||||
else
|
||||
_ -> changeset
|
||||
end
|
||||
end
|
||||
|
||||
defp fetch(changeset, field) do
|
||||
case Ash.Changeset.fetch_change(changeset, field) do
|
||||
{:ok, value} when not is_nil(value) ->
|
||||
{:ok, value}
|
||||
|
||||
_ ->
|
||||
case Map.get(changeset.data, field) do
|
||||
nil -> :error
|
||||
value -> {:ok, value}
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
defp resolve_interval(changeset, fee_type_id) do
|
||||
case Ash.Changeset.get_argument(changeset, :interval) do
|
||||
nil -> fetch_interval(fee_type_id)
|
||||
interval -> {:ok, interval}
|
||||
end
|
||||
end
|
||||
|
||||
defp fetch_interval(fee_type_id) do
|
||||
case Ash.get(MembershipFeeType, fee_type_id, authorize?: false) do
|
||||
{:ok, %{interval: interval}} -> {:ok, interval}
|
||||
_ -> :error
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -305,7 +305,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
|||
# Only generate if start_date <= end_date
|
||||
if start_date && Date.compare(start_date, end_date) != :gt do
|
||||
cycle_starts = generate_cycle_starts(start_date, end_date, interval)
|
||||
create_cycles(cycle_starts, member.id, fee_type.id, amount, opts)
|
||||
create_cycles(cycle_starts, member.id, fee_type.id, amount, interval, opts)
|
||||
else
|
||||
{:ok, [], []}
|
||||
end
|
||||
|
|
@ -400,7 +400,7 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
|||
end
|
||||
end
|
||||
|
||||
defp create_cycles(cycle_starts, member_id, fee_type_id, amount, opts) do
|
||||
defp create_cycles(cycle_starts, member_id, fee_type_id, amount, interval, opts) do
|
||||
actor = get_actor(opts)
|
||||
create_opts = Helpers.ash_actor_opts(actor)
|
||||
|
||||
|
|
@ -414,7 +414,10 @@ defmodule Mv.MembershipFees.CycleGenerator do
|
|||
member_id: member_id,
|
||||
membership_fee_type_id: fee_type_id,
|
||||
amount: amount,
|
||||
status: :unpaid
|
||||
status: :unpaid,
|
||||
# Pass the already-resolved interval so SetCycleEnd does not re-fetch the
|
||||
# fee type for every cycle in the batch (avoids an N+1 lookup).
|
||||
interval: interval
|
||||
}
|
||||
|
||||
handle_cycle_creation_result(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue