From b22a1644e2e052a6e78c0254a8583b548f5fe6cf Mon Sep 17 00:00:00 2001 From: Guillem Arias Date: Thu, 14 May 2026 19:41:30 +0200 Subject: [PATCH] fix(goals/pledge-modal): use StyledFormBuilder + restore live preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `
` + `

` 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 `

` 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. --- app/controllers/goal_pledges_controller.rb | 22 +++++- ...r.js => goal_pledge_preview_controller.js} | 7 +- app/views/goal_pledges/new.html.erb | 73 +++++++++---------- config/locales/views/goal_pledges/en.yml | 8 +- 4 files changed, 61 insertions(+), 49 deletions(-) rename app/javascript/controllers/{goal_contribution_preview_controller.js => goal_pledge_preview_controller.js} (89%) diff --git a/app/controllers/goal_pledges_controller.rb b/app/controllers/goal_pledges_controller.rb index c88c87c78..66c80fec5 100644 --- a/app/controllers/goal_pledges_controller.rb +++ b/app/controllers/goal_pledges_controller.rb @@ -4,9 +4,11 @@ class GoalPledgesController < ApplicationController rescue_from ActiveRecord::RecordNotFound, with: :record_not_found def new + account = preselected_account @pledge = @goal.goal_pledges.new( currency: @goal.currency, - kind: default_kind_for(@goal), + account: account, + kind: kind_for_account(account), amount: params[:amount].presence ) end @@ -14,7 +16,7 @@ class GoalPledgesController < ApplicationController def create @pledge = @goal.goal_pledges.new(pledge_params) @pledge.account = lookup_account(params.dig(:goal_pledge, :account_id)) - @pledge.kind = default_kind_for(@goal) + @pledge.kind = kind_for_account(@pledge.account) @pledge.currency = @goal.currency if @pledge.save @@ -62,8 +64,20 @@ class GoalPledgesController < ApplicationController @goal.linked_accounts.find_by(id: id) end - def default_kind_for(goal) - goal.any_connected_account? ? "transfer" : "manual_save" + def preselected_account + requested = params[:account_id].presence && @goal.linked_accounts.find_by(id: params[:account_id]) + requested || @goal.linked_accounts.first + end + + # Per-account: manual accounts get a `manual_save` pledge (resolves on the + # user's next valuation), connected accounts get a `transfer` pledge + # (resolves when the synced deposit posts). Account-level avoids the + # mixed-funding goal bug where the goal-level toggle picked one kind for + # all pledges regardless of which account the user actually moved money + # into. + def kind_for_account(account) + return "transfer" if account.nil? + account.manual? ? "manual_save" : "transfer" end def record_not_found diff --git a/app/javascript/controllers/goal_contribution_preview_controller.js b/app/javascript/controllers/goal_pledge_preview_controller.js similarity index 89% rename from app/javascript/controllers/goal_contribution_preview_controller.js rename to app/javascript/controllers/goal_pledge_preview_controller.js index 22299f69a..8d7601aa6 100644 --- a/app/javascript/controllers/goal_contribution_preview_controller.js +++ b/app/javascript/controllers/goal_pledge_preview_controller.js @@ -1,9 +1,8 @@ import { Controller } from "@hotwired/stimulus"; -// Live impact preview for the add-contribution modal. Reads current -// balance + target amount from values and updates a preview sentence -// each keystroke. Template strings come from ERB so the wording stays -// localized. +// Live impact preview for the record-pledge modal. Reads current balance + +// target amount from values and updates a preview sentence each keystroke. +// Template strings come from ERB so the wording stays localized. export default class extends Controller { static targets = ["amountInput", "preview"]; static values = { diff --git a/app/views/goal_pledges/new.html.erb b/app/views/goal_pledges/new.html.erb index 409c37cae..ee0ab326a 100644 --- a/app/views/goal_pledges/new.html.erb +++ b/app/views/goal_pledges/new.html.erb @@ -1,52 +1,49 @@ <%= render DS::Dialog.new do |dialog| %> <% dialog.with_header(title: t(@goal.pledge_action_label_key)) %> <% dialog.with_body do %> - <%= styled_form_with model: @pledge, url: goal_pledges_path(@goal), class: "form" do |form| %> - <% if @pledge.errors.any? %> -

- <% @pledge.errors.full_messages.each do |msg| %> -

<%= msg %>

- <% end %> -
- <% end %> + <% if @pledge.errors.any? %> + <%= render "shared/form_errors", model: @pledge %> + <% end %> -

+ <%= styled_form_with model: @pledge, + url: goal_pledges_path(@goal), + class: "space-y-3", + data: { + controller: "goal-pledge-preview", + goal_pledge_preview_current_balance_value: @goal.current_balance.to_f, + goal_pledge_preview_target_amount_value: @goal.target_amount.to_f, + goal_pledge_preview_currency_value: @goal.currency, + goal_pledge_preview_template_zero_value: t(".preview_zero"), + goal_pledge_preview_template_nonzero_value: t(".preview_nonzero"), + goal_pledge_preview_template_reached_value: t(".preview_reached") + } do |f| %> +

<% if @goal.any_connected_account? %> - <%= t("goal_pledges.new.helper_transfer") %> + <%= t(".helper_transfer") %> <% else %> - <%= t("goal_pledges.new.helper_manual") %> + <%= t(".helper_manual") %> <% end %>

-
- <%= form.money_field :amount, - label: t("goal_pledges.new.amount_label"), - hide_currency: true, - required: true %> + <%= f.money_field :amount, + label: t(".amount_label"), + hide_currency: true, + autofocus: true, + required: true, + amount_data: { + goal_pledge_preview_target: "amountInput", + action: "input->goal-pledge-preview#update" + } %> -
-
- <%= form.label :account_id, - t("goal_pledges.new.account_label"), - class: "form-field__label" %> - <%= form.select :account_id, - @goal.linked_accounts.map { |a| [ a.name, a.id ] }, - { include_blank: false }, - class: "form-field__input" %> -
-
-
+

-
-
- <%= render DS::Button.new( - text: t("goal_pledges.new.cancel"), - variant: "ghost", - data: { action: "click->dialog#close" } - ) %> - <%= form.submit t("goal_pledges.new.submit"), - class: "btn btn--primary", - data: { turbo_frame: "_top" } %> + <%= f.select :account_id, + options_from_collection_for_select(@goal.linked_accounts, :id, :name, @pledge.account_id), + { label: t(".account_label") } %> + +
+ <%= f.submit t(".submit") %>
<% end %> <% end %> diff --git a/config/locales/views/goal_pledges/en.yml b/config/locales/views/goal_pledges/en.yml index a15325bde..7e5fe295c 100644 --- a/config/locales/views/goal_pledges/en.yml +++ b/config/locales/views/goal_pledges/en.yml @@ -2,12 +2,14 @@ en: goal_pledges: new: - helper_transfer: Sure will look for a matching deposit in your linked account. The pledge stays pending for 7 days (±5 days, ±$0.50 or ±1%%), then auto-confirms once it lands. - helper_manual: This will record on your next manual balance edit and confirm the contribution. + helper_transfer: Sure will look for a matching deposit in your linked account. The pledge stays pending for 7 days, then auto-confirms once Sure spots it. + helper_manual: We'll record this on your next manual balance edit and confirm the contribution. amount_label: Amount account_label: Into account - cancel: Cancel submit: Record pledge + preview_zero: "Currently {current} of {target} saved." + preview_nonzero: "Reaches {percent}% — {newTotal} of {target}." + preview_reached: "Hits your {target} target — goal reached." create: success: Pledge recorded. Sure will confirm it on the next sync. extend: