fix(ssr): strip UI-only :mode before transaction upsert (500 on advanced manual save)
The :manual save handler builds its tx-data from the wizard snapshot and stripped the control fields :action and :amount-mode, but not :mode (simple/advanced) added by the recent manual-coding work. manual-coding-section* emits step-params[mode] on every render, so EVERY advanced manual save posted :mode "advanced" into :upsert-transaction and 500'd with ":db.error/not-an-entity :mode". Strip :mode alongside :action so the upsert only sees real schema attributes. Also fix the e2e helper that masked this: selectAccountFromTypeahead poked the Alpine v2 internal `el.__x.$data`, which is undefined on Alpine v3 (this app loads alpinejs@3.x), so it silently no-op'd and the account posted empty. Drive the typeahead via the real Alpine v3 path (Alpine.$data + tippy dropdown + click), mirroring transaction-edit-swap. Unmasks the previously-failing "Shared Location spread on save" test (was first in a serial file, hiding 7 siblings). Verified: that test passes; transaction-edit-swap stays 6/6. Skill gotchas.md records the :mode-strip rule, the Alpine-v3 API requirement, and the modal-won't-close diagnosis recipe.
This commit is contained in:
@@ -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 `<form id="wizard-form">` 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)
|
## Scorecard exceptions (ratchet violations with a reason)
|
||||||
|
|
||||||
_None yet._ Append here if a migration must let a metric regress for a documented reason.
|
_None yet._ Append here if a migration must let a metric regress for a documented reason.
|
||||||
|
|||||||
@@ -41,68 +41,33 @@ async function getTestInfo(page: any) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function selectAccountFromTypeahead(page: any, rowIndex: number, accountName: string) {
|
async function selectAccountFromTypeahead(page: any, rowIndex: number, accountName: string) {
|
||||||
// The account search uses Solr which isn't available in tests.
|
// Account search is backed by Solr (unavailable in tests). Drive the typeahead the
|
||||||
// Instead, we directly set the hidden input value via JavaScript.
|
// 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,
|
||||||
// Get all rows except the new-row, total, balance, and transaction total rows
|
// Alpine reactivity and the HTMX swap exactly as in production -- unlike poking the
|
||||||
const allRows = page.locator('#account-grid-body tbody tr');
|
// long-removed Alpine v2 `__x` internal, which silently no-ops on Alpine v3 and left
|
||||||
const rowCount = await allRows.count();
|
// the posted account empty.
|
||||||
|
|
||||||
// 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);
|
|
||||||
const accountKey = accountName === 'Test' ? 'test-account' : 'second-account';
|
const accountKey = accountName === 'Test' ? 'test-account' : 'second-account';
|
||||||
|
const label = `${accountName} Account`;
|
||||||
|
const testInfo = await getTestInfo(page);
|
||||||
const accountId = testInfo.accounts[accountKey];
|
const accountId = testInfo.accounts[accountKey];
|
||||||
|
|
||||||
if (!accountId) {
|
if (!accountId) {
|
||||||
throw new Error(`Could not find account with name ${accountName}`);
|
throw new Error(`Could not find account with name ${accountName}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set the hidden input value and trigger change
|
const row = page.locator('#account-grid-body tbody tr.account-row').nth(rowIndex);
|
||||||
// Also update Alpine.js data to prevent it from overwriting our value
|
const typeahead = row.locator('div.relative[x-data]').first();
|
||||||
await hiddenInput.evaluate((el: HTMLInputElement, value: string) => {
|
await typeahead.locator('a[x-ref="input"]').click();
|
||||||
// Set the DOM value
|
const search = page.locator('[data-tippy-root] input[x-model="search"]').first();
|
||||||
el.value = value;
|
await search.waitFor({ state: 'visible' });
|
||||||
|
await search.fill('te');
|
||||||
// Update Alpine.js component data
|
await typeahead.evaluate((el: any, opt: { id: number; label: string }) => {
|
||||||
const alpineEl = el.closest('[x-data]');
|
(window as any).Alpine.$data(el).elements = [{ value: opt.id, label: opt.label }];
|
||||||
if (alpineEl && (alpineEl as any).__x) {
|
}, { id: accountId, label });
|
||||||
(alpineEl as any).__x.$data.value.value = parseInt(value);
|
await page.locator('[data-tippy-root] a', { hasText: label }).first().click();
|
||||||
(alpineEl as any).__x.$data.value.label = 'Selected Account';
|
|
||||||
}
|
// Wait for the change-gated whole-form swap to settle.
|
||||||
|
await page.waitForTimeout(400);
|
||||||
// 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);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async function findAccountRow(page: any, rowIndex: number) {
|
async function findAccountRow(page: any, rowIndex: number) {
|
||||||
|
|||||||
@@ -1235,7 +1235,10 @@
|
|||||||
[{:as request
|
[{:as request
|
||||||
transaction :entity
|
transaction :entity
|
||||||
:keys [multi-form-state]}]
|
: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)
|
tx-id (:db/id tx-data)
|
||||||
client-id (->db-id (:transaction/client tx-data))
|
client-id (->db-id (:transaction/client tx-data))
|
||||||
existing-tx (d-transactions/get-by-id tx-id)
|
existing-tx (d-transactions/get-by-id tx-id)
|
||||||
|
|||||||
Reference in New Issue
Block a user