mirror of
https://github.com/we-promise/sure.git
synced 2026-05-29 23:39:03 +00:00
fix(goals): pace counts transfers, family rollup currency-scoped
Two semantic shifts in V2 that drove the worst on-screen confusion.
B3/B4 — `Goal#pace` excluded `Transaction::TRANSFER_KINDS`. When a
user tapped "I just transferred…" and the deposit landed, the linked
account's balance went up but pace did not: pace ignored transfer-
kind entries, so the goal stayed `:behind` against `monthly_target`
and the catch-up callout kept demanding $X/mo even though the user
had just moved the money in. Same root cause hit any long-time saver
whose 90-day net was zero — pace=0, status=:behind, projection says
"At $0.00/mo you'll miss your target date" while the ring sits at
80%.
Drop the transfer-kind exclusion. Pace is now net inflow into linked
accounts over 90 days. Transfers between linked accounts already net
out (both legs land inside the same account set); transfers from
outside (checking → linked savings) net positive, which is exactly
the case the pledge flow records.
B19 — `Family#savings_inflow_velocity` summed entry amounts across
every depository account linked to any goal regardless of currency,
then rendered the result in the family's primary currency. A family
with one USD goal and one EUR goal saw `usd_inflow + eur_inflow`
reported as USD with no FX conversion. Scope the account set to the
family's primary currency until proper FX-conversion lands. Also
let the result go negative (net outflow) — clamping to ≥0 lost
signal; the controller decides how to render the sign.
V20 (controller) — `velocity_30d_sign` was wired off the *delta*
direction, so a $1,234 down-month rendered as "−$1,234 ↓ 27% vs
prior 30d". The minus read as a loss but $1,234 was the (positive)
contribution. Re-wire the headline sign off the headline value
itself; the delta-direction stays on the subline as ↑/↓ N%. With
the family-rollup change above, the headline can now legitimately
be negative — UI now says "−$200 ↓ 50% vs prior 30d" when the
family had net outflow.
B21 — KPI tile `on_track_count` lumped `:reached` goals into "on
track", inflating the numerator while the sort order placed reached
goals at the bottom of the list. Split `reached_count` out and
render it as its own segment in the on-track subline ("1 reached ·
1 behind · 1 paused").
Test: rename the pace=zero test to match its new premise (no
transactions vs. no non-transfer entries). The fixture still has no
entries, so the assertion holds.
This commit is contained in:
@@ -190,8 +190,15 @@ class GoalsController < ApplicationController
|
||||
velocity_30d = family.savings_inflow_velocity(range: (today - 30)..today)
|
||||
velocity_prior_30d = family.savings_inflow_velocity(range: (today - 60)..(today - 31))
|
||||
delta_amount = velocity_30d - velocity_prior_30d
|
||||
delta_percent = velocity_prior_30d.zero? ? nil : ((delta_amount / velocity_prior_30d) * 100).round(1)
|
||||
velocity_direction = if delta_amount.positive? then :up
|
||||
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
|
||||
@@ -200,19 +207,21 @@ class GoalsController < ApplicationController
|
||||
.select { |g| g.status == :behind }
|
||||
.sum { |g| g.monthly_target_amount.to_d }
|
||||
behind = active_goals.count { |g| g.status == :behind }
|
||||
on_track = active_goals.count { |g| g.status == :on_track || g.status == :reached }
|
||||
on_track = active_goals.count { |g| 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?)
|
||||
|
||||
{
|
||||
currency: currency,
|
||||
velocity_30d_money: Money.new(velocity_30d.abs, currency),
|
||||
velocity_prior_30d_money: Money.new(velocity_prior_30d, currency),
|
||||
velocity_30d_sign: velocity_direction == :down ? "−" : (velocity_direction == :up ? "+" : ""),
|
||||
velocity_prior_30d_money: Money.new(velocity_prior_30d.abs, currency),
|
||||
velocity_30d_sign: headline_sign,
|
||||
velocity_delta_percent: delta_percent,
|
||||
velocity_direction: velocity_direction,
|
||||
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,
|
||||
|
||||
@@ -45,13 +45,24 @@ class Family < ApplicationRecord
|
||||
|
||||
has_many :goals, dependent: :destroy
|
||||
|
||||
# Net non-transfer inflow into every depository account linked to any goal,
|
||||
# over the given window. Entry amount convention in Sure: inflow is negative,
|
||||
# so we flip the sign for the user-facing "contributed" value.
|
||||
# Net inflow into every depository account linked to any primary-currency
|
||||
# goal, over the given window. Transfers between linked accounts net to zero
|
||||
# because both sides of an internal move land inside the same account set;
|
||||
# external transfers (e.g. checking → linked savings) net positive.
|
||||
#
|
||||
# Scoped to the family's primary currency: mixed-currency families would
|
||||
# otherwise sum raw EUR + USD numbers and surface the result as primary.
|
||||
# Foreign-currency goals are excluded from this KPI until FX conversion is
|
||||
# added.
|
||||
#
|
||||
# Entry amount convention in Sure: inflow is negative, so flip the sign.
|
||||
# Result is allowed to go negative (net outflow last 30d) so the headline
|
||||
# reflects reality; the controller decides how to render.
|
||||
def savings_inflow_velocity(range: 30.days.ago.to_date..Date.current)
|
||||
account_ids = Account
|
||||
.joins(:goal_accounts)
|
||||
.where(goal_accounts: { goal_id: goals.select(:id) })
|
||||
.where(currency: primary_currency_code)
|
||||
.distinct
|
||||
.pluck(:id)
|
||||
return 0 if account_ids.empty?
|
||||
@@ -59,11 +70,10 @@ class Family < ApplicationRecord
|
||||
net = Entry
|
||||
.joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'")
|
||||
.where(account_id: account_ids, date: range)
|
||||
.where.not(transactions: { kind: Transaction::TRANSFER_KINDS })
|
||||
.where(excluded: false)
|
||||
.sum(:amount)
|
||||
|
||||
(-net.to_d).clamp(0, Float::INFINITY)
|
||||
-net.to_d
|
||||
end
|
||||
|
||||
has_many :llm_usages, dependent: :destroy
|
||||
|
||||
@@ -111,8 +111,13 @@ class Goal < ApplicationRecord
|
||||
end
|
||||
end
|
||||
|
||||
# 90-day rolling monthly pace: average net non-transfer inflow into linked
|
||||
# accounts. Entry amount sign convention in Sure: inflow is negative.
|
||||
# 90-day rolling monthly pace: net inflow into linked accounts divided by
|
||||
# three months. Transfers between linked accounts net to zero — both sides
|
||||
# land inside this account set. Transfers from outside (e.g. checking →
|
||||
# linked savings) net positive, which is the behaviour we want: the user
|
||||
# taps "I just transferred…", 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.
|
||||
def pace
|
||||
return @pace if defined?(@pace)
|
||||
|
||||
@@ -123,7 +128,6 @@ class Goal < ApplicationRecord
|
||||
net = Entry
|
||||
.joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'")
|
||||
.where(account_id: account_ids, date: 90.days.ago.to_date..Date.current)
|
||||
.where.not(transactions: { kind: Transaction::TRANSFER_KINDS })
|
||||
.where(excluded: false)
|
||||
.sum(:amount)
|
||||
(-net.to_d / 3).round(2)
|
||||
|
||||
@@ -51,6 +51,7 @@
|
||||
<p class="text-xs text-secondary mt-1">
|
||||
<%
|
||||
parts = []
|
||||
parts << t(".kpi.on_track_sub_parts.reached", count: @kpi[:reached_count]) if @kpi[:reached_count].positive?
|
||||
parts << t(".kpi.on_track_sub_parts.behind", count: @kpi[:behind_count]) if @kpi[:behind_count].positive?
|
||||
parts << t(".kpi.on_track_sub_parts.no_date", count: @kpi[:no_date_count]) if @kpi[:no_date_count].positive?
|
||||
parts << t(".kpi.on_track_sub_parts.paused", count: @kpi[:paused_count]) if @kpi[:paused_count].positive?
|
||||
|
||||
@@ -21,6 +21,9 @@ en:
|
||||
on_track_label: Goals on track
|
||||
on_track_value: "%{on_track} of %{total}"
|
||||
on_track_sub_parts:
|
||||
reached:
|
||||
one: 1 reached
|
||||
other: "%{count} reached"
|
||||
behind:
|
||||
one: 1 behind
|
||||
other: "%{count} behind"
|
||||
|
||||
@@ -97,7 +97,7 @@ class GoalTest < ActiveSupport::TestCase
|
||||
assert_equal 0, @goal.remaining_amount
|
||||
end
|
||||
|
||||
test "pace is zero on a goal whose linked accounts have no non-transfer entries" do
|
||||
test "pace is zero on a goal whose linked accounts have no transactions" do
|
||||
fresh_account = Account.create!(
|
||||
family: @family,
|
||||
accountable: Depository.new,
|
||||
|
||||
Reference in New Issue
Block a user