Commit Graph

4 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
4a8f6557e6 fix(goals/pledge-modal): helper text reacts to selected account
The helper paragraph above the amount field was painted off
`@goal.any_connected_account?` — a goal-level decision that fires
once at modal render and never updates. On a mixed-funding goal
(one connected + one manual account linked), the helper read the
"transfer" copy regardless of which account the user picked from
the dropdown, even though the saved pledge's `kind` is decided
per-account by `kind_for_account` in the controller. Stale UI vs.
correct data.

Render both helper paragraphs hidden, then toggle the right one
visible via Stimulus on `change->goal-pledge-preview#accountChanged`.
The select renders its `<option>` tags with `data-manual="true|false"`
from `Account#manual?`; the controller reads that attribute off
the currently-selected option and flips `hidden` on the two helper
targets.

`connect()` also calls `accountChanged()` so the initial paint
matches the preselected account from `?account_id=…` (the catch-up
callout's amount-specific deep link).

Switch from `options_from_collection_for_select` to
`options_for_select` with the `[label, value, { data: { manual: } }]`
3-tuple form — Rails supports per-option data attributes there but
not on the collection helper.
2026-05-14 20:37:16 +02:00
Guillem Arias
b22a1644e2 fix(goals/pledge-modal): use StyledFormBuilder + restore live preview
V2 rebuilt the pledge create modal but bypassed the DS form helpers
inherited from `StyledFormBuilder`, lost the inline impact preview
from V1's contribution form, and shipped a goal-level "transfer vs
manual_save" toggle that broke on mixed-funding goals.

- Manual `form-field/__body/__label/__input` div-wrapping for the
  account select → idiomatic `f.select :account_id, choices,
  { label: t(".account_label") }`. The builder applies the required
  marker, error state, and inline-label handling automatically; the
  hand-built version drifted from that path and applied
  `form-field__input` directly onto the select element, where the
  builder picks the correct input class per field type.

- Hand-rolled `<div class="form-error">` + `<p>` loop for errors →
  `render "shared/form_errors", model: @pledge` (the shared partial
  with the destructive-icon prefix). Matches V1's contribution modal
  and the rest of the codebase.

- Drop `class: "btn btn--primary"` on `f.submit` → bare
  `f.submit t(".submit")`. The builder's `submit` is wired to
  `DS::Button.new(text:, full_width: true)`; the explicit class was
  redundant.

- Drop the duplicate "Cancel" button. DS::Dialog already renders an
  X in the header; the in-form ghost Cancel was a second close
  affordance with no analogue in the new-goal stepper or V1's
  contribution form.

- Drop `data: { turbo_frame: "_top" }` on submit. Success already
  flows through the controller's `turbo_stream.action(:redirect, …)`
  and on 422 the modal frame is the right swap target; the explicit
  `_top` was at best redundant and at worst a future Turbo footgun.

- Wire `data-controller="goal-pledge-preview"` on the form and add
  an inline preview `<p>` below the amount field. As the user types
  the amount, the line updates to "Reaches 75% — $3,750 of $5,000."
  or "Hits your $5,000 target — goal reached." Mirrors V1's
  contribution preview that V2 dropped on the floor.

- Rename `goal_contribution_preview_controller.js` →
  `goal_pledge_preview_controller.js`. Pure rename; the controller
  was already domain-neutral.

- Per-account pledge kind. The controller's `default_kind_for(goal)`
  picked `transfer` whenever the goal had ANY connected account —
  meaning a goal that linked a Plaid checking account AND a manual
  cash envelope routed every pledge as `transfer`, including those
  the user submitted against the manual account. The reconciler
  would then watch for a Transaction that never arrives. Replace
  with `kind_for_account(account)` that picks per-account: manual →
  `manual_save`, anything else → `transfer`.

- `new` action now respects `?account_id=…` query params and
  preselects that account (helpful for the catch-up callout's
  inline "Save $X/mo" CTA, which can target a specific account).

Locale: drop the hardcoded "(±5 days, ±$0.50 or ±1%)" tolerance
copy from the helper text — that detail belongs in docs, not in a
modal that fires on every pledge create. Currency-aware copy lands
in commit I. Drop the now-unused `cancel:` key. Add the three
preview templates (`preview_zero`, `preview_nonzero`,
`preview_reached`) consumed by the Stimulus controller.
2026-05-14 19:41:30 +02:00