refactor(ssr): Phase 4 — full Selmer migration of POS Sales Summary; remove the wizard; fix add-item + totals

Migrates the POS Sales Summary edit modal off the wizard to a plain Selmer form,
building on the parity gate committed earlier. Largest migration so far and the
first with no prior test coverage.

What changed
- Wizard removed: deleted MainStep/EditWizard records, MultiStepFormState, the
  step-params[...] prefix, the EDN snapshot round-trip, and all mm/* middleware.
  Replaced with a plain handler + flat wrap-decode/wrap-derive-state. The 51 fc/
  cursor refs are de-cursored into explicit data + Selmer templates.
- db/id-keyed item merge: wrap-derive-state overlays posted items onto the
  persisted items by :db/id, so read-only fields the form doesn't post
  (ledger-side, amount) survive a re-render and the debit/credit split + totals
  stay correct. New manual rows (temp db/id) ride through as-is.
- Inline click-to-edit account cell preserved as three small targeted
  .account-cell-swap routes (edit/save/cancel-item-account), ported to Selmer
  with the new field-name scheme.
- 100% Selmer modal render path (the remaining Hiccup / hx-swap-oob / "hx-"
  strings are all grid-page code — grid render lambdas, the filters form, and the
  submit response-header map — not the modal).
- Routes: dropped edit-wizard-navigate + new-summary-item; added form-changed.

Fixes (two pre-existing bugs, per request)
- "New Summary Item" add button (was throwing `newRowIndex is not defined` and
  targeting a non-existent `.new-row`) is now a whole-form-swap op=new-item that
  adds an editable manual row (category + account typeahead + debit/credit money
  inputs + remove).
- The dead totals/balance display (malformed Hiccup that discarded its labels) is
  replaced by a proper #summary-totals block showing running Total +
  Balanced/Unbalanced, refreshed via a Rule-4 targeted swap on manual amount edits.

Scorecard delta (pos/sales_summaries.clj): LOC 790->732, mm coupling 20->0,
wizard records 4->0, fc/ cursor 51->0, step-params 27->0 (2 comments), modal
routes 8->6. (hx-swap-oob 1 and mixed-hx live in the grid page, not the modal.)

Verification: sales-summary spec 7/7 (incl. the two fixes); full Playwright suite
46/46; cljfmt clean. Skill fed: scorecard row + narrative; gotchas (parity-gate-
first, characterize-then-fix, keyup-trigger tests); cookbook (inline click-to-edit
cell, db/id-keyed item merge).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-24 22:13:19 -07:00
parent a289ff2557
commit 599b849e6f
8 changed files with 524 additions and 484 deletions

View File

@@ -165,6 +165,28 @@ the interop bridge; dynamic HTMX/Alpine attrs go through `sc/attrs->str` →
|---------|---------|-------|
| `sc/hidden` / `sc/text-input` / `sc/money-input` | `hidden`/`text-input`/`money-input`.html | leaf inputs; class via `inputs/default-input-classes` + `use-size` |
| `sc/select` (Phase 3) | `select.html` | generic `<select>`; `options [[value label] …]`, `:value` (string/keyword) marks selected, extra hx-/x- attrs ride through. `location-select.html` generalized — reach for this before `com/select`. Added for the bulk-code status field. |
## inline click-to-edit cell (Phase 4) — targeted `.account-cell` swap, not a whole-form op
A "display value + pencil → edit-in-place → check/cancel" cell. Three tiny **stateless** routes,
each swapping just the cell (`hx-target="closest .account-cell"`, `outerHTML`): a `display` cell
(value + pencil `hx-get edit`), an `edit` cell (typeahead + check `hx-put save` / cancel
`hx-get cancel`). State rides in the request (item index + current value via `hx-vals`), so no
server-side "which cell is editing" flag is needed. Keep it as its own routes — it is a distinct
feature, *not* folded into the whole-form `form-changed` dispatcher (that would lose the targeted
swap and re-render the whole modal on every pencil click). The cells are assembled with `sc/*` +
`sel/raw` strings (like `edit.clj`'s `footer*`); SVGs ride in as `svg/*` Hiccup via the
`sc/a-icon-button` body (no `[:svg]` literal lands in the modal file).
## db/id-keyed item merge (Phase 4) — for rows the form posts only partially
When a row renders some fields read-only (so they aren't posted) but the entity holds them
(sales-summary auto items post only db/id/category/account — not ledger-side/amount), the flat
`wrap-derive-state` must **overlay posted items onto the persisted items by `:db/id`** so the
unposted fields survive a re-render: `(merge (by-id (:db/id posted)) posted)`. New rows (temp
`:db/id` not in the entity) ride through as-is. This is the row-level analog of edit's
"entity-only fields always from the entity"; without it, a re-render drops ledger-side/amount and
the debit/credit split + totals break.
| `sc/validated-field` | `validated-field.html` | label + body + always-present error `<p>`; pass-through attrs land on the wrapping div (the per-row location cell hangs its swap wiring here) |
| `sc/button` / `sc/a-button` / `sc/a-icon-button` | `button`/`a-button`/`a-icon-button`.html | spinner via `{% include "spinner.html" %}`; class via `btn/bg-colors` |
| `sc/badge` / `sc/link` | `badge`/`link`.html | |

View File

@@ -224,6 +224,35 @@ on each post (`[:vector {:coerce? true} entity-id]` + the `coerce-vector` transf
— you code exactly the rows the user saw, immune to data changing between open and submit. This
is heuristic 2 → 0 for a multi-select modal.
## No parity gate? Build one first — seed + characterization spec, before touching code
A modal with **no e2e coverage** (and no test-server seed for its domain) cannot be migrated
safely — "behavior parity is proven by tests, not by reading" is the skill's #1 non-negotiable.
Phase 4 (POS Sales Summary) had zero coverage. The fix: (1) seed a representative entity in
`test_server.clj`'s `seed-test-data` and surface its id via `/test-info`; (2) write a
characterization spec against the **unmodified** modal and confirm it green; (3) commit the gate
*separately, ahead of the rewrite*. Reach the modal the real way (grid → row's edit button), not
a direct fragment URL. To discover the actual rendered structure (field names, ids, swap targets)
— especially when the code has dead/buggy render fns — dump the live modal HTML with a throwaway
spec first; assert against what *renders*, not what the code looks like.
## Characterize before you fix; never assert a bug as working
Writing the gate often surfaces pre-existing bugs (Phase 4: a "New Summary Item" button that
threw `newRowIndex is not defined`, and a totals display whose malformed Hiccup discarded its
own labels). Do **not** assert the broken behavior as if it works, and do **not** silently "fix"
it mid-refactor — surface it and let the user decide fix-vs-preserve. If they choose *fix*: the
spec first documents the break (a passing test of the *current* inert behavior or an explicit
note), then is rewritten to assert the *fixed* behavior as part of the migration commit.
## htmx `keyup`-triggered inputs need real keystrokes in tests
A money/text input wired `hx-trigger="keyup changed delay:300ms"` does **not** fire on Playwright
`.fill()` + `dispatchEvent('change')` — `fill` sets the value without keyup events. Use
`.click()` then `.pressSequentially('500')` (types char-by-char, firing keyup) so the targeted
swap actually triggers. (A `change`-triggered control is the opposite — `dispatchEvent('change')`
is fine there.)
## Scorecard exceptions (ratchet violations with a reason)
**Heuristic 4 (LOC net ↓) — exception (Phase 3, Transaction Bulk Code: 420→506).** When the

View File

@@ -44,6 +44,9 @@ Each migration appends one row (after-numbers), referencing the before in the di
| 3 | Transaction Bulk Code `transaction/bulk_code.clj` | 506 (was 420 — see exception) | **3** | 0 | 0 | **0** | 0 | 0† | reused **all** of Phase-2's `sc/*` lib + `account-typeahead*`/`location-select*` + `edit-modal`/`transitioner` chrome / added **`sc/select`** |
† The one `"hx-..."` string hit is a response-header map (`{"hx-trigger" "refreshTable, reset-selection"}`), not a mixed attribute encoding. mm coupling 19→**0**, wizard records 3→**0**, step-params 10→**0** (the 2 hits are comments), Hiccup-in-render → **0** except the shared `com/success-modal` (heuristic-9 exception, as in Phase 2).
| 4 | POS Sales Summary `pos/sales_summaries.clj` | **732** (was 790) | **6** modal | 0 | 0 | **0** | 1✦ | 0✦ | reused `sc/*` lib + `edit-modal`/`transitioner` chrome / added the inline click-to-edit **account-cell** + **manual-items** patterns |
✦ The residual 1 `hx-swap-oob` and the `"hx-..."` string hits all live in the **grid page** code (the `grid-page` render lambdas + the `filters` form + the submit response-header map) — none are in the migrated **modal** render path, which is 100% Selmer. `defrecord` count **0** (all 4 wizard records gone), `fc/` cursor refs 51→**0**, mm coupling 20→**0**, step-params 27→**0** (2 comments). LOC dropped (this wizard held real custom code, unlike bulk-code's thin shell). **Two pre-existing bugs fixed** (per the user's call): the "New Summary Item" add button (was throwing `newRowIndex is not defined`) and the dead totals/balance display.
### New heuristics introduced at 2-final (full Selmer)
@@ -108,3 +111,25 @@ Each migration appends one row (after-numbers), referencing the before in the di
> (resolve the non-editable selection to a concrete id vector at open, ride it in hidden
> fields — the bulk analog of edit's single `db/id`), and the **`:id`-keyed vendor typeahead**
> (a value-bound hidden must be keyed or its posted value goes stale across a whole-form swap).
> **Phase 4 — POS Sales Summary (first modal with no prior test coverage).** The largest
> migration so far and the first that required **building the parity gate first**: the modal
> had zero e2e/clj tests and the test server seeded no POS data, so the work began by seeding a
> balanced sales summary + writing a 7-test characterization spec (committed separately, ahead
> of the rewrite). Then the standard wizard→plain-Selmer migration: `MainStep`/`EditWizard` +
> `MultiStepFormState` deleted, the 51 `fc/` cursor refs de-cursored into explicit data +
> Selmer, `step-params` dropped, the EDN snapshot replaced by flat `wrap-decode`/`wrap-derive-state`
> (with a **db/id-keyed item merge** so the read-only fields the form doesn't post —
> ledger-side, amount — survive a re-render). The **inline click-to-edit account cell** (pencil →
> typeahead editor → check/cancel) was preserved as three small targeted `.account-cell`-swap
> routes (a distinct feature, not folded into the form-changed dispatcher). LOC 790→**732** (net
> ↓ — a fat wizard, opposite of bulk-code).
>
> **Characterize-then-fix.** Writing the gate surfaced two pre-existing bugs: the "New Summary
> Item" button threw `newRowIndex is not defined` (dead since forever) and the totals/balance
> display was dead code (malformed Hiccup that discarded its labels). The spec first *documented*
> them as broken (never assert a bug as working); then, on the user's call, the migration **fixed
> both** — add-item is now a whole-form-swap `op=new-item` adding an editable manual row, and a
> proper `#summary-totals` block shows running Total + Balanced/Unbalanced (a Rule-4 targeted swap
> on manual amount edits). The spec was updated to assert the fixed behavior. New cookbook entries:
> the **inline click-to-edit cell** and the **db/id-keyed item merge** for partially-posted rows.