fix(goals): jjmata review — reconciler guard, chart i18n, pace test

Three issues raised on PR #1798 review:

- ProviderImportAdapter now memoizes account.goal_accounts.exists?
  per-account so a bulk historical import on an unlinked account
  short-circuits the reconciler instead of paying one SELECT per row.
  Linked accounts still hit the per-row reconciler with no change.
- goal_projection_chart_controller.js reads Today / Projected /
  Saved labels via Stimulus values fed from
  goals.show.projection.* locale keys instead of inlining English.
- goal_test.rb now covers Goal#pace with real inflows, asserting
  the 90-day window cutoff plus the Transaction.excluding_pending
  and entries.excluded = false filters.
This commit is contained in:
Guillem Arias
2026-05-17 16:54:13 +02:00
parent 2872f3798e
commit 89bae8a59b
5 changed files with 71 additions and 6 deletions

View File

@@ -11,7 +11,14 @@ import * as d3 from "d3";
// Data shape passed via `data-goal-projection-chart-data-value`
// matches Goal#projection_payload.
export default class extends Controller {
static values = { data: Object, ariaLabel: String, ariaDescription: String };
static values = {
data: Object,
ariaLabel: String,
ariaDescription: String,
todayLabel: { type: String, default: "Today" },
projectedTemplate: { type: String, default: "Projected: {amount}" },
savedTemplate: { type: String, default: "Saved: {amount}" },
};
connect() {
this._resize = this._draw.bind(this);
@@ -400,7 +407,7 @@ export default class extends Controller {
.attr("text-anchor", "middle")
.attr("font-size", 12)
.attr("fill", textSecondary)
.text("Today");
.text(this.todayLabelValue);
}
// Full 4-digit year so the terminal "Jan 2027" reads as the year, not
@@ -515,7 +522,7 @@ export default class extends Controller {
const projValue = currentAmount + tFrac * (projectionEnd - currentAmount);
hoverProjDot.attr("cx", hoverX).attr("cy", y(projValue)).style("display", null);
hoverSavedDot.style("display", "none");
lines.push(`Projected: ${this._fmtMoney(projValue, data.currency)}`);
lines.push(this.projectedTemplateValue.replace("{amount}", this._fmtMoney(projValue, data.currency)));
} else {
// Saved segment: hoverDate is already snapped to nearest savedSeries
// entry above, so reuse that entry directly instead of running
@@ -523,7 +530,7 @@ export default class extends Controller {
const savedPoint = savedSeries.find((p) => p.date.getTime() === hoverDate.getTime()) || savedSeries[savedSeries.length - 1];
hoverSavedDot.attr("cx", x(savedPoint.date)).attr("cy", y(savedPoint.value)).style("display", null);
hoverProjDot.style("display", "none");
lines.push(`Saved: ${this._fmtMoney(savedPoint.value, data.currency)}`);
lines.push(this.savedTemplateValue.replace("{amount}", this._fmtMoney(savedPoint.value, data.currency)));
}
tooltip.textContent = lines.join("\n");

View File

@@ -223,7 +223,15 @@ class Account::ProviderImportAdapter
# Auto-resolve any open Goal pledges on this account whose tolerance
# window matches the posted transaction. Idempotent via the partial-unique
# index on transactions.extra->'goal'->>'pledge_id'.
GoalPledge::Reconciler.new(entry).run unless incoming_pending
#
# Short-circuit when the account isn't linked to any goal: a 2k-row
# historical Plaid import on an unlinked account otherwise pays one
# SELECT per row. goal_accounts membership is stable across a sync
# batch, so memoize once per adapter instance (one query per account
# synced, not per transaction).
if !incoming_pending && account_linked_to_any_goal?
GoalPledge::Reconciler.new(entry).run
end
# AFTER save: For NEW posted transactions, check for fuzzy matches to SUGGEST (not auto-claim)
# This handles tip adjustments where auto-matching is too risky
@@ -968,4 +976,12 @@ class Account::ProviderImportAdapter
account_name: entry.account.name
}
end
# Memoized per adapter instance (which is per-account). Membership in
# goal_accounts is stable across a sync batch.
def account_linked_to_any_goal?
return @account_linked_to_any_goal if defined?(@account_linked_to_any_goal)
@account_linked_to_any_goal = account.goal_accounts.exists?
end
end

View File

@@ -246,7 +246,10 @@
data-controller="goal-projection-chart"
data-goal-projection-chart-data-value="<%= @goal.projection_payload.to_json %>"
data-goal-projection-chart-aria-label-value="<%= t("goals.show.projection.aria_label", name: @goal.name) %>"
data-goal-projection-chart-aria-description-value="<%= strip_tags(@goal.projection_summary) %>"></div>
data-goal-projection-chart-aria-description-value="<%= strip_tags(@goal.projection_summary) %>"
data-goal-projection-chart-today-label-value="<%= t("goals.show.projection.today_marker") %>"
data-goal-projection-chart-projected-template-value="<%= t("goals.show.projection.tooltip_projected", amount: "{amount}") %>"
data-goal-projection-chart-saved-template-value="<%= t("goals.show.projection.tooltip_saved", amount: "{amount}") %>"></div>
<% if @goal.target_date.nil? %>
<div class="mt-3 flex items-center gap-2 text-xs text-secondary">
<span class="text-subdued"><%= icon("calendar-plus", size: "sm") %></span>

View File

@@ -139,6 +139,9 @@ en:
behind: Falling short at current pace.
on_track_html: At your current pace, you'll reach this goal around <strong class="text-primary">%{date}</strong>.
aria_label: "Projection chart for %{name}"
today_marker: Today
tooltip_projected: "Projected: %{amount}"
tooltip_saved: "Saved: %{amount}"
catch_up:
title: "Save %{amount}/mo more to catch up"
body: "Current pace %{avg}/mo · required %{required}/mo to hit your target."

View File

@@ -1,6 +1,8 @@
require "test_helper"
class GoalTest < ActiveSupport::TestCase
include EntriesTestHelper
setup do
@family = families(:dylan_family)
@depository = accounts(:depository)
@@ -114,6 +116,40 @@ class GoalTest < ActiveSupport::TestCase
assert_equal 0, fresh.pace.to_d
end
test "pace averages 90-day net inflow, excluding pending and excluded entries" do
account = Account.create!(
family: @family,
accountable: Depository.new,
name: "Pace Savings",
currency: "USD",
balance: 0
)
goal = @family.goals.create!(
name: "Pace goal",
target_amount: 10_000,
currency: "USD"
) { |g| g.goal_accounts.build(account: account) }
# Three inflows over the 90-day window. Sure convention: inflows are
# negative. Net = -900 → pace = 900 / 3 = 300.
create_transaction(account: account, amount: -300, date: 80.days.ago.to_date)
create_transaction(account: account, amount: -300, date: 40.days.ago.to_date)
create_transaction(account: account, amount: -300, date: 5.days.ago.to_date)
# Pending inflow that must be excluded by `Transaction.excluding_pending`.
pending_entry = create_transaction(account: account, amount: -1_000, date: 10.days.ago.to_date)
pending_entry.transaction.update!(extra: { "plaid" => { "pending" => true } })
# User-excluded outflow that must be excluded by `entries.excluded = false`.
excluded_entry = create_transaction(account: account, amount: 500, date: 20.days.ago.to_date)
excluded_entry.update!(excluded: true)
# Entry outside the 90-day window — must be ignored.
create_transaction(account: account, amount: -10_000, date: 200.days.ago.to_date)
assert_equal 300, goal.pace.to_d
end
test "months_of_runway is nil when goal has a target date" do
assert_not_nil @goal.target_date
assert_nil @goal.months_of_runway