test(ssr): Phase 7 parity gate — characterization spec for the Invoice Pay wizard

The Invoice Pay wizard is the first GENUINE multi-data-step wizard: choose-method
(collects bank-account + method) -> payment-details (collects check-number /
handwritten-date / amounts), merged only at submit. This gate characterizes that
flow before migrating it onto the session-backed engine, so the merge can be proven
behavior-preserving.

- Seed: make the TEST client's check bank account visible (+ name "Test Checking")
  so the choose-method step renders a usable method card. The pay flow had no e2e
  coverage, so the bank account was never visible in tests before.
- Spec drives the real 2-step flow against the unmodified wizard: choose-method
  renders the bank account + its methods (print-check/debit/handwrite-check, in the
  card tooltip); picking handwrite-check advances to payment-details (check-number +
  date + Pay); filling the check number and submitting shows the completion modal.
  The handwrite-check path is used because it transacts a pending check payment
  directly (no PDF/S3), making the success assertion stable.

Notes for the migration: the method controls live in a <template x-ref="tooltip">
revealed by the card button; the footer Pay submit is x-ref="next"; both the grid
filters and the modal carry a check-number input, so the modal selectors are scoped
to #wizard-form.

Verification: invoice-pay spec 3/3; full suite 58/58 (no regressions from the seed
change).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

.claude/skills/ssr-form-migration/reference/gotchas.md

@@ -265,23 +265,31 @@ carve-out. Verify with `load-file` (compile) + `lein cljfmt check`, not by eyeba
 diff is contained with `git diff -U0 <file> | grep '^@@'` — the hunks should cluster only where you
 edited (requires + the modal region), nothing else.
 
-## Wiring a modal onto the wizard2 engine — three traps that cost a debug cycle each
+## Wiring a modal onto the wizard2 engine — use the engine's primitives, don't re-roll them
 
-1. **Strip the engine's nav fields in the step `:decode`.** The posted form carries
-   `wizard-id` / `current-step` / `direction` alongside the real fields. If the step schema is
-   an open `:map` (most are), `mc/decode` keeps them, they ride into `get-all`, and the save's
-   `:upsert-entity` dies with `:db.error/not-an-entity ... :current-step`. Fix: `select-keys`
-   the decode to the schema's known top-level keys (the same allowlist trick as the flat-form
-   migrations). Symptom is a **500 on save**, not a validation message.
-2. **New repeated-row needs a temp `:db/id` or the step can't advance.** If the row schema
-   requires `[:db/id [:or entity-id temp-id]]`, an added row with no id fails per-step
-   validation, so the engine re-renders the *same* step instead of advancing — looks like "the
-   Next/Test button does nothing." Give new rows `(str (java.util.UUID/randomUUID))`.
-3. **Nav is a `direction` field, and Back/Save are both submit buttons.** The footer buttons
-   are plain `<button type="submit" name="direction" value="next|back|submit">`; the clicked
-   one's value rides in the POST and the engine branches on it. In tests, a selector like
-   `button:has-text("Save"), button[type=submit]` also matches **Back** (also a submit) and
-   `.first()` clicks Back — target the button by its text/value precisely.
+Phase 6's first migration (Transaction Rule) hit three traps; an adversarial review pointed
+out the engine had the information to prevent all three, so **the engine now absorbs them**.
+A consumer is just a config map + the step `:render` fns — reach for these instead of
+re-implementing them (and re-hitting the bug):
+
+- **Nav fields are stripped for you.** `handle-step-submit` `dissoc`s its own
+  `wizard-id`/`current-step`/`direction` from `:form-params` before calling a step's
+  `:decode` (`wizard2.clj`), so your decode sees only real fields and they can't ride into
+  the saved entity. (The old failure was a **500 on save** — `:db.error/not-an-entity
+  :current-step` — because an open `:map` decode kept them. No allowlist needed anymore.)
+- **`wizard2/open-wizard` owns the modal wrap.** Give the config an `:open-response` fn
+  (e.g. `(fn [form] (modal-response [:div#transitioner.flex-1 form]))`); then the
+  new/edit routes are literally `(partial wizard2/open-wizard config)`. Don't hand-roll
+  `create!/render/wrap/thread` — that boilerplate was duplicating engine internals.
+- **Add rows with `wizard2/blank-row`.** It supplies a temp `:db/id` (so a row schema
+  requiring `[:db/id [:or entity-id temp-id]]` validates and the step actually advances —
+  the old symptom was "the Next/Test button does nothing") plus `:new?` for the appear
+  animation: `(wizard2/blank-row :foo/location "Shared")`.
+- **Footer with `wizard2/nav-footer`.** It emits the `direction` submit buttons (Back /
+  primary advance / Save), marks the advance/save button `data-primary`, and the form's
+  Enter guard (`wizard2/wizard-form`) triggers `data-primary` — so Enter and Back/Save
+  aren't left to per-consumer convention. (Testing note that survives: Back and Save are
+  *both* `type=submit`, so target a save button by its text, not `button[type=submit]`.)
 
 ## Scorecard exceptions (ratchet violations with a reason)
 

