Compare commits
6 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 0d971cda15 | |||
| 914527edc6 | |||
| ff513e1712 | |||
| 1a91b8a242 | |||
| 2bce469ed2 | |||
| fbb7a297a6 |
@@ -185,9 +185,15 @@ pub(crate) async fn add_term(
|
|||||||
.await
|
.await
|
||||||
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
|
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
|
||||||
|
|
||||||
let term_id = db::vocab::add_term(&mut tx, &new)
|
let term_id = db::vocab::add_term(&mut tx, &new).await.map_err(|err| {
|
||||||
.await
|
// A well-formed id for a missing vocabulary hits the FK constraint (23503).
|
||||||
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
|
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
|
||||||
|
}
|
||||||
|
})?;
|
||||||
|
|
||||||
tx.commit()
|
tx.commit()
|
||||||
.await
|
.await
|
||||||
|
|||||||
@@ -71,11 +71,17 @@ pub(crate) async fn list_objects(
|
|||||||
// public read surface.
|
// public read surface.
|
||||||
let objects = db::catalog::list_public_objects(state.db.pool(), limit, offset)
|
let objects = db::catalog::list_public_objects(state.db.pool(), limit, offset)
|
||||||
.await
|
.await
|
||||||
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
|
.map_err(|err| {
|
||||||
|
tracing::error!(?err, "listing public objects");
|
||||||
|
StatusCode::INTERNAL_SERVER_ERROR
|
||||||
|
})?;
|
||||||
|
|
||||||
let total = db::catalog::count_public_objects(state.db.pool())
|
let total = db::catalog::count_public_objects(state.db.pool())
|
||||||
.await
|
.await
|
||||||
.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
|
.map_err(|err| {
|
||||||
|
tracing::error!(?err, "counting public objects");
|
||||||
|
StatusCode::INTERNAL_SERVER_ERROR
|
||||||
|
})?;
|
||||||
|
|
||||||
Ok(Json(PublicObjectPage {
|
Ok(Json(PublicObjectPage {
|
||||||
items: objects.iter().map(PublicView::from_object).collect(),
|
items: objects.iter().map(PublicView::from_object).collect(),
|
||||||
@@ -106,7 +112,10 @@ pub(crate) async fn get_object(
|
|||||||
match db::catalog::public_object_by_id(state.db.pool(), object_id).await {
|
match db::catalog::public_object_by_id(state.db.pool(), object_id).await {
|
||||||
Ok(Some(object)) => Json(PublicView::from_object(&object)).into_response(),
|
Ok(Some(object)) => Json(PublicView::from_object(&object)).into_response(),
|
||||||
Ok(None) => StatusCode::NOT_FOUND.into_response(),
|
Ok(None) => StatusCode::NOT_FOUND.into_response(),
|
||||||
Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(),
|
Err(err) => {
|
||||||
|
tracing::error!(?err, "fetching public object");
|
||||||
|
StatusCode::INTERNAL_SERVER_ERROR.into_response()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -263,3 +263,30 @@ async fn create_and_list_authorities_by_kind(pool: PgPool) {
|
|||||||
let bad = app2_get(&app, &cookie, "/api/admin/authorities?kind=alien").await;
|
let bad = app2_get(&app, &cookie, "/api/admin/authorities?kind=alien").await;
|
||||||
assert_eq!(bad, StatusCode::UNPROCESSABLE_ENTITY);
|
assert_eq!(bad, StatusCode::UNPROCESSABLE_ENTITY);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[sqlx::test(migrations = "../db/migrations")]
|
||||||
|
async fn add_term_to_missing_vocabulary_is_404(pool: PgPool) {
|
||||||
|
migrate_sessions(&db::Db::from_pool(pool.clone()))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
seed_user(&pool, "ed@example.com", "pw-editor-123", Role::Editor).await;
|
||||||
|
|
||||||
|
let app = build_app(state(pool));
|
||||||
|
let cookie = login(&app, "ed@example.com", "pw-editor-123").await;
|
||||||
|
|
||||||
|
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);
|
||||||
|
}
|
||||||
|
|||||||
@@ -4,6 +4,10 @@ use time::OffsetDateTime;
|
|||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
|
|
||||||
/// What kind of change an audit entry records.
|
/// What kind of change an audit entry records.
|
||||||
|
///
|
||||||
|
/// NOTE: kept in sync by hand with the
|
||||||
|
/// `CHECK (action IN ('created', 'updated', 'deleted'))` constraint in
|
||||||
|
/// `crates/db/migrations/0001_audit_log.sql` — add a variant in both places.
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
#[serde(rename_all = "lowercase")]
|
#[serde(rename_all = "lowercase")]
|
||||||
pub enum AuditAction {
|
pub enum AuditAction {
|
||||||
|
|||||||
@@ -3,6 +3,10 @@ use serde::{Deserialize, Serialize};
|
|||||||
use crate::{AuthorityId, LocalizedLabel};
|
use crate::{AuthorityId, LocalizedLabel};
|
||||||
|
|
||||||
/// The kind of authority record.
|
/// The kind of authority record.
|
||||||
|
///
|
||||||
|
/// NOTE: kept in sync by hand with the
|
||||||
|
/// `CHECK (kind IN ('person', 'organisation', 'place'))` constraint in
|
||||||
|
/// `crates/db/migrations/0002_vocabularies_authorities.sql` — add a variant in both places.
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
#[serde(rename_all = "lowercase")]
|
#[serde(rename_all = "lowercase")]
|
||||||
pub enum AuthorityKind {
|
pub enum AuthorityKind {
|
||||||
|
|||||||
@@ -50,7 +50,7 @@ pub async fn run(config: Config) -> anyhow::Result<()> {
|
|||||||
|
|
||||||
let state = AppState {
|
let state = AppState {
|
||||||
db,
|
db,
|
||||||
app_name: config.app_name.clone(),
|
app_name: config.app_name,
|
||||||
cookie_secure: config.cookie_secure,
|
cookie_secure: config.cookie_secure,
|
||||||
search,
|
search,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -22,13 +22,21 @@ async fn serves_health_live_over_tcp() {
|
|||||||
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
|
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
|
||||||
let addr: SocketAddr = listener.local_addr().unwrap();
|
let addr: SocketAddr = listener.local_addr().unwrap();
|
||||||
|
|
||||||
let handle = tokio::spawn(async move {
|
let handle = tokio::spawn(async move { serve(listener, state).await });
|
||||||
serve(listener, state).await.unwrap();
|
|
||||||
});
|
|
||||||
|
|
||||||
let url = format!("http://{addr}/health/live");
|
let url = format!("http://{addr}/health/live");
|
||||||
let body: serde_json::Value = reqwest::get(&url)
|
let response = reqwest::get(&url).await;
|
||||||
.await
|
|
||||||
|
// If the request failed and the server task already ended, it errored — surface that
|
||||||
|
// (a clear server error) instead of the opaque reqwest failure.
|
||||||
|
if response.is_err() && handle.is_finished() {
|
||||||
|
match handle.await {
|
||||||
|
Ok(Err(err)) => panic!("server failed: {err:?}"),
|
||||||
|
other => panic!("server task ended unexpectedly: {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
let body: serde_json::Value = response
|
||||||
.expect("request succeeds")
|
.expect("request succeeds")
|
||||||
.json()
|
.json()
|
||||||
.await
|
.await
|
||||||
|
|||||||
@@ -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 && (
|
||||||
|
<p className="p-4 text-sm text-red-600">
|
||||||
|
{search.error instanceof HttpError && search.error.status === 503
|
||||||
|
? t("search.unavailable")
|
||||||
|
: t("search.loadError")}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
|
```
|
||||||
|
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 `<select>` (label `/authority kind/i`) appears, `selectOptions` it to `"person"`, submit, and assert the POST body's `authority_kind === "person"` (use a `server.use` POST handler that captures the body, like the Term test does).
|
||||||
|
|
||||||
|
- [ ] **Step 11: Verify** — `cd web && pnpm test && pnpm typecheck && pnpm lint && pnpm build && pnpm check:size`. All green; bundle ≤150 KB. Commit:
|
||||||
|
```bash
|
||||||
|
cd /Users/olsson/Laboratory/biggus-dickus
|
||||||
|
git add web
|
||||||
|
git commit -m "fix(web): search 503 vs error (#34); terms/authorities list error states (#31); authority-tab a11y + dead keys (#32); authority-kind test (#37)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 4: Verification
|
||||||
|
|
||||||
|
- [ ] **Step 1: i18n parity** —
|
||||||
|
```bash
|
||||||
|
cd web
|
||||||
|
node -e "const a=require('./src/i18n/en.json'),b=require('./src/i18n/sv.json');const k=o=>Object.entries(o).flatMap(([K,v])=>typeof v==='object'?k(v).map(s=>K+'.'+s):[K]);const ka=k(a).sort(),kb=k(b).sort();console.log(JSON.stringify(ka)===JSON.stringify(kb)?'PARITY OK':'MISMATCH '+JSON.stringify({onlyEn:ka.filter(x=>!kb.includes(x)),onlySv:kb.filter(x=>!ka.includes(x))}))"
|
||||||
|
```
|
||||||
|
Expected `PARITY OK`.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Frontend** — `pnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size` (report bundle gz).
|
||||||
|
|
||||||
|
- [ ] **Step 3: Backend** —
|
||||||
|
```bash
|
||||||
|
cd /Users/olsson/Laboratory/biggus-dickus
|
||||||
|
DATABASE_URL=postgres://postgres:postgres@localhost:5442/cms_dev \
|
||||||
|
MEILI_URL=http://localhost:7700 MEILI_MASTER_KEY=masterKey \
|
||||||
|
cargo test -p api -p domain -p server
|
||||||
|
cargo clippy --workspace --all-targets
|
||||||
|
cargo +nightly fmt --check
|
||||||
|
```
|
||||||
|
All pass; clippy + fmt clean.
|
||||||
|
|
||||||
|
- [ ] **Step 4:** No codename: `git grep -in 'biggus\|dickus' -- crates web/src` → no matches.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Self-Review (completed)
|
||||||
|
- **Spec coverage:** #22 (404), #18 (log 500s) → Task 1; #9 (Rust cross-ref comments), #4 (clone + smoke test) → Task 2; #34, #31, #32, #37 → Task 3; parity + suites → Task 4. ✓
|
||||||
|
- **Scope adjustments baked in:** #8 already closed (thiserror is used); #37 backend-403 omitted (no non-EditCatalogue role exists); #9 Rust-side only (migration checksums). ✓
|
||||||
|
- **Placeholder scan:** none — code is concrete; the "match the existing harness" notes are verification instructions against named files.
|
||||||
|
- **Type consistency:** `HttpError` defined in queries.ts and imported in search-panel; the 23503/FK pattern matches the field-def handler; `authorities.loadError` (existing key) now consumed; `search.unavailable` added at parity.
|
||||||
@@ -3,6 +3,13 @@ import { keepPreviousData, useInfiniteQuery, useMutation, useQuery, useQueryClie
|
|||||||
import { api } from "./client";
|
import { api } from "./client";
|
||||||
import type { components } from "./schema";
|
import type { components } from "./schema";
|
||||||
|
|
||||||
|
export class HttpError extends Error {
|
||||||
|
constructor(public readonly status: number) {
|
||||||
|
super(`HTTP ${status}`);
|
||||||
|
this.name = "HttpError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
type UserView = components["schemas"]["UserView"];
|
type UserView = components["schemas"]["UserView"];
|
||||||
type LoginRequest = components["schemas"]["LoginRequest"];
|
type LoginRequest = components["schemas"]["LoginRequest"];
|
||||||
|
|
||||||
@@ -291,7 +298,7 @@ export function useSearch(q: string, visibility: string | null) {
|
|||||||
enabled: term.length > 0,
|
enabled: term.length > 0,
|
||||||
initialPageParam: 0,
|
initialPageParam: 0,
|
||||||
queryFn: async ({ pageParam }) => {
|
queryFn: async ({ pageParam }) => {
|
||||||
const { data, error } = await api.GET("/api/admin/search", {
|
const { data, error, response } = await api.GET("/api/admin/search", {
|
||||||
params: {
|
params: {
|
||||||
query: {
|
query: {
|
||||||
q: term,
|
q: term,
|
||||||
@@ -302,7 +309,7 @@ export function useSearch(q: string, visibility: string | null) {
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
if (error || !data) throw new Error("search failed");
|
if (error || !data) throw new HttpError(response.status);
|
||||||
|
|
||||||
return data;
|
return data;
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -18,8 +18,9 @@ export function AuthoritiesPage() {
|
|||||||
const lang = i18n.language.startsWith("sv") ? "sv" : "en";
|
const lang = i18n.language.startsWith("sv") ? "sv" : "en";
|
||||||
|
|
||||||
const isValidKind = (KINDS as readonly string[]).includes(kind ?? "");
|
const isValidKind = (KINDS as readonly string[]).includes(kind ?? "");
|
||||||
|
const currentKind = isValidKind ? (kind as string) : "person";
|
||||||
|
|
||||||
const { data: authorities } = useAuthorities(isValidKind ? (kind as string) : "person");
|
const { data: authorities, isLoading, isError } = useAuthorities(currentKind);
|
||||||
const create = useCreateAuthority();
|
const create = useCreateAuthority();
|
||||||
|
|
||||||
const [labels, setLabels] = useState<LabelInput[]>([]);
|
const [labels, setLabels] = useState<LabelInput[]>([]);
|
||||||
@@ -44,11 +45,13 @@ export function AuthoritiesPage() {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="overflow-auto p-4">
|
<div className="overflow-auto p-4">
|
||||||
<div className="mb-3 flex gap-2">
|
<div role="tablist" className="mb-3 flex gap-2">
|
||||||
{KINDS.map((k) => (
|
{KINDS.map((k) => (
|
||||||
<NavLink
|
<NavLink
|
||||||
key={k}
|
key={k}
|
||||||
to={`/authorities/${k}`}
|
to={`/authorities/${k}`}
|
||||||
|
role="tab"
|
||||||
|
aria-selected={k === currentKind}
|
||||||
className={({ isActive }) =>
|
className={({ isActive }) =>
|
||||||
`rounded px-3 py-1 text-sm ${isActive ? "bg-neutral-800 text-white" : "border"}`
|
`rounded px-3 py-1 text-sm ${isActive ? "bg-neutral-800 text-white" : "border"}`
|
||||||
}
|
}
|
||||||
@@ -59,7 +62,13 @@ export function AuthoritiesPage() {
|
|||||||
</div>
|
</div>
|
||||||
|
|
||||||
<ul className="mb-4">
|
<ul className="mb-4">
|
||||||
{authorities?.length === 0 && (
|
{isLoading && (
|
||||||
|
<li className="text-sm text-neutral-400">…</li>
|
||||||
|
)}
|
||||||
|
{isError && (
|
||||||
|
<li className="text-sm text-red-600">{t("authorities.loadError")}</li>
|
||||||
|
)}
|
||||||
|
{!isLoading && !isError && authorities?.length === 0 && (
|
||||||
<li className="text-sm text-neutral-500">{t("authorities.empty")}</li>
|
<li className="text-sm text-neutral-500">{t("authorities.empty")}</li>
|
||||||
)}
|
)}
|
||||||
{authorities?.map((a) => (
|
{authorities?.map((a) => (
|
||||||
@@ -71,7 +80,7 @@ export function AuthoritiesPage() {
|
|||||||
|
|
||||||
<form onSubmit={onCreate} className="space-y-2 border-t pt-3">
|
<form onSubmit={onCreate} className="space-y-2 border-t pt-3">
|
||||||
<div className="text-sm font-medium">
|
<div className="text-sm font-medium">
|
||||||
{t("authorities.new")} · {t(`authorities.${kind}`)}
|
{t("authorities.new")} · {t(`authorities.${currentKind}`)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<LabelEditor value={labels} onChange={setLabels} />
|
<LabelEditor value={labels} onChange={setLabels} />
|
||||||
|
|||||||
@@ -33,7 +33,13 @@ test("lists authorities for the kind and creates one", async () => {
|
|||||||
|
|
||||||
test("kind tabs link to the other kinds", async () => {
|
test("kind tabs link to the other kinds", async () => {
|
||||||
renderApp(tree(), { route: "/authorities/person" });
|
renderApp(tree(), { route: "/authorities/person" });
|
||||||
expect(await screen.findByRole("link", { name: /place/i })).toHaveAttribute("href", "/authorities/place");
|
expect(await screen.findByRole("tab", { name: /place/i })).toHaveAttribute("href", "/authorities/place");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("aria-selected is on the tab element and reflects the active kind", async () => {
|
||||||
|
renderApp(tree(), { route: "/authorities/person" });
|
||||||
|
expect(await screen.findByRole("tab", { name: /^person$/i })).toHaveAttribute("aria-selected", "true");
|
||||||
|
expect(screen.getByRole("tab", { name: /^place$/i })).toHaveAttribute("aria-selected", "false");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("create without EN label shows required alert and does not POST", async () => {
|
test("create without EN label shows required alert and does not POST", async () => {
|
||||||
@@ -51,6 +57,14 @@ test("create without EN label shows required alert and does not POST", async ()
|
|||||||
expect(posted).toBe(false);
|
expect(posted).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("authorities endpoint error shows loadError", async () => {
|
||||||
|
server.use(
|
||||||
|
http.get("/api/admin/authorities", () => new HttpResponse(null, { status: 500 })),
|
||||||
|
);
|
||||||
|
renderApp(tree(), { route: "/authorities/person" });
|
||||||
|
expect(await screen.findByText(/could not load/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
test("unknown kind redirects to person list", async () => {
|
test("unknown kind redirects to person list", async () => {
|
||||||
renderApp(tree(), { route: "/authorities/banana" });
|
renderApp(tree(), { route: "/authorities/banana" });
|
||||||
expect(await screen.findByText("Ada Lovelace")).toBeInTheDocument();
|
expect(await screen.findByText("Ada Lovelace")).toBeInTheDocument();
|
||||||
|
|||||||
@@ -43,6 +43,27 @@ test("creates a text field — posts the body and clears the key input", async (
|
|||||||
await waitFor(() => expect(screen.getByLabelText(/^key$/i)).toHaveValue(""));
|
await waitFor(() => expect(screen.getByLabelText(/^key$/i)).toHaveValue(""));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("selecting Authority reveals the kind picker and posts the chosen kind", async () => {
|
||||||
|
let body: { authority_kind: string | null } | undefined;
|
||||||
|
|
||||||
|
server.use(
|
||||||
|
http.post("/api/admin/field-definitions", async ({ request }) => {
|
||||||
|
body = (await request.json()) as { authority_kind: string | null };
|
||||||
|
return HttpResponse.json({ key: "maker" }, { status: 201 });
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
renderApp(tree(), { route: "/fields" });
|
||||||
|
|
||||||
|
await userEvent.type(screen.getByLabelText(/^key$/i), "maker");
|
||||||
|
await userEvent.type(screen.getByLabelText(/label \(en\)/i), "Maker");
|
||||||
|
await userEvent.selectOptions(screen.getByLabelText(/^type$/i), "authority");
|
||||||
|
const kind = await screen.findByLabelText(/authority kind/i);
|
||||||
|
await userEvent.selectOptions(kind, "person");
|
||||||
|
await userEvent.click(screen.getByRole("button", { name: /create field/i }));
|
||||||
|
|
||||||
|
await waitFor(() => expect(body?.authority_kind).toBe("person"));
|
||||||
|
});
|
||||||
|
|
||||||
test("selecting Term reveals the vocabulary picker and blocks submit until chosen", async () => {
|
test("selecting Term reveals the vocabulary picker and blocks submit until chosen", async () => {
|
||||||
let posted = false;
|
let posted = false;
|
||||||
|
|
||||||
|
|||||||
@@ -9,13 +9,13 @@
|
|||||||
"actions": { "edit": "Edit", "delete": "Delete", "confirmDelete": "Delete this object? This cannot be undone." },
|
"actions": { "edit": "Edit", "delete": "Delete", "confirmDelete": "Delete this object? This cannot be undone." },
|
||||||
"labels": { "en": "Label (EN)", "sv": "Label (SV)", "externalUri": "External URI (optional)" },
|
"labels": { "en": "Label (EN)", "sv": "Label (SV)", "externalUri": "External URI (optional)" },
|
||||||
"vocab": {
|
"vocab": {
|
||||||
"title": "Vocabularies", "newVocabulary": "New vocabulary", "key": "Key",
|
"newVocabulary": "New vocabulary", "key": "Key",
|
||||||
"create": "Create", "selectPrompt": "Select a vocabulary to manage its terms",
|
"create": "Create", "selectPrompt": "Select a vocabulary to manage its terms",
|
||||||
"terms": "Terms", "addTerm": "Add term", "empty": "No vocabularies yet",
|
"terms": "Terms", "addTerm": "Add term", "empty": "No vocabularies yet",
|
||||||
"noTerms": "No terms yet", "loadError": "Could not load"
|
"noTerms": "No terms yet", "loadError": "Could not load"
|
||||||
},
|
},
|
||||||
"authorities": {
|
"authorities": {
|
||||||
"title": "Authorities", "person": "Person", "organisation": "Organisation", "place": "Place",
|
"person": "Person", "organisation": "Organisation", "place": "Place",
|
||||||
"new": "New", "create": "Create", "empty": "No authorities yet", "loadError": "Could not load"
|
"new": "New", "create": "Create", "empty": "No authorities yet", "loadError": "Could not load"
|
||||||
},
|
},
|
||||||
"search": {
|
"search": {
|
||||||
@@ -24,6 +24,7 @@
|
|||||||
"prompt": "Type to search",
|
"prompt": "Type to search",
|
||||||
"empty": "No results",
|
"empty": "No results",
|
||||||
"loadError": "Search is unavailable",
|
"loadError": "Search is unavailable",
|
||||||
|
"unavailable": "Search is not available on this server",
|
||||||
"loadMore": "Load more",
|
"loadMore": "Load more",
|
||||||
"resultCount_one": "{{count}} result",
|
"resultCount_one": "{{count}} result",
|
||||||
"resultCount_other": "{{count}} results",
|
"resultCount_other": "{{count}} results",
|
||||||
|
|||||||
@@ -9,13 +9,13 @@
|
|||||||
"actions": { "edit": "Redigera", "delete": "Ta bort", "confirmDelete": "Ta bort detta föremål? Detta kan inte ångras." },
|
"actions": { "edit": "Redigera", "delete": "Ta bort", "confirmDelete": "Ta bort detta föremål? Detta kan inte ångras." },
|
||||||
"labels": { "en": "Etikett (EN)", "sv": "Etikett (SV)", "externalUri": "Extern URI (valfritt)" },
|
"labels": { "en": "Etikett (EN)", "sv": "Etikett (SV)", "externalUri": "Extern URI (valfritt)" },
|
||||||
"vocab": {
|
"vocab": {
|
||||||
"title": "Vokabulär", "newVocabulary": "Ny vokabulär", "key": "Nyckel",
|
"newVocabulary": "Ny vokabulär", "key": "Nyckel",
|
||||||
"create": "Skapa", "selectPrompt": "Välj en vokabulär för att hantera dess termer",
|
"create": "Skapa", "selectPrompt": "Välj en vokabulär för att hantera dess termer",
|
||||||
"terms": "Termer", "addTerm": "Lägg till term", "empty": "Inga vokabulärer ännu",
|
"terms": "Termer", "addTerm": "Lägg till term", "empty": "Inga vokabulärer ännu",
|
||||||
"noTerms": "Inga termer ännu", "loadError": "Kunde inte ladda"
|
"noTerms": "Inga termer ännu", "loadError": "Kunde inte ladda"
|
||||||
},
|
},
|
||||||
"authorities": {
|
"authorities": {
|
||||||
"title": "Auktoriteter", "person": "Person", "organisation": "Organisation", "place": "Plats",
|
"person": "Person", "organisation": "Organisation", "place": "Plats",
|
||||||
"new": "Ny", "create": "Skapa", "empty": "Inga auktoriteter ännu", "loadError": "Kunde inte ladda"
|
"new": "Ny", "create": "Skapa", "empty": "Inga auktoriteter ännu", "loadError": "Kunde inte ladda"
|
||||||
},
|
},
|
||||||
"search": {
|
"search": {
|
||||||
@@ -24,6 +24,7 @@
|
|||||||
"prompt": "Skriv för att söka",
|
"prompt": "Skriv för att söka",
|
||||||
"empty": "Inga träffar",
|
"empty": "Inga träffar",
|
||||||
"loadError": "Sök är inte tillgängligt",
|
"loadError": "Sök är inte tillgängligt",
|
||||||
|
"unavailable": "Sök är inte tillgängligt på den här servern",
|
||||||
"loadMore": "Visa fler",
|
"loadMore": "Visa fler",
|
||||||
"resultCount_one": "{{count}} träff",
|
"resultCount_one": "{{count}} träff",
|
||||||
"resultCount_other": "{{count}} träffar",
|
"resultCount_other": "{{count}} träffar",
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import { useEffect, useState } from "react";
|
|||||||
import { useSearchParams } from "react-router-dom";
|
import { useSearchParams } from "react-router-dom";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
|
|
||||||
import { useSearch } from "../api/queries";
|
import { useSearch, HttpError } from "../api/queries";
|
||||||
import { useDebouncedValue } from "../lib/use-debounced-value";
|
import { useDebouncedValue } from "../lib/use-debounced-value";
|
||||||
import { SearchResultRow } from "./search-result-row";
|
import { SearchResultRow } from "./search-result-row";
|
||||||
import { Button } from "@/components/ui/button";
|
import { Button } from "@/components/ui/button";
|
||||||
@@ -92,7 +92,11 @@ export function SearchPanel() {
|
|||||||
)}
|
)}
|
||||||
|
|
||||||
{hasQuery && search.isError && (
|
{hasQuery && search.isError && (
|
||||||
<p className="p-4 text-sm text-red-600">{t("search.loadError")}</p>
|
<p className="p-4 text-sm text-red-600">
|
||||||
|
{search.error instanceof HttpError && search.error.status === 503
|
||||||
|
? t("search.unavailable")
|
||||||
|
: t("search.loadError")}
|
||||||
|
</p>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{hasQuery && !search.isLoading && !search.isError && hits.length === 0 && (
|
{hasQuery && !search.isLoading && !search.isError && hits.length === 0 && (
|
||||||
|
|||||||
@@ -76,6 +76,20 @@ test("clicking a result shows the object in the detail pane", async () => {
|
|||||||
expect(await screen.findByText(amphora.object_name)).toBeInTheDocument();
|
expect(await screen.findByText(amphora.object_name)).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("a 503 shows the search-unavailable message", async () => {
|
||||||
|
server.use(http.get("/api/admin/search", () => new HttpResponse(null, { status: 503 })));
|
||||||
|
renderApp(tree(), { route: "/search" });
|
||||||
|
await userEvent.type(screen.getByLabelText(/search the collection/i), "bronze");
|
||||||
|
expect(await screen.findByText(/not available on this server/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a 500 shows the generic search error", async () => {
|
||||||
|
server.use(http.get("/api/admin/search", () => new HttpResponse(null, { status: 500 })));
|
||||||
|
renderApp(tree(), { route: "/search" });
|
||||||
|
await userEvent.type(screen.getByLabelText(/search the collection/i), "bronze");
|
||||||
|
expect(await screen.findByText(/^search is unavailable$/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
test("hydrates query and visibility from the initial URL", async () => {
|
test("hydrates query and visibility from the initial URL", async () => {
|
||||||
renderApp(tree(), { route: "/search?q=bronze" });
|
renderApp(tree(), { route: "/search?q=bronze" });
|
||||||
|
|
||||||
|
|||||||
@@ -52,6 +52,14 @@ test("selecting a vocabulary shows its terms and adds one", async () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("terms endpoint error shows vocab loadError", async () => {
|
||||||
|
server.use(
|
||||||
|
http.get("/api/admin/vocabularies/:id/terms", () => new HttpResponse(null, { status: 500 })),
|
||||||
|
);
|
||||||
|
renderApp(tree(), { route: "/vocabularies/v-material" });
|
||||||
|
expect(await screen.findByText(/could not load/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
test("add term without EN label shows required alert and does not POST", async () => {
|
test("add term without EN label shows required alert and does not POST", async () => {
|
||||||
let posted = false;
|
let posted = false;
|
||||||
server.use(
|
server.use(
|
||||||
|
|||||||
@@ -19,7 +19,7 @@ export function VocabularyTerms() {
|
|||||||
|
|
||||||
const lang = i18n.language.startsWith("sv") ? "sv" : "en";
|
const lang = i18n.language.startsWith("sv") ? "sv" : "en";
|
||||||
|
|
||||||
const { data: terms } = useTerms(id);
|
const { data: terms, isLoading, isError } = useTerms(id);
|
||||||
|
|
||||||
const addTerm = useAddTerm();
|
const addTerm = useAddTerm();
|
||||||
|
|
||||||
@@ -53,7 +53,13 @@ export function VocabularyTerms() {
|
|||||||
{t("vocab.terms")}
|
{t("vocab.terms")}
|
||||||
</h3>
|
</h3>
|
||||||
<ul className="mb-4">
|
<ul className="mb-4">
|
||||||
{terms?.length === 0 && (
|
{isLoading && (
|
||||||
|
<li className="text-sm text-neutral-400">…</li>
|
||||||
|
)}
|
||||||
|
{isError && (
|
||||||
|
<li className="text-sm text-red-600">{t("vocab.loadError")}</li>
|
||||||
|
)}
|
||||||
|
{!isLoading && !isError && terms?.length === 0 && (
|
||||||
<li className="text-sm text-neutral-500">{t("vocab.noTerms")}</li>
|
<li className="text-sm text-neutral-500">{t("vocab.noTerms")}</li>
|
||||||
)}
|
)}
|
||||||
{terms?.map((term) => (
|
{terms?.map((term) => (
|
||||||
|
|||||||
Reference in New Issue
Block a user