notid/integreat
1
0
Fork 0
Code Issues Pull Requests 3 Actions Packages Projects Releases Wiki Activity

refactor(ssr): wizard2 engine absorbs the per-consumer boilerplate (review follow-up)

Browse Source 
Adversarial review of Phase 6 found the engine's coupling had relocated rather than
dissolved: every wizard consumer had to hand-build a decode allowlist, re-implement the
open-handler modal wrap, mint temp ids for added rows, and hand-roll the nav buttons +
Enter guard. The engine had the information to prevent all four. Now it does:

- handle-step-submit strips its own nav fields (wizard-id/current-step/direction) from
  form-params before calling a step's :decode -- no per-consumer allowlist, and they can
  no longer leak into the saved entity (the Phase-6 "500 on save" class of bug is
  structurally impossible).
- open-wizard takes an :open-response config fn and owns the create!/render/wrap/thread
  flow, so modal wizards route through (partial wizard2/open-wizard config) directly.
- wizard2/blank-row supplies a temp :db/id (+ :new?) so an added row passes schema
  validation and the step actually advances.
- wizard2/nav-footer emits the direction buttons (Back/advance/Save), marks the primary,
  and wizard-form guards Enter to trigger the primary button.

Consumer (transaction_rules.clj) gets correspondingly leaner: deleted rule-form-keys +
the decode allowlist, rule-nav, and the hand-rolled open-rule-wizard; new/edit routes are
now (partial wizard2/open-wizard config). A new wizard is now just a config map + the step
:render fns. LOC 964 -> 932, and the deleted code was exactly the cross-consumer
boilerplate, not modal-specific logic.

Verification: rule spec 4/4; full suite 55/55; cljfmt clean. Skill gotchas updated from
"three traps" to "use the engine's primitives" (the engine now absorbs them).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Bryce
2026-06-25 14:38:07 -07:00
parent 107a02f4f1
commit a2d8517668
3 changed files with 86 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
src/clj/auto_ap/ssr/admin/transaction_rules.clj
View File

@@ -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)

54
src/clj/auto_ap/ssr/components/wizard2.clj
View File

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