e2e/invoice-pay.spec.ts

@@ -0,0 +1,73 @@
+import { test, expect } from '@playwright/test';
+
+// Characterization spec for the Invoice Pay wizard (the first genuine multi-data-step
+// wizard: choose-method -> payment-details, merged at submit). Captures CURRENT
+// (pre-migration) behavior so the migration onto the session-backed engine can be proven
+// behavior-preserving. The seed's lone unpaid invoice (UNPAID-001, Test Vendor, $150,
+// client TEST) is payable; its client has one visible check bank account (Test Checking).
+test.beforeEach(async ({ request }) => { await request.post('/test-reset'); });
+
+// Select the unpaid invoice on the grid and open the pay wizard (choose-method step).
+async function openPayWizard(page: any) {
+  await page.setExtraHTTPHeaders({ 'x-clients': '"mine"' });
+  await page.goto('/invoice');
+  await page.waitForSelector('#entity-table tbody tr');
+  await page.locator('#entity-table tbody input[type="checkbox"]').first().click();
+  await page.waitForTimeout(300);
+  // #pay-button's container hx-gets /invoice/pay on click; wait for the wizard to land.
+  await page.locator('#pay-button').first().click();
+  await page.waitForTimeout(900);
+}
+
+// The bank-account card's method options (print-check / debit / handwrite-check) live in a
+// <template x-ref="tooltip"> revealed by clicking the card's tooltip button; open it.
+async function openMethodTooltip(page: any) {
+  await page.locator('button[x-ref="button"]').first().click();
+  await page.waitForTimeout(400);
+}
+
+// Advance choose-method -> payment-details by picking a method (each is an hx-put to
+// .../pay/navigate?to=:payment-details carrying step-params[method]).
+async function pickMethod(page: any, method: string) {
+  await openMethodTooltip(page);
+  await page.locator(`[hx-vals*="${method}"]`).first().click();
+  await page.waitForTimeout(900);
+}
+
+test.describe.configure({ mode: 'serial' });
+
+test.describe('Invoice Pay wizard (characterization)', () => {
+  test('choose-method step renders the bank account and its payment methods', async ({ page }) => {
+    await openPayWizard(page);
+    const body = page.locator('body');
+    await expect(body).toContainText('Payment method');
+    await expect(body).toContainText('Test Checking');
+    // a check account offers print-check / debit / handwrite-check (in the card's tooltip)
+    await openMethodTooltip(page);
+    expect(await page.locator('[hx-vals*="handwrite-check"]').count()).toBeGreaterThan(0);
+    expect(await page.locator('[hx-vals*="print-check"]').count()).toBeGreaterThan(0);
+  });
+
+  test('picking handwrite-check advances to the payment-details step', async ({ page }) => {
+    await openPayWizard(page);
+    await pickMethod(page, 'handwrite-check');
+    const body = page.locator('body');
+    await expect(body).toContainText('Check number'); // handwrite-check-only field
+    await expect(body).toContainText('Date');         // check date
+    await expect(body.locator('button:has-text("Pay"), a:has-text("Pay")').first()).toBeVisible();
+  });
+
+  test('completing a handwritten-check payment shows the success modal', async ({ page }) => {
+    await openPayWizard(page);
+    await pickMethod(page, 'handwrite-check');
+    // step 2 collects the check number; method (step 1) + check-number (step 2) combine at submit
+    // scope to the wizard form (the background grid filters also have a check-number input)
+    await page.locator('#wizard-form input[name*="check-number"]').first().fill('10001');
+    await page.waitForTimeout(150);
+    // the footer submit button (x-ref="next"), not the background #pay-button
+    await page.locator('[x-ref="next"]').first().click();
+    await page.waitForTimeout(1500);
+    // the submit transacts a pending check payment and swaps in the completion modal
+    await expect(page.locator('body')).toContainText('payment is complete');
+  });
+});

