Commit Graph

7 Commits

Author SHA1 Message Date
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
880ca69657 fix(goals): demote Behind pill to neutral surface + drop em-dashes
Behavioural + RUI audit follow-ups.

The yellow overload finding flagged three concurrent yellow surfaces
on the show page: the "Behind" status pill, the catch-up alert, and
the open-pledge banner(s). Demoting the alert to outline ownership
of the primary CTA addressed one layer, but the pill kept fighting
the alert for hue attention. "Behind" is a state, not a call to
action; the alert owns the action signal.

Switch the pill's classes from `bg-yellow-500/10 text-yellow-700`
to `bg-surface-inset text-yellow-700` (with the same dark-mode
override). Background goes neutral (matches paused/archived chips);
the text keeps the warning hue and the triangle-alert icon stays.
Signal preserved, weight reduced. The yellow alert below now reads
as the primary nudge instead of one of three matching tones.

Also: copy/em-dash sweep across goal surfaces. User-facing strings
that contained em-dashes ("Reaches 70% — $X of $Y", "into your
linked account — Sure will catch it", "You're at 80% — $X of $Y")
read as a stylistic tic; replace with comma/period/period
respectively. Form-stepper review placeholders "—" become "…"
(ellipsis reads as "not yet set" without the typographic weight).
Code comments + log messages also scrubbed for consistency; awkward
sed artifacts (//. its...) restored to readable English.

No locale-key shape changes; pure string-content edits + one
component-style tweak.
2026-05-14 22:12:52 +02:00
Guillem Arias
da4af43a7d fix(goals/new): avatar default icon + restore .goal-avatar color-mix
Two interlocking bugs on the new-goal modal's color/icon preview.

1. Avatar fell back to a literal "?" when icon + name were both
   blank — `form.object.name.to_s.strip.first&.upcase || "?"`. User
   reported the avatar looked empty on a fresh modal because the
   "?" disappears against many palette tints. Categories handle
   this by always showing the category icon. Replace the "?"
   fallback chain with a default `target` icon (matches the goal
   creation header's iconography):
     • icon present       → render that icon
     • icon blank, name   → render first letter
     • icon blank, no name → render default "target" icon

2. Picking a color via the Pickr color picker called
   `updateAvatarColors(color)` which inlined `style.backgroundColor`
   + `style.color = color` — overriding the `.goal-avatar` class's
   `color-mix(in oklab, var(--avatar-color) 55%, black)` rule. The
   class handles theme-aware contrast (darken text in light mode,
   full color in dark mode); the inline override killed it and
   text rendered at the same lightness as the 10% tint background.
   Update only the `--avatar-color` CSS variable; let the class
   continue computing the resolved colors.

Wire the avatar to the goal-stepper controller properly:
`_color_picker.html.erb` gains `data-goal-stepper-target="avatarPreview"`
on the span. `nameChanged` now updates the avatar directly (the
previous selector queried `[data-testid="goal-avatar"]` which
doesn't exist on the color_picker span) and:
- swaps to the first letter as the user types,
- restores the default-icon HTML (captured at connect) when the
  name is cleared,
- bails when the user has explicitly checked an icon radio (don't
  undo their choice).
2026-05-14 22:07:32 +02:00
Guillem Arias
ff98a9cee2 fix(goals/stepper): inactive connector line uses border-subdued
RUI audit: form-stepper progress line used `border-secondary` for
the inactive state — same weight as the active step's border, so
the active-step circle didn't visually pop against the line
connecting it to the inactive step. Recede passive states.

Swap to `border-subdued` (the DS's quieter divider) for the
inactive (step 1) line state. The active state stays `border-inverse`.
JS toggle in `goal_stepper_controller.js#updateStepperState` follows.
2026-05-14 21:51:55 +02:00
Guillem Arias
5530ff5f06 chore(goals): drop dead V1 hooks + surface chart errors
Loose ends from the V1 → V2 refactor that the architecture commit
didn't sweep.

- Demo generator (B14): the `goal_spec[:contributions]` arrays
  + the `wedding_contribs` / `house_contribs` builders still
  shipped in the file, but the seeding loop that consumed them
  was deleted alongside `GoalContribution`. Dead data. Strip both
  the per-goal arrays and the two locals. Goal balance/pace in
  the demo family now derives from the linked depository
  accounts' own seeded entries elsewhere in the generator.

- Goal stepper controller (B16): the `static targets` declaration
  still listed `initialContributionAmount` and
  `initialContributionAccountSelect`, and `refreshAccountSelect`
  + its two callsites still ran every time a linked-account
  checkbox flipped. The HTML targets disappeared with the V2
  stepper rebuild, so `has*Target` guards short-circuited and the
  method was a no-op — but it was still dispatched on every
  change. Drop the targets, the method, and the two callsites.

- Chart series rescue (B25): `Goal#balance_series_values` and
  `FundingAccountsBreakdownComponent#sparkline_map` both swallowed
  `StandardError` with a `Rails.logger.warn(…)`. The chart then
  degraded to "target line only" silently. Promote the log to
  `error` level and forward to Sentry when present (matching the
  pattern in `Account::Syncer`, `Sync`, `PlaidItem`). Fallback to
  empty result still preserved so the surface degrades instead of
  500-ing.
2026-05-14 19:48:32 +02:00
Guillem Arias
3fa762289a feat(goals/stepper+chart): Step 2 derived projection + JS i18n + Intl.NumberFormat
B — Step 2 of the create stepper used to echo Step 1 fields back at
the user in three labelled rows (Funding accounts: 2 · $123,456 balance;
Suggested monthly: $1,003/mo over 12 months). Replaces those rows with
a single derived sentence:

  "Save $1,003/mo across 2 accounts to hit it on time."

If no target date is set: "Set a target date to project a finish line."
The previous "Suggested monthly" + "Funding accounts" rows are dropped;
review block shows only Name, "$12,000 by May 11 2027", and the
derived insight sentence.

L — All hard-coded English templates + currency symbols in the JS
controllers go through Stimulus values now:

- goal_stepper_controller: new {currency, summaryWithDate, summaryNoDate,
  accountCountOne, accountCountOther, suggestedWithDate, suggestedNoDate}
  values. Money formatted via Intl.NumberFormat(undefined, { style:
  "currency", currency: this.currencyValue, maximumFractionDigits: 0 }).
- goal_projection_chart_controller: _fmtMoney upgraded to Intl.NumberFormat
  (was $/€/£ ternary fallback that lost JPY/INR/CHF/...).

Locale: new goals.form_stepper.step2.review.{summary_*,account_count,
suggested_*}. Old funding_accounts / suggested_monthly keys retained
(unused by the new ERB) so any translator paths in flight don't break.

Verified live via Playwright: step-2 review reads "Save $1,003/mo
across 2 accounts to hit it on time." for a $12,000 / 12-month / 2-
account goal.
2026-05-11 20:42:13 +02:00
Guillem Arias
9b61e4a41b refactor: rename Savings Goals feature to Goals
User-facing rename + structural rename. Feature is now called just
"Goals" everywhere — page title, sidebar nav, modal headings, flash
messages, AI assistant tool. Code identifiers follow:

- Models: SavingsGoal → Goal, SavingsContribution → GoalContribution,
  SavingsGoalAccount → GoalAccount.
- Tables: savings_goals → goals, savings_contributions → goal_contributions,
  savings_goal_accounts → goal_accounts. FK columns savings_goal_id →
  goal_id. New migration db/migrate/20260511100003_rename_savings_to_goals.rb
  uses rename_table + rename_column; PG handles index renaming and FK
  redirection automatically.
- Controllers: SavingsGoalsController → GoalsController,
  SavingsContributionsController → GoalContributionsController.
- Routes: /savings_goals → /goals, nested /goals/:id/contributions
  (resource name shifts; old route name aliases dropped).
- ViewComponent namespace: Savings::* → Goals::*. Component class
  names drop their redundant "Goal" prefix where the namespace already
  carries it: Savings::GoalCardComponent → Goals::CardComponent,
  Savings::GoalAvatarComponent → Goals::AvatarComponent. Others keep
  their names (Goals::ProgressRingComponent, Goals::StatusPillComponent,
  Goals::AccountStackComponent, Goals::FundingAccountsBreakdownComponent).
- Stimulus controllers: savings_goal_* → goal_*, savings_goals_filter
  → goals_filter. Stimulus identifiers in data-controller / data-*
  attributes follow.
- Locale keys: savings_goals: → goals: (top level), savings_contributions:
  → goal_contributions: (top level). All t() callers updated.
- AI assistant tool: Assistant::Function::CreateSavingsGoal →
  Assistant::Function::CreateGoal, tool name "create_savings_goal" →
  "create_goal", description / response text updated.
- Sidebar nav label "Savings" → "Goals". Goals/show + index page title
  "Savings" → "Goals". Empty goals_section heading/subtitle dropped
  (duplicated the page title post-rename).

Original migrations create_savings_goals / create_savings_goal_accounts /
create_savings_contributions remain untouched so historical replay
still works; the rename migration runs on top.
2026-05-11 20:08:32 +02:00