Commit Graph

9 Commits

Author SHA1 Message Date
Guillem Arias
839d6b36ad fix(retirement): isolate retirement goals from savings goal routes
Addresses Codex P2 on #2044. A Goal::Retirement row lives in
Current.family.goals, so the shared GoalsController and
GoalPledgesController loaded it through `family.goals.find(...)` —
never calling Goal::Retirement#editable_by?. Any preview-enabled
family member could therefore open /goals/:id and edit/archive/delete
another member's owner-scoped retirement plan, hit its pledge routes,
and see it listed in the savings Goals grid.

Adds `Goal.savings` (base type only) and scopes both savings
controllers to it, so retirement goals are unreachable through the
shared routes (RecordNotFound -> goals_path redirect) and absent from
the savings index. Owner-only retirement access stays in
RetirementController; editable_by? is retained for it.

Tests: savings scope excludes retirement; retirement goal absent from
goals index; show + pledge routes redirect not-found for retirement.

(The Codex schema.rb null:false finding is a false positive — this
branch's schema.rb retains null:false on all IBKR payload columns and
the diff vs the base branch touches no IBKR lines; Codex compared
against main rather than the PR base.)
2026-05-29 10:25:05 +02:00
Guillem Arias
e16d9c4a6a Merge origin/feat/goals-v2-architecture; reconcile beta→preview rename
Remote branch added a beta_gated_nav_item helper + 'Gating the main nav'
docs section. Main concurrently renamed the beta-features gate to
preview-features (concern, predicate, JSONB key, locale flash). Rename
the new helper / partial local / pill marker to match preview naming and
port the nav-gating docs into gating-a-preview-feature.md so the
improvement survives the rename.

Resolved conflicts:
- db/schema.rb: take the later schema version (2026_05_19_100000).
- docs/llm-guides/gating-a-beta-feature.md: accept main's deletion;
  port the 'Gating the main nav' section into the preview guide.

Renames carried through to keep the gate wired end-to-end:
- application_helper.rb: beta_gated_nav_item → preview_gated_nav_item;
  beta_features_enabled? → preview_features_enabled?; beta: → preview:.
- _nav_item.html.erb: beta: local → preview: local; shared.beta i18n
  key → shared.preview.
- application.html.erb: caller renamed to preview_gated_nav_item.
- goals/index.html.erb: pill label uses shared.preview.
- shared/en.yml: 'beta: Beta' → 'preview: Preview'.
- goals_controller, goal_pledges_controller: require_beta_features! →
  require_preview_features!.
- goals_controller_test, goal_pledges_controller_test: flip the
  preference key, flash matcher, and test names to 'preview'.
2026-05-20 21:47:27 +02:00
Guillem Arias
5c7babc44e feat(goals): gate Goals v2 behind beta features toggle
Add require_beta_features! to GoalsController and GoalPledgesController,
hide the Goals nav item for non-beta users, and tag index/show headers
with the Beta pill marker. Update controller tests to enable the
preference in setup and assert the redirect for users without access.
2026-05-18 20:13:44 +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
cd2bfa8eb5 fix(goals/pledge): redirect non-turbo-frame GET to goal show
UX audit: `app/views/goal_pledges/new.html.erb` unconditionally
renders the form in a DS::Dialog wrapper — when the user lands on
`/goals/:id/pledges/new` directly (F5, bookmark, stale deep-link),
the dialog renders as a freestanding modal over an otherwise-empty
page. Compare to `goals/new.html.erb` which has a
`turbo_frame_request?` branch with a full-page fallback.

Pledges aren't usefully standalone — the modal only makes sense
in goal context. Redirect non-frame GETs back to the goal show
page instead of rendering the broken-looking standalone dialog.

If we want a deep-linkable "open the pledge modal" experience
later, that lands as a `?open=pledge` query on the show page that
auto-fires the modal — out of scope here.
2026-05-14 21:59:11 +02:00
Guillem Arias
51fca464b5 fix(goals): reconciler logs to Sentry + rename :extend route to :renew
Two Ruby idiom audit fixes.

The Reconciler's outer `rescue StandardError` was logging at error
level and moving on. Pipeline-protective (we don't want a Goal
reconcile failure to break the Plaid/SimpleFIN/etc importer it's
hooked into) but invisible — real bugs hid behind a warn log
forever. Add `Sentry.capture_exception(e) if defined?(Sentry)`
alongside the log, matching the pattern in `Account::Syncer`,
`Sync`, `PlaidItem`, and the chart-series rescues this branch
already added. Keep the rescue's protective function.

`member do patch :extend end` shadows `Module#extend` — the
controller action name competes with Ruby's most-common
mixin entry point. `before_action :foo, only: %i[extend destroy]`
reads as "extend this controller with :foo, only: …" to a casual
reader, and stack traces against `def extend` look misleading.
Rename to `:renew` (matches the existing copy: the button says
"Extend 7 days," but the API verb is "renew the watching window"):

  - config/routes.rb: `patch :renew`
  - GoalPledgesController#extend → #renew
  - locale `goal_pledges.extend` → `goal_pledges.renew`
  - banner `extend_goal_pledge_path` → `renew_goal_pledge_path`
  - test refs updated

The user-facing button text is unchanged.
2026-05-14 21:50:01 +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
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