refactor(ssr): Phase 9 — migrate New/Edit Vendor onto the engine (5-step wizard)
A five-step linear wizard (info → terms → account → address → legal) plus a
separate Merge dialog, migrated off mm/* + form-cursor + the EDN snapshot onto the
session-backed engine (wizard2), following the Phase 8 template.
Latent bug found + fixed: the old "Next" PUT /admin/vendor/navigat carried a
[:map [:db/id entity-id]] route-schema on a route with no :db/id path param, so empty
route-params {} → main-transformer's parse-empty-as-nil → nil → 500 on every advance
(the same quirk as Phase 8's query-params, now via route-params). The engine's submit
is a POST with no such schema; the dead navigate route is deleted.
What changed:
- defrecord 5 → 0 (InfoModal/TermsModal/AccountModal/AddressModal/LegalEntityModal +
VendorWizard), mm/ 0, fc/ cursor refs 0 (wizard AND the de-cursored Merge dialog),
step-params[…] 0.
- 5 de-cursored step renders (plain data + path->name2 + a *errors* binding); the 3
repeated grids became add-row-handler + a blank-row row render; the timeline is
preserved as a per-step side panel.
- :init-fn branches new (empty) vs edit (entity split across the 5 steps' :init-data,
seeded as per-step step-data so edit opens populated); per-step :validate via
mc/validate + me/humanize replaces wrap-ensure-step; vendor-step wraps
handle-step-submit in try+ to surface create-time validation as a 4xx.
Two new gotchas found + fixed + documented:
- empty-step decode: an all-blank step collapses to nil (parse-empty-as-nil), which a
schema :validate rejects as "invalid type"; decode-with coerces nil → {} so optional-
only steps advance while required-field steps still fail on the missing key.
- blank nested entity: an untouched Address (all-nil, no :db/id) makes :upsert-entity
mint a tempid used only as value (datomic error); blank-address? drops it.
Verification: full e2e suite 65/65 (61 prior + 4 new: info renders + timeline; create
across all 5 steps persists; edit opens prefilled and a rename persists; a too-short
name blocks advancing). Create + edit confirmed at the REPL incl. the cookie-session
EDN round-trip. Skill fed (scorecard Phase 9; gotchas for both new traps).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -365,3 +365,35 @@ before they land in step-data, and coerce back to clj-time only for display
|
||||
(`->edn-safe-dates`) right after `mc/decode` is the clean seam — both the step `:decode` and
|
||||
the edit `:init-fn` run the posted/persisted map through it. Datomic's upsert wants
|
||||
`java.util.Date` anyway, so the done-fn needs no extra conversion.
|
||||
|
||||
## The `{}`→nil trap has a THIRD face: empty-step decode → validation "invalid type"
|
||||
|
||||
Beyond query-params (Phase 8) and route-params (Phase 9's `/navigat`), the same
|
||||
`parse-empty-as-nil` `:map` decoder bites a wizard step whose fields are all blank: an
|
||||
all-empty step posts only blank inputs → the decoded all-nil map collapses to `nil`. If that
|
||||
`nil` then flows into a `:validate` that does `(mc/validate step-schema data)`, validation
|
||||
fails with `[invalid type]` (nil isn't a map) and the step can never advance — even though
|
||||
every field is optional. The legal/address steps (all-optional) hit this.
|
||||
|
||||
Fix at the seam: have the step `:decode` coerce nil back to `{}`:
|
||||
```clojure
|
||||
(defn- decode-with [schema request]
|
||||
(or (mc/decode schema (... nested form-params ...) main-transformer) {}))
|
||||
```
|
||||
Now an optional-only step validates `{}` (passes, advances) while a required-field step
|
||||
(e.g. account needs `:vendor/default-account`) still fails on the *missing key*, not on a
|
||||
spurious nil. Don't "fix" it by skipping validation when data is nil — that lets a genuinely
|
||||
empty required step through.
|
||||
|
||||
## A new (db/id-less) nested entity with all-nil fields → datomic "tempid used only as value"
|
||||
|
||||
The empty Address step decodes to `{:vendor/address {:address/street1 nil, …}}` — a map of
|
||||
nils with no `:db/id`. `:upsert-entity` mints a tempid for that nested map but, since every
|
||||
attribute is nil, the address entity has nothing transacted, so the tempid is referenced as
|
||||
a ref value but never defined → `:db.error/tempid-not-an-entity … used only as value`. Drop
|
||||
such blank nested maps before the upsert:
|
||||
```clojure
|
||||
(defn- blank-address? [a] (and (map? a) (not (:db/id a)) (every? nil? (vals a))))
|
||||
```
|
||||
This is the nested-entity analogue of "don't create empty rows"; the engine's `blank-row`
|
||||
gives *added* rows a tempid, but a never-touched optional nested entity must be elided.
|
||||
|
||||
@@ -260,3 +260,36 @@ now / Add another / Close), matching the Phase 7 pay-success shape.
|
||||
default-path create → next-steps; customize-path → accounts grid → create → next-steps); the
|
||||
`maybe-spread-locations` unit test still 6/6; create semantics + edit prefill confirmed at the
|
||||
REPL; dates ride as `#inst` so step-data is EDN-safe across the non-terminal step.
|
||||
|
||||
---
|
||||
|
||||
## Phase 9 — New / Edit Vendor (5-step linear wizard)
|
||||
|
||||
`auto-ap.ssr.admin.vendors` — a five-step linear wizard (info → terms → account → address →
|
||||
legal) plus a separate Merge dialog. Migrated onto the engine following the Phase 8 template.
|
||||
|
||||
**Latent bug found + fixed (again):** the old "Next" PUT `/admin/vendor/navigat` (note the
|
||||
typo) carried a `[:map [:db/id entity-id]]` route-schema on a route with **no** `:db/id` path
|
||||
param, so empty route-params `{}`→nil 500d every advance (same `main-transformer` quirk as
|
||||
Phase 8's query-params). The engine's submit is a POST with no such schema → gone.
|
||||
|
||||
**Coupling outcome:** `defrecord` **5 → 0** (InfoModal / TermsModal / AccountModal /
|
||||
AddressModal / LegalEntityModal + VendorWizard all gone), `mm/` **0**, `fc/` cursor refs
|
||||
**0** (the wizard *and* the de-cursored Merge dialog), `step-params[…]` **0**. Routes: the
|
||||
broken `navigate` is deleted (open + submit + 3 add-rows + account-typeahead remain). The 5
|
||||
step renders are plain data + `path->name2` + a `*errors*` binding; the 3 repeated grids
|
||||
(terms-overrides / automatic-payment / account-overrides) became `add-row-handler` + a
|
||||
`blank-row` row render. The wizard timeline is preserved as a per-step side panel.
|
||||
|
||||
**Engine refinements exercised:** conditionless linear `:next`; `:init-fn` branches new
|
||||
(empty) vs edit (entity split across the 5 steps' `:init-data`, which `create-wizard!` seeds
|
||||
as per-step step-data so edit opens fully populated); per-step `:validate` via
|
||||
`mc/validate` + `me/humanize` replaces the old `wrap-ensure-step` schema assertion;
|
||||
`vendor-step` wraps `handle-step-submit` in `try+` to surface create-time validation as a
|
||||
4xx. Two new gotchas surfaced and are documented: the empty-step `{}`→nil decode trap and the
|
||||
blank-nested-entity upsert error (see `gotchas.md`).
|
||||
|
||||
**Verification:** full e2e suite **65/65** (61 prior + 4 new: info renders + timeline;
|
||||
create across all 5 steps persists; edit opens prefilled and a rename persists; a too-short
|
||||
name blocks advancing). Create + edit semantics also confirmed at the REPL (incl. the
|
||||
cookie-session EDN round-trip). `maybe-spread-locations`-style domain helpers untouched.
|
||||
|
||||
103
e2e/vendor-wizard.spec.ts
Normal file
103
e2e/vendor-wizard.spec.ts
Normal file
@@ -0,0 +1,103 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
|
||||
// Acceptance spec for the New/Edit Vendor wizard (info → terms → account → address → legal),
|
||||
// migrated onto the session-backed engine (wizard2). Like New Invoice, the pre-migration
|
||||
// "Next" PUT /admin/vendor/navigat 500d on the empty route-params {}→nil quirk (a
|
||||
// [:map [:db/id …]] route-schema on a route with no path param), so this is an ACCEPTANCE
|
||||
// gate: green on the engine. Required fields: vendor/name (min 3) on info, vendor/default-account
|
||||
// on account.
|
||||
test.beforeEach(async ({ request }) => { await request.post('/test-reset'); });
|
||||
|
||||
async function seedAccount(page: any): Promise<number> {
|
||||
const info = await (await page.request.get('/test-info')).json();
|
||||
return info.accounts['test-account'];
|
||||
}
|
||||
|
||||
async function openNewVendor(page: any) {
|
||||
await page.goto('/admin/vendor');
|
||||
await page.waitForSelector('#entity-table');
|
||||
await page.locator('button:has-text("New Vendor")').first().click();
|
||||
await page.waitForSelector('#wizard-form');
|
||||
await page.waitForTimeout(400);
|
||||
}
|
||||
|
||||
// The advance/save button is the engine's data-primary nav button.
|
||||
const primary = (page: any) => page.locator('#wizard-form button[data-primary]').first().click();
|
||||
|
||||
async function setHidden(page: any, name: string, value: number) {
|
||||
await page.evaluate(({ name, value }: { name: string; value: number }) => {
|
||||
const h = document.querySelector(`input[name="${name}"]`) as HTMLInputElement;
|
||||
h.value = String(value);
|
||||
h.dispatchEvent(new Event('change', { bubbles: true }));
|
||||
}, { name, value });
|
||||
}
|
||||
|
||||
test.describe.configure({ mode: 'serial' });
|
||||
|
||||
test.describe('Vendor wizard (acceptance)', () => {
|
||||
test('info step renders with the name field and a timeline', async ({ page }) => {
|
||||
await openNewVendor(page);
|
||||
const form = page.locator('#wizard-form');
|
||||
await expect(form).toContainText('Basic Info');
|
||||
await expect(form).toContainText('Terms'); // timeline step
|
||||
await expect(form.locator('input[name="vendor/name"]')).toBeVisible();
|
||||
});
|
||||
|
||||
test('create a vendor across all 5 steps adds it to the grid', async ({ page }) => {
|
||||
const account = await seedAccount(page);
|
||||
await openNewVendor(page);
|
||||
// info
|
||||
await page.locator('input[name="vendor/name"]').fill('Acme Supplies');
|
||||
await primary(page); // -> terms
|
||||
await page.waitForTimeout(500);
|
||||
await expect(page.locator('#wizard-form')).toContainText('Terms Overrides');
|
||||
await primary(page); // -> account
|
||||
await page.waitForTimeout(500);
|
||||
await expect(page.locator('#wizard-form')).toContainText('Default Account');
|
||||
await setHidden(page, 'vendor/default-account', account);
|
||||
await primary(page); // -> address
|
||||
await page.waitForTimeout(500);
|
||||
await expect(page.locator('#wizard-form')).toContainText('Street');
|
||||
await primary(page); // -> legal
|
||||
await page.waitForTimeout(500);
|
||||
await expect(page.locator('#wizard-form')).toContainText('Legal Entity');
|
||||
await primary(page); // Save
|
||||
await page.waitForTimeout(1200);
|
||||
// the vendor persists: reload the grid and it's there
|
||||
await page.goto('/admin/vendor');
|
||||
await page.waitForSelector('#entity-table tbody tr');
|
||||
await expect(page.locator('#entity-table')).toContainText('Acme Supplies');
|
||||
});
|
||||
|
||||
test('edit opens prefilled and a rename persists', async ({ page }) => {
|
||||
const account = await seedAccount(page);
|
||||
await page.goto('/admin/vendor');
|
||||
await page.waitForSelector('#entity-table tbody tr');
|
||||
// open the edit wizard for the seeded "Test Vendor" (its row pencil)
|
||||
await page.locator('#entity-table tbody tr', { hasText: 'Test Vendor' }).first()
|
||||
.locator('[hx-get*="/edit"]').first().click();
|
||||
await page.waitForSelector('#wizard-form');
|
||||
await page.waitForTimeout(400);
|
||||
// info step is prefilled with the existing name
|
||||
await expect(page.locator('input[name="vendor/name"]')).toHaveValue('Test Vendor');
|
||||
await page.locator('input[name="vendor/name"]').fill('Test Vendor RENAMED');
|
||||
await primary(page); await page.waitForTimeout(400); // terms
|
||||
await primary(page); await page.waitForTimeout(400); // account (default-account already set)
|
||||
await setHidden(page, 'vendor/default-account', account);
|
||||
await primary(page); await page.waitForTimeout(400); // address
|
||||
await primary(page); await page.waitForTimeout(400); // legal
|
||||
await primary(page); await page.waitForTimeout(1000); // save
|
||||
await page.goto('/admin/vendor');
|
||||
await page.waitForSelector('#entity-table tbody tr');
|
||||
await expect(page.locator('#entity-table')).toContainText('Test Vendor RENAMED');
|
||||
});
|
||||
|
||||
test('info step blocks advancing when the name is too short', async ({ page }) => {
|
||||
await openNewVendor(page);
|
||||
await page.locator('input[name="vendor/name"]').fill('ab'); // < 3 chars
|
||||
await primary(page);
|
||||
await page.waitForTimeout(500);
|
||||
// still on the info step (validation re-renders it, no advance)
|
||||
await expect(page.locator('#wizard-form')).toContainText('Basic Info');
|
||||
});
|
||||
});
|
||||
File diff suppressed because it is too large
Load Diff
@@ -9,7 +9,6 @@
|
||||
"/account-override" ::new-account-override
|
||||
"/account-typeahead" ::account-typeahead
|
||||
"/validate" ::validate
|
||||
"/navigat" ::navigate
|
||||
"/new" {:get ::new}
|
||||
"/merge" {:get ::merge
|
||||
:put ::merge-submit}
|
||||
|
||||
Reference in New Issue
Block a user