fix(ssr): operation handlers read live step-params, not the stale snapshot (rewrite stage 1)

apply-new-account / apply-remove-account / apply-toggle-amount-mode rebuilt the account
rows from the decoded :snapshot, dropping any value the user typed but that had not yet
round-tripped into the snapshot (type 50%, click "New account" -> first row reverts,
giving a 66.67/33.33 split instead of 50/50). Read the live :step-params rows instead
(already schema-decoded by mm/wrap-wizard, so typed), falling back to snapshot only when
absent. First stage of removing the snapshot round-trip; fixes a real user-facing bug
(typed amounts lost on add/remove/$%-toggle).

Restore the percentage-split e2e to the realistic type-then-add ordering as a regression
guard. Modal stays green: swap 6/6, transaction-edit 8/8.
This commit is contained in:
2026-06-03 08:18:58 -07:00
parent 38ad665726
commit 0b5bfd9c84
3 changed files with 32 additions and 25 deletions

View File

@@ -108,13 +108,12 @@ should ride in the form, not a parallel snapshot). It bit the percentage-split e
50% then adding a row reverted the first row to its snapshot value, yielding a 66.67/33.33 50% then adding a row reverted the first row to its snapshot value, yielding a 66.67/33.33
split. Two ways it shows up and how to handle until the snapshot is gone: split. Two ways it shows up and how to handle until the snapshot is gone:
- **In tests:** order interactions so no whole-form operation runs between typing and the **Fixed (Stage 1 of the wizard→plain-form rewrite):** the operation handlers now read the
save (toggle/add/remove *first*, then pick accounts and type, then save). The live `:step-params` rows (already schema-decoded by `mm/wrap-wizard`, so typed) and fall
account→location and amount→totals swaps are *targeted* (don't rebuild rows), so they're back to the snapshot only when absent — typed values survive add/remove/toggle. The
safe between typing and save. percentage e2e was restored to the realistic type-then-add ordering as a regression guard.
- **The real fix** (deferred to the wizard→plain-form rewrite): operations read the live The *full* removal (no snapshot at all; the posted form *is* the state) is the remaining
`:step-params` rows (coercing string amounts/ids), or there is no snapshot at all and the rewrite stages.
posted form *is* the state.
## Characterization tests rot against table order and removed wizard chrome ## Characterization tests rot against table order and removed wizard chrome

View File

@@ -214,25 +214,20 @@ test.describe('Transaction Edit Full Workflow', () => {
// Transaction 0 is $100. Code it 50% / 50% across two accounts in percentage mode and // Transaction 0 is $100. Code it 50% / 50% across two accounts in percentage mode and
// verify the save-time %->$ conversion stores/displays $50 + $50 on reopen. // verify the save-time %->$ conversion stores/displays $50 + $50 on reopen.
// //
// Ordering matters with the current snapshot machinery: a whole-form operation // This intentionally types a percentage and THEN adds another row -- a whole-form
// (add/remove row, mode toggle) rebuilds the rows from the server snapshot and drops // operation. The operation handlers now rebuild from the live posted form, not the
// any value only present in the live form. So we add the rows and toggle to % FIRST, // stale snapshot, so the first row's typed 50% survives (it used to revert, yielding a
// then pick accounts and type the percentages, with no operation between typing and // 66.67/33.33 split).
// the save -- those values ride straight through. (The underlying snapshot-vs-form
// gap is the heuristic-2 work tracked in the ssr-form-migration skill.)
await openEditModal(page, 0); await openEditModal(page, 0);
await removeAllAccounts(page); await removeAllAccounts(page);
// Two empty rows, then switch to percentage mode (both are whole-form operations).
await addNewAccount(page);
await addNewAccount(page);
await toggleToPercentMode(page); await toggleToPercentMode(page);
// Now pick the accounts (targeted location swap) and set 50% / 50% (targeted totals await addNewAccount(page);
// swap). Neither re-renders the rows from the snapshot, so the form keeps these.
await selectAccountFromTypeahead(page, 0, 'Test'); await selectAccountFromTypeahead(page, 0, 'Test');
await selectAccountFromTypeahead(page, 1, 'Second');
await setAccountAmount(page, 0, '50'); await setAccountAmount(page, 0, '50');
await addNewAccount(page);
await selectAccountFromTypeahead(page, 1, 'Second');
await setAccountAmount(page, 1, '50'); await setAccountAmount(page, 1, '50');
await saveTransaction(page); await saveTransaction(page);

View File

@@ -537,10 +537,15 @@
"edit-form-changed op: convert account amounts between $ and % and record the new mode." "edit-form-changed op: convert account amounts between $ and % and record the new mode."
[request] [request]
(let [snapshot (-> request :multi-form-state :snapshot) (let [snapshot (-> request :multi-form-state :snapshot)
step-params (-> request :multi-form-state :step-params)
old-mode (or (:amount-mode snapshot) "$") old-mode (or (:amount-mode snapshot) "$")
new-mode (or (get-in request [:multi-form-state :step-params :amount-mode]) "$") new-mode (or (:amount-mode step-params) "$")
total (Math/abs (or (:transaction/amount snapshot) 0.0)) total (Math/abs (or (:transaction/amount snapshot) 0.0))
accounts (convert-accounts-mode (:transaction/accounts snapshot) old-mode new-mode total)] ;; Convert the LIVE rows (step-params), not the stale snapshot, so amounts the
;; user typed before toggling survive. step-params is already schema-decoded.
accounts (convert-accounts-mode (or (seq (:transaction/accounts step-params))
(:transaction/accounts snapshot))
old-mode new-mode total)]
(-> request (-> request
(assoc-in [:multi-form-state :snapshot :transaction/accounts] accounts) (assoc-in [:multi-form-state :snapshot :transaction/accounts] accounts)
(assoc-in [:multi-form-state :snapshot :amount-mode] new-mode)))) (assoc-in [:multi-form-state :snapshot :amount-mode] new-mode))))
@@ -1456,13 +1461,17 @@
"edit-form-changed op: append a fresh account row." "edit-form-changed op: append a fresh account row."
[request] [request]
(let [snapshot (-> request :multi-form-state :snapshot) (let [snapshot (-> request :multi-form-state :snapshot)
amount-mode (or (:amount-mode snapshot) "$") step-params (-> request :multi-form-state :step-params)
amount-mode (or (:amount-mode step-params) (:amount-mode snapshot) "$")
total (Math/abs (or (:transaction/amount snapshot) 0.0)) total (Math/abs (or (:transaction/amount snapshot) 0.0))
new-account {:db/id (str (java.util.UUID/randomUUID)) new-account {:db/id (str (java.util.UUID/randomUUID))
:new? true :new? true
:transaction-account/location "Shared" :transaction-account/location "Shared"
:transaction-account/amount (if (= amount-mode "%") 100.0 total)} :transaction-account/amount (if (= amount-mode "%") 100.0 total)}
accounts (vec (or (:transaction/accounts snapshot) [])) ;; Append to the LIVE rows (step-params) so values typed before clicking
;; "New account" are not reverted to the stale snapshot.
accounts (vec (or (seq (:transaction/accounts step-params))
(:transaction/accounts snapshot) []))
updated-accounts (conj accounts new-account) updated-accounts (conj accounts new-account)
updated-request (-> request updated-request (-> request
(assoc-in [:multi-form-state :snapshot :transaction/accounts] updated-accounts) (assoc-in [:multi-form-state :snapshot :transaction/accounts] updated-accounts)
@@ -1474,7 +1483,11 @@
[request] [request]
(let [row-index (some-> request :form-params (get "row-index") Integer/parseInt) (let [row-index (some-> request :form-params (get "row-index") Integer/parseInt)
snapshot (-> request :multi-form-state :snapshot) snapshot (-> request :multi-form-state :snapshot)
accounts (vec (or (:transaction/accounts snapshot) [])) step-params (-> request :multi-form-state :step-params)
;; Remove from the LIVE rows (step-params) so the surviving rows keep the values
;; the user typed, rather than reverting to the stale snapshot.
accounts (vec (or (seq (:transaction/accounts step-params))
(:transaction/accounts snapshot) []))
updated-accounts (if (and row-index (< row-index (count accounts))) updated-accounts (if (and row-index (< row-index (count accounts)))
(vec (concat (subvec accounts 0 row-index) (vec (concat (subvec accounts 0 row-index)
(subvec accounts (inc row-index)))) (subvec accounts (inc row-index))))