Commit Graph

5 Commits

Author SHA1 Message Date
Guillem Arias
f672aae3cf perf + tests(goals): share account-ids across velocity windows + cover gaps
- Family#savings_inflow_windows wraps the current/prior 30d sums in a
  single helper that memoizes the linked-account-id lookup. The KPI tile
  on the goals index used to run the join+pluck twice per request.
- Replace two instance_variable_set pokes and one any_instance.stubs in
  the goal/controller tests. Refetching the goal exercises the real
  request lifecycle and stops the tests from leaning on implementation
  details. The 'All caught up' assertion now relies on a real reached
  state (target 1 vs the depository fixture's 5000 balance) rather than
  stubbing :status.
- Add tests covering: hex format validation on Goal#color, AASM cache
  reset (display_status reads the new state on the same instance after
  pause!), negative pledge amount rejection, expire! no-op on already-
  expired pledge, cancel! NotOpenError on non-open pledge, sweep job
  idempotency on a second pass, and strong-params rejection of state /
  family_id on goal create.
2026-05-18 21:11:30 +02:00
Guillem Arias
9f29185160 fix(goals): address AI review on PR #1798 (CodeRabbit + Codex)
Correctness:
- GoalPledge#matches? rejects outflows on transfer pledges so a +$200
  purchase no longer satisfies a $200 deposit pledge after .abs
- GoalsController#sync_linked_accounts! saves through the goal so
  currency/depository/family validations actually run on update
- AlreadyClaimedError replaces empty RecordInvalid in resolve_with! and
  reconciler rescues the dedicated class
- SweepExpiredGoalPledgesJob wraps each expire! in a per-record rescue
- Assistant::Function::CreateGoal disambiguates duplicate account names
  and returns an absolute URL via mailer host config
- Family#savings_inflow_velocity defensively scopes from the family's
  accounts (was Account.joins(:goal_accounts).where(goal_id: ...))
- GoalPledgesController#set_goal preloads linked_accounts + providers
  to drop the N+1 on any_connected_account?
- Stepper subtitle update walks to the enclosing dialog before
  querySelector so two stepper instances don't fight over one header
- categories/_form.html.erb data-action targets color-icon-picker, not
  the non-existent "category" controller

UX / visual:
- Projection chart drops preserveAspectRatio="none" and pins endDate at
  today for past-due goals so the today marker stays in-domain
- _color_picker / categories form swap non-standard border-1 for border
- Goals index search input uses ring-alpha-black-100 (was raw gray-500)

Refactors:
- Goal#header_summary extracts the multi-line ERB header block
- Goal#catch_up_delta_money sums open_pledges in SQL
- Goal#projection_summary uses I18n.l for the on-track month label
- Account#default_pledge_kind moves the manual/transfer decision out of
  GoalPledgesController
- GoalPledge::Reconciler iterates ordered (created_at, id) so first-claim
  wins is deterministic under non-sequential PKs
- Goals::FundingAccountsBreakdownComponent + Goals::AccountStackComponent
  use clamp(0..) instead of Float::INFINITY / [x, 0].max
- Goals::StatusPillComponent#label provides a titleize fallback
- Goal projection chart skips the redundant initial _draw and reuses
  the snapped point in the past branch (no double-bisect)
- Goal pledge preview drops maximumFractionDigits: 0 so USD/EUR show
  cents while JPY/KRW stay whole-unit
- Demo generator captures the Wedding fund goal in the seed loop
  instead of looking it up by hardcoded name

Tests:
- GoalPledgeTest: outflow rejection
- GoalsControllerTest: cross-currency attachment rejected on update
- SweepExpiredGoalPledgesJobTest: cancelled coverage + per-record rescue
- GoalTest: pledge_action_label_key flips to manual_save without an
  unconditional guard
2026-05-15 00:01:13 +02:00
Guillem Arias
83c64b9e94 fix(goals): pledge lifecycle + connected-account detection
Behavioural fixes touching Goal, GoalPledge, the reconciler and the
goals controller. No schema change.

B5 — connected-account detection covered only Plaid. SimpleFIN, Brex,
Enable Banking, IBKR, Kraken, SnapTrade and Lunchflow users got
"manual_save" pledges by default; their auto-synced Transactions then
failed to match (reconciler matches Transactions to "transfer" pledges
only). Pledges sat in the yellow banner until expiry. Switch the
detection to !Account#manual?, which mirrors the existing
`Account.manual` scope (no account_providers, no plaid_account_id, no
simplefin_account_id). Add `Account#manual?` so the per-instance and
per-query checks can't drift.

B7 — `extend!` widens `expires_at` but `matches?` was anchored on
`created_at ± 5d`, so an extension that pushed the expiry past day 5
didn't actually buy any match runway. Widen the upper bound to
`max(created_at + 5d, expires_at)`. The lower bound stays at
`created_at − 5d`.

B8 — `Goal#open_pledges` returned `status: open` regardless of expiry.
Between a pledge timing out (day 7) and the 15-min sweep job marking
it `expired`, the show page rendered a ghost yellow banner with
"0 days left" that the reconciler would no longer touch. Add
`expires_at >= NOW` to the scope so the visible state matches the
match-eligible state.

B9 — Double-click on Record pledge produced two identical open
pledges, which then stacked as two yellow banners. Add a create-time
validation rejecting duplicates against (goal_id, account_id, amount,
status=open, expires_at >= NOW).

B10 — The reconciler used `transaction.with_lock` but didn't lock the
pledge. Two concurrent reconcile attempts on different transactions
could both target the same pledge; one would lose to the partial
unique index on `transactions.extra->'goal'->>'pledge_id'` and the
RecordNotUnique was caught by the outer StandardError rescue, which
silently dropped the other transaction's match attempt entirely.
Lock the pledge first, re-check `status_open?` inside the lock, and
catch RecordNotUnique alongside RecordInvalid/NotOpenError in the
reconciler — so on a lost race we fall through to the next candidate
pledge instead of exiting the loop. Extract the Valuation-match path
to `GoalPledge#resolve_with_valuation!` so it goes through the same
locked status-recheck.

B12 — When a goal is destroyed, `dependent: :destroy` reaped pledges
but left `transactions.extra["goal"]["pledge_id"]` pointing at the
now-deleted UUIDs. The partial unique index on that JSON path then
indexed stale references. Add a `before_destroy` on GoalPledge that
clears the matching transaction's `extra` if it still points back to
the pledge.

B6 — `last_matched_pledge_at` used `goal_pledges.maximum(:updated_at)`
on matched rows. Any backfill or sync-resync that touches a matched
pledge bumped `updated_at`, so a single resync set every goal's "Last
saved N days ago" header back to "today". Switch to the entry's
`date` via a join through `matched_transaction_id`, which reflects the
date the money actually moved.

B22 — `scope :chronological` ordered DESC, the opposite of what the
name promises. Rename to `:reverse_chronological` and update the one
caller in `goals#show`. (Other models' `chronological` scopes are
unrelated and ordered correctly.)

Also: preload `account_providers` on `linked_accounts` in the index
and show controllers so `Account#manual?` walks the in-memory
collection instead of triggering N queries.

Tests: add fixture-backed coverage for extend-widens-match-window,
post-extend rejection beyond expiry, and the duplicate-pledge
validation. Existing assertions still hold against the new
`matches?` window math.
2026-05-14 19:12:28 +02:00
Guillem Arias
eb7ef50eed fix(goals): CI green — schema, brakeman, pledge modal, error class
Regenerate schema.rb after the three v2 migrations so CI's db:schema:load
picks up goal_pledges, the dropped goal_contributions, and the partial
unique pledge_id index.

Brakeman:
- Drop :account_id and :kind from goal_pledge permit; look the account
  up via @goal.linked_accounts.find_by(id:) instead and set kind
  server-side from goal.any_connected_account?.
- Rename goals.show.projection.on_track to .on_track_html so I18n
  marks the result html_safe automatically; drop the unconditional
  .html_safe call in show.html.erb.

Pledge modal: rewrite app/views/goal_pledges/new.html.erb to use
DS::Dialog (the Sure convention for create modals — matches
categories/transfers).

Error handling: replace `raise ActiveRecord::RecordInvalid, "string"`
in GoalPledge#extend!/cancel! with a dedicated GoalPledge::NotOpenError;
the controller rescues that specifically.

Tests: rewrite the "pace is zero" test to create a fresh account with
no entries (the fixture's depository accounts carry transaction history
that produces a non-zero pace). All goal tests now green (73 runs,
157 assertions, 0 failures).
2026-05-14 17:54:08 +02:00
Guillem Arias
88032ce020 feat(goals): v2 architecture — drop ledger, derive balance, add pledge
Reshape the goals feature to live on top of linked-account balances.
A goal's balance is now the live balance of every depository account
linked to it — no parallel ledger, no "log a contribution" step.

The "Add contribution" affordance is replaced by a 7-day GoalPledge
(kind: transfer | manual_save). GoalPledge::Reconciler matches incoming
Transactions (via Account::ProviderImportAdapter) and Valuations (via
Account::ReconciliationManager) against open pledges within ±5 days,
±$0.50, or ±1% — single hook covers every provider (Plaid, SimpleFIN,
Lunchflow, Enable Banking, Brex, IBKR, Kraken, SnapTrade) plus manual
balance edits. A 15-minute Sidekiq cron sweeps expired pledges.

Goal model: balance derived from linked_accounts.sum(&:balance), new
pace (90-day net non-transfer inflow), months_of_runway,
last_matched_pledge_*, pledge_action_label_key (the "I just
transferred…" vs "I just saved…" verb switch).

UI:
- Index gets a 3-card KPI strip (Contributed last 30d / Needs this
  month / On track) plus a pending-pledges callout.
- Show page swaps the "Add contribution" CTA for the pledge modal,
  replaces the contribution list with a pending-pledge banner, and
  rebuilds the funding widget into per-account rows with a 12-bucket
  weekly sparkline and last-30 inflow.
- Projection chart adds a required-line (dashed light from
  today → target) and a translucent pending-pledge bump at today's X.

Schema (3 migrations):
1. goal_pledges table with PG enums (goal_pledge_kind, goal_pledge_status),
   open-by-expiry index, and unique-when-not-null matched_transaction_id.
2. Drop goal_contributions.
3. Partial unique index on
   transactions ((extra -> 'goal' ->> 'pledge_id')) built CONCURRENTLY
   so it doesn't block prod.

After pulling: run bin/rails db:migrate, then commit the schema.rb sync
separately (or let CI regenerate).

Deferred to v1.1: allocation columns, contention/archived banners,
"why is this behind?" diagnostic, reallocate flow, refresh-sync +
Plaid throttle, unallocated-cash chip, joint-account approval,
goal_activities log, polymorphic matched_entry_id/type for manual
pledge audit.
2026-05-14 16:07:14 +02:00