diff --git a/docs/superpowers/plans/2026-06-07-object-form-robustness.md b/docs/superpowers/plans/2026-06-07-object-form-robustness.md new file mode 100644 index 0000000..449a4ef --- /dev/null +++ b/docs/superpowers/plans/2026-06-07-object-form-robustness.md @@ -0,0 +1,372 @@ +# Object Form Robustness Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the object create/edit form safe for long daily sessions — no double-submit, an unsaved-changes guard, one consistent partial-failure recovery, code-aware validation messages, and batch-entry ergonomics. + +**Architecture:** Migrate to a React Router data router (enables `useBlocker`) keeping the route tree verbatim. The form is react-hook-form; `isSubmitting` (made real by returning the async `onSubmit` from `handleSubmit`) drives submit-disable, and `useBlocker(isDirty && !isSubmitting)` drives the dirty guard — so save-driven navigation is never falsely blocked and Cancel flows through the same dialog. `onSubmit` returns a success boolean so the form can reset for "Save & create another". + +**Tech Stack:** React 19 + TS + pnpm, react-router-dom 7 (data router), react-hook-form, react-i18next, Base UI (alert-dialog), Vitest + RTL + MSW. Test runner: `pnpm test` (single pass). + +**Conventions:** pnpm; **no `any`/`eslint-disable`/`@ts-ignore`**; no codename; en/sv parity; ui/ no-semicolon, app source double-quote+semicolon; token classes only; guard DOM globals; **run tests exactly once per task.** + +**Spec:** `docs/superpowers/specs/2026-06-07-object-form-robustness-design.md` + +**Key facts (from the code):** +- `object-form.tsx`: RHF `useForm`; `submit = handleSubmit((data) => { onSubmit(...) })` — **does not return the promise** (so `isSubmitting` never tracks it; fix in T2). Props: `mode/defaults/onSubmit/onCancel/formError/fieldErrorKey`. `coreField` renders `errors.core?.[key] && t("form.required")` always. `number_of_objects` registered via `coreField(..., { type: "number", required: true })`. +- `object-new-page.tsx`: `onSubmit` create→setFields; on setFields fail `navigate(\`/objects/${id}/edit\`, { state: { fieldsError, fieldErrorKey } })`; success → `/objects/${id}`. +- `object-edit-form.tsx`: split into `ObjectEditFormLoaded`; reads `location.state` (`fieldsError`/`fieldErrorKey`) to seed the banner; `onSubmit` update→setFields; on `FieldRejection` sets `fieldErrorKey` + banner, stays. +- `FieldRejection` carries `field` + `code`. `useCreateObject/useUpdateObject/useSetFields` expose `.isPending` (unused today). +- Router: `app.tsx` `` (3 top-level siblings). `renderApp` wraps `ui` in `` with no ``. + +--- + +# Task 1: Migrate to a data router (foundation) + +**Files:** `web/src/app.tsx`, `web/src/test/render.tsx`. (Possibly `main.tsx` — only if needed; it should NOT need changes since `App` stays the exported component.) + +- [ ] **Step 1: `app.tsx` → data router.** Convert the JSX route tree verbatim using `createRoutesFromElements`. Replace the `import { BrowserRouter, Navigate, Route, Routes }` with `import { createBrowserRouter, createRoutesFromElements, Navigate, Route, RouterProvider } from "react-router-dom";`. Keep all the `lazy`/`Suspense` wrappers and every `` exactly as-is. New shape: +```tsx +const router = createBrowserRouter( + createRoutesFromElements( + <> + } /> + }> + }> + {/* ...all the existing nested elements, verbatim... */} + + + } /> + , + ), +); + +export function App() { + return ; +} +``` +Do NOT change any path, element, Suspense, or nesting. (The `FormFallback` + lazy imports stay.) + +- [ ] **Step 2: `test/render.tsx` → `createMemoryRouter`.** Replace `MemoryRouter` usage: +```tsx +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render } from "@testing-library/react"; +import type { ReactElement } from "react"; +import { createMemoryRouter, RouterProvider } from "react-router-dom"; + +import "../i18n"; + +export function renderApp(ui: ReactElement, { route = "/" } = {}) { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const router = createMemoryRouter([{ path: "*", element: ui }], { initialEntries: [route] }); + + return render( + + + , + ); +} +``` +This is behavior-preserving: `ui` renders at `route`; tests that include their own `` still nest under the `*` route; now a data-router context exists (so `useBlocker` works later). + +- [ ] **Step 3: Full suite must stay green (the migration gate). Run ONCE:** +```bash +cd web && pnpm typecheck && pnpm lint && pnpm test && pnpm build +``` +Expected: ALL existing tests pass unchanged. If a test fails because it relied on `MemoryRouter`-specific behavior (e.g., asserting a redirect, or a component that rendered without its own `` and needs params), investigate and fix the test's setup to the data-router equivalent WITHOUT weakening it. Report any test that needed adjustment and why. If many break, STOP and report (the migration approach may need a tweak) rather than mass-editing. + +- [ ] **Step 4: Commit** +```bash +git add web/src/app.tsx web/src/test/render.tsx +git commit -m "refactor(web): migrate to data router (createBrowserRouter) to enable useBlocker (#46)" +``` + +--- + +# Task 2: Submit-disable + keyboard submit + "Save & create another" + +**Files:** `web/src/objects/object-form.tsx`, `web/src/objects/object-new-page.tsx`, `web/src/objects/object-edit-form.tsx`, `web/src/i18n/en.json`, `web/src/i18n/sv.json`, tests. + +- [ ] **Step 1: i18n** — add to the `form` namespace in BOTH locales (parity): + - en: `"saving": "Saving…"`, `"createAnother": "Save & create another"` + - sv: `"saving": "Sparar…"`, `"createAnother": "Spara & skapa ny"` + +- [ ] **Step 2: ObjectForm — make `isSubmitting` real + disable + the new button + Cmd/Ctrl+Enter.** + - Change the `onSubmit` prop type to: `onSubmit: (values: ObjectFormValues, opts?: { createAnother?: boolean }) => Promise | boolean;` + - Destructure `isSubmitting`: `const { register, handleSubmit, formState: { errors, isSubmitting } } = form;` + - Add a ref: `const createAnotherRef = useRef(false);` + - Rewrite `submit` to RETURN/await the promise (so RHF tracks it) and reset on create-another success: +```tsx +const submit = handleSubmit(async (data) => { + const fields = pruneFields(data.fields, localizedTextKeys, default_language); + const values = + mode === "create" + ? { core: data.core, visibility: data.visibility, fields } + : { core: data.core, fields }; + const createAnother = createAnotherRef.current; + createAnotherRef.current = false; + const ok = await onSubmit(values, { createAnother }); + if (ok && createAnother) { + form.reset({ core: EMPTY_CORE, visibility: "draft", fields: {} }); + document.getElementById("object_number")?.focus(); + } +}); +``` + - Add a keydown handler on the `
` for Cmd/Ctrl+Enter: + `onKeyDown={(e) => { if ((e.metaKey || e.ctrlKey) && e.key === "Enter") { e.preventDefault(); void submit(); } }}` + - Footer buttons: +```tsx +
+ + {mode === "create" && ( + + )} + +
+``` + (Add `useRef` to the React import. `variant="secondary"` — confirm it exists in `ui/button.tsx`; if not, use `variant="outline"` or default — check.) + +- [ ] **Step 3: Update pages' `onSubmit` to return `boolean` + honor `createAnother`.** + - `object-new-page.tsx`: +```tsx +const onSubmit = async (values: ObjectFormValues, opts?: { createAnother?: boolean }): Promise => { + setError(null); + let id: string; + try { + const created = await create.mutateAsync({ ...values.core, visibility: values.visibility ?? "draft" }); + id = created.id; + } catch { + setError(t("form.rejected")); + return false; + } + if (Object.keys(values.fields).length > 0) { + try { + await setFields.mutateAsync({ id, fields: values.fields }); + } catch (e) { + const fieldErrorKey = e instanceof FieldRejection ? e.field : undefined; + const fieldErrorCode = e instanceof FieldRejection ? e.code : undefined; + navigate(`/objects/${id}/edit`, { state: { created: true, fieldErrorKey, fieldErrorCode } }); + return true; + } + } + if (opts?.createAnother) return true; // success; ObjectForm resets, stays on /objects/new + navigate(`/objects/${id}`); + return true; +}; +``` + - `object-edit-form.tsx` `ObjectEditFormLoaded.onSubmit`: return `false` in the catch, `true` after the success navigate. (Edit mode never passes `createAnother`.) + +- [ ] **Step 4: Tests.** Extend `object-form.test.tsx` / `object-new-page.test.tsx`: + - During an in-flight create (MSW delayed handler, or assert the button is `disabled` + shows "Saving…" synchronously after submit), the create mutation is called exactly once on a double-click. (If timing is hard, at minimum assert the button becomes `disabled` while submitting and reads `t("form.saving")`.) + - "Save & create another": click it in create mode with a delayed/immediate success handler → after success the form is reset (e.g., `object_number` input is empty) and the location is still `/objects/new` (no navigation to detail). Use the renderApp data-router harness; assert location via a probe or that the form is still present + cleared. + - Cmd/Ctrl+Enter triggers submit (fireEvent.keyDown with `{ key: "Enter", metaKey: true }` → the create mutation fires). + +- [ ] **Step 5: Verify (vitest ONCE).** `cd web && pnpm vitest run src/objects && pnpm typecheck && pnpm lint`. Expected: PASS. + +- [ ] **Step 6: Commit** +```bash +git add web/src/objects/object-form.tsx web/src/objects/object-new-page.tsx web/src/objects/object-edit-form.tsx web/src/i18n/en.json web/src/i18n/sv.json web/src/objects/object-form.test.tsx web/src/objects/object-new-page.test.tsx +git commit -m "feat(web): disable submit while saving + Save & create another + Cmd/Ctrl+Enter (#46)" +``` + +--- + +# Task 3: Validation messages (server code echo, type-specific core errors, min count) + +**Files:** `web/src/objects/object-form.tsx`, `web/src/objects/object-new-page.tsx`, `web/src/objects/object-edit-form.tsx`, `web/src/i18n/en.json`, `web/src/i18n/sv.json`, tests. + +- [ ] **Step 1: i18n** — add to the `form` namespace (both locales, parity): + - en: `"minCount": "Must be at least 1"`, and a nested `"fieldError": { "type_mismatch": "Wrong type for this field", "unresolved": "Referenced value not found", "unknown": "Unknown field" }`. + - sv: `"minCount": "Måste vara minst 1"`, `"fieldError": { "type_mismatch": "Fel typ för detta fält", "unresolved": "Refererat värde hittades inte", "unknown": "Okänt fält" }`. + +- [ ] **Step 2: ObjectForm — carry the rejection `code` + type-specific messages.** + - Add prop `fieldErrorCode?: string | null;` (alongside `fieldErrorKey`). + - The highlight effect picks the code-specific message: +```tsx +useEffect(() => { + if (fieldErrorKey) { + const codeKey = fieldErrorCode ? `form.fieldError.${fieldErrorCode}` : ""; + const message = + fieldErrorCode && t(codeKey) !== codeKey ? t(codeKey) : t("form.fieldRejected", { field: fieldErrorKey }); + form.setError(`fields.${fieldErrorKey}` as never, { type: "server", message }); + } +}, [fieldErrorKey, fieldErrorCode, form, t]); +``` + - Core error render → message-aware (so `min` shows minCount, required falls back): +```tsx +{errors.core?.[key] && ( +

+ {errors.core[key]?.message || t("form.required")} +

+)} +``` + - `number_of_objects` min: in `coreField`, when registering a number with required, also pass `min`. Simplest: special-case the count field by giving `coreField` an optional `min` and rendering. Concretely change the `number_of_objects` registration to include `min: { value: 1, message: t("form.minCount") }`. Implement by extending `coreField`'s `opts` with `min?: number` and, when set, register `{ valueAsNumber: true, required, min: { value: opts.min, message: t("form.minCount") } }`; call `coreField("number_of_objects", "count", { type: "number", required: true, min: 1 })`. + +- [ ] **Step 3: Pass the code through the pages.** + - `object-edit-form.tsx`: in the `FieldRejection` catch, also `setFieldErrorCode(e.code)` (add a `fieldErrorCode` state) and pass `fieldErrorCode` to ``. Also seed it from `location.state.fieldErrorCode` (set by the create teleport). The banner stays `form.fieldRejected` (or upgrade to code-specific too — optional; the field highlight is the key UX). + - ``. + +- [ ] **Step 4: Tests.** Extend `object-edit-form.test.tsx`: + - A `setFields` 422 with `{ field: "...", code: "type_mismatch" }` → the field shows the `form.fieldError.type_mismatch` message (assert the text). + - `number_of_objects` set to `0` and submit → the `form.minCount` message shows and NO create/update mutation is called (client-side block). (In `object-form.test.tsx` or a page test.) + +- [ ] **Step 5: Verify (vitest ONCE).** `cd web && pnpm vitest run src/objects && pnpm typecheck && pnpm lint`. PASS. + +- [ ] **Step 6: Commit** +```bash +git add web/src/objects/object-form.tsx web/src/objects/object-edit-form.tsx web/src/objects/object-new-page.tsx web/src/i18n/en.json web/src/i18n/sv.json web/src/objects/object-edit-form.test.tsx web/src/objects/object-form.test.tsx +git commit -m "feat(web): code-aware field errors + min count validation (#46)" +``` + +--- + +# Task 4: Unsaved-changes guard + +**Files:** `web/src/lib/use-unsaved-changes.tsx` (new), `web/src/objects/object-form.tsx`, `web/src/i18n/en.json`, `web/src/i18n/sv.json`, `web/src/lib/use-unsaved-changes.test.tsx` (new) or extend object-form tests. + +- [ ] **Step 1: i18n** — add a `form.unsaved` namespace (both locales, parity): + - en: `"unsaved": { "title": "Discard unsaved changes?", "body": "You have unsaved changes that will be lost.", "stay": "Keep editing", "leave": "Discard" }` + - sv: `"unsaved": { "title": "Kasta osparade ändringar?", "body": "Du har osparade ändringar som går förlorade.", "stay": "Fortsätt redigera", "leave": "Kasta" }` + +- [ ] **Step 2: The hook + dialog** `web/src/lib/use-unsaved-changes.tsx`: +```tsx +import { useEffect } from "react"; +import { useBlocker } from "react-router-dom"; + +export function useUnsavedChanges(active: boolean) { + const blocker = useBlocker(active); + + useEffect(() => { + if (!active) return; + const handler = (e: BeforeUnloadEvent) => { + e.preventDefault(); + e.returnValue = ""; + }; + window.addEventListener("beforeunload", handler); + return () => window.removeEventListener("beforeunload", handler); + }, [active]); + + return blocker; +} +``` +And an `UnsavedChangesDialog` component (same file or a sibling) using `ui/alert-dialog`, driven by the blocker: +```tsx +import { useTranslation } from "react-i18next"; +import type { Blocker } from "react-router-dom"; +import { + AlertDialog, AlertDialogContent, AlertDialogHeader, AlertDialogTitle, + AlertDialogDescription, AlertDialogFooter, AlertDialogCancel, AlertDialogAction, +} from "@/components/ui/alert-dialog"; + +export function UnsavedChangesDialog({ blocker }: { blocker: Blocker }) { + const { t } = useTranslation(); + const open = blocker.state === "blocked"; + return ( + { if (!o) blocker.reset?.(); }}> + + + {t("form.unsaved.title")} + {t("form.unsaved.body")} + + + blocker.reset?.()}>{t("form.unsaved.stay")} + blocker.proceed?.()}>{t("form.unsaved.leave")} + + + + ); +} +``` +IMPORTANT: open `web/src/components/ui/alert-dialog.tsx` and match the EXACT exported part names/props (`AlertDialog` may take `open`/`onOpenChange`, or be trigger-driven — adapt to the real API; the delete dialogs in `web/src/objects/delete-object-dialog.tsx` / `components/delete-confirm-dialog.tsx` show the controlled usage to mirror). The dialog must be openable WITHOUT a trigger (controlled by `open`). Validate by running the test. + +- [ ] **Step 3: Wire into ObjectForm.** + - `const isDirty = form.formState.isDirty;` + - `const blocker = useUnsavedChanges(isDirty && !isSubmitting);` + - Render `` inside the form's container. + - **Cancel** now just calls `onCancel` (which navigates) — the blocker intercepts it and shows the dialog automatically when dirty. (No separate confirm needed; confirm this in the test.) + +- [ ] **Step 4: Tests** `web/src/lib/use-unsaved-changes.test.tsx` (and/or extend object-form): + - Render a small component (or the ObjectForm) under the `renderApp` data-router harness with two routes; with a dirty form, click a ``/Cancel → the dialog appears; "Keep editing" stays (location unchanged), "Discard" proceeds (location changes). + - `beforeunload`: with `active=true`, a `beforeunload` event is registered (spy on `window.addEventListener`) and not when inactive. + - A clean form navigates without the dialog. + - Saving (isSubmitting true) does NOT block — simulate or assert via the `isDirty && !isSubmitting` condition (e.g., the blocker arg is false during submit). + +- [ ] **Step 5: Verify (vitest ONCE).** `cd web && pnpm vitest run src/objects src/lib && pnpm typecheck && pnpm lint`. PASS. + +- [ ] **Step 6: Commit** +```bash +git add web/src/lib/use-unsaved-changes.tsx web/src/objects/object-form.tsx web/src/i18n/en.json web/src/i18n/sv.json web/src/lib/use-unsaved-changes.test.tsx +git commit -m "feat(web): unsaved-changes guard (useBlocker + beforeunload) on the object form (#46)" +``` + +--- + +# Task 5: Partial-failure unification + final gate + +**Files:** `web/src/objects/object-edit-form.tsx`, `web/src/i18n/en.json`, `web/src/i18n/sv.json`, tests. + +(Task 2 already changed the create page to pass `state: { created: true, fieldErrorKey, fieldErrorCode }`. This task handles the edit page's reading of it + messaging.) + +- [ ] **Step 1: i18n** — add (both locales, parity): en `"createdButFieldRejected": "Object created, but a field was rejected — fix it below."`; sv `"createdButFieldRejected": "Föremålet skapades, men ett fält avvisades — åtgärda nedan."`. + +- [ ] **Step 2: Edit page reads `created`.** In `ObjectEditFormLoaded`, broaden the `locationState` type to `{ created?: boolean; fieldsError?: boolean; fieldErrorKey?: string; fieldErrorCode?: string } | null` and seed the banner: +```tsx +const [error, setError] = useState(() => { + if (locationState?.created) return t("form.createdButFieldRejected"); + if (locationState?.fieldErrorKey) return t("form.fieldRejected", { field: locationState.fieldErrorKey }); + if (locationState?.fieldsError) return t("form.rejected"); + return null; +}); +const [fieldErrorKey, setFieldErrorKey] = useState(locationState?.fieldErrorKey ?? null); +const [fieldErrorCode, setFieldErrorCode] = useState(locationState?.fieldErrorCode ?? null); +``` + (Keep backward compatibility: the create page from T2 sends `created`; older `fieldsError` branch can remain or be removed since T2 replaced it — remove `fieldsError` seeding if no longer sent.) + +- [ ] **Step 3: Tests.** Update `object-new-page.test.tsx`: the create→setFields-422 path now navigates to `/objects/:id/edit` and the edit form shows the `form.createdButFieldRejected` banner + highlights the field. (The existing partial-failure test asserted the old `fieldsError` flow — update it to the new `created` message, not weakened.) + +- [ ] **Step 4: FULL GATE (run tests EXACTLY ONCE):** +```bash +cd web && pnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size && pnpm check:colors +``` +All green. Report test totals, largest chunk, check:colors line. + +- [ ] **Step 5: Codename + status:** +```bash +cd /Users/olsson/Laboratory/biggus-dickus +git grep -in 'biggus\|dickus' -- web/src; echo "codename-exit=$?" +git status --short +``` + +- [ ] **Step 6: Manual smoke (recommended).** `pnpm dev`: create with a bad field → lands on edit with "Object created, but a field was rejected"; submit disables + "Saving…"; edit a field then try to leave (sidebar/Cancel/reload) → guard prompts; "Save & create another" resets; count 0 blocked client-side; Cmd+Enter submits. + +- [ ] **Step 7: Commit** +```bash +git add web/src/objects/object-edit-form.tsx web/src/i18n/en.json web/src/i18n/sv.json web/src/objects/object-new-page.test.tsx +git commit -m "feat(web): unify create/edit partial-failure recovery with 'created' banner (#46)" +``` + +--- + +## Self-Review (completed) + +**Spec coverage:** data-router migration + harness, full suite green (T1); submit-disable via real `isSubmitting` + Cmd/Ctrl+Enter + Save-&-create-another (T2); code-aware field errors + type-specific core errors + min count (T3); unsaved-changes guard via `useBlocker(isDirty && !isSubmitting)` + beforeunload + dialog, Cancel through the blocker (T4); partial-failure unified to the edit route with a "created" banner (T5, building on T2's create-side state). All acceptance criteria 1–7 mapped. ✓ + +**Placeholder scan:** the alert-dialog wiring says "match the exact exported parts" with the delete dialogs named as the reference — a concrete adapt-to-real-API step, not a TODO. `variant="secondary"` flagged to verify against button.tsx. No vague steps; all code blocks complete. ✓ + +**Type/flow consistency:** `onSubmit` returns `Promise|boolean` (T2) — both pages updated to return booleans; `createAnotherRef` gates the reset; `fieldErrorCode` prop added (T3) and threaded from both the edit catch and the create teleport state (T2/T5); the guard condition `isDirty && !isSubmitting` ensures save/teleport navigation (still submitting) is never blocked — consistent across T2/T4/T5. ✓ + +## Notes +- The single biggest correctness lever: `handleSubmit` must RETURN/await `onSubmit` so `isSubmitting` is real (T2) — both the submit-disable AND the guard's non-blocking-while-saving depend on it. +- `useBlocker(boolean)` (RR v7) blocks all nav when true; Cancel and sidebar links both flow through the one dialog. Save-driven nav happens while `isSubmitting` → condition false → not blocked. +- No new dependency. New i18n keys: `form.saving/createAnother/minCount/createdButFieldRejected` + `form.fieldError.*` + `form.unsaved.*` (en+sv parity). No keys removed. diff --git a/docs/superpowers/specs/2026-06-07-object-form-robustness-design.md b/docs/superpowers/specs/2026-06-07-object-form-robustness-design.md new file mode 100644 index 0000000..7041c2b --- /dev/null +++ b/docs/superpowers/specs/2026-06-07-object-form-robustness-design.md @@ -0,0 +1,164 @@ +# Object Form Robustness — Design + +**Date:** 2026-06-07 +**Status:** Approved (brainstorming) — ready for implementation planning. +**Issue:** #46. + +## Context + +The object create/edit form has data-integrity gaps that compound in long daily cataloguing sessions +(High severity): +- **Submit never disabled during save.** `object-form.tsx` submit button has no `disabled`/pending. + Both create (`object-new-page.tsx`) and edit (`object-edit-form.tsx`) do **two sequential awaited + mutations** (create/update, then `setFields`) with no feedback → a second click can duplicate-create + or race the two-step write. `publish-control.tsx` is the model (disables on `isPending`). +- **No unsaved-changes guard.** No `isDirty`/`useBlocker`/`beforeunload`; Cancel/nav loses work + silently. +- **Two-phase write recovers inconsistently.** Create teleports to the edit page on `setFields` + failure; edit stays put with a banner. The partial write (core saved, field rejected) reads as + nothing happened. +- **Thin validation.** Core errors always show `form.required`; the server `code` + (`type_mismatch`/`unresolved`/`unknown`) on `FieldRejection` is discarded; no `number_of_objects >= 1`. +- No "Save & create another" / keyboard submit for batch entry. + +**Facts established:** the form uses **react-hook-form** (so `formState.isDirty` + `isSubmitting` are +available; `isSubmitting` stays true across both awaited mutations — the unified pending signal). +`ObjectForm` props: `mode`, `defaults`, `onSubmit`, `onCancel`, `formError`, `fieldErrorKey`; the +422→highlight path uses `form.setError`. `FieldRejection` (`queries.ts`) carries `field` + `code`. +The router is `` + `` (component router) in `app.tsx`; **`useBlocker` requires +a data router** (`createBrowserRouter` + `RouterProvider`). `renderApp` wraps `ui` in `` +with no `` (tests supply their own when they need params). + +### Decisions (from brainstorming) +1. **Migrate to a data router** (the long-term-correct foundation; unblocks `useBlocker` and React + Router v7 data APIs). Done via `createRoutesFromElements` to keep the exact route tree (minimal + diff), isolated as the first task, with the test harness moved to `createMemoryRouter`. +2. **Unify create partial-failure** to the edit route with a distinct "Object created, but a field was + rejected" banner (single recovery surface). +3. **Include batch entry** ("Save & create another" + Cmd/Ctrl+Enter). + +## Components + +### 0. Data-router migration (foundation — isolated) +- **`app.tsx`:** replace `…` with a module-level + `const router = createBrowserRouter(createRoutesFromElements(<>…the existing tree…))` + and `export function App() { return ; }`. The route JSX (login, + `RequireAuth`→`AppShell` nest with all nested routes, lazy `Suspense` wrappers, splat) moves + **verbatim** into `createRoutesFromElements`. No behavior/path change. +- **`main.tsx`:** unchanged provider stack (QueryClient → Config → Toast) now wraps `` which is + ``. +- **`test/render.tsx`:** `renderApp(ui, { route })` switches to + `createMemoryRouter([{ path: "*", element: ui }], { initialEntries: [route] })` rendered via + `` (inside `QueryClientProvider`). Behavior-preserving: `ui` renders at `route`, + and tests that supply their own `` still nest correctly; now a data-router context exists so + `useBlocker` works. **Acceptance: the entire existing suite stays green** before any feature is built + on top. + +### 1. Submit pending + keyboard + batch (`object-form.tsx`) +- Submit ` - + )} + + diff --git a/web/src/objects/object-new-page.test.tsx b/web/src/objects/object-new-page.test.tsx index 18014dc..b682655 100644 --- a/web/src/objects/object-new-page.test.tsx +++ b/web/src/objects/object-new-page.test.tsx @@ -1,24 +1,19 @@ import { expect, test } from "vitest"; import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { http, HttpResponse } from "msw"; -import { Routes, Route, useLocation } from "react-router-dom"; +import { delay, http, HttpResponse } from "msw"; +import { Routes, Route } from "react-router-dom"; import { server } from "../test/server"; import { renderApp } from "../test/render"; import { ObjectNewPage } from "./object-new-page"; - -function EditStub() { - const location = useLocation(); - const flagged = (location.state as { fieldsError?: boolean } | null)?.fieldsError === true; - return
edit page{flagged ? " (fields error)" : ""}
; -} +import { ObjectEditForm } from "./object-edit-form"; function tree() { return ( } /> detail view} /> - } /> + } /> ); } @@ -50,13 +45,15 @@ test("create: POST then PUT fields, then navigate to the new object's detail", a expect((fieldsBody as { inscription: string }).inscription).toBe("To the gods"); }); -test("partial create: fields PUT fails -> navigate to edit with an error banner", async () => { +test("partial create: fields PUT fails -> edit page shows the 'created' banner and highlights the field", async () => { + // POST returns the amphora id so the default GET /api/admin/objects/:id handler + // resolves and the real ObjectEditForm renders at /objects/:id/edit. server.use( http.post("/api/admin/objects", () => - HttpResponse.json({ id: "new-id-2" }, { status: 201 }), + HttpResponse.json({ id: "11111111-1111-1111-1111-111111111111" }, { status: 201 }), ), http.put("/api/admin/objects/:id/fields", () => - new HttpResponse(null, { status: 422 }), + HttpResponse.json({ field: "inscription", code: "type_mismatch" }, { status: 422 }), ), ); @@ -67,5 +64,57 @@ test("partial create: fields PUT fails -> navigate to edit with an error banner" await userEvent.type(screen.getByLabelText(/inscription/i), "x"); await userEvent.click(screen.getByRole("button", { name: /create object/i })); - await waitFor(() => expect(screen.getByText(/edit page \(fields error\)/i)).toBeInTheDocument()); + await waitFor(() => + expect(screen.getByText(/object created, but a field was rejected/i)).toBeInTheDocument(), + ); + expect(await screen.findByText(/wrong type for this field/i)).toBeInTheDocument(); +}); + +test("in-flight submit: button disabled + shows Saving…, create fires exactly once on double-click", async () => { + let postCount = 0; + + server.use( + http.post("/api/admin/objects", async () => { + postCount += 1; + await delay(50); + return HttpResponse.json({ id: "new-id-3" }, { status: 201 }); + }), + ); + + renderApp(tree(), { route: "/objects/new" }); + + await userEvent.type(await screen.findByLabelText(/object number/i), "A-9"); + await userEvent.type(screen.getByLabelText(/^name/i), "Amphora"); + await userEvent.type(screen.getByLabelText(/inscription/i), "To the gods"); + + const button = screen.getByRole("button", { name: /create object/i }); + await userEvent.click(button); + await userEvent.click(button); + + await waitFor(() => expect(screen.getByText(/saving…/i)).toBeInTheDocument()); + expect(screen.getByRole("button", { name: /saving…/i })).toBeDisabled(); + + await waitFor(() => expect(screen.getByText("detail view")).toBeInTheDocument()); + expect(postCount).toBe(1); +}); + +test("Save & create another: resets the form and stays on /objects/new", async () => { + server.use( + http.post("/api/admin/objects", () => + HttpResponse.json({ id: "new-id-4" }, { status: 201 }), + ), + ); + + renderApp(tree(), { route: "/objects/new" }); + + const numberInput = (await screen.findByLabelText(/object number/i)) as HTMLInputElement; + await userEvent.type(numberInput, "A-9"); + await userEvent.type(screen.getByLabelText(/^name/i), "Amphora"); + await userEvent.type(screen.getByLabelText(/inscription/i), "To the gods"); + + await userEvent.click(screen.getByRole("button", { name: /save & create another/i })); + + await waitFor(() => expect(numberInput.value).toBe("")); + expect(screen.queryByText("detail view")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: /save & create another/i })).toBeInTheDocument(); }); diff --git a/web/src/objects/object-new-page.tsx b/web/src/objects/object-new-page.tsx index d541b76..7940935 100644 --- a/web/src/objects/object-new-page.tsx +++ b/web/src/objects/object-new-page.tsx @@ -18,7 +18,10 @@ export function ObjectNewPage() { useDocumentTitle(t("objects.new")); useBreadcrumb([{ label: t("nav.objects"), to: "/objects" }, { label: t("objects.new") }]); - const onSubmit = async (values: ObjectFormValues) => { + const onSubmit = async ( + values: ObjectFormValues, + opts?: { createAnother?: boolean }, + ): Promise => { setError(null); let id: string; @@ -32,7 +35,7 @@ export function ObjectNewPage() { id = created.id; } catch { setError(t("form.rejected")); - return; + return false; } if (Object.keys(values.fields).length > 0) { @@ -40,12 +43,16 @@ export function ObjectNewPage() { await setFields.mutateAsync({ id, fields: values.fields }); } catch (e) { const fieldErrorKey = e instanceof FieldRejection ? e.field : undefined; - navigate(`/objects/${id}/edit`, { state: { fieldsError: true, fieldErrorKey } }); - return; + const fieldErrorCode = e instanceof FieldRejection ? e.code : undefined; + navigate(`/objects/${id}/edit`, { state: { created: true, fieldErrorKey, fieldErrorCode } }); + return true; } } + if (opts?.createAnother) return true; + navigate(`/objects/${id}`); + return true; }; return ( diff --git a/web/src/search/search.test.tsx b/web/src/search/search.test.tsx index 66ba609..f69fea6 100644 --- a/web/src/search/search.test.tsx +++ b/web/src/search/search.test.tsx @@ -1,8 +1,9 @@ import { expect, test } from "vitest"; -import { screen, waitFor, within } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { http, HttpResponse } from "msw"; -import { Route, Routes } from "react-router-dom"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { createMemoryRouter, Route, RouterProvider, Routes } from "react-router-dom"; import { server } from "../test/server"; import { renderApp } from "../test/render"; @@ -10,6 +11,7 @@ import { amphora } from "../test/fixtures"; import { SearchPage } from "./search-page"; import { SelectSearchPrompt } from "./select-search-prompt"; import { ObjectDetail } from "../objects/object-detail"; +import "../i18n"; function tree() { return ( @@ -22,6 +24,35 @@ function tree() { ); } +// The search rows are s. Under the shared `renderApp` harness the test +// subtree lives in a descendant under a catch-all `*` data route, where +// the data router does not intercept the link click (it falls through to a real +// browser navigation that jsdom rejects). Mounting the search routes as real +// data-router routes lets RouterProvider intercept the NavLink, which is the +// data-router equivalent of the old MemoryRouter behavior. +function renderSearchRouter(route = "/search") { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const router = createMemoryRouter( + [ + { + path: "/search", + element: , + children: [ + { index: true, element: }, + { path: ":id", element: }, + ], + }, + ], + { initialEntries: [route] }, + ); + + return render( + + + , + ); +} + test("typing searches and renders highlighted rich rows", async () => { renderApp(tree(), { route: "/search" }); await userEvent.type(screen.getByLabelText(/search the collection/i), "bronze"); @@ -69,7 +100,7 @@ test("empty query shows the prompt; zero results shows empty", async () => { }); test("clicking a result shows the object in the detail pane", async () => { - renderApp(tree(), { route: "/search" }); + renderSearchRouter(); await userEvent.type(screen.getByLabelText(/search the collection/i), "bronze"); await userEvent.click(await screen.findByText("Bronze figurine")); diff --git a/web/src/test/render.tsx b/web/src/test/render.tsx index b9f5edd..abf83fa 100644 --- a/web/src/test/render.tsx +++ b/web/src/test/render.tsx @@ -1,16 +1,17 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { render } from "@testing-library/react"; import type { ReactElement } from "react"; -import { MemoryRouter } from "react-router-dom"; +import { createMemoryRouter, RouterProvider } from "react-router-dom"; import "../i18n"; export function renderApp(ui: ReactElement, { route = "/" } = {}) { const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const router = createMemoryRouter([{ path: "*", element: ui }], { initialEntries: [route] }); return render( - {ui} + , ); } diff --git a/web/vite.config.ts b/web/vite.config.ts index 628630d..b777c42 100644 --- a/web/vite.config.ts +++ b/web/vite.config.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { defineConfig } from "vite"; import { fileURLToPath } from 'node:url'; import { storybookTest } from '@storybook/addon-vitest/vitest-plugin'; +import { playwright } from '@vitest/browser-playwright'; const dirname = typeof __dirname !== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url)); // More info at: https://storybook.js.org/docs/next/writing-tests/integrations/vitest-addon @@ -48,7 +49,7 @@ export default defineConfig({ browser: { enabled: true, headless: true, - provider: 'playwright', + provider: playwright(), instances: [{ browser: 'chromium' }]