From 03620e9d420046247637e3c0618de832a373cd57 Mon Sep 17 00:00:00 2001 From: Bryce Date: Wed, 24 Jun 2026 19:38:09 -0700 Subject: [PATCH] =?UTF-8?q?refactor(ssr):=20Phase=203=20=E2=80=94=20full?= =?UTF-8?q?=20Selmer=20migration=20of=20Transaction=20Bulk=20Code;=20remov?= =?UTF-8?q?e=20the=20wizard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrates the Transaction Bulk Code modal (a single-step form wearing a full wizard costume) to a plain Selmer form, cold-applying the ssr-form-migration skill. Almost entirely reuse of the Phase-2 work: the whole `sc/*` Selmer component library, `account-typeahead*` / `location-select*`, and the `edit-modal` / `transitioner` chrome are imported wholesale. What changed - Wizard removed: deleted `BulkCodeWizard` / `AccountsStep` records, `MultiStepFormState`, the `step-params[...]` prefix, and all `mm/*` middleware. Replaced with a plain handler + flat `wrap-bulk-state` (decode straight into `bulk-code-schema`, no snapshot round-trip). - Selection round-trip: the non-editable transaction selection is resolved to a concrete not-locked id vector at open and ridden back in hidden `ids[]` fields (the bulk analog of edit's single `db/id`) — no EDN snapshot, no filter re-query, and more correct (codes exactly the rows the user saw). - 100% Selmer render path (only the shared terminal `com/success-modal` keeps Hiccup — heuristic-9 exception). New shared component `sc/select` (`location-select.html` generalized) for the status dropdown. - Routes 4 -> 3: GET `bulk-code` (open), POST `bulk-code-submit`, POST `bulk-code-form-changed` (one whole-form op dispatcher folding the old `new-account` + `vendor-changed` routes). Location swap moved off `find *` onto explicit `#account-location-` + `hx-select`. - Fixed a latent correctness bug surfaced by the migration: the vendor typeahead needs `:id` (value-keyed `:key`) or its value-bound hidden goes stale across a whole-form swap and posts blank. Scorecard delta (transaction/bulk_code.clj): mm coupling 19->0, snapshot merges 4->0, wizard records 3->0, step-params 10->0, routes 4->3, OOB 0, Hiccup-in-render ->0 (bar success-modal). LOC 420->506 (documented exception: the wizard was a thin shell over mm/* defaults, so explicitness moves shared plumbing into the file). Cookbook: reused the entire Phase-2 sc/* lib + chrome, added sc/select. Verification: bulk-code-transactions.spec.ts 13/13; full Playwright suite 39/39; cljfmt clean. Skill fed: scorecard row + narrative + LOC exception; gotchas (value-bound typeahead keying, selection-as-ids round-trip); cookbook (sc/select). Co-Authored-By: Claude Opus 4.8 --- .../reference/component-cookbook.md | 1 + .../ssr-form-migration/reference/gotchas.md | 37 + .../ssr-form-migration/reference/scorecard.md | 26 + e2e/bulk-code-transactions.spec.ts | 36 +- resources/templates/components/select.html | 4 + .../transaction-bulk-code/bulk-code-body.html | 3 + .../transaction-bulk-code/bulk-code-form.html | 5 + src/clj/auto_ap/ssr/components/selmer.clj | 18 + src/clj/auto_ap/ssr/transaction/bulk_code.clj | 713 ++++++++++-------- src/cljc/auto_ap/routes/transactions.cljc | 5 +- 10 files changed, 515 insertions(+), 333 deletions(-) create mode 100644 resources/templates/components/select.html create mode 100644 resources/templates/transaction-bulk-code/bulk-code-body.html create mode 100644 resources/templates/transaction-bulk-code/bulk-code-form.html diff --git a/.claude/skills/ssr-form-migration/reference/component-cookbook.md b/.claude/skills/ssr-form-migration/reference/component-cookbook.md index af778820..79919252 100644 --- a/.claude/skills/ssr-form-migration/reference/component-cookbook.md +++ b/.claude/skills/ssr-form-migration/reference/component-cookbook.md @@ -164,6 +164,7 @@ the interop bridge; dynamic HTMX/Alpine attrs go through `sc/attrs->str` → | Wrapper | Partial | Notes | |---------|---------|-------| | `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 `` +whose DOM `.value` is set by Alpine, not by the server-rendered static `value` attr. After a +**whole-form `outerHTML` swap** that re-renders the typeahead, Alpine may preserve the *previous* +component's empty `.value` instead of binding the new server value — so the field posts blank +on the next submit. Fix: pass **`:id`** to `sc/typeahead` (the account typeahead already does). +`:id` makes the wrapper emit `:key (str id "--" value)`, and the value-keyed `:key` forces a +clean Alpine re-init that lands the server value. The bulk-code *vendor* typeahead hit this +(account rows didn't, because they pass `:id`) — symptom: "vendor not preserved on a validation +re-render." Note the testing trap: reading the hidden's `.value` in isolation +(`inputValue()` / `toHaveValue`) is an unreliable probe — it lags Alpine. Assert what the form +**actually posts** instead: `new FormData(form).get('vendor')` (wrap in `expect.poll`). + +## Round-trip a multi-row selection as `ids[]`, not as an EDN/filter snapshot + +A bulk modal acts on a *selection* of N entities (bulk-code: the checked transactions), the +analog of a single modal's one `db/id`. The wizard stashed the whole search-params blob (filters ++ `selected` + `all-selected`) in the EDN snapshot and re-ran the filter query on every post. +Don't carry that forward. Instead **resolve the selection to a concrete id vector once at open** +(`selected->ids` → the not-locked set) and ride it back in hidden `ids[0..n]` fields; re-read it +on each post (`[:vector {:coerce? true} entity-id]` + the `coerce-vector` transformer turns the +`{"0" "123"}` index-map into `[123]`). No snapshot, no filter round-trip, and it's *more* correct +— 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. + ## Scorecard exceptions (ratchet violations with a reason) +**Heuristic 4 (LOC net ↓) — exception (Phase 3, Transaction Bulk Code: 420→506).** When the +modal's wizard was a *thin* shell that delegated almost everything to `mm/*` defaults +(`default-render-step`, `default-render-wizard`, `submit-handler`, `open-wizard-handler`), +ripping the wizard out moves that previously-shared plumbing **into the file** as explicit +render/decode/submit/handler code, so the single-file LOC rises even though total system +complexity drops. This is the opposite of a fat wizard (edit went 1608→1548). The trade is +intended and every other heuristic improved sharply (mm coupling 19→0, snapshot merges 4→0, +wizard records 3→0, routes 4→3, `find *`→explicit-id swap). Watch for it on the small +"single-step wearing a wizard costume" modals — LOC is the wrong headline metric there; +the mm-coupling / snapshot / route counts are. + **Heuristic 9 (Hiccup in render path) — partial exception (Phase 2-final).** The post-save `com/success-modal` confirmation dialogs in `save-handler` keep ~6 `[:p …]` Hiccup lines. They are terminal responses (shown after the form closes), reuse a shared dialog component, diff --git a/.claude/skills/ssr-form-migration/reference/scorecard.md b/.claude/skills/ssr-form-migration/reference/scorecard.md index f74491cf..8e9b6449 100644 --- a/.claude/skills/ssr-form-migration/reference/scorecard.md +++ b/.claude/skills/ssr-form-migration/reference/scorecard.md @@ -41,6 +41,9 @@ Each migration appends one row (after-numbers), referencing the before in the di | 1 (baseline) | Transaction Edit `transaction/edit.clj` | 1608 | ~12 | 1 | 2 | ~75 | 0 | 8 | — / seeded 7 entries | | 2 | Transaction Edit `transaction/edit.clj` | 1593 | **~5** | **0** | **0** | **0 round-trip** | 0 | 8 (shared) | location-select / **1 Selmer** | | 2-final | Transaction Edit (full Selmer + wizard removed) | 1548 | **5** | 0 | 0 | 0 | 0 | **0** | full `sc/*` lib / **~30 partials** | +| 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). ### New heuristics introduced at 2-final (full Selmer) @@ -82,3 +85,26 @@ Each migration appends one row (after-numbers), referencing the before in the di > shared component — out of the form's render path). See `form-vs-wizard.md` (drop-the- > wizard test), `selmer-conventions.md` (composition mechanics), and `gotchas.md` > (stray-field decode leak; jetty reload staleness). + +> **Phase 3 — Transaction Bulk Code (first cold apply of the mature skill).** Single-step +> form wearing a full wizard costume (`BulkCodeWizard`/`AccountsStep`, `MultiStepFormState`, +> the `step-params[...]` prefix, the old `find *` location swap). Migrated to a plain form by +> mirroring Phase 2 — and it was mostly **reuse**: the entire `sc/*` Selmer component library, +> `account-typeahead*`/`location-select*`, and the `edit-modal`/`transitioner` chrome were +> imported wholesale; the only new shared component was **`sc/select`** (the status dropdown — +> `location-select.html` generalized). Parity held: bulk-code spec **13/13**, full suite +> **39/39** (up from the Phase-2 baseline of 38–39). mm coupling 19→0, snapshot merges 4→0, +> wizard records 3→0, routes 4→3 (open / submit / `form-changed` — the per-op `new-account` + +> `vendor-changed` routes folded into one `form-changed` op dispatcher), the location swap moved +> off `find *` onto explicit `#account-location-` + `hx-select`. +> +> **The one regression — LOC 420→506 (documented exception, see `gotchas.md`).** Unlike edit +> (whose wizard held real custom code), bulk-code's wizard was a *thin* shell that delegated +> almost everything to `mm/*` defaults (`default-render-step`, `default-render-wizard`, +> `submit-handler`, `open-wizard-handler`). Ripping the wizard out moves that +> previously-shared plumbing **into the file** as explicit render/decode/submit/handler code. +> The trade is intended: every other heuristic improved and the modal is now self-contained +> and wizard-free. New patterns added to the cookbook: the **selection-as-`ids[]` round-trip** +> (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). diff --git a/e2e/bulk-code-transactions.spec.ts b/e2e/bulk-code-transactions.spec.ts index 0b9c385d..eb0d71e6 100644 --- a/e2e/bulk-code-transactions.spec.ts +++ b/e2e/bulk-code-transactions.spec.ts @@ -40,7 +40,7 @@ async function openBulkCodeModal(page: any) { const codeButton = page.locator('button:has-text("Code")').first(); await codeButton.click(); await page.waitForSelector('#modal-holder[x-show="open"]', { state: 'visible' }); - await page.waitForSelector('#wizardmodal'); + await page.waitForSelector('#bulkcodemodal'); } async function closeBulkCodeModal(page: any) { @@ -156,7 +156,7 @@ async function addNewAccount(page: any) { } async function submitBulkCodeForm(page: any) { - const form = page.locator('#wizard-form'); + const form = page.locator('#bulk-code-form'); await form.evaluate((el: HTMLFormElement) => { el.dispatchEvent(new Event('submit', { bubbles: true })); }); @@ -184,7 +184,7 @@ test.describe('Bulk Code Transactions - Happy Path', () => { await expect(page.locator('text=Bulk editing 1 transactions')).toBeVisible(); // Select vendor - const vendorHidden = page.locator('input[type="hidden"][name="step-params[vendor]"]').first(); + const vendorHidden = page.locator('input[type="hidden"][name="vendor"]').first(); const testInfo = await getTestInfo(page); await vendorHidden.evaluate((el: HTMLInputElement, value: string) => { const newInput = document.createElement('input'); @@ -196,7 +196,7 @@ test.describe('Bulk Code Transactions - Happy Path', () => { await page.waitForTimeout(300); // Select approval status - const statusSelect = page.locator('select[name="step-params[approval-status]"]').first(); + const statusSelect = page.locator('select[name="approval-status"]').first(); await statusSelect.selectOption('approved'); // Add account @@ -278,7 +278,7 @@ test.describe('Bulk Code Transactions - Validation', () => { const testInfo = await getTestInfo(page); const vendorId = testInfo.accounts.vendor; - const vendorContainer = page.locator('div[hx-post*="vendor-changed"]').first(); + const vendorContainer = page.locator('div[hx-vals*="vendor-changed"]').first(); const vendorHidden = vendorContainer.locator('input[type="hidden"]').first(); await vendorHidden.evaluate((el: HTMLInputElement, value: string) => { @@ -293,11 +293,11 @@ test.describe('Bulk Code Transactions - Validation', () => { el.dispatchEvent(new Event('change', { bubbles: true })); }); - await page.waitForResponse(response => response.url().includes('/vendor-changed') && response.status() === 200); + await page.waitForResponse(response => response.url().includes('/form-changed') && response.status() === 200); await page.waitForTimeout(500); // Select approval status - const statusSelect = page.locator('select[name="step-params[approval-status]"]').first(); + const statusSelect = page.locator('select[name="approval-status"]').first(); await statusSelect.selectOption('approved'); // Vendor selection pre-populated a default account row at 100%. @@ -310,11 +310,15 @@ test.describe('Bulk Code Transactions - Validation', () => { // Modal should still be open await expect(page.locator('#modal-holder[x-show="open"]')).toBeVisible(); - // Vendor should still be selected - const vendorHiddenAfter = page.locator('input[type="hidden"][name="step-params[vendor]"]').first(); - const vendorValueAfter = await vendorHiddenAfter.inputValue(); - expect(vendorValueAfter).toBe(vendorId.toString()); - + // Vendor should still be selected. The vendor typeahead is Alpine-managed and posts + // its value via an x-bound hidden input, so the right correctness check is what the + // form actually submits (the value that gets saved), not the lagging DOM .value of the + // hidden read in isolation. + await expect.poll(async () => + page.locator('#bulk-code-form').evaluate((f: HTMLFormElement) => + new FormData(f).get('vendor')) + ).toBe(vendorId.toString()); + // Status should still be selected const statusValueAfter = await statusSelect.inputValue(); expect(statusValueAfter).toBe('approved'); @@ -464,7 +468,7 @@ test.describe('Bulk Code Transactions - Vendor Pre-population', () => { // The vendor typeahead dispatches change from its parent div // We need to set the hidden input and dispatch change on the container - const vendorContainer = page.locator('div[hx-post*="vendor-changed"]').first(); + const vendorContainer = page.locator('div[hx-vals*="vendor-changed"]').first(); const vendorHidden = vendorContainer.locator('input[type="hidden"]').first(); await vendorHidden.evaluate((el: HTMLInputElement, value: string) => { @@ -481,7 +485,7 @@ test.describe('Bulk Code Transactions - Vendor Pre-population', () => { }); // Wait for HTMX response - await page.waitForResponse(response => response.url().includes('/vendor-changed') && response.status() === 200); + await page.waitForResponse(response => response.url().includes('/form-changed') && response.status() === 200); await page.waitForTimeout(500); // Account should be pre-populated - check for account row @@ -521,7 +525,7 @@ test.describe('Bulk Code Transactions - Vendor Pre-population', () => { const testInfo = await getTestInfo(page); const vendorId = testInfo.accounts.vendor; - const vendorContainer = page.locator('div[hx-post*="vendor-changed"]').first(); + const vendorContainer = page.locator('div[hx-vals*="vendor-changed"]').first(); const vendorHidden = vendorContainer.locator('input[type="hidden"]').first(); await vendorHidden.evaluate((el: HTMLInputElement, value: string) => { @@ -538,7 +542,7 @@ test.describe('Bulk Code Transactions - Vendor Pre-population', () => { }); // Wait for HTMX response - await page.waitForResponse(response => response.url().includes('/vendor-changed') && response.status() === 200); + await page.waitForResponse(response => response.url().includes('/form-changed') && response.status() === 200); await page.waitForTimeout(500); // Should pre-populate the vendor's default account (non-clientized) plus the "New account" row diff --git a/resources/templates/components/select.html b/resources/templates/components/select.html new file mode 100644 index 00000000..cc7eab4e --- /dev/null +++ b/resources/templates/components/select.html @@ -0,0 +1,4 @@ +{# Generic {% for opt in options %}{% endfor %} diff --git a/resources/templates/transaction-bulk-code/bulk-code-body.html b/resources/templates/transaction-bulk-code/bulk-code-body.html new file mode 100644 index 00000000..52b17f1a --- /dev/null +++ b/resources/templates/transaction-bulk-code/bulk-code-body.html @@ -0,0 +1,3 @@ +{# Bulk-code modal body: vendor field (a change repopulates the default account via a + whole-form swap), status select, and the expense-account grid. #} +
{{ vendor_field|safe }}
{{ status_field|safe }}

Expense Accounts

{{ accounts_field|safe }}
diff --git a/resources/templates/transaction-bulk-code/bulk-code-form.html b/resources/templates/transaction-bulk-code/bulk-code-form.html new file mode 100644 index 00000000..266a0923 --- /dev/null +++ b/resources/templates/transaction-bulk-code/bulk-code-form.html @@ -0,0 +1,5 @@ +{# Top-level plain form for bulk-code (no wizard). The resolved (not-locked) transaction + id set rides in hidden ids[] fields -- the analog of the edit modal's single db/id + hidden -- so the selection survives form-changed / submit posts without an EDN snapshot + or a filter round-trip. #} +
{{ ids_hidden|safe }}{{ modal|safe }}
diff --git a/src/clj/auto_ap/ssr/components/selmer.clj b/src/clj/auto_ap/ssr/components/selmer.clj index 1226145a..e1330219 100644 --- a/src/clj/auto_ap/ssr/components/selmer.clj +++ b/src/clj/auto_ap/ssr/components/selmer.clj @@ -81,6 +81,24 @@ (assoc :type "number" :step "0.01"))] (render "templates/components/money-input.html" {:attrs (attrs->str attrs)}))) +(defn select + "Generic