Files
sure/app/controllers/goals_controller.rb
T
buzzromainandClaude Opus 5 6d7ca3584c feat(goals): reserves you maintain, not goals you finish (#3167)
* feat(goals): reserves you maintain, not goals you finish

An emergency fund is not a goal you reach and close — it is a level you
hold, and every withdrawal is a shortfall to make good. Sure treated it
like anything else: at 100% it offered to close it, which would release
the very money being set aside; a withdrawal dropped the bar with no
sign that anything was owed.

`kind` (added by the lifecycle lot without behavior) now means something.
A maintained goal is `funded` or `depleted`, never `reached` — sitting at
its floor is a steady state, not an achievement to file away. `complete`
is refused by an AASM guard rather than merely hidden, so no path can
release a reserve's earmark.

Two ordering traps, both of which would have made a drained reserve
invisible:

`ACTIVE_DISPLAY_STATUS_RANK` falls back to 4 for any status it does not
know, so an unranked `:depleted` would sort a drained emergency fund
below everything else — the exact opposite of what it means. It ranks
alongside `:behind` now, and `:funded` sorts near the end with the goals
that need nothing.

`behind_pace?` excludes reserves. `monthly_target_amount` and `pace` both
derive from `target_date`, which a reserve does not have, so "save X/month
to catch up" would be advice about a deadline that does not exist.

The form leads with the choice, since it changes what the rest of it
means, and hides the target date for a reserve rather than disabling it —
a hidden field cannot submit a stale value that would then drive a pace.
The card states the shortfall, which is exactly `remaining_amount`. The
panel that offers a one-off its closing action tells a reserve it is
intact and offers nothing, because there is nothing to do.

Scope: fixed targets only. Targets expressed in months of expenses, the
monthly refresh job, and the depletion insight are the next two PRs.

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

* fix(goals): let a reserve behave like one everywhere it is shown

Addresses review feedback on #3167.

The kind selector never hid the target date. `data-controller="goal-kind"`
sat on the selector div while its `dateField` target is a sibling, so
`dateFieldTargets` came back empty and picking "Reserve to maintain" left the
deadline on screen and submittable. The controller moves to the form wrapper,
which encloses both.

Hiding a field is not enforcement, so the model now clears `target_date` for a
maintained goal. Normalising rather than rejecting: the field is hidden, and an
error about something the user cannot see is not actionable. A date could only
arrive through a conversion or a crafted request, and either way a stored
deadline would drive a pace the reserve does not have.

A completed goal could be switched to `maintained` from the edit form. It then
sat in a released state — one that has handed its earmark back — while the show
page promised its money stays reserved, and `complete` for reserves is refused
precisely to prevent that state. `kind` is now locked while released: reopen
first.

Reserves counted against the "goals on track" tile. Their statuses are
`funded`/`depleted`, which match none of the exclusions in `tracked_total`, so
they could never reach the numerator and a family with one reserve read
"0 of 1 on track" for a goal working exactly as intended.

Two more places still spoke of pace to something that has none. `pace_line` is
suppressed for reserves on the card, and a depleted reserve gets its own panel
before the projection card — the projection's summary, catch-up line and colour
are all built from a deadline. What a drained reserve needs is the number the
projection cannot show: how much is missing from the floor.

The French celebration copy read "Votre réserve est à son niveau", which never
says which level.

Each guard was confirmed load-bearing by removing it and watching its test
fail. bin/rails test: 6962 runs, 28003 assertions, 0 failures. RuboCop,
erb_lint and Brakeman clean.

Left open deliberately: extracting the show page's lifecycle panel into a
ViewComponent. The guideline behind it is right, but the refactor is wider than
this round of fixes and belongs on its own.

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

* fix(goals): finish teaching the status consumers about reserves

Second round of review feedback on #3167: two consumers still had no branch
for the reserve statuses.

`ProgressRingComponent#percent_text_class` styled only `:reached` as success,
so a funded reserve — a floor the user is holding exactly as intended — fell
back to the neutral colour and read as unfinished.

`status_callout_context` had no `:depleted` branch, so a drained reserve showed
no callout at all: the one status that most deserves a line of explanation was
the only one saying nothing. It now names the shortfall.

`:funded` deliberately keeps no callout — a reserve at its level has nothing to
report, and the celebration panel already says so. A test pins that, so the
silence reads as a decision rather than another missing branch.

bin/rails test: 6964 runs, 28008 assertions, 0 failures. RuboCop and Brakeman
clean.

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

* fix(goals): lock the kind on the state the goal is actually in

Addresses review feedback on #3167, on the guard that landed in c1c7f4f7.

`kind_locked_while_released` read the in-memory `state`, so a single write
setting `state: "active"` alongside the new kind saw the goal as already
reopened and waved it through.

The end state looks legitimate — active and maintained — which is why the hole
is easy to miss. It is not: the direct write skipped the `reopen` transition,
and with it `thaw_completed_amount!`. `completed_amount` survived, so
`current_balance` returned that frozen snapshot forever on a live reserve.
Reopening has to be its own gesture, because it is the gesture that thaws.

Now reads `state_in_database`, with a regression test on the combined write
asserting both that it is refused and that the frozen amount is untouched.

Confirmed load-bearing by reading the attribute again and watching it fail.
bin/rails test: 6969 runs, 28019 assertions, 0 failures. RuboCop and Brakeman
clean.

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

* fix(goals): send an empty reserve to the shortfall panel, not the empty state

Addresses the last review thread on #3167.

The `maintained?` branch sat after the zero-balance/zero-pace one, so a
brand-new reserve matched the generic "make your first transfer" card. I had
put it there on purpose, thinking a reserve with nothing in it wanted the
first-transfer nudge. The review is right that it does not: it is still a
reserve short of its floor, and the shortfall panel says so with the saved,
target and missing amounts, where the generic card says none of them.

Ordering it after also meant evaluating `pace` on a goal that has no pace to
evaluate.

bin/rails test: 6965 runs, 0 failures. RuboCop and erb_lint clean.

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

* fix(goals): stop a paused goal outranking a reserve that is whole

Review on #3175.

`:funded` and paused both ranked 3 in `active_display_sort`, so the tie
broke on name and a paused goal called "Alpha" sat above a reserve called
"Zeta" that was fully funded — the list saying the paused one wanted
attention more. Paused now ranks behind every status, which is what the
comment above the table already claimed.

The seven panels on the goal page were hand-rolled repetitions of
`DS::Card`'s exact shell, two of them adjacent and identical. They render
through the primitive now, so their surface styling cannot drift apart.

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

* refactor(goals): move the lifecycle panel decision out of the template

Review on #3167 and #3180.

Which panel a goal gets is a lifecycle question with five answers, and the
template worked it out inline from `completed?`, `maintained?`, `one_off?`,
`status` and `may_complete?` — five predicates deep in ERB where the
ordering between them was load-bearing and nothing said so.

`Goals::LifecyclePanelComponent` answers it in Ruby and the template
renders the answer. The markup moves across unchanged, keys made absolute
because a relative `t(".x")` in a component resolves against the
component's own path rather than the page these strings belong to.

The order is now stated once, where it can be read and tested:
`:reserve_shortfall` before `:empty`, because a brand-new reserve sits at
zero balance and zero pace and the generic "make your first transfer" card
would otherwise swallow it.

Closing from the panel now confirms, as the header menu already did.
Completing releases the goal's earmarked money, and the panel offered that
in one click. Both go through `goal_complete_confirm` rather than building
the wording twice — two copies drifting apart is how one ends up
describing the wrong consequence.

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

* fix(goals): refresh the pace suggestion when the deadline is cleared

Assigning `input.value = ""` fires no event, so `goal-form#suggestedChanged`
never ran: selecting "Reserve to maintain" cleared the date but left the
monthly pace suggestion on screen, derived from a deadline the goal no
longer has.

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

* fix(goals): let a depleted reserve look as urgent as it is

Review on #3179 and #3180.

`Goal#needs_attention?` names the pair of statuses that mean "this one wants
looking at" — a goal off its pace and a reserve below its floor. Three
places were spelling that out and the Plan hub's progress bar had fallen
behind, so a depleted reserve got a neutral bar an inch from its own amber
status pill: the same goal reported as needing attention and not.

`projection_summary` told a funded reserve it had "hit the target, no
projection needed". A reserve holds a level; there is no finish line to
project toward and no target to have hit. It does not reach that panel
today — the shortfall and celebration panels catch it first — but the
method reads as the single source of truth for that subtitle and should not
hand a caller a one-off's wording.

The legend swatches are bordered spans now rather than inline SVG, and the
label takes `text-xs` instead of an arbitrary 11px. The projection swatch
keeps the chart's own colour variables in an inline style rather than
`border-success` / `border-warning`: the chart hard-codes green-600 and
yellow-600, and a legend whose colour does not match the line it describes
is worse than the markup it would save.

The Plan-card test counts warning bars rather than matching one. A fixture
goal is already off its pace, so the markup is on the page either way and a
presence check passed without the fix.

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 08:20:43 +02:00

340 lines
14 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]
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
@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 = []
@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 }
render :new, status: :unprocessable_entity
end
def edit
@linkable_accounts = linkable_accounts_for_new
@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
@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
@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
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)
end
def goal_update_params
params.require(:goal).permit(:name, :target_amount, :target_date, :color, :icon, :notes, :kind)
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
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