From 1b403d64e5fbea32ecf4b0694660768d4e1cd7fd Mon Sep 17 00:00:00 2001 From: Guillem Arias Fauste Date: Tue, 30 Jun 2026 06:55:29 +0200 Subject: [PATCH] feat(goals): earmark a portion of an account toward a goal (Phase 1) (#2490) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(goals): earmark a portion of an account toward a goal Goals currently count each linked account's whole balance, so an account shared across goals double-counts and one account can't fund several goals in distinct slices. Add a per-account earmark — the "GoalBacking" the v1 model already foreshadowed (goal.rb). - goal_accounts.allocated_amount (nullable). NULL = "dedicate the whole balance" (the v1 default: no backfill, existing goals unchanged); a set amount reserves a fixed slice. - Goal#current_balance is now the single chokepoint computing each account's backing under a family-wide shared pool: fixed earmarks take their slice, an unallocated link takes the remainder, and when fixed earmarks exceed the balance every slice is scaled down pro-rata so the goals' shares can never sum past the account balance (no double-counting). - Account#free_to_earmark / #goal_earmarked_total (mirror Budget's available_to_allocate) back a soft, non-blocking over-allocation hint. - GoalsController threads a goal[allocations] hash through create/update. Phase 1 of the goals earmarking work; investment-backed goals follow. * feat(goals): earmark UI on the goal form + backing-aware funding breakdown - Goal form: a per-account "earmark amount" input (blank = whole balance) next to each funding-account checkbox, prefilled from the saved allocation on edit. - Goal#account_backing exposes a single linked account's share so the funding-accounts breakdown shows each account's earmarked contribution and percent instead of its whole balance — keeping the show page consistent with the (now allocation-aware) progress ring. - English strings for the earmark controls and the "earmarked of balance" breakdown line. * fix(goals): address review on the earmark shared-pool math - Overdrawn (<= 0 balance) accounts now back nothing on both the fixed and whole-balance paths. The fixed path previously produced negative backing and let a goal claim money the account doesn't hold. - An archived goal reads its OWN earmark from its own goal_accounts instead of the shared pool (which excludes archived goals), so it no longer mis-reports the whole account balance for itself. - goals#index injects one family-wide earmark pool into every card (Goal.pooled_allocations_for) instead of querying once per goal (N+1), and preloads goal_accounts. - The projection chart scales its whole-account historical series by the backing ratio so the saved line meets current_balance at "today" rather than dropping off a cliff for earmarked goals. - Honest comments: free_to_earmark no longer claims a form warning that doesn't exist yet; pace documents its deliberate whole-account basis. * fix(goals): widen the earmark input so the 'Whole balance' placeholder isn't clipped * fix(goals): address review on #2490 - autosave: true on goal_accounts so earmark edits to already-linked accounts persist through goal.save! (Rails only auto-saves newly built children, so changing/clearing an existing earmark was silently dropped). + test. - Reset the balance/progress memos on AASM transitions, not just the status memos, so a same-instance render after complete!/archive! isn't stale. + test. - backing_ratio is 0 (not 1) when the linked-account total is non-positive, so the projection saved series ends at 0 to match the forced-zero current_balance. - Localize the funding-row subtype label via goals.form.subtypes.*. - Add the earmark strings to zh-CN (the maintained second locale; goals has no ca locale, so Catalan keeps falling back to en like the rest of goals). --------- Signed-off-by: Juan José Mata Co-authored-by: Juan José Mata --- ...ding_accounts_breakdown_component.html.erb | 12 +- .../funding_accounts_breakdown_component.rb | 18 ++- app/controllers/goals_controller.rb | 46 +++++-- app/models/account.rb | 20 +++ app/models/goal.rb | 120 ++++++++++++++++-- app/models/goal_account.rb | 12 ++ app/views/goals/_form.html.erb | 46 ++++--- config/locales/views/goals/en.yml | 6 +- config/locales/views/goals/zh-CN.yml | 6 +- ...5120000_add_allocation_to_goal_accounts.rb | 14 ++ db/schema.rb | 2 + test/models/goal_account_test.rb | 32 +++++ test/models/goal_test.rb | 112 ++++++++++++++++ 13 files changed, 398 insertions(+), 48 deletions(-) create mode 100644 db/migrate/20260625120000_add_allocation_to_goal_accounts.rb create mode 100644 test/models/goal_account_test.rb diff --git a/app/components/goals/funding_accounts_breakdown_component.html.erb b/app/components/goals/funding_accounts_breakdown_component.html.erb index 587038a8d..f98118fb4 100644 --- a/app/components/goals/funding_accounts_breakdown_component.html.erb +++ b/app/components/goals/funding_accounts_breakdown_component.html.erb @@ -13,8 +13,8 @@ <% if rows.size > 1 && total.positive? %>
<% rows.each do |row| %> - <% next if row[:balance].to_d.zero? %> -
+ <% next if row[:backing].to_d.zero? %> +
<% end %>
<% end %> @@ -31,13 +31,17 @@

<%= account.name %>

- <%= accountable_label(account) %> · <%= row[:balance_money].format(precision: 0) %> + <% if row[:earmarked] %> + <%= t("goals.show.funding_accounts.earmarked_of", earmarked: row[:backing_money].format(precision: 0), balance: row[:balance_money].format(precision: 0)) %> + <% else %> + <%= accountable_label(account) %> · <%= row[:backing_money].format(precision: 0) %> + <% end %>

<% if rows.size > 1 %> <% else %> diff --git a/app/components/goals/funding_accounts_breakdown_component.rb b/app/components/goals/funding_accounts_breakdown_component.rb index 345d40988..6917bf47c 100644 --- a/app/components/goals/funding_accounts_breakdown_component.rb +++ b/app/components/goals/funding_accounts_breakdown_component.rb @@ -9,12 +9,16 @@ class Goals::FundingAccountsBreakdownComponent < ApplicationComponent attr_reader :goal def rows - @rows ||= goal.linked_accounts.sort_by { |a| -a.balance.to_d }.map do |account| + @rows ||= goal.linked_accounts.sort_by { |a| -goal.account_backing(a).amount.to_d }.map do |account| totals = inflow_totals_for(account) + backing = goal.account_backing(account).amount.to_d + goal_account = goal_account_by_id[account.id] { account: account, - balance: account.balance.to_d, + backing: backing, + backing_money: Money.new(backing, goal.currency), balance_money: Money.new(account.balance.to_d, goal.currency), + earmarked: goal_account&.allocated_amount.present?, last_30_money: Money.new(totals[:last_30], goal.currency), last_90_money: Money.new(totals[:last_90], goal.currency) } @@ -22,12 +26,12 @@ class Goals::FundingAccountsBreakdownComponent < ApplicationComponent end def total - @total ||= rows.sum { |r| r[:balance].to_d } + @total ||= rows.sum { |r| r[:backing].to_d } end - def percent_for(balance) + def percent_for(backing) return 0 if total.zero? - ((balance.to_d / total) * 100).round + ((backing.to_d / total) * 100).round end # Pull from the goal's per-goal account color map so the colors here @@ -52,6 +56,10 @@ class Goals::FundingAccountsBreakdownComponent < ApplicationComponent end private + def goal_account_by_id + @goal_account_by_id ||= goal.goal_accounts.index_by(&:account_id) + end + # Per-account net inflow for both windows in one pass over the 90-day # entries set. Entry amount sign in Sure: inflow is negative; flip and # clamp ≥ 0. diff --git a/app/controllers/goals_controller.rb b/app/controllers/goals_controller.rb index 1bb8f3c8a..e77db0367 100644 --- a/app/controllers/goals_controller.rb +++ b/app/controllers/goals_controller.rb @@ -14,7 +14,7 @@ class GoalsController < ApplicationController all_goals = Current.family.goals .alphabetically - .includes(:open_pledges, linked_accounts: :account_providers) + .includes(:open_pledges, :goal_accounts, linked_accounts: :account_providers) .to_a @active_goals = all_goals.reject { |g| %w[completed archived].include?(g.state) } .sort_by { |g| [ g.paused? ? 3 : ACTIVE_STATUS_RANK.fetch(g.status, 4), g.name.downcase ] } @@ -26,6 +26,11 @@ class GoalsController < ApplicationController # entirely (rendered with filterable: false). @grid_goals = @active_goals + @completed_goals + # One family-wide earmark-pool query injected into every rendered goal so + # the shared-pool backing math doesn't fire a query per card (N+1). + pooled = Goal.pooled_allocations_for(Current.family) + (@grid_goals + @archived_goals).each { |goal| goal.pooled_allocations = pooled } + @linkable_account_count = Current.user.accessible_accounts.where(accountable_type: "Depository").visible.count @kpi = kpi_payload(@active_goals) @any_pending_pledge = @active_goals.any? { |g| g.open_pledges.any? } @@ -63,8 +68,9 @@ class GoalsController < ApplicationController 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) } + accounts.each { |a| @goal.goal_accounts.build(account: a, allocated_amount: allocations[a.id.to_s]) } @goal.save! end @@ -100,7 +106,7 @@ class GoalsController < ApplicationController Goal.transaction do @goal.update!(goal_update_params) - sync_linked_accounts!(@goal, accounts) if accounts_supplied + sync_linked_accounts!(@goal, accounts, submitted_allocations) if accounts_supplied end flash[:notice] = t(".success") @@ -176,7 +182,7 @@ class GoalsController < ApplicationController Current.user.accessible_accounts.where(accountable_type: "Depository").visible.alphabetically.to_a end - def sync_linked_accounts!(goal, accounts) + def sync_linked_accounts!(goal, accounts, allocations = {}) desired_ids = accounts.map(&:id).to_set current_ids = goal.goal_accounts.pluck(:account_id).to_set @@ -189,14 +195,36 @@ class GoalsController < ApplicationController ((current_ids & removable_ids) - desired_ids).each do |id| goal.goal_accounts.where(account_id: id).destroy_all end - additions = accounts.reject { |a| current_ids.include?(a.id) } - additions.each { |a| goal.goal_accounts.build(account: a) } - # Save through the goal so currency / depository / family - # validations fire. `create!` on goal_accounts directly bypasses them - # and let cross-currency / non-depository attachments through. + 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 diff --git a/app/models/account.rb b/app/models/account.rb index 8ed07849d..d137831d8 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -410,6 +410,26 @@ class Account < ApplicationRecord manual? ? "manual_save" : "transfer" end + # Total fixed earmark this account currently has reserved across every + # non-archived goal (unallocated/whole-balance links reserve no fixed + # slice). Mirrors Budget#allocated_spending. + def goal_earmarked_total + GoalAccount.joins(:goal) + .where(account_id: id) + .where.not(allocated_amount: nil) + .where.not(goals: { state: "archived" }) + .sum(:allocated_amount) + .to_d + end + + # Headroom left to earmark toward goals before fixed allocations exceed the + # balance. Negative means the account is over-earmarked. Intended to back a + # non-blocking over-allocation warning (UI is a follow-up). Mirrors + # Budget#available_to_allocate. + def free_to_earmark + balance.to_d - goal_earmarked_total + end + def logo_url if institution_domain.present? && Setting.brand_fetch_client_id.present? logo_size = Setting.brand_fetch_logo_size diff --git a/app/models/goal.rb b/app/models/goal.rb index 67c4f6dae..3dbc78ac1 100644 --- a/app/models/goal.rb +++ b/app/models/goal.rb @@ -8,7 +8,10 @@ class Goal < ApplicationRecord validates :color, format: { with: /\A#[0-9A-Fa-f]{6}\z/ }, allow_nil: true belongs_to :family - has_many :goal_accounts, dependent: :destroy + # autosave so earmark (allocated_amount) edits on already-linked accounts + # persist through goal.save! — without it Rails only saves newly built + # children, silently dropping changes to existing goal_accounts. + has_many :goal_accounts, dependent: :destroy, autosave: true has_many :linked_accounts, through: :goal_accounts, source: :account has_many :goal_pledges, dependent: :destroy has_many :open_pledges, @@ -35,6 +38,24 @@ class Goal < ApplicationRecord Digest::SHA1.hexdigest("goals:family:#{family_id}").to_i(16) % (2**63) end + # Family-wide map of non-archived goal earmarks, grouped by account_id: + # { account_id => [{ goal_id:, allocated_amount: }, ...] }. The controller + # assigns this to each goal on index (goal.pooled_allocations = ...) so the + # shared-pool backing math runs ONE query for the whole page instead of one + # per goal. + def self.pooled_allocations_for(family) + GoalAccount.joins(:goal) + .where(goals: { family_id: family.id }) + .where.not(goals: { state: "archived" }) + .pluck(:account_id, :goal_id, :allocated_amount) + .group_by(&:first) + .transform_values do |triples| + triples.map { |(_, goal_id, amount)| { goal_id: goal_id, allocated_amount: amount } } + end + end + + attr_writer :pooled_allocations + aasm column: :state do after_all_transitions :reset_state_dependent_caches! @@ -64,13 +85,14 @@ class Goal < ApplicationRecord end end - # Balance is the live balance of every linked depository account that - # matches the goal's currency. The model validates this invariant at - # write time, but defensive filter + telemetry here guards against any - # drift caused by direct DB writes, account-currency edits outside - # goal validation, or future code that bypasses the validation chain. - # v1.1+: minus other goals' allocations via the upcoming GoalBacking - # query. + # Balance is this goal's backing across its linked depository accounts that + # match the goal's currency. Each linked account contributes either its + # earmarked slice (goal_accounts.allocated_amount) or — when unallocated — + # the whole balance left after other goals' earmarks (see + # #backing_balance_for). The model validates the currency invariant at write + # time, but the defensive filter + telemetry here guards against drift from + # direct DB writes, account-currency edits outside goal validation, or + # future code that bypasses the validation chain. def current_balance @current_balance ||= begin matching = linked_accounts.select { |a| a.currency == currency } @@ -78,7 +100,7 @@ class Goal < ApplicationRecord Rails.logger.warn("Goal##{id} linked-account currency drift: #{linked_accounts.size - matching.size} of #{linked_accounts.size} mismatched (expected #{currency})") Sentry.capture_message("Goal linked-account currency drift", level: :warning, extra: { goal_id: id, expected_currency: currency }) if defined?(Sentry) end - matching.sum { |a| a.balance.to_d } + matching.sum { |account| backing_balance_for(account) } end end @@ -86,6 +108,13 @@ class Goal < ApplicationRecord @current_balance_money ||= Money.new(current_balance, currency) end + # This goal's backing from a single linked account — the earmarked slice, or + # the whole-balance remainder when the link is unallocated — as Money. Used + # by the funding breakdown so the per-account rows reconcile with the ring. + def account_backing(account) + Money.new(backing_balance_for(account), currency) + end + def remaining_amount @remaining_amount ||= [ target_amount - current_balance, 0 ].max end @@ -139,6 +168,11 @@ class Goal < ApplicationRecord # user records a pledge, the transfer arrives, balance goes up, pace # goes up, status flips off "behind". Excludes user-flagged-excluded # entries. Entry amount sign convention in Sure: inflow is negative. + # + # NOTE: pace is whole-account inflow by design in this phase, even for an + # earmarked goal whose current_balance is only a slice — so runway/status + # mix a whole-account numerator with an earmark-scoped balance. Earmark-aware + # pace is a deliberate follow-up; don't "fix" the basis without that work. def pace return @pace if defined?(@pace) @@ -190,7 +224,16 @@ class Goal < ApplicationRecord # strings server-side rather than build them with its own Intl calls. def projection_payload series_values = balance_series_values - saved_series = series_values.map { |v| { date: v.date.to_s, value: v.value.amount.to_f } } + # The historical series tracks the whole linked-account balances. Scale it + # to this goal's backing so the saved line meets current_balance at "today" + # instead of dropping off a cliff for earmarked goals. Assumes the earmark + # ratio held over the window (an approximation); exact for unallocated + # goals, where ratio == 1 and the series is unchanged. + whole_total = linked_accounts.select { |a| a.currency == currency }.sum { |a| a.balance.to_d } + # 0 when the linked-account total is non-positive: current_balance is forced + # to 0 there, so the saved series must end at 0 too (no stray non-zero tail). + backing_ratio = whole_total.positive? ? (current_balance.to_d / whole_total) : 0.to_d + saved_series = series_values.map { |v| { date: v.date.to_s, value: (v.value.amount.to_d * backing_ratio).to_f } } earliest = series_values.first&.date || created_at.to_date target_amt = target_amount.to_d @@ -402,12 +445,67 @@ class Goal < ApplicationRecord end private + # This goal's share of `account`'s live balance under the family-wide + # shared pool. The goal's OWN earmark is read from its own goal_accounts + # (reliable even for an archived goal, which is excluded from the pool); + # OTHER non-archived goals' fixed earmarks come from the shared pool. A + # fixed earmark takes its slice; an unallocated link takes the balance left + # after others' fixed earmarks (so it keeps the v1 whole-balance behaviour + # when nothing else earmarks the account). When the fixed earmarks on an + # account exceed its balance every fixed slice is scaled down pro-rata (to + # within sub-cent rounding) so the goals' shares effectively never sum past + # the account's balance — no double-counting. An overdrawn (<= 0) account + # backs nothing. + def backing_balance_for(account) + balance = account.balance.to_d + return 0.to_d if balance <= 0 + + mine = own_allocation_for(account) + others_fixed = (pooled_allocations[account.id] || []) + .reject { |r| r[:goal_id] == id } + .sum { |r| r[:allocated_amount].to_d } + + if mine + total_fixed = others_fixed + mine + if total_fixed > balance && total_fixed.positive? + (mine * (balance / total_fixed)).round(4) # pro-rata haircut + else + mine + end + else + [ balance - others_fixed, 0 ].max # unallocated link: the remainder + end + end + + # This goal's own earmark on `account` (a BigDecimal, or nil for a + # whole-balance link). Read from the loaded goal_accounts association so it + # is correct even for archived goals, which are excluded from the pool. + def own_allocation_for(account) + goal_accounts.find { |ga| ga.account_id == account.id }&.allocated_amount + end + + # Family-wide map of non-archived goal earmarks. Injected once per request + # by the controller on index (one query for the whole page); falls back to + # a single query for the standalone (show) case. + def pooled_allocations + @pooled_allocations ||= self.class.pooled_allocations_for(family) + end + # Cleared after every AASM transition. The state column drives the # display_status / projection_summary memos; without this the same # instance keeps returning the pre-transition value if a controller # calls archive! / pause! and then renders without reload. def reset_state_dependent_caches! - %i[@display_status @projection_summary].each do |ivar| + # current_balance now depends on the goal's own archived state (an + # archived goal is excluded from the shared pool), so the balance-derived + # memos must be cleared on a transition too, not just the status memos. + %i[ + @display_status @projection_summary + @current_balance @current_balance_money + @remaining_amount @remaining_amount_money + @progress_percent @monthly_target_amount + @pace @pace_money @status @pooled_allocations + ].each do |ivar| remove_instance_variable(ivar) if instance_variable_defined?(ivar) end end diff --git a/app/models/goal_account.rb b/app/models/goal_account.rb index 4ab38bb68..57f906db6 100644 --- a/app/models/goal_account.rb +++ b/app/models/goal_account.rb @@ -3,4 +3,16 @@ class GoalAccount < ApplicationRecord belongs_to :account validates :account_id, uniqueness: { scope: :goal_id } + validates :allocated_amount, + numericality: { greater_than_or_equal_to: 0 }, + allow_nil: true + + # nil allocated_amount means "dedicate the whole account balance" (the v1 + # default). A set amount earmarks a fixed slice of the account toward this + # goal. The share that actually counts toward the goal — after sibling + # earmarks and the pro-rata over-allocation haircut — is computed by + # Goal#current_balance, which owns the shared-pool math. + def whole_account? + allocated_amount.nil? + end end diff --git a/app/views/goals/_form.html.erb b/app/views/goals/_form.html.erb index 252bc4259..6b53c230d 100644 --- a/app/views/goals/_form.html.erb +++ b/app/views/goals/_form.html.erb @@ -44,30 +44,42 @@
<%= t("goals.form.fields.funding_accounts") %>

<%= t("goals.form.fields.funding_accounts_hint") %>

+

<%= t("goals.form.fields.earmark_hint") %>

+ <% linked_allocation_by_account = goal.goal_accounts.index_by(&:account_id) %> <% grouped = linkable_accounts.group_by { |a| a.subtype.to_s.presence || "other" } %> <% grouped.each_with_index do |(subtype, accts), group_idx| %>
<%= t("goals.form.subtypes.#{subtype}", default: subtype.titleize) %>
"> <% accts.each_with_index do |account, idx| %> - + <% linked_ga = linked_allocation_by_account[account.id] %> +
0 %>"> + + <%= text_field_tag "goal[allocations][#{account.id}]", + linked_ga&.allocated_amount&.to_s("F"), + placeholder: t("goals.form.fields.whole_balance"), + inputmode: "decimal", + autocomplete: "off", + aria: { label: t("goals.form.fields.earmark_for", account: account.name) }, + class: "shrink-0 w-36 rounded-md border border-primary bg-container px-2.5 py-1.5 text-sm text-right tabular-nums text-primary placeholder:text-subdued focus-ring privacy-sensitive" %> +
<% end %>
<% end %> diff --git a/config/locales/views/goals/en.yml b/config/locales/views/goals/en.yml index c8b04b9d2..11e4a65f3 100644 --- a/config/locales/views/goals/en.yml +++ b/config/locales/views/goals/en.yml @@ -105,6 +105,7 @@ en: pledge_just_saved: Log money you set aside funding_accounts_heading: Funding accounts funding_accounts: + earmarked_of: "%{earmarked} earmarked of %{balance}" empty: heading: No funding accounts linked yet body: Edit the goal to link the depository accounts you save into. @@ -258,7 +259,10 @@ en: notes: Notes (optional) notes_placeholder: A reminder for future you… funding_accounts: Funding accounts - funding_accounts_hint: This goal's balance is the balance of these accounts. + funding_accounts_hint: This goal's balance is the balance (or earmarked portion) of these accounts. + whole_balance: Whole balance + earmark_for: Earmark amount for %{account} + earmark_hint: Leave an amount blank to dedicate that account's whole balance. subtypes: checking: Checking savings: Savings diff --git a/config/locales/views/goals/zh-CN.yml b/config/locales/views/goals/zh-CN.yml index 6f9ab8b29..9f7f36556 100644 --- a/config/locales/views/goals/zh-CN.yml +++ b/config/locales/views/goals/zh-CN.yml @@ -105,6 +105,7 @@ zh-CN: pledge_just_saved: 记录你留出的资金 funding_accounts_heading: 资金账户 funding_accounts: + earmarked_of: 已预留 %{earmarked}（共 %{balance}） empty: heading: 尚未关联资金账户 body: 编辑目标以关联用于储蓄的存款账户。 @@ -258,7 +259,10 @@ zh-CN: notes: 备注（可选） notes_placeholder: 给未来自己的提醒… funding_accounts: 资金账户 - funding_accounts_hint: 此目标余额等于这些账户的余额。 + funding_accounts_hint: 此目标余额等于这些账户的余额（或预留的部分）。 + whole_balance: 全部余额 + earmark_for: 为「%{account}」预留的金额 + earmark_hint: 留空则使用该账户的全部余额。 subtypes: checking: 支票账户 savings: 储蓄账户 diff --git a/db/migrate/20260625120000_add_allocation_to_goal_accounts.rb b/db/migrate/20260625120000_add_allocation_to_goal_accounts.rb new file mode 100644 index 000000000..cdbfe0d39 --- /dev/null +++ b/db/migrate/20260625120000_add_allocation_to_goal_accounts.rb @@ -0,0 +1,14 @@ +class AddAllocationToGoalAccounts < ActiveRecord::Migration[7.2] + def change + # Per-account earmark toward a goal. NULL = "dedicate the whole account + # balance" (the v1 behaviour), so every existing goal_accounts row keeps + # its current semantics with no backfill. A set amount reserves a fixed + # slice of the account, letting one account fund several goals without + # double-counting (Goal#current_balance applies the shared-pool math). + add_column :goal_accounts, :allocated_amount, :decimal, precision: 19, scale: 4, null: true + + add_check_constraint :goal_accounts, + "allocated_amount IS NULL OR allocated_amount >= 0", + name: "chk_goal_accounts_allocation_non_negative" + end +end diff --git a/db/schema.rb b/db/schema.rb index 4255f5d2b..145b7c728 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -813,9 +813,11 @@ ActiveRecord::Schema[7.2].define(version: 2026_06_25_230639) do t.uuid "account_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.decimal "allocated_amount", precision: 19, scale: 4 t.index ["account_id"], name: "index_goal_accounts_on_account_id" t.index ["goal_id", "account_id"], name: "index_savings_goal_accounts_on_goal_and_account", unique: true t.index ["goal_id"], name: "index_goal_accounts_on_goal_id" + t.check_constraint "allocated_amount IS NULL OR allocated_amount >= 0::numeric", name: "chk_goal_accounts_allocation_non_negative" end create_table "goal_pledges", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| diff --git a/test/models/goal_account_test.rb b/test/models/goal_account_test.rb new file mode 100644 index 000000000..c0c8b9609 --- /dev/null +++ b/test/models/goal_account_test.rb @@ -0,0 +1,32 @@ +require "test_helper" + +class GoalAccountTest < ActiveSupport::TestCase + setup do + @goal = goals(:emergency_fund) + @account = Account.create!( + family: families(:dylan_family), + accountable: Depository.new, + name: "Allocation Test", + currency: "USD", + balance: 1_000 + ) + end + + test "allocated_amount may be nil, meaning dedicate the whole balance" do + ga = GoalAccount.new(goal: @goal, account: @account, allocated_amount: nil) + assert ga.valid?, ga.errors.full_messages.to_sentence + assert ga.whole_account? + end + + test "a set allocated_amount is not a whole-account link" do + ga = GoalAccount.new(goal: @goal, account: @account, allocated_amount: 250) + assert ga.valid?, ga.errors.full_messages.to_sentence + assert_not ga.whole_account? + end + + test "allocated_amount must be non-negative" do + ga = GoalAccount.new(goal: @goal, account: @account, allocated_amount: -1) + assert_not ga.valid? + assert_includes ga.errors[:allocated_amount], "must be greater than or equal to 0" + end +end diff --git a/test/models/goal_test.rb b/test/models/goal_test.rb index e7d5959fe..0333b680e 100644 --- a/test/models/goal_test.rb +++ b/test/models/goal_test.rb @@ -290,4 +290,116 @@ class GoalTest < ActiveSupport::TestCase reloaded = Goal.find(@goal.id) assert_equal "goals.show.pledge_just_saved", reloaded.pledge_action_label_key end + + test "explicit allocation backs only the earmarked slice" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Split Savings", currency: "USD", balance: 5_000) + goal = @family.goals.create!(name: "Earmarked", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 1_000) + end + assert_equal BigDecimal("1000"), goal.current_balance.to_d + end + + test "explicit allocation is capped at the account balance via the haircut" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Over Earmark", currency: "USD", balance: 800) + goal = @family.goals.create!(name: "Over", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 5_000) + end + assert_equal BigDecimal("800"), goal.current_balance.to_d + end + + test "unallocated link claims the balance left after another goal's earmark" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Shared Savings", currency: "USD", balance: 5_000) + earmarked = @family.goals.create!(name: "Earmarked", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 2_000) + end + whole = @family.goals.create!(name: "Whole", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account) # NULL = whole-balance remainder + end + assert_equal BigDecimal("2000"), earmarked.current_balance.to_d + assert_equal BigDecimal("3000"), whole.current_balance.to_d + # The two goals' shares of the shared account never exceed its balance. + assert_equal account.balance.to_d, earmarked.current_balance.to_d + whole.current_balance.to_d + end + + test "over-earmarked account scales fixed slices pro-rata" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Contested Savings", currency: "USD", balance: 5_000) + a = @family.goals.create!(name: "Goal A", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 4_000) + end + b = @family.goals.create!(name: "Goal B", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 4_000) + end + # sum_fixed 8000 > balance 5000 -> each scaled by 5000/8000 -> 2500. + assert_equal BigDecimal("2500"), a.current_balance.to_d + assert_equal BigDecimal("2500"), b.current_balance.to_d + assert_equal account.balance.to_d, a.current_balance.to_d + b.current_balance.to_d + end + + test "archived goals release their earmark from the shared pool" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Release Savings", currency: "USD", balance: 5_000) + whole = @family.goals.create!(name: "Whole", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account) + end + earmarked = @family.goals.create!(name: "Earmarked", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 2_000) + end + assert_equal BigDecimal("3000"), Goal.find(whole.id).current_balance.to_d + earmarked.archive! + # Archived goal no longer reserves its slice -> whole reclaims it. + assert_equal BigDecimal("5000"), Goal.find(whole.id).current_balance.to_d + end + + test "account free_to_earmark subtracts non-archived fixed earmarks" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Headroom Savings", currency: "USD", balance: 5_000) + @family.goals.create!(name: "Earmarker", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 1_500) + end + assert_equal BigDecimal("1500"), account.goal_earmarked_total + assert_equal BigDecimal("3500"), account.free_to_earmark + end + + test "an overdrawn account backs nothing for fixed or whole-balance links" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Overdrawn", currency: "USD", balance: BigDecimal("-100")) + fixed = @family.goals.create!(name: "Fixed OD", target_amount: 1_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 50) + end + whole = @family.goals.create!(name: "Whole OD", target_amount: 1_000, currency: "USD") do |g| + g.goal_accounts.build(account: account) + end + assert_equal 0.to_d, fixed.current_balance.to_d + assert_equal 0.to_d, whole.current_balance.to_d + end + + test "an archived goal still shows its own earmark, not the whole balance" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Archived Earmark", currency: "USD", balance: 5_000) + earmarked = @family.goals.create!(name: "Archived Fixed", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account, allocated_amount: 2_000) + end + earmarked.archive! + # Excluded from the shared pool, but its own earmark is read from its own + # goal_accounts — so it still reports 2,000, not the whole 5,000. + assert_equal BigDecimal("2000"), Goal.find(earmarked.id).current_balance.to_d + end + + test "earmark edits to an existing linked account persist via goal.save!" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Autosave Savings", currency: "USD", balance: 5_000) + goal = @family.goals.create!(name: "Autosave goal", target_amount: 10_000, currency: "USD") do |g| + g.goal_accounts.build(account: account) # NULL = whole balance + end + ga = goal.goal_accounts.first + assert_nil ga.allocated_amount + ga.allocated_amount = 1_500 + goal.save! # autosave: true must persist the dirty existing child + assert_equal BigDecimal("1500"), goal.goal_accounts.first.reload.allocated_amount + end + + test "progress_percent memo resets after complete! on the same instance" do + account = Account.create!(family: @family, accountable: Depository.new, name: "Memo Savings", currency: "USD", balance: 100) + goal = @family.goals.create!(name: "Memo goal", target_amount: 1_000, currency: "USD") do |g| + g.goal_accounts.build(account: account) + end + assert_operator goal.progress_percent, :<, 100 # memoize the underfunded value + goal.complete! + assert_equal 100, goal.progress_percent, "stale memo would still report the pre-complete percent" + end end