Files
sure/app/controllers/goals_controller.rb
T
buzzromainandClaude Opus 5 74bb980271 feat(goals): a reserve measured in months of spending, not a fixed sum (#3180)
* feat(goals): a reserve measured in months of spending, not a fixed sum

"Six months of expenses" is the way people actually describe an emergency
fund, and it is a moving number: what covered six months last January
does not cover six months today. A reserve pinned to a figure typed once
drifts quietly out of date, and the drift always runs the wrong way — the
bar reads full while the cover shrinks.

`target_amount` stays the single source of truth, rewritten monthly by
RefreshMaintainedGoalTargetsJob. That is the whole architectural decision
here. An `effective_target_amount` would have been the obvious shape and
the wrong one: `remaining_amount`, `progress_percent`, `Goal.summary_for`,
the ring, the card and every future caller would each have had to learn
which target to read. None of them change.

The job refuses to write more often than it writes, on purpose:

- a family with no spending history yet computes a floor of zero, which
  would both violate the `target_amount > 0` constraint and read to the
  user as "your reserve is complete". The previous target stands.
- a figure identical to the current one is not rewritten, so a reserve
  does not collect a fresh updated_at every month for nothing.
- a write that fails validation leaves the target alone and is recorded
  through DebugLogEntry, not just the application log: a reserve frozen
  at a stale floor is invisible to the user, who has no reason to suspect
  the number stopped moving.

The job reads the family's spending, not a member's view. IncomeStatement
falls back to Current.user when nobody says otherwise, which in a
background job is nobody — so the scope is the whole family, and the
number is the same whoever is looking. That is deliberate, and matches
how the rollover chain had to be pinned.

`target_months` is refused outside a months-mode reserve rather than
tolerated: a number nothing reads would sit there looking meaningful,
and the job would skip it for reasons no one could see.

schema.rb is hand-edited again — verified against a real migration on a
throwaway database, structures identical. The dumper on this Rails
version also rewrites every check-constraint cast, so the new constraint
is written in the file's existing style rather than the dumper's.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ1npaGEHr6t2HW1rYZdt4

* fix(goals): pin the reserve calculation to the family, and get it right on day one

Review of the previous commit raised two things, and they are the same
thing seen from either end.

`IncomeStatement.new(family)` looked family-wide but was only so by
accident. Its constructor falls back to `Current.user`, and eligible_accounts
narrows to that user's accounts when one is present. The single caller was
a background job, where nobody is current — so the figure was correct for
the reason that it happened to be computed nowhere else. `target_amount`
belongs to the whole family: derived from a viewer's slice of the accounts,
it would have started moving with whoever last triggered it. This is the
same fallback that made the budget rollover carry depend on its reader.
The account scope is now passed explicitly, so the calculation is safe
whatever calls it.

That mattered immediately, because the second point required a new caller.
A reserve created as "6 months of expenses" had no floor computed until
the 1st of the following month: the user chose the mode, guessed an
amount, and lived with a wrong target for up to a month. The feature's
first impression was its least convincing moment. The floor is now
computed on creation, and whenever the mode or the number of months
changes — but never on an unrelated save, so the monthly job keeps owning
the cadence and renaming a goal cannot silently move a financial figure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJ1npaGEHr6t2HW1rYZdt4

* fix(goals): keep a months-based floor derived, and in the right currency

Review on #3180.

The median comes back in FAMILY currency and `target_amount` is stored in
the GOAL's, so a EUR reserve in a USD family read a 3,000 dollar floor as
3,000 euros — and rewrote it that way every month, silently. Converted
now, and when there is no rate for the day the previous target stands:
the same safe failure the method already took for a family with no
spending history, because a stale floor beats a wrong one.

The callback also skipped a target_amount edit, so the form could persist
an arbitrary figure under a "six months of expenses" label until the next
monthly refresh. It runs on that edit now and overwrites it — and when
there is nothing to derive from, restores what the reserve already had
rather than accepting the typed figure.

The form marks the field read-only in that mode. The model does not depend
on it, but a field that silently discards what you type is worse than one
you cannot type into.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-26 22:02:57 +02:00

417 lines
17 KiB
Ruby
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
class GoalsController < ApplicationController
before_action :require_preview_features!
before_action :set_goal, only: %i[show edit update destroy pause resume complete archive unarchive reopen consume record_consumption]
FUNDABLE_TYPES = Goal::FUNDABLE_ACCOUNT_TYPES
rescue_from ActiveRecord::RecordNotFound, with: :goal_not_found
STATE_FILTERS = %w[all active paused completed archived].freeze
def index
state_counts = Current.family.goals.group(:state).count
@counts = STATE_FILTERS.each_with_object({}) do |state, h|
h[state] = state == "all" ? state_counts.values.sum : (state_counts[state] || 0)
end
# Preloads + the family-wide backing-math injection (N+1 guard) live in
# Goal.prepared_for, shared with the Plan hub's active_prepared_for.
all_goals = Goal.prepared_for(Current.family)
@active_goals = Goal.active_display_sort(all_goals.reject { |g| %w[completed archived].include?(g.state) })
@completed_goals = all_goals.select { |g| g.state == "completed" }.sort_by { |g| g.name.downcase }
@archived_goals = all_goals.select { |g| g.state == "archived" }
# Completed goals join the chip-filterable grid below the active ones
# so the `completed` chip can isolate them. Archived stays in a
# separate collapsed-by-default section, opted out of the filter
# entirely (rendered with filterable: false).
@grid_goals = @active_goals + @completed_goals
@linkable_account_count = Current.user.accessible_accounts.where(accountable_type: FUNDABLE_TYPES).visible.count
@kpi = kpi_payload(@active_goals)
@any_pending_pledge = @active_goals.any? { |g| g.open_pledges.any? }
@show_search = @grid_goals.size > 6
@breadcrumbs = plan_breadcrumb_prefix + [ [ t("goals.index.title"), nil ] ]
end
def show
@open_pledges = @goal.open_pledges.reverse_chronological.to_a
@unattributed_outflows = withdrawal_detector.unattributed_outflows.to_a
@breadcrumbs = plan_breadcrumb_prefix + [
[ t("goals.index.title"), goals_path ],
[ @goal.name, nil ]
]
end
def new
@goal = Current.family.goals.new(
color: Goal::COLORS.sample,
currency: Current.family.primary_currency_code
)
@linkable_accounts = linkable_accounts_for_new
@currently_linked_account_ids = []
@pooled_allocations = Goal.pooled_allocations_for(Current.family)
@breadcrumbs = plan_breadcrumb_prefix + [
[ t("goals.index.title"), goals_path ],
[ t("goals.new.heading"), nil ]
]
end
def create
@goal = Current.family.goals.new(goal_params)
accounts = lookup_accounts(params.dig(:goal, :account_ids))
@goal.currency = (accounts.first&.currency || Current.family.primary_currency_code) if @goal.currency.blank?
allocations = submitted_allocations
Goal.transaction do
accounts.each { |a| @goal.goal_accounts.build(account: a, allocated_amount: allocations[a.id.to_s]) }
@goal.save!
end
flash[:notice] = t(".success")
respond_to do |format|
format.html { redirect_to goal_path(@goal) }
format.turbo_stream do
render turbo_stream: turbo_stream.action(:redirect, goal_path(@goal))
end
end
rescue ActiveRecord::RecordInvalid
@linkable_accounts = linkable_accounts_for_new
# From the in-memory links, not `pluck`: nothing is persisted on a failed
# create, so a query would come back empty and every box the user ticked
# would render unchecked. The amounts survived — the form reads those off
# the same built records — so the user was left staring at an error telling
# them to enter an amount, on a form whose accounts had silently cleared.
@currently_linked_account_ids = @goal.goal_accounts.map { |ga| ga.account_id.to_s }
@pooled_allocations = Goal.pooled_allocations_for(Current.family)
render :new, status: :unprocessable_entity
end
def edit
@linkable_accounts = linkable_accounts_for_new
@pooled_allocations = Goal.pooled_allocations_for(Current.family)
@currently_linked_account_ids = @goal.goal_accounts.pluck(:account_id).map(&:to_s)
end
def update
account_ids = params.dig(:goal, :account_ids)
accounts_supplied = !account_ids.nil?
accounts = accounts_supplied ? lookup_accounts(account_ids) : []
if accounts_supplied && accounts.empty?
@goal.errors.add(:base, :at_least_one_linked_account_required)
@linkable_accounts = linkable_accounts_for_new
@pooled_allocations = Goal.pooled_allocations_for(Current.family)
@currently_linked_account_ids = @goal.goal_accounts.pluck(:account_id).map(&:to_s)
render :edit, status: :unprocessable_entity
return
end
# Assign first, sync the links, then persist once. Saving the attributes
# up front ran `must_have_at_least_one_linked_account` against the goal's
# OLD links, so a goal orphaned by account deletion (Account has_many
# :goal_accounts, dependent: :destroy) could never be edited back to
# health: the save raised before the submitted accounts were attached.
# One save over the fully-assembled goal validates what the user actually
# submitted.
Goal.transaction do
@goal.assign_attributes(goal_update_params)
if accounts_supplied
sync_linked_accounts!(@goal, accounts, submitted_allocations)
else
@goal.save!
end
end
flash[:notice] = t(".success")
respond_to do |format|
format.html { redirect_to goal_path(@goal) }
format.turbo_stream do
render turbo_stream: turbo_stream.action(:redirect, goal_path(@goal))
end
end
rescue ActiveRecord::RecordInvalid
@linkable_accounts = linkable_accounts_for_new
@pooled_allocations = Goal.pooled_allocations_for(Current.family)
@currently_linked_account_ids = @goal.goal_accounts.pluck(:account_id).map(&:to_s)
render :edit, status: :unprocessable_entity
end
# Deletable from any state. Destroying a goal cascades only to its own
# goal_accounts / goal_pledges — and GoalPledge#clear_matched_transaction_extra
# unstamps the pledge id it wrote onto a matched transaction. No account,
# balance, entry or transaction is removed, so the archive-first gate this
# used to enforce bought no safety; it only hid the action behind a two-step
# flow no other Sure resource requires.
def destroy
@goal.destroy!
redirect_to goals_path, notice: t(".success")
end
def pause
perform_transition!(:pause)
end
def resume
perform_transition!(:resume)
end
def complete
perform_transition!(:complete)
end
def archive
perform_transition!(:archive)
end
def unarchive
perform_transition!(:unarchive)
end
# Renders the dialog. The write lives in its own action below.
def consume
@consumption_accounts = eligible_consumption_accounts
end
def record_consumption
txn = consumption_transaction
amount = txn ? txn.entry.amount.to_d : params[:amount].to_d
@goal.consume!(amount, account: consumption_account(txn), transaction: txn)
redirect_to goal_path(@goal),
notice: t("goals.consume.success", amount: Money.new(amount, @goal.currency).format)
rescue Goal::ConsumptionRefused => e
redirect_to goal_path(@goal), alert: t("goals.consume.errors.#{e.reason}")
rescue ActiveRecord::RecordInvalid => e
redirect_to goal_path(@goal), alert: e.record.errors.full_messages.to_sentence
end
def reopen
perform_transition!(:reopen)
end
private
def set_goal
@goal = Current.family.goals
.includes(:open_pledges, linked_accounts: :account_providers)
.find(params[:id])
end
def goal_not_found
redirect_to goals_path, alert: t("goals.errors.not_found")
end
def goal_params
params.require(:goal).permit(:name, :target_amount, :target_date, :color, :icon, :notes, :kind, :target_mode, :target_months)
end
def goal_update_params
params.require(:goal).permit(:name, :target_amount, :target_date, :color, :icon, :notes, :kind, :target_mode, :target_months)
end
def lookup_accounts(ids)
return [] if ids.blank?
ids = Array(ids).reject(&:blank?)
Current.user.accessible_accounts.where(accountable_type: FUNDABLE_TYPES).visible.where(id: ids).to_a
end
def linkable_accounts_for_new
Current.user.accessible_accounts.where(accountable_type: FUNDABLE_TYPES).visible.alphabetically.to_a
end
def sync_linked_accounts!(goal, accounts, allocations = {})
desired_ids = accounts.map(&:id).to_set
current_ids = goal.goal_accounts.pluck(:account_id).to_set
# Only unlink accounts the current user can actually see in the picker.
# A family goal may be linked to another member's private account, which
# never renders as a checkbox — so its absence from the submitted set is
# not an intentional removal and must not destroy the link.
removable_ids = Current.user.accessible_accounts.where(id: current_ids.to_a).pluck(:id).to_set
((current_ids & removable_ids) - desired_ids).each do |id|
goal.goal_accounts.where(account_id: id).destroy_all
end
goal.goal_accounts.reload
# Add new links and refresh the earmark on kept links. Only touch the
# allocation when the form actually submitted a value for that account
# (allocations.key?), so a caller that omits the hash leaves earmarks
# untouched. Save through the goal so currency / depository / family
# validations fire — create! on goal_accounts bypasses them.
accounts.each do |account|
existing = goal.goal_accounts.find { |ga| ga.account_id == account.id }
if existing
existing.allocated_amount = allocations[account.id.to_s] if allocations.key?(account.id.to_s)
else
goal.goal_accounts.build(account: account, allocated_amount: allocations[account.id.to_s])
end
end
goal.save!
end
# { account_id_string => amount_string_or_nil } from goal[allocations].
# A blank amount means "dedicate the whole balance" (NULL allocated_amount).
def submitted_allocations
raw = params.dig(:goal, :allocations)
return {} if raw.blank?
hash = raw.respond_to?(:to_unsafe_h) ? raw.to_unsafe_h : raw
hash.each_with_object({}) do |(account_id, amount), memo|
memo[account_id.to_s] = amount.to_s.strip.presence
end
end
def kpi_payload(active_goals)
family = Current.family
currency = family.primary_currency_code
today = Date.current
windows = family.savings_inflow_windows(window_days: 30, now: today)
velocity_30d = windows[:current]
velocity_prior_30d = windows[:prior]
delta_amount = velocity_30d - velocity_prior_30d
delta_percent = velocity_prior_30d.zero? ? nil : ((delta_amount / velocity_prior_30d.abs) * 100).round(1)
# Sign decoupling: the headline-amount sign reflects this month's
# direction ("$200 last 30d" = net outflow); the delta direction
# (↑/↓ vs prior 30d) goes on the subline. Conflating them produced the
# "$1234" + "↓ 27%" tile where the minus looked like a loss but the
# $1234 was actually the (positive) amount contributed.
headline_sign = velocity_30d.negative? ? "" : ""
delta_direction = if delta_amount.positive? then :up
elsif delta_amount.negative? then :down
else :flat
end
# behind_pace? excludes paused goals — pausing stops the pace clock,
# so a paused-but-behind goal belongs in the paused count (below),
# not in "N behind" or the "needs this month" sum. Keeps this tile
# consistent with the Plan hub's summary (Goal.summary_for).
needs = active_goals
.select(&:behind_pace?)
.sum { |g| g.monthly_target_amount.to_d }
behind = active_goals.count(&:behind_pace?)
# Paused goals are excluded from the on-track numerator for the same
# reason they're excluded from tracked_total below — otherwise the
# "X of Y" fraction could exceed its own denominator.
on_track = active_goals.count { |g| !g.paused? && g.status == :on_track }
reached = active_goals.count { |g| g.status == :reached }
no_date = active_goals.count { |g| g.status == :no_target_date }
paused = active_goals.count(&:paused?)
# Denominator of the "Goals on track" tile. A goal only belongs in
# the fraction if there is a benchmark to compare against:
# - reached → target already hit, no longer tracked toward pace
# - paused → user stopped the pace clock on purpose
# - no_target_date → open-ended saving (emergency fund, sabbatical
# fund, etc.) has no required monthly pace, so "on track" is
# undefined. Counting it would penalise the user for having
# open-ended goals — they'd never improve the ratio.
# - maintained → a reserve has no deadline and no pace benchmark at
# all. Its statuses are `funded`/`depleted`, which match none of the
# exclusions above, so without this it would land in the denominator
# and never in the numerator: a family with one reserve read
# "0 of 1 on track" for a goal that is working exactly as intended.
# When this hits zero the tile swaps to a celebration / empty
# state in the view.
tracked_total = active_goals.count do |g|
!g.paused? && !g.maintained? && g.status != :reached && g.status != :no_target_date
end
{
currency: currency,
velocity_30d_money: Money.new(velocity_30d.abs, currency),
velocity_prior_30d_money: Money.new(velocity_prior_30d.abs, currency),
velocity_30d_sign: headline_sign,
velocity_delta_percent: delta_percent,
velocity_direction: delta_direction,
needs_this_month_money: Money.new(needs, currency),
on_track_count: on_track,
reached_count: reached,
behind_count: behind,
no_date_count: no_date,
paused_count: paused,
tracked_total: tracked_total,
active_total: active_goals.size
}
end
# A blank id means "the goal has one link, use it" and the model decides
# whether that is true. An id that resolves to nothing is refused here
# rather than falling back to nil: on a single-link goal, nil would consume
# from that link and the user would see a spend recorded against an account
# they did not name.
# Attributing a detected outflow: the transaction says both how much and
# from where, so neither is taken from the form.
def consumption_transaction
return nil if params[:transaction_id].blank?
withdrawal_detector.unattributed_outflows(limit: 50)
.find_by(entryable_id: params[:transaction_id])
&.entryable ||
raise(Goal::ConsumptionRefused.new(:transaction_not_found))
end
# The goal's links narrowed to what the VIEWER may see. A goal can be
# backed by a private account, and every half of that leak matters: the
# dialog would name an account the reader is not allowed to know exists,
# the outflow panel would list its transactions, and a POST would reduce
# its earmark — the figures moving afterwards saying how much was in it.
def eligible_consumption_accounts
@eligible_consumption_accounts ||= begin
linked = @goal.linked_accounts
visible_ids = Current.user.accessible_accounts.where(id: linked.map(&:id)).pluck(:id).to_set
linked.select { |account| visible_ids.include?(account.id) }
end
end
def withdrawal_detector
Goal::WithdrawalDetector.new(@goal, accounts: eligible_consumption_accounts)
end
def consumption_account(txn = nil)
eligible = eligible_consumption_accounts
raise Goal::ConsumptionRefused.new(:account_not_linked) if eligible.empty?
# An attributed outflow names its own account; a typed spend names one
# only when the goal has a choice to offer.
account_id = txn ? txn.entry.account_id : params[:account_id]
if account_id.present?
return eligible.find { |account| account.id.to_s == account_id.to_s } ||
raise(Goal::ConsumptionRefused.new(:account_not_linked))
end
# Named explicitly even when the goal has several links, because the one
# the viewer can reach is not necessarily the one the model would pick on
# its own. With more than one eligible link it stays nil, and the model
# refuses rather than guessing.
eligible.size == 1 ? eligible.first : nil
end
def perform_transition!(event)
unless @goal.aasm.may_fire_event?(event)
redirect_to goal_path(@goal), alert: t(".invalid_transition")
return
end
# AASM's bang event returns false — it does NOT raise — when the save
# that persists the new state fails validation. The return value used to
# be discarded, so an invalid goal (e.g. orphaned by account deletion)
# flashed "Goal archived." while the state never moved. Surface the
# validation error instead of claiming success.
unless @goal.public_send("#{event}!")
redirect_to goal_path(@goal),
alert: @goal.errors.full_messages.to_sentence.presence || t(".invalid_transition")
return
end
respond_to do |format|
format.html { redirect_to goal_path(@goal), notice: t(".success") }
format.turbo_stream do
render turbo_stream: turbo_stream.action(:redirect, goal_path(@goal))
end
end
end
end