mirror of
https://github.com/we-promise/sure.git
synced 2026-09-06 07:11:14 +00:00
* feat(goals): surface money that left a goal's accounts unexplained Goals never read transactions. `current_balance` is a stock summed from account balances, so an outflow reaches a goal only as a smaller number, with nothing saying which goal it belonged to. `consume!` closes that gap, but only for a user who thinks to declare it — and the whole difficulty is that they have no reason to think of it. `Goal::WithdrawalDetector` surfaces the outflows nothing has claimed, and the goal page offers them: *if any of this was spent on Trip, say so*. One click records it with the transaction as evidence. This is the pull half of what `GoalPledge` does for money coming in. A pledge asks first and matches later; here there is nothing to promise, so the outflow is surfaced after the fact and attributed — or not. **Anchored on the transaction, not declared.** `consume!` now takes one and stamps `extra["goal"]["consumed_goal_id"]`, the same namespace the pledges write into. That is what makes attribution idempotent: replaying it cannot credit a goal twice for one spend, and the stamp happens inside the consumption's own transaction so a refusal rolls the whole thing back. **Sign matters more than it reads.** In Sure an inflow carries a NEGATIVE amount, so the detector selects the positive side. Reading it the other way round would have offered to attribute the user's deposits as spending, and the mistake would look right in a diff. A test pins it. **A reserve is excluded.** It is drawn down and refilled, not spent, and asking someone to attribute a withdrawal from one invites them to erase the very shortfall it exists to report. Known limitation, unchanged by this: `GoalPledge::Reconciler` only runs on provider imports, never on a hand-entered transaction. This detector reads entries directly and so has no such gap, but the two halves are not symmetric and that is worth knowing. bin/rails test: 7100 runs, 28540 assertions, 0 failures. RuboCop, erb_lint and Brakeman clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye * fix(goals): only offer an outflow the goal could still have spent Review on #3177. The panel offered outflows for completed and archived goals. Those have handed their accounts back, so a later transaction on one is not evidence about this goal — attributing it writes spending into a history that is already closed. The detector now returns nothing for a released goal; `consume!` refuses these too, but the panel should not ask in the first place. Provisional transactions were offered as well. A pending charge can be reversed or replaced by its posted form, leaving the goal consumed for a transaction that no longer exists while the posted twin arrives unstamped and gets offered again. Filtered through the pending-provider SQL the rest of the app already uses. `thaw_completed_amount!` wiped `consumed_amount` unconditionally, so a goal that recorded a spend and was then archived straight from active lost that history on unarchive — and dropped its progress with it. Restarting is what clears the figure, and a direct archive never closed a lifecycle to restart from. Cleared now only when a frozen figure exists. The attribution button was a hand-rolled `button_to` with raw `btn` classes; it is `DS::Button` now, the same primitive the consumption dialog uses, so the two ways of recording a spend do not read as two features. Carried down from #3176 by rebase: the goal-level lock, the `:not_active` guard, and the success notice, which was blank on this path because the form posts only `transaction_id`. The resolved amount is formatted through `Money` and the account behind an attributed outflow now resolves through `accessible_accounts` like the named one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye * fix(goals): keep a private backing account out of the outflow panel The same leak #3176 closed on the dialog, through a third door. A goal can be backed by an account private to another family member, and the panel listed its outflows — naming the account, what was spent on it and roughly its size to someone with no access to it. `WithdrawalDetector` takes `accounts:` now, and the controller passes the links narrowed to what the viewer may see. It defaults to every linked account for callers with no viewer to speak for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GTNba5qE5NwzaHzbp27ye * fix(goals): use the shared separator key in the outflow panel DS Drift Patrol on #3177. The `·` between an outflow's date and its account was a bare literal. `shared.dot_separator` already exists and is already used three times in the goals views, so this was drift rather than a missing mechanism. Wrapped in `aria-hidden` like the existing uses: the separator is decorative, and a screen reader was reading it out between the two values. 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>
417 lines
17 KiB
Ruby
417 lines
17 KiB
Ruby
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)
|
||
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.
|
||
# 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
|