Files
biggus-dickus/docs/superpowers/plans/2026-06-07-object-form-robustness.md

373 lines
23 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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<FormShape>`; `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` `<BrowserRouter><Routes>` (3 top-level siblings). `renderApp` wraps `ui` in `<MemoryRouter>` with no `<Routes>`.
---
# 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 `<Route>` exactly as-is. New shape:
```tsx
const router = createBrowserRouter(
createRoutesFromElements(
<>
<Route path="/login" element={<LoginPage />} />
<Route element={<RequireAuth />}>
<Route element={<AppShell />}>
{/* ...all the existing nested <Route> elements, verbatim... */}
</Route>
</Route>
<Route path="*" element={<Navigate to="/objects" replace />} />
</>,
),
);
export function App() {
return <RouterProvider router={router} />;
}
```
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(
<QueryClientProvider client={qc}>
<RouterProvider router={router} />
</QueryClientProvider>,
);
}
```
This is behavior-preserving: `ui` renders at `route`; tests that include their own `<Routes>` 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 `<Routes>` 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> | 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 `<form>` for Cmd/Ctrl+Enter:
`onKeyDown={(e) => { if ((e.metaKey || e.ctrlKey) && e.key === "Enter") { e.preventDefault(); void submit(); } }}`
- Footer buttons:
```tsx
<div className="flex gap-2 pt-2">
<Button type="submit" disabled={isSubmitting} onClick={() => (createAnotherRef.current = false)}>
{isSubmitting ? t("form.saving") : mode === "create" ? t("form.create") : t("form.save")}
</Button>
{mode === "create" && (
<Button
type="submit"
variant="secondary"
disabled={isSubmitting}
onClick={() => (createAnotherRef.current = true)}
>
{t("form.createAnother")}
</Button>
)}
<Button type="button" variant="ghost" disabled={isSubmitting} onClick={onCancel}>
{t("form.cancel")}
</Button>
</div>
```
(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<boolean> => {
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] && (
<p role="alert" className="text-xs text-destructive">
{errors.core[key]?.message || t("form.required")}
</p>
)}
```
- `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 `<ObjectForm>`. 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).
- `<ObjectForm ... fieldErrorKey={fieldErrorKey} fieldErrorCode={fieldErrorCode} />`.
- [ ] **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 (
<AlertDialog open={open} onOpenChange={(o) => { if (!o) blocker.reset?.(); }}>
<AlertDialogContent>
<AlertDialogHeader>
<AlertDialogTitle>{t("form.unsaved.title")}</AlertDialogTitle>
<AlertDialogDescription>{t("form.unsaved.body")}</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel onClick={() => blocker.reset?.()}>{t("form.unsaved.stay")}</AlertDialogCancel>
<AlertDialogAction onClick={() => blocker.proceed?.()}>{t("form.unsaved.leave")}</AlertDialogAction>
</AlertDialogFooter>
</AlertDialog>
</AlertDialog>
);
}
```
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 `<UnsavedChangesDialog blocker={blocker} />` 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 `<Link>`/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<string | null>(() => {
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<string | null>(locationState?.fieldErrorKey ?? null);
const [fieldErrorCode, setFieldErrorCode] = useState<string | null>(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 17 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>|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.