214 lines
13 KiB
Markdown
214 lines
13 KiB
Markdown
# Consistent, Status-Aware Mutation Error Feedback — Design
|
|
|
|
**Date:** 2026-06-08
|
|
**Status:** Approved (brainstorming) — ready for implementation planning.
|
|
**Issue:** #63 (silent update failures + dead/unreachable mutation error strings).
|
|
|
|
## Context
|
|
|
|
A frontend deep audit flagged the TanStack Query mutation layer for inconsistent error feedback.
|
|
Grounding in the code refined the picture:
|
|
|
|
- **The update mutations are not silent — they're inconsistent.** `useUpdateTerm` (`queries.ts:444`) and
|
|
`useUpdateAuthority` (`:521`) are the **only 2 of 18 mutations** without `meta.suppressErrorToast`, so on
|
|
failure they fire the global `MutationCache` error toast while the row stays in edit mode. The matching
|
|
**create** and **delete** actions on the same screens suppress the toast and show an **inline** message
|
|
instead. So within one screen, create-failure → inline, delete-failure → inline, edit-failure → a
|
|
disconnected global toast.
|
|
- **Once the 2 update mutations also suppress (the consistency fix), all 18 mutations suppress** → the
|
|
global error toast becomes vestigial. Therefore status differentiation only delivers value at the
|
|
**inline** error sites, which today all render a generic `t("form.rejected")` ("Could not be saved").
|
|
- **The generic error strings are dead.** 16 mutations `throw new Error("update failed" | …)`, but
|
|
`mutationErrorMessage` (`query-client.ts:11-23`) never reads `.message` — it falls back to
|
|
`t("toast.error")`. The bespoke strings are unreachable.
|
|
- **`object-edit-form` mislabels a fetch failure.** `objects/object-edit-form.tsx:17` destructures only
|
|
`{ data, isLoading }` from `useObject`; a non-404 fetch error (network/500) falls through to the
|
|
"object not found" branch. (The **submit** path already handles `FieldRejection` and a generic else
|
|
correctly.)
|
|
|
|
### Decisions (from brainstorming)
|
|
1. Differentiate errors **by HTTP status**, surfaced via one shared mapping.
|
|
2. Because all mutations suppress the toast, apply status-aware messages at **all inline error sites**
|
|
(via one shared `<MutationError>` component), not the (now-vestigial) toast. Half-applying would
|
|
create a new inline inconsistency.
|
|
3. Make `useUpdateTerm`/`useUpdateAuthority` consistent with their create/delete siblings (suppress +
|
|
inline).
|
|
4. Fix the `object-edit-form` fetch mislabel.
|
|
|
|
## Components
|
|
|
|
### `web/src/api/error-message.ts` (new) — single source of truth
|
|
```ts
|
|
import { HttpError, InUseError } from "./queries";
|
|
|
|
/** Maps a caught mutation error to an i18n key (+ interpolation opts). Used by
|
|
* both the global toast fallback and every inline error display. */
|
|
export function errorMessageKey(error: unknown): { key: string; opts?: Record<string, unknown> } {
|
|
if (error instanceof InUseError) return { key: "actions.inUse", opts: { count: error.count } };
|
|
if (error instanceof HttpError) return { key: statusKey(error.status) };
|
|
return { key: "toast.error" };
|
|
}
|
|
|
|
function statusKey(status: number): string {
|
|
if (status === 403) return "errors.forbidden";
|
|
if (status === 404) return "errors.notFound";
|
|
if (status === 409) return "errors.conflict";
|
|
if (status === 422) return "errors.validation";
|
|
if (status >= 500) return "errors.server";
|
|
return "toast.error";
|
|
}
|
|
```
|
|
Returns a key+opts (not a resolved string) so callers render with their own `t` (reactive to language).
|
|
`InUseError` is checked first so 409-with-count keeps its richer message. (No circular import:
|
|
`queries.ts` does not import this module; `query-client.ts` imports both.)
|
|
|
|
### `web/src/components/mutation-error.tsx` (new) — shared inline display
|
|
```tsx
|
|
import { useTranslation } from "react-i18next";
|
|
|
|
import { errorMessageKey } from "../api/error-message";
|
|
|
|
/** Renders a caught mutation error as an inline alert, or nothing when error is falsy.
|
|
* Replaces the duplicated `<p role="alert" className="text-xs text-destructive">` markup. */
|
|
export function MutationError({ error }: { error: unknown }) {
|
|
const { t } = useTranslation();
|
|
if (!error) return null;
|
|
const { key, opts } = errorMessageKey(error);
|
|
return (
|
|
<p role="alert" className="text-xs text-destructive">
|
|
{t(key, opts)}
|
|
</p>
|
|
);
|
|
}
|
|
```
|
|
|
|
### `web/src/api/query-client.ts` (rewire)
|
|
`mutationErrorMessage` becomes:
|
|
```ts
|
|
function mutationErrorMessage(error: unknown, meta: MutationMeta | undefined): string {
|
|
if (meta?.errorMessage) return i18n.t(meta.errorMessage);
|
|
const { key, opts } = errorMessageKey(error);
|
|
return i18n.t(key, opts);
|
|
}
|
|
```
|
|
This keeps the toast path as a one-source-of-truth fallback (for any future non-suppressed mutation) and
|
|
drops the old special-cases (`InUseError` and `503 → search.unavailable` now flow through
|
|
`errorMessageKey`; no mutation throws 503, and the search **query**'s own inline 503 handling in
|
|
`search-panel.tsx` is untouched). Import moves from `{ HttpError, InUseError }` to `{ errorMessageKey }`.
|
|
|
|
### `web/src/api/queries.ts` (load-bearing errors)
|
|
Replace the 16 **mutation** `throw new Error("…")` with `throw new HttpError(response.status)`, preserving
|
|
the existing `InUseError` (409) and `FieldRejection` (422 on `setFields`) branches. Two POSTs
|
|
(`useCreateObject` `:184`, `useCreateVocabulary` `:279`) destructure only `{ data, error }` — add
|
|
`response` so the status is available. **Query** fetch errors (`:38,73,90,105,152,169,264`) and the login
|
|
`"invalid"/"network"` mapping (`:121`) are NOT changed (they're consumed by component `isError`/login
|
|
mapping, not the mutation toast). `useUpdateObject`'s 422 maps to the generic `errors.validation`;
|
|
surfacing a *structured* core-field rejection there is uncertain backend behavior → out of scope.
|
|
|
|
### Inline-site adoption — replace generic `form.rejected` with `<MutationError>`
|
|
- `web/src/components/delete-confirm-dialog.tsx:33-42` — the `confirm` catch currently sets
|
|
`err instanceof InUseError ? t("actions.inUse", {count}) : t("form.rejected")`. Replace with
|
|
`const { key, opts } = errorMessageKey(err); setMessage(t(key, opts));` — `errorMessageKey` already
|
|
handles `InUseError`, so the dialog simplifies and a 403/404 delete now reads a specific message. (The
|
|
dialog keeps its own `message` state because it must stay open on failure; it does not render
|
|
`<MutationError>` directly.)
|
|
- `web/src/authorities/authorities-page.tsx:144-148` — replace the `{create.isError && <p…>form.rejected}`
|
|
with `<MutationError error={create.error} />`.
|
|
- `web/src/vocab/vocabulary-terms.tsx:119-123` — `<MutationError error={addTerm.error} />`.
|
|
- `web/src/vocab/vocabulary-list.tsx:57-61` (create) and `:109-113` (rename) — `<MutationError error={create.error} />`
|
|
and `<MutationError error={renameVocabulary.error} />`.
|
|
- `web/src/fields/field-form.tsx:202-204` — replace the `{failed && <p…>form.rejected}` with
|
|
`<MutationError error={isEdit ? update.error : create.error} />`.
|
|
- **Object create/edit:** `object-new-page.tsx` and `object-edit-form.tsx` keep the `FieldRejection`
|
|
field-specific branch; in the non-`FieldRejection` else, replace `setError(t("form.rejected"))` with
|
|
`const { key, opts } = errorMessageKey(e); setError(t(key, opts));` (these set a string passed to
|
|
`ObjectForm`'s `formError` prop, so they use `errorMessageKey` directly, not `<MutationError>`).
|
|
|
|
### Edit-row consistency — `term-row.tsx`, `authority-row.tsx`, `queries.ts`
|
|
- Add `suppressErrorToast: true` to `useUpdateTerm` (`:444`) and `useUpdateAuthority` (`:521`) meta.
|
|
- In each row's edit view, render `<MutationError error={updateTerm.error} />` (resp. `updateAuthority.error`)
|
|
below the save/cancel buttons.
|
|
- Call `updateTerm.reset()` (resp. `updateAuthority.reset()`) inside the Edit button's `onClick` (alongside
|
|
the existing `setLabels`/`setUri`/`setEditing`) so a stale error from a prior failed save doesn't linger
|
|
when re-entering edit mode. On a failed save the row already stays editable (the `onSuccess`
|
|
`setEditing(false)` doesn't fire), preserving the user's input.
|
|
|
|
### `web/src/objects/object-edit-form.tsx` (fetch fix)
|
|
```tsx
|
|
const { data: object, isLoading, isError } = useObject(id!);
|
|
if (isLoading) return <FormSkeleton />;
|
|
if (isError) return <p className="p-4 text-sm text-destructive">{t("objects.loadError")}</p>;
|
|
if (!object) return <p className="p-4 text-sm text-muted-foreground">{t("objects.notFound")}</p>;
|
|
```
|
|
`objects.loadError` ("Could not load objects") already exists in both locales.
|
|
|
|
### i18n (en + sv parity — 5 new keys under `errors`)
|
|
| key | en | sv |
|
|
|-----|----|----|
|
|
| `errors.forbidden` | You don't have permission to do that. | Du har inte behörighet att göra det. |
|
|
| `errors.notFound` | That item no longer exists. | Objektet finns inte längre. |
|
|
| `errors.conflict` | That conflicts with existing data. | Det står i konflikt med befintliga data. |
|
|
| `errors.validation` | Some values weren't accepted. | Vissa värden godtogs inte. |
|
|
| `errors.server` | The server had a problem. Please try again. | Servern hade ett problem. Försök igen. |
|
|
|
|
## Data flow
|
|
mutation rejects → `HttpError(status)` (or `InUseError`/`FieldRejection`) → caught by the component (or
|
|
the `MutationCache` fallback) → `errorMessageKey(error)` → `{key, opts}` → `t(key, opts)` rendered inline
|
|
via `<MutationError>` (or as a `formError` string / dialog message). One mapping, one component, every
|
|
surface.
|
|
|
|
## Error handling / edges
|
|
- **All mutations suppress** after this change → the `MutationCache.onError` toast path is a dormant
|
|
fallback; kept (not deleted) so a future non-suppressed mutation still gets a sensible message.
|
|
- **401** isn't in the status map → falls to `toast.error`; the `client.ts` middleware still redirects to
|
|
login (unchanged). A brief generic toast before redirect is pre-existing, not worsened.
|
|
- **Network error with no response:** openapi-fetch may reject before a `response` exists; those paths
|
|
keep throwing a generic `Error` → `errorMessageKey` returns `toast.error`. (Only status-bearing
|
|
failures get differentiated.)
|
|
- **Language reactivity:** `<MutationError>` and the object-form `formError` re-render with the active
|
|
locale because they call `t` at render (the `formError` string is recomputed on the next failed submit;
|
|
acceptable — error strings are transient).
|
|
- **Stale row error:** cleared via `mutation.reset()` on re-edit.
|
|
|
|
## Testing
|
|
- **`web/src/api/error-message.test.ts`** (unit): each status → expected key (403/404/409/422/500/502);
|
|
`InUseError(3)` → `{ key: "actions.inUse", opts: { count: 3 } }`; a bare `Error`/unknown →
|
|
`{ key: "toast.error" }`.
|
|
- **`web/src/components/mutation-error.test.tsx`**: renders the mapped text for an `HttpError(403)`;
|
|
renders nothing for `null`/`undefined`; renders the in-use count for `InUseError`.
|
|
- **`web/src/api/mutation-feedback.test.tsx`** (rework): the "non-suppressed → catch-all toast" test is
|
|
obsolete (all mutations now suppress). Replace it with a test that a suppressed mutation failing adds
|
|
**no** toast (keep the existing delete case), plus assert the success-toast case still works. The
|
|
status→message behavior is covered by `error-message.test.ts`.
|
|
- **`term-row` / `authority-row`** (extend existing or via `mutation-feedback`): a failed update renders
|
|
the inline `MutationError` text and the row stays editable; a successful update closes the editor;
|
|
re-entering edit after a failure shows no stale error.
|
|
- **`object-edit-form`**: a `useObject` fetch error (mock `/api/admin/objects/{id}` → 500) renders
|
|
`objects.loadError`, not `objects.notFound`.
|
|
- **Inline adoption**: at least one create-form test (e.g. authorities) asserts a failed create shows the
|
|
status-aware message via `MutationError` (e.g. 403 → `errors.forbidden`).
|
|
- **Gate:** `typecheck`/`lint`/`test`/`build`/`check:size`/`check:colors` green; en/sv parity (the #60
|
|
parity test guards the 5 new keys); no codename; no new dependency.
|
|
|
|
## Acceptance criteria
|
|
1. A shared `errorMessageKey(error)` maps `InUseError` and `HttpError` (by status: 403/404/409/422/≥500)
|
|
to i18n keys, with a `toast.error` fallback; unit-tested.
|
|
2. A shared `<MutationError error>` component renders the mapped inline alert (or nothing) and replaces
|
|
the duplicated `form.rejected` markup at every inline mutation-error site (delete dialog via the helper
|
|
directly, create/rename forms, edit rows, object-form `formError` via the helper).
|
|
3. The 16 mutation throws use `HttpError(status)` (queries unchanged); `query-client.ts` routes the toast
|
|
fallback through `errorMessageKey`.
|
|
4. `useUpdateTerm`/`useUpdateAuthority` suppress the toast and show an inline error at the row, staying
|
|
editable on failure and clearing stale errors on re-edit.
|
|
5. `object-edit-form` distinguishes a fetch error (`objects.loadError`) from "not found."
|
|
6. `typecheck`/`lint`/`test`/`build`/`check:colors` green; `check:size` reported; en/sv parity (5 new
|
|
keys); no codename; no new dependency.
|
|
|
|
## Out of scope → follow-ups
|
|
- Structured field-level rejection on **core-object** update 422 (uncertain backend shape).
|
|
- The broader `queries.ts` split + relocating the error classes to `api/errors.ts` (tracked in **#65** —
|
|
`error-message.ts` imports the classes from `queries.ts` for now).
|
|
- Toast-based error surfacing (deliberately superseded by inline; the toast path remains only as a
|
|
fallback).
|
|
- Retry affordances / optimistic updates.
|