Files
sure/app/controllers/goals_controller.rb
T
b6029c1e28 feat(goals): show what each account still has room to earmark (#3166)
* feat(goals): show what each account still has room to earmark

`Account#free_to_earmark` has existed, unused, since earmarks shipped —
its own comment said the UI was a follow-up. This is that follow-up, and
the wording is the substance of it.

It does not say "over-allocated". `free_to_earmark` is negative for as
long as the saving is unfinished, which is the normal condition of anyone
with goals in progress: a 6,000 account backing two goals of 5,000 gives
−4,000 and is a perfectly correct setup. A warning phrased as a fault
would fire permanently and teach people to ignore it. The message states
the consequence instead — the goals come to X for a balance of Y, so they
progress pro rata — and is never styled as an error.

The trap is the goal being edited. `goal_earmarked_total` counts every
goal including that one, so reopening a goal that earmarks 5,000 on a
6,000 account shows 1,000 of headroom, and re-entering the same 5,000
trips a message about a setup the user has not touched.
`earmarked_by_other_goals` excludes it, and only when it is persisted —
a goal being created has nothing to exclude.

The pool is read once per render and passed down, never per account: the
form lists every fundable account the user can see. A test counts the
query and fails at two.

The Stimulus controller is its own, with 3 targets. goal_form_controller
is at 10 against the 7 the project guidelines suggest, needs none of this
state, and is untouched.

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

* fix(goals): read the typed amount strictly, and format it in the app's locale

Addresses review feedback on #3166.

`Number.parseFloat` accepts prefixes, so "500abc" became 500, and the bare
comma-to-dot swap turned a thousands-separated "1,500" into 1.5. Either way the
preview described an amount the user had not typed — and the second case is a
habit from another locale, not a typo, so it would have gone unnoticed. The
value now has to match a complete number before anything is computed.

`Intl.NumberFormat(undefined, ...)` let the BROWSER pick the locale, so a
French user on an English-locale browser read separators and symbol placement
matching nothing else on the page. The amounts cannot be formatted server-side
— they change with every keystroke — so the server passes `I18n.locale` and the
client applies it. That puts the decision where the rest of the app's
formatting already lives.

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

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

* fix(goals): let the assistant create a second goal on a claimed account

Review on #3166. The function always built whole-account links and had no
way to express an earmark, so once exclusivity landed, asking for a second
goal on an account another goal already claimed came back as a bare
`validation_failed` — while the account list still advertised the account
as available. A common request became an unexplained refusal.

Three changes, and the list is the important one: it now says what is left
on each account and which are claimed in full, because the assistant
reasons from that list and had no way to know otherwise.

`earmarks` is an optional map of account name to amount, so the assistant
can reserve a slice rather than the whole balance. Accounts left out keep
the previous behaviour and take whatever is spare.

The refusal is named before the save — `account_claimed_in_full`, with the
account names — so the assistant gets a reason it can act on and ask about,
rather than a validation message it can only relay. Checked after the
currency check, which is the more fundamental of the two.

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

* test(goals): move the spend tests back out of the private section

The merge of `main` into this branch landed #3176's tests between
`count_pool_queries` and the helpers below it, inside the `private`
section and at the wrong indentation. `ci / lint` has been failing on
`Layout/IndentationConsistency` since.

They still ran — `test` is a class method, so `private` does not hide them
— which is why the unit job stayed green while lint went red.

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

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
2026-08-26 21:12:06 +02:00

395 lines
16 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
@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
amount = params[:amount].to_d
@goal.consume!(amount, account: consumption_account)
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)
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
# 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.
# The goal's links narrowed to what the VIEWER may see. A goal can be
# backed by a private account, and both halves of that leak matter: the
# dialog would name an account the reader is not allowed to know exists,
# and a direct 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 consumption_account
eligible = eligible_consumption_accounts
raise Goal::ConsumptionRefused.new(:account_not_linked) if eligible.empty?
if params[:account_id].present?
return eligible.find { |account| account.id.to_s == params[: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