src/clj/auto_ap/ssr/admin/transaction_rules.clj

@@ -16,7 +16,6 @@
    [auto-ap.ssr-routes :as ssr-routes]
    [auto-ap.ssr.company :refer [bank-account-typeahead*]]
    [auto-ap.ssr.components :as com]
-   [auto-ap.ssr.components.wizard-state :as ws]
    [auto-ap.ssr.components.wizard2 :as wizard2]
    [auto-ap.ssr.grid-page-helper :as helper :refer [wrap-apply-sort]]
    [auto-ap.ssr.hx :as hx]
@@ -664,19 +663,6 @@
    (com/modal-body {} body)
    (com/modal-footer {} footer)))
 
-(defn- rule-nav
-  "Footer step controls. Buttons post a `direction` field the engine reads:
-   next = validate + advance, back = no validate, submit = finish."
-  [{:keys [next back? save?]}]
-  [:div.flex.justify-end.gap-x-4
-   [:div#form-errors]
-   (when back?
-     (com/button {:type "submit" :name "direction" :value "back" :class "w-24"} "Back"))
-   (when next
-     (com/button {:type "submit" :name "direction" :value "next" :color :primary :class "w-24"} next))
-   (when save?
-     (com/button {:type "submit" :name "direction" :value "submit" :color :primary :class "w-24" :x-ref "next"} "Save"))])
-
 (defn render-edit-step
   "Edit step: the rule form, de-cursored (explicit data + path->name2 + *errors*)."
   [{:keys [step-data errors]}]
@@ -783,7 +769,7 @@
                                                       :name (fname :transaction-rule/transaction-approval-status)
                                                       :size :small
                                                       :orientation :horizontal}))]]]
-       :footer (rule-nav {:next "Test"})))))
+       :footer (wizard2/nav-footer {:next "Test"})))))
 
 (defn render-test-step
   "Test step: a read-only preview of the transactions the rule (the combined session
@@ -793,24 +779,15 @@
    :head [:div.p-2.flex.space-x-4 [:div "Transaction Rule"] [:div ">"] [:div "Results"]]
    :body [:div.space-y-1 {:class "w-[850px] h-[600px]"}
           (transaction-rule-test-table* {:entity all-data :clients (:clients request)})]
-   :footer (rule-nav {:back? true :save? true})))
-
-(def ^:private rule-form-keys
-  "Top-level keys form-schema recognises. The posted form also carries the engine's nav
-   fields (wizard-id / current-step / direction); without this allowlist they'd ride into
-   the decoded rule (form-schema is an open :map) and break the upsert."
-  [:db/id :transaction-rule/client :transaction-rule/client-group :transaction-rule/description
-   :transaction-rule/bank-account :transaction-rule/amount-gte :transaction-rule/amount-lte
-   :transaction-rule/dom-gte :transaction-rule/dom-lte :transaction-rule/vendor
-   :transaction-rule/transaction-approval-status :transaction-rule/accounts])
+   :footer (wizard2/nav-footer {:back? true :save? true})))
 
 (defn- decode-rule-form
-  "Parse the posted edit-step fields straight into the rule map (no step-params prefix);
-   strip the stray engine nav fields."
+  "Parse the posted edit-step fields straight into the rule map (no step-params prefix).
+   The engine has already stripped its own nav fields (wizard-id / current-step /
+   direction), so they can't leak into the decoded rule."
   [request]
-  (let [nested (:form-params (nfp/nested-params-request request {}))
-        decoded (mc/decode form-schema nested main-transformer)]
-    (if (map? decoded) (select-keys decoded rule-form-keys) {})))
+  (let [nested (:form-params (nfp/nested-params-request request {}))]
+    (mc/decode form-schema nested main-transformer)))
 
 (defn- rule-form-errors
   "Per-step validation: schema-validate so an invalid form can't advance to the test step
@@ -846,6 +823,11 @@
    :init-fn (fn [request]
               {:context {}
                :init-data (when-let [e (:entity request)] {:edit e})})
+   ;; The engine owns the modal wrap: open-wizard applies this to the rendered form, so the
+   ;; new/edit routes are just (partial wizard2/open-wizard config) -- no hand-rolled
+   ;; create!/render/wrap/thread boilerplate.
+   :open-response (fn [form]
+                    (modal-response [:div#transitioner.flex-1 form]))
    :steps [{:key :edit
             :decode decode-rule-form
             :validate rule-form-errors
@@ -857,18 +839,6 @@
             :next (fn [_] :done)}]
    :done-fn save-rule!})
 
-(defn open-rule-wizard
-  "Open handler (new or edit): create the wizard instance, render its first step, and
-   wrap it in the modal shell the stack expects."
-  [request]
-  (let [cfg transaction-rule-wizard-config
-        {:keys [context init-data]} ((:init-fn cfg) request)
-        [id session'] (ws/create-wizard! (:session request) (:name cfg)
-                                         {:first-step :edit :context context :init-data init-data})
-        form (wizard2/render-wizard {:config cfg :wizard-id id :session session' :request request})]
-    (-> (modal-response [:div#transitioner.flex-1 form])
-        (assoc :session session'))))
-
 (defn save-step
   "POST handler for every step transition (next / back / save) -- the engine reads the
    `direction` field and either advances, goes back, or finishes via done-fn."
@@ -884,9 +854,7 @@
         client-id (-> request :query-params :client-id)
         client-locations (some->> client-id (pull-attr (dc/db conn) :client/locations))]
     (html-response
-     (transaction-rule-account-row* {:db/id (str (java.util.UUID/randomUUID))
-                                     :new? true
-                                     :transaction-rule-account/location "Shared"}
+     (transaction-rule-account-row* (wizard2/blank-row :transaction-rule-account/location "Shared")
                                     idx client-id client-locations))))
 
 (def key->handler
@@ -948,11 +916,11 @@
                                 (wrap-entity [:route-params :db/id] default-read)
                                 (wrap-schema-enforce :route-schema [:map [:db/id entity-id]]))
 
-     ::route/edit-dialog (-> open-rule-wizard
+     ::route/edit-dialog (-> (partial wizard2/open-wizard transaction-rule-wizard-config)
                              (wrap-entity [:route-params :db/id] default-read)
                              (wrap-schema-enforce :route-schema [:map [:db/id entity-id]]))
 
-     ::route/new-dialog  open-rule-wizard})
+     ::route/new-dialog  (partial wizard2/open-wizard transaction-rule-wizard-config)})
    (fn [h]
      (-> h
          (wrap-copy-qp-pqp)

src/clj/auto_ap/ssr/components/wizard2.clj

@@ -47,17 +47,46 @@
 
 (defn wizard-form
   "Wrap a step body in the wizard <form>: the form posts to the submit route, and only the
-   wizard-id + current-step ride along (no accumulated data — that lives in the session)."
+   wizard-id + current-step ride along (no accumulated data — that lives in the session).
+   Enter is guarded so it triggers the step's primary nav button (the one marked
+   `data-primary`) rather than whichever submit button the browser picks first."
   [config wizard-id current-step body]
   [:form (merge {:id (:form-id config "wizard-form")
                  :hx-post (:submit-route config)
                  :hx-target "this"
-                 :hx-swap "outerHTML"}
+                 :hx-swap "outerHTML"
+                 "@keydown.enter.prevent.stop" "$el.querySelector('[data-primary]')?.click()"}
                 (:form-attrs config))
    (com/hidden {:name "wizard-id" :value wizard-id})
    (com/hidden {:name "current-step" :value (name current-step)})
    body])
 
+(defn nav-footer
+  "Standard wizard footer controls — so consumers don't hand-roll the `direction` buttons
+   (and mis-target Back vs Save, or forget the Enter guard). Buttons post a `direction`
+   field the engine branches on; the advance/save button is marked `data-primary` so the
+   form's Enter guard triggers it. Also renders the `#form-errors` slot.
+
+     (nav-footer {:next \"Test\"})            ; an intermediate step: Next
+     (nav-footer {:back? true :save? true})  ; the last step: Back + Save"
+  [{:keys [next back? save?]}]
+  [:div.flex.justify-end.items-baseline.gap-x-4
+   [:div#form-errors]
+   (when back?
+     (com/button {:type "submit" :name "direction" :value "back" :class "w-24"} "Back"))
+   (when next
+     (com/button {:type "submit" :name "direction" :value "next" :data-primary "" :color :primary :class "w-24"} next))
+   (when save?
+     (com/button {:type "submit" :name "direction" :value "submit" :data-primary "" :color :primary :class "w-24"} "Save"))])
+
+(defn blank-row
+  "A fresh repeated-row map for an 'add row' interaction, with a temp `:db/id` (so a row
+   schema requiring `[:db/id [:or entity-id temp-id]]` validates and the step can advance,
+   instead of the add button silently doing nothing) plus `:new?` for the appear
+   animation. Merge in any field defaults: `(blank-row :foo/location \"Shared\")`."
+  [& {:as defaults}]
+  (merge {:db/id (str (java.util.UUID/randomUUID)) :new? true} defaults))
+
 (defn render-wizard
   "Render the current step's body inside the wizard form. `step-data`/`errors` let a
    validation re-render show the just-posted values + messages."
@@ -86,16 +115,23 @@
       (assoc :session session)))
 
 (defn open-wizard
-  "Create a wizard instance in the session and render its first step. `:init-fn` returns
-   {:context ..., :init-data ...} (both optional)."
+  "Create a wizard instance in the session, render its first step, and return a Ring
+   response with the updated session threaded. `:init-fn` returns {:context ..., :init-data
+   ...} (both optional). If the config supplies an `:open-response` fn it is applied to the
+   rendered form hiccup to build the response (e.g. wrap it in a modal shell via
+   modal-response); otherwise a bare html-response is returned. This makes open-wizard
+   directly usable as a route handler — `(partial open-wizard config)` — for modal
+   wizards, instead of every consumer re-implementing create!/render/wrap/thread."
   [config request]
   (let [{:keys [context init-data]} ((:init-fn config) request)
         first-step (-> config :steps first :key)
         [id session'] (ws/create-wizard! (:session request) (:name config)
                                          {:first-step first-step
                                           :context context
-                                          :init-data init-data})]
-    (render-response config id session' request)))
+                                          :init-data init-data})
+        form (render-wizard {:config config :wizard-id id :session session' :request request})
+        resp ((or (:open-response config) html-response) form)]
+    (assoc resp :session session')))
 
 (defn- expired-response
   "The wizard instance is gone from the session (server restart / session expiry / a stale
@@ -127,7 +163,11 @@
 
       :else
       (let [step (step-by-key config current-step)
-            posted ((:decode step) request)
+            ;; The engine owns wizard-id / current-step / direction. Strip them so the
+            ;; step's :decode never sees them and can decode straight into its schema --
+            ;; no per-consumer allowlist, and they can't leak into the saved entity.
+            clean (update request :form-params dissoc "wizard-id" "current-step" "direction")
+            posted ((:decode step) clean)
             errors (when-let [v (:validate step)] (v posted request))]
         (if (seq errors)
           (render-response config wizard-id session request

test/clj/auto_ap/test_server.clj

@@ -69,7 +69,8 @@
                                 [(assoc (test-client :db/id "client-id"
                                                      :client/code "TEST"
                                                      :client/locations ["DT"])
-                                        :client/bank-accounts [(test-bank-account :db/id "bank-account-id" :bank-account/code "TEST-CHK")])
+                                        :client/bank-accounts [(test-bank-account :db/id "bank-account-id" :bank-account/code "TEST-CHK"
+                                                                                  :bank-account/visible true :bank-account/name "Test Checking")])
                                  (test-client :db/id "client-id-2"
                                               :client/code "TEST2"
                                               :client/locations ["NY"])