diff --git a/.claude/skills/ssr-form-migration/reference/gotchas.md b/.claude/skills/ssr-form-migration/reference/gotchas.md index 56846ae2..e0ccfdee 100644 --- a/.claude/skills/ssr-form-migration/reference/gotchas.md +++ b/.claude/skills/ssr-form-migration/reference/gotchas.md @@ -50,6 +50,39 @@ come from `GET /test-info`. --- +## UI-only control fields must be stripped before a Datomic upsert + +The wizard snapshot/step-params carry UI control fields that are **not** schema +attributes — `:action`, `:amount-mode`, and (added by the simple/advanced work) `:mode`. +The `:manual` save handler stripped `:action`/`:amount-mode` but not `:mode`, so every +*advanced* manual save passed `:mode "advanced"` into `:upsert-transaction` and 500'd with +`:db.error/not-an-entity :mode`. Lesson: when a save derives its tx-data from the form +snapshot, **strip every non-schema control key** before transacting. The session-backed +wizard engine (Phase 6) avoids this class of bug by storing per-step *validated* data +only — UI control fields never enter the combined data. This was a real production bug +surfaced by the e2e gate, not a test artifact. + +## E2E helpers must use the Alpine **v3** API, not the v2 `__x` internal + +The app loads Alpine v3 (`cdn.jsdelivr.net/npm/alpinejs@3.x.x`). The v2 internal +`el.__x.$data` is **gone** — `el.__x` is `undefined`, so any helper that pokes it silently +no-ops. A stale `selectAccountFromTypeahead` did this and left the posted account empty +(account-controlled by `x-model`, so the raw DOM `.value` you set is overwritten from +Alpine's empty state). Drive components the real way instead: `window.Alpine.$data(el)`, +open the tippy dropdown, inject `elements`, click the result — exactly as +`transaction-edit-swap.spec.ts` does. Probe with +`{ hasLegacy__x: !!el.__x, hasAlpineData: !!window.Alpine.$data(el) }`. + +## Diagnosing a "modal won't close after save" + +The edit modal closes on an `hx-trigger: modalclose` from a *successful* save; a +validation failure re-renders the `#wizard-form` (200), and a server exception returns 500 +(caught by `wrap-error`). To find which: capture POST responses in Playwright +(`page.on('response', …)`), read the `edit-submit` body — a `
` means +validation re-render; a `#error {…}` stack means a 500. Then serialize the form right +before save (`new FormData(document.querySelector('#wizard-form'))`) to see exactly what +posts. This is how the `:mode` 500 and the empty-account bugs above were isolated. + ## Scorecard exceptions (ratchet violations with a reason) _None yet._ Append here if a migration must let a metric regress for a documented reason. diff --git a/e2e/transaction-edit.spec.ts b/e2e/transaction-edit.spec.ts index fc941d96..3f7d948e 100644 --- a/e2e/transaction-edit.spec.ts +++ b/e2e/transaction-edit.spec.ts @@ -41,68 +41,33 @@ async function getTestInfo(page: any) { } async function selectAccountFromTypeahead(page: any, rowIndex: number, accountName: string) { - // The account search uses Solr which isn't available in tests. - // Instead, we directly set the hidden input value via JavaScript. - - // Get all rows except the new-row, total, balance, and transaction total rows - const allRows = page.locator('#account-grid-body tbody tr'); - const rowCount = await allRows.count(); - - // Find the row that has a hidden input for account (actual account rows) - let accountRow = null; - let accountRowIndex = 0; - for (let i = 0; i < rowCount; i++) { - const row = allRows.nth(i); - const hasAccountInput = await row.locator('input[name*="transaction-account/account"]').count() > 0; - if (hasAccountInput) { - if (accountRowIndex === rowIndex) { - accountRow = row; - break; - } - accountRowIndex++; - } - } - - if (!accountRow) { - throw new Error(`Could not find account row at index ${rowIndex}`); - } - - // Find the hidden input for the account - const hiddenInput = accountRow.locator('input[type="hidden"][name*="transaction-account/account"]').first(); - - // Get account IDs from test-info endpoint - const testInfo = await getTestInfo(page); + // Account search is backed by Solr (unavailable in tests). Drive the typeahead the + // way a user does, using the Alpine v3 API: open the tippy dropdown, inject a result + // into the component's `elements`, then click it. This runs the real click handler, + // Alpine reactivity and the HTMX swap exactly as in production -- unlike poking the + // long-removed Alpine v2 `__x` internal, which silently no-ops on Alpine v3 and left + // the posted account empty. const accountKey = accountName === 'Test' ? 'test-account' : 'second-account'; + const label = `${accountName} Account`; + const testInfo = await getTestInfo(page); const accountId = testInfo.accounts[accountKey]; - if (!accountId) { throw new Error(`Could not find account with name ${accountName}`); } - - // Set the hidden input value and trigger change - // Also update Alpine.js data to prevent it from overwriting our value - await hiddenInput.evaluate((el: HTMLInputElement, value: string) => { - // Set the DOM value - el.value = value; - - // Update Alpine.js component data - const alpineEl = el.closest('[x-data]'); - if (alpineEl && (alpineEl as any).__x) { - (alpineEl as any).__x.$data.value.value = parseInt(value); - (alpineEl as any).__x.$data.value.label = 'Selected Account'; - } - - // Also update any parent Alpine model (accountId) - const rowEl = el.closest('tr[x-data]'); - if (rowEl && (rowEl as any).__x) { - (rowEl as any).__x.$data.accountId = parseInt(value); - } - - el.dispatchEvent(new Event('change', { bubbles: true })); - }, accountId.toString()); - - // Wait for any HTMX updates - await page.waitForTimeout(300); + + const row = page.locator('#account-grid-body tbody tr.account-row').nth(rowIndex); + const typeahead = row.locator('div.relative[x-data]').first(); + await typeahead.locator('a[x-ref="input"]').click(); + const search = page.locator('[data-tippy-root] input[x-model="search"]').first(); + await search.waitFor({ state: 'visible' }); + await search.fill('te'); + await typeahead.evaluate((el: any, opt: { id: number; label: string }) => { + (window as any).Alpine.$data(el).elements = [{ value: opt.id, label: opt.label }]; + }, { id: accountId, label }); + await page.locator('[data-tippy-root] a', { hasText: label }).first().click(); + + // Wait for the change-gated whole-form swap to settle. + await page.waitForTimeout(400); } async function findAccountRow(page: any, rowIndex: number) { diff --git a/src/clj/auto_ap/ssr/transaction/edit.clj b/src/clj/auto_ap/ssr/transaction/edit.clj index 7b59ff84..1d264bd0 100644 --- a/src/clj/auto_ap/ssr/transaction/edit.clj +++ b/src/clj/auto_ap/ssr/transaction/edit.clj @@ -1235,7 +1235,10 @@ [{:as request transaction :entity :keys [multi-form-state]}] - (let [tx-data (-> multi-form-state :snapshot (dissoc :action)) + (let [;; :mode is a UI-only field (simple/advanced); :action/:amount-mode are control + ;; fields. None are Datomic attributes, so strip them before building the upsert + ;; (otherwise :upsert-transaction fails with :db.error/not-an-entity :mode). + tx-data (-> multi-form-state :snapshot (dissoc :action :mode)) tx-id (:db/id tx-data) client-id (->db-id (:transaction/client tx-data)) existing-tx (d-transactions/get-by-id tx-id)