diff --git a/app/controllers/goals_controller.rb b/app/controllers/goals_controller.rb index e71135eb2..390a8a79e 100644 --- a/app/controllers/goals_controller.rb +++ b/app/controllers/goals_controller.rb @@ -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, diff --git a/app/models/family.rb b/app/models/family.rb index 959ef6a29..c421b6ef7 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -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 diff --git a/app/models/goal.rb b/app/models/goal.rb index e3ce6249c..5f9005c6e 100644 --- a/app/models/goal.rb +++ b/app/models/goal.rb @@ -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) diff --git a/app/views/goals/index.html.erb b/app/views/goals/index.html.erb index a41e5a961..7368ad3e5 100644 --- a/app/views/goals/index.html.erb +++ b/app/views/goals/index.html.erb @@ -51,6 +51,7 @@

<% 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? diff --git a/config/locales/views/goals/en.yml b/config/locales/views/goals/en.yml index 4ba3639fe..5d3c4c221 100644 --- a/config/locales/views/goals/en.yml +++ b/config/locales/views/goals/en.yml @@ -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" diff --git a/test/models/goal_test.rb b/test/models/goal_test.rb index ed622a098..d4ff54d58 100644 --- a/test/models/goal_test.rb +++ b/test/models/goal_test.rb @@ -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,