From 0b5bfd9c84142cf7ead069a97461479aa31654ee Mon Sep 17 00:00:00 2001 From: Bryce Date: Wed, 3 Jun 2026 08:18:58 -0700 Subject: [PATCH] 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. --- .../ssr-form-migration/reference/gotchas.md | 13 +++++------ e2e/transaction-edit.spec.ts | 21 +++++++---------- src/clj/auto_ap/ssr/transaction/edit.clj | 23 +++++++++++++++---- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/.claude/skills/ssr-form-migration/reference/gotchas.md b/.claude/skills/ssr-form-migration/reference/gotchas.md index 29f63dcf..9758af0d 100644 --- a/.claude/skills/ssr-form-migration/reference/gotchas.md +++ b/.claude/skills/ssr-form-migration/reference/gotchas.md @@ -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 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 - save (toggle/add/remove *first*, then pick accounts and type, then save). The - account→location and amount→totals swaps are *targeted* (don't rebuild rows), so they're - safe between typing and save. -- **The real fix** (deferred to the wizard→plain-form rewrite): operations read the live - `:step-params` rows (coercing string amounts/ids), or there is no snapshot at all and the - posted form *is* the state. +**Fixed (Stage 1 of the wizard→plain-form rewrite):** the operation handlers now read the +live `:step-params` rows (already schema-decoded by `mm/wrap-wizard`, so typed) and fall +back to the snapshot only when absent — typed values survive add/remove/toggle. The +percentage e2e was restored to the realistic type-then-add ordering as a regression guard. +The *full* removal (no snapshot at all; the posted form *is* the state) is the remaining +rewrite stages. ## Characterization tests rot against table order and removed wizard chrome diff --git a/e2e/transaction-edit.spec.ts b/e2e/transaction-edit.spec.ts index 5c9691cc..7b9dcafa 100644 --- a/e2e/transaction-edit.spec.ts +++ b/e2e/transaction-edit.spec.ts @@ -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 // verify the save-time %->$ conversion stores/displays $50 + $50 on reopen. // - // Ordering matters with the current snapshot machinery: a whole-form operation - // (add/remove row, mode toggle) rebuilds the rows from the server snapshot and drops - // any value only present in the live form. So we add the rows and toggle to % FIRST, - // then pick accounts and type the percentages, with no operation between typing and - // 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.) + // This intentionally types a percentage and THEN adds another row -- a whole-form + // operation. The operation handlers now rebuild from the live posted form, not the + // stale snapshot, so the first row's typed 50% survives (it used to revert, yielding a + // 66.67/33.33 split). await openEditModal(page, 0); 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); - // Now pick the accounts (targeted location swap) and set 50% / 50% (targeted totals - // swap). Neither re-renders the rows from the snapshot, so the form keeps these. + await addNewAccount(page); await selectAccountFromTypeahead(page, 0, 'Test'); - await selectAccountFromTypeahead(page, 1, 'Second'); await setAccountAmount(page, 0, '50'); + + await addNewAccount(page); + await selectAccountFromTypeahead(page, 1, 'Second'); await setAccountAmount(page, 1, '50'); await saveTransaction(page); diff --git a/src/clj/auto_ap/ssr/transaction/edit.clj b/src/clj/auto_ap/ssr/transaction/edit.clj index 5e5ad1f8..ed9c1518 100644 --- a/src/clj/auto_ap/ssr/transaction/edit.clj +++ b/src/clj/auto_ap/ssr/transaction/edit.clj @@ -537,10 +537,15 @@ "edit-form-changed op: convert account amounts between $ and % and record the new mode." [request] (let [snapshot (-> request :multi-form-state :snapshot) + step-params (-> request :multi-form-state :step-params) 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)) - 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 (assoc-in [:multi-form-state :snapshot :transaction/accounts] accounts) (assoc-in [:multi-form-state :snapshot :amount-mode] new-mode)))) @@ -1456,13 +1461,17 @@ "edit-form-changed op: append a fresh account row." [request] (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)) new-account {:db/id (str (java.util.UUID/randomUUID)) :new? true :transaction-account/location "Shared" :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-request (-> request (assoc-in [:multi-form-state :snapshot :transaction/accounts] updated-accounts) @@ -1474,7 +1483,11 @@ [request] (let [row-index (some-> request :form-params (get "row-index") Integer/parseInt) 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))) (vec (concat (subvec accounts 0 row-index) (subvec accounts (inc row-index))))