diff --git a/docs/superpowers/plans/2026-06-04-tier2-papercuts.md b/docs/superpowers/plans/2026-06-04-tier2-papercuts.md new file mode 100644 index 0000000..e990ed1 --- /dev/null +++ b/docs/superpowers/plans/2026-06-04-tier2-papercuts.md @@ -0,0 +1,217 @@ +# Tier 2 Papercuts Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax. + +**Goal:** Clear a batch of small, well-specified correctness/observability/UX fixes from the issue tracker (#22, #18, #9, #4, #34, #31, #32, #37) — no new features. + +**Architecture:** Independent small fixes grouped by area into four tasks: backend API behaviour (#22, #18), backend cleanup (#9, #4), frontend states/a11y (#34, #31, #32, #37), then verification. + +**Tech Stack:** Rust (axum, sqlx, tracing), React + TS, TanStack Query, react-i18next, Vitest + RTL + MSW. + +**Conventions (every task):** nightly `cargo +nightly fmt`; `cargo clippy`. Frontend: no `any`/`eslint-disable`/`@ts-ignore`; en/sv i18n parity; no codename "biggus"/"dickus". Test infra via compose: `DATABASE_URL=postgres://postgres:postgres@localhost:5442/cms_dev` (this machine's override port), `MEILI_URL=http://localhost:7700`, `MEILI_MASTER_KEY=masterKey`. cargo from repo root; web from `web/`. + +--- + +## Task 1: Backend API — 404 for missing vocabulary (#22) + log public 500s (#18) + +**Files:** Modify `crates/api/src/admin_vocab.rs`, `crates/api/src/public.rs`; Test in the existing `crates/api/tests/admin_catalog.rs` (vocab/authority harness). + +### #22 — `add_term` returns 404 when the vocabulary doesn't exist +Today `db::vocab::add_term(...)` maps every error to 500; a well-formed `{id}` for a missing vocabulary triggers a foreign-key violation (SQLSTATE 23503) that should be **404**. + +- [ ] **Step 1: Failing test** — add to `crates/api/tests/admin_catalog.rs` (mirror its existing seed-editor/login/oneshot harness). Read the file first to reuse its helpers: +```rust +#[sqlx::test(migrations = "../db/migrations")] +async fn add_term_to_missing_vocabulary_is_404(pool: PgPool) { + // (use this file's existing migrate_sessions + seed editor + login helpers) + let app = /* build_app with state */; + let cookie = /* login as editor */; + let resp = app + .oneshot( + Request::builder() + .method("POST") + .uri("/api/admin/vocabularies/00000000-0000-0000-0000-000000000000/terms") + .header(header::COOKIE, &cookie) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(r#"{"labels":[{"lang":"en","label":"X"}]}"#)) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); +} +``` +(Match the exact helper names/signatures already in `admin_catalog.rs`. If that file doesn't have a login helper, copy the pattern from `crates/api/tests/admin_fields.rs`.) + +- [ ] **Step 2: Run → fails** (currently 500): `cargo test -p api --test admin_catalog add_term_to_missing_vocabulary`. + +- [ ] **Step 3: Fix** — in `crates/api/src/admin_vocab.rs` `add_term`, replace: +```rust + let term_id = db::vocab::add_term(&mut tx, &new) + .await + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; +``` +with: +```rust + let term_id = db::vocab::add_term(&mut tx, &new).await.map_err(|err| { + // A well-formed id for a missing vocabulary hits the FK constraint (23503). + if err.as_database_error().and_then(|e| e.code()).as_deref() == Some("23503") { + StatusCode::NOT_FOUND + } else { + tracing::error!(?err, "adding term"); + StatusCode::INTERNAL_SERVER_ERROR + } + })?; +``` + +- [ ] **Step 4: Run → passes**, and confirm adding a term to an existing vocab still returns 201 (existing tests cover this). + +### #18 — log the discarded `sqlx::Error` on public 500 paths +`crates/api/src/public.rs` discards errors via `.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)` (lines ~74, ~78) and `Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response()` (line ~109). `tracing` is already a dependency of the `api` crate — just log. + +- [ ] **Step 5:** In `list_objects`, change both `.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?` to: +```rust + .map_err(|err| { + tracing::error!(?err, "listing public objects"); + StatusCode::INTERNAL_SERVER_ERROR + })?; +``` +(use a message specific to each call site — e.g. "listing public objects" and "counting public objects" — match what each query does). + +- [ ] **Step 6:** In `get_object`, change the `Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response()` arm to bind and log the error: +```rust + Err(err) => { + tracing::error!(?err, "fetching public object"); + StatusCode::INTERNAL_SERVER_ERROR.into_response() + } +``` + +- [ ] **Step 7: Verify** — `cargo +nightly fmt`, `cargo clippy -p api --all-targets`, `cargo test -p api`. Commit: +```bash +git add crates/api +git commit -m "fix(api): 404 when adding a term to a missing vocabulary (#22); log public 500s (#18)" +``` + +--- + +## Task 2: Backend cleanup — enum/CHECK cross-refs (#9) + dead clone & test handle (#4) + +**Files:** Modify `crates/domain/src/authority.rs`, `crates/domain/src/audit.rs`, `crates/server/src/lib.rs`, `crates/server/tests/serve.rs`. + +> Do **not** edit any file under `crates/db/migrations/` — `sqlx::migrate!()` checksums applied migrations, so editing them (even a comment) breaks existing databases. The cross-reference comments go in the Rust enums only. + +- [ ] **Step 1: #9 — cross-reference comments.** + - In `crates/domain/src/authority.rs`, above `pub enum AuthorityKind`, add: + ```rust + /// Allowed kinds. NOTE: kept in sync by hand with the + /// `CHECK (kind IN ('person','organisation','place'))` constraint in + /// `crates/db/migrations/0002_vocabularies_authorities.sql` — update both together. + ``` + - In `crates/domain/src/audit.rs`, above `pub enum AuditAction`, add an equivalent comment pointing at the `action` CHECK in `crates/db/migrations/0001_*.sql` (open the migration to name the exact file + values). + +- [ ] **Step 2: #4 — remove the dead clone.** In `crates/server/src/lib.rs` `run`, the `AppState` is built with `app_name: config.app_name.clone()`. Since `config.app_name` is a `String` and the only later use of `config` is the disjoint field `config.bind_addr`, change it to a move: +```rust + app_name: config.app_name, +``` +Confirm it still compiles (partial move of one field; `&config.bind_addr` afterward is fine). + +- [ ] **Step 3: #4 — smoke-test handle.** Open `crates/server/tests/serve.rs`. The spawned `serve(...)` task's `.unwrap()` swallows server errors as a task panic, surfacing as a confusing client error. Capture the `JoinHandle` and, after the assertions, either abort it cleanly or check it didn't error — make a server-start failure surface as a clear test failure rather than a `reqwest` error. Read the file and apply the minimal change that propagates/surfaces the server error (e.g. keep the handle, assert it hasn't finished-with-error, or `handle.abort()` at the end). Keep the test green. + +- [ ] **Step 4: Verify** — `cargo +nightly fmt`, `cargo clippy --workspace --all-targets`, `cargo test -p server -p domain`. Commit: +```bash +git add crates/domain crates/server +git commit -m "chore: cross-ref enum/CHECK constraints (#9); drop dead clone + harden smoke test (#4)" +``` + +--- + +## Task 3: Frontend — search 503 (#34), list error states (#31), a11y + dead keys (#32), authority-kind test (#37) + +**Files:** Modify `web/src/api/queries.ts`, `web/src/search/search-panel.tsx`, `web/src/vocab/vocabulary-terms.tsx`, `web/src/authorities/authorities-page.tsx`, `web/src/i18n/{en,sv}.json`, `web/src/fields/fields.test.tsx`; Tests in `web/src/search/search.test.tsx`, plus the vocab/authorities test files. + +### #34 — distinguish search 503 ("unavailable") from a generic error +- [ ] **Step 1:** In `web/src/api/queries.ts`, add a tiny typed error and have `useSearch` throw it with the HTTP status (so the UI can branch without `any`). Near the top: +```ts +export class HttpError extends Error { + constructor(public readonly status: number) { + super(`HTTP ${status}`); + this.name = "HttpError"; + } +} +``` +In `useSearch`'s `queryFn`, replace `if (error || !data) throw new Error("search failed");` with: +```ts + if (error || !data) throw new HttpError(response.status); +``` +(`response` is already destructured from `api.GET`; if not, add it.) + +- [ ] **Step 2: i18n** — add `search.unavailable` to BOTH `en.json` and `sv.json` (parity): + - en: `"unavailable": "Search is not available on this server"` + - sv: `"unavailable": "Sök är inte tillgängligt på den här servern"` + +- [ ] **Step 3:** In `web/src/search/search-panel.tsx`, where `search.isError` renders `t("search.loadError")`, branch on a 503: +```tsx + {hasQuery && search.isError && ( +
+ {search.error instanceof HttpError && search.error.status === 503 + ? t("search.unavailable") + : t("search.loadError")} +
+ )} +``` +Import `HttpError` from `../api/queries`. + +- [ ] **Step 4: Tests** — in `web/src/search/search.test.tsx`, add: a `503` response → renders `search.unavailable`; a `500` response → renders `search.loadError`. (Use `server.use(http.get("/api/admin/search", () => new HttpResponse(null, { status: 503 })))` etc., then type a query and assert the text.) + +### #31 — loading/error states on the terms + authorities lists +- [ ] **Step 5:** In `web/src/vocab/vocabulary-terms.tsx`, the terms list uses `useTerms(id)` but renders empty/data only. Add `isLoading` (skeleton or `…`) and `isError` (`t("vocab.loadError")`) branches before the empty/data render, mirroring `vocabulary-list.tsx`'s state ladder. +- [ ] **Step 6:** In `web/src/authorities/authorities-page.tsx`, the list uses `useAuthorities(kind)`; add an `isError` branch rendering `t("authorities.loadError")` (currently a dead key — this uses it) and a loading branch. Keep the existing empty/data render. +- [ ] **Step 7: Tests** — add an error-state test to the vocab and authorities test files: MSW returns 500 for the terms / authorities GET → the respective `loadError` text appears. (Override the default handler with `server.use(...)`.) + +### #32 — ARIA tab semantics + remove dead i18n keys +- [ ] **Step 8:** In `web/src/authorities/authorities-page.tsx`, the kind tabs are `NavLink`s. Add tab semantics: wrap them in a container with `role="tablist"`, give each `role="tab"` and `aria-selected={isActive}` (the `NavLink` className callback already exposes `isActive` — use the render-prop form to set `aria-selected`). Keep the existing styling. +- [ ] **Step 9:** Remove the unused keys `vocab.title` and `authorities.title` from BOTH `en.json` and `sv.json` (grep first: `grep -rn "vocab.title\|authorities.title\|\.title" web/src` — confirm only the i18n definitions match; nothing references them). + +### #37 — frontend authority-kind reveal test +- [ ] **Step 10:** In `web/src/fields/fields.test.tsx`, add a test mirroring the existing Term test: type a key + EN label, `selectOptions(type, "authority")`, assert the authority-kind `