diff --git a/crates/api/src/admin_vocab.rs b/crates/api/src/admin_vocab.rs index e42d446..18a2e71 100644 --- a/crates/api/src/admin_vocab.rs +++ b/crates/api/src/admin_vocab.rs @@ -185,9 +185,15 @@ pub(crate) async fn add_term( .await .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - let term_id = db::vocab::add_term(&mut tx, &new) - .await - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + 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 + } + })?; tx.commit() .await diff --git a/crates/api/src/public.rs b/crates/api/src/public.rs index b36f2c7..15a5d05 100644 --- a/crates/api/src/public.rs +++ b/crates/api/src/public.rs @@ -71,11 +71,17 @@ pub(crate) async fn list_objects( // public read surface. let objects = db::catalog::list_public_objects(state.db.pool(), limit, offset) .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()) .await - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + .map_err(|err| { + tracing::error!(?err, "counting public objects"); + StatusCode::INTERNAL_SERVER_ERROR + })?; Ok(Json(PublicObjectPage { 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 { Ok(Some(object)) => Json(PublicView::from_object(&object)).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() + } } } diff --git a/crates/api/tests/admin_catalog.rs b/crates/api/tests/admin_catalog.rs index 9ef9e8a..d9e4594 100644 --- a/crates/api/tests/admin_catalog.rs +++ b/crates/api/tests/admin_catalog.rs @@ -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; 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); +} diff --git a/crates/domain/src/audit.rs b/crates/domain/src/audit.rs index f9d7114..238dd3c 100644 --- a/crates/domain/src/audit.rs +++ b/crates/domain/src/audit.rs @@ -4,6 +4,10 @@ use time::OffsetDateTime; use uuid::Uuid; /// 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)] #[serde(rename_all = "lowercase")] pub enum AuditAction { diff --git a/crates/domain/src/authority.rs b/crates/domain/src/authority.rs index 299b1a1..7cc550e 100644 --- a/crates/domain/src/authority.rs +++ b/crates/domain/src/authority.rs @@ -3,6 +3,10 @@ use serde::{Deserialize, Serialize}; use crate::{AuthorityId, LocalizedLabel}; /// 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)] #[serde(rename_all = "lowercase")] pub enum AuthorityKind { diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index ae630b8..a355cd6 100644 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -50,7 +50,7 @@ pub async fn run(config: Config) -> anyhow::Result<()> { let state = AppState { db, - app_name: config.app_name.clone(), + app_name: config.app_name, cookie_secure: config.cookie_secure, search, }; diff --git a/crates/server/tests/serve.rs b/crates/server/tests/serve.rs index aa15497..b3dd99d 100644 --- a/crates/server/tests/serve.rs +++ b/crates/server/tests/serve.rs @@ -22,13 +22,21 @@ async fn serves_health_live_over_tcp() { let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr: SocketAddr = listener.local_addr().unwrap(); - let handle = tokio::spawn(async move { - serve(listener, state).await.unwrap(); - }); + let handle = tokio::spawn(async move { serve(listener, state).await }); let url = format!("http://{addr}/health/live"); - let body: serde_json::Value = reqwest::get(&url) - .await + let response = reqwest::get(&url).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") .json() .await diff --git a/web/src/api/queries.ts b/web/src/api/queries.ts index 0825ec9..d53d672 100644 --- a/web/src/api/queries.ts +++ b/web/src/api/queries.ts @@ -3,6 +3,13 @@ import { keepPreviousData, useInfiniteQuery, useMutation, useQuery, useQueryClie import { api } from "./client"; 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 LoginRequest = components["schemas"]["LoginRequest"]; @@ -291,7 +298,7 @@ export function useSearch(q: string, visibility: string | null) { enabled: term.length > 0, initialPageParam: 0, queryFn: async ({ pageParam }) => { - const { data, error } = await api.GET("/api/admin/search", { + const { data, error, response } = await api.GET("/api/admin/search", { params: { query: { 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; }, diff --git a/web/src/authorities/authorities-page.tsx b/web/src/authorities/authorities-page.tsx index 06d03f2..13aa20d 100644 --- a/web/src/authorities/authorities-page.tsx +++ b/web/src/authorities/authorities-page.tsx @@ -18,8 +18,9 @@ export function AuthoritiesPage() { const lang = i18n.language.startsWith("sv") ? "sv" : "en"; 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 [labels, setLabels] = useState([]); @@ -44,11 +45,13 @@ export function AuthoritiesPage() { return (
-
+
{KINDS.map((k) => ( `rounded px-3 py-1 text-sm ${isActive ? "bg-neutral-800 text-white" : "border"}` } @@ -59,7 +62,13 @@ export function AuthoritiesPage() {
    - {authorities?.length === 0 && ( + {isLoading && ( +
  • + )} + {isError && ( +
  • {t("authorities.loadError")}
  • + )} + {!isLoading && !isError && authorities?.length === 0 && (
  • {t("authorities.empty")}
  • )} {authorities?.map((a) => ( @@ -71,7 +80,7 @@ export function AuthoritiesPage() {
    - {t("authorities.new")} · {t(`authorities.${kind}`)} + {t("authorities.new")} · {t(`authorities.${currentKind}`)}
    diff --git a/web/src/authorities/authorities.test.tsx b/web/src/authorities/authorities.test.tsx index 58e49c3..67406a7 100644 --- a/web/src/authorities/authorities.test.tsx +++ b/web/src/authorities/authorities.test.tsx @@ -33,7 +33,13 @@ test("lists authorities for the kind and creates one", async () => { test("kind tabs link to the other kinds", async () => { 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 () => { @@ -51,6 +57,14 @@ test("create without EN label shows required alert and does not POST", async () 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 () => { renderApp(tree(), { route: "/authorities/banana" }); expect(await screen.findByText("Ada Lovelace")).toBeInTheDocument(); diff --git a/web/src/fields/fields.test.tsx b/web/src/fields/fields.test.tsx index 87fdceb..a3df0e8 100644 --- a/web/src/fields/fields.test.tsx +++ b/web/src/fields/fields.test.tsx @@ -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("")); }); +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 () => { let posted = false; diff --git a/web/src/i18n/en.json b/web/src/i18n/en.json index 9c27be1..0e61bc9 100644 --- a/web/src/i18n/en.json +++ b/web/src/i18n/en.json @@ -9,13 +9,13 @@ "actions": { "edit": "Edit", "delete": "Delete", "confirmDelete": "Delete this object? This cannot be undone." }, "labels": { "en": "Label (EN)", "sv": "Label (SV)", "externalUri": "External URI (optional)" }, "vocab": { - "title": "Vocabularies", "newVocabulary": "New vocabulary", "key": "Key", + "newVocabulary": "New vocabulary", "key": "Key", "create": "Create", "selectPrompt": "Select a vocabulary to manage its terms", "terms": "Terms", "addTerm": "Add term", "empty": "No vocabularies yet", "noTerms": "No terms yet", "loadError": "Could not load" }, "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" }, "search": { @@ -24,6 +24,7 @@ "prompt": "Type to search", "empty": "No results", "loadError": "Search is unavailable", + "unavailable": "Search is not available on this server", "loadMore": "Load more", "resultCount_one": "{{count}} result", "resultCount_other": "{{count}} results", diff --git a/web/src/i18n/sv.json b/web/src/i18n/sv.json index 833b5ff..844a80a 100644 --- a/web/src/i18n/sv.json +++ b/web/src/i18n/sv.json @@ -9,13 +9,13 @@ "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)" }, "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", "terms": "Termer", "addTerm": "Lägg till term", "empty": "Inga vokabulärer ännu", "noTerms": "Inga termer ännu", "loadError": "Kunde inte ladda" }, "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" }, "search": { @@ -24,6 +24,7 @@ "prompt": "Skriv för att söka", "empty": "Inga träffar", "loadError": "Sök är inte tillgängligt", + "unavailable": "Sök är inte tillgängligt på den här servern", "loadMore": "Visa fler", "resultCount_one": "{{count}} träff", "resultCount_other": "{{count}} träffar", diff --git a/web/src/search/search-panel.tsx b/web/src/search/search-panel.tsx index 701e9b5..3cf9de9 100644 --- a/web/src/search/search-panel.tsx +++ b/web/src/search/search-panel.tsx @@ -2,7 +2,7 @@ import { useEffect, useState } from "react"; import { useSearchParams } from "react-router-dom"; import { useTranslation } from "react-i18next"; -import { useSearch } from "../api/queries"; +import { useSearch, HttpError } from "../api/queries"; import { useDebouncedValue } from "../lib/use-debounced-value"; import { SearchResultRow } from "./search-result-row"; import { Button } from "@/components/ui/button"; @@ -92,7 +92,11 @@ export function SearchPanel() { )} {hasQuery && search.isError && ( -

    {t("search.loadError")}

    +

    + {search.error instanceof HttpError && search.error.status === 503 + ? t("search.unavailable") + : t("search.loadError")} +

    )} {hasQuery && !search.isLoading && !search.isError && hits.length === 0 && ( diff --git a/web/src/search/search.test.tsx b/web/src/search/search.test.tsx index 0edfb79..66ba609 100644 --- a/web/src/search/search.test.tsx +++ b/web/src/search/search.test.tsx @@ -76,6 +76,20 @@ test("clicking a result shows the object in the detail pane", async () => { 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 () => { renderApp(tree(), { route: "/search?q=bronze" }); diff --git a/web/src/vocab/vocabularies.test.tsx b/web/src/vocab/vocabularies.test.tsx index 9e7b30a..38e731c 100644 --- a/web/src/vocab/vocabularies.test.tsx +++ b/web/src/vocab/vocabularies.test.tsx @@ -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 () => { let posted = false; server.use( diff --git a/web/src/vocab/vocabulary-terms.tsx b/web/src/vocab/vocabulary-terms.tsx index f6cb86f..1d8d886 100644 --- a/web/src/vocab/vocabulary-terms.tsx +++ b/web/src/vocab/vocabulary-terms.tsx @@ -19,7 +19,7 @@ export function VocabularyTerms() { const lang = i18n.language.startsWith("sv") ? "sv" : "en"; - const { data: terms } = useTerms(id); + const { data: terms, isLoading, isError } = useTerms(id); const addTerm = useAddTerm(); @@ -53,7 +53,13 @@ export function VocabularyTerms() { {t("vocab.terms")}
      - {terms?.length === 0 && ( + {isLoading && ( +
    • + )} + {isError && ( +
    • {t("vocab.loadError")}
    • + )} + {!isLoading && !isError && terms?.length === 0 && (
    • {t("vocab.noTerms")}
    • )} {terms?.map((term) => (