marcel/familienarchiv
1
0
Fork 0
Code Issues 118 Pull Requests Actions Packages Projects Releases Wiki Activity

feat(geschichten): visible filter chip for the ?documentId list filter #803

New Issue
Closed
opened 2026-06-11 08:34:52 +02:00 by marcel · 1 comment
marcel commented 2026-06-11 08:34:52 +02:00
Owner
Copy Link

Context

PR #792 wired the headless ?documentId=<uuid> filter on GET /api/geschichten (commit 98e3d924): the document-detail drawer links to /geschichten?documentId=<uuid> to surface Lesereisen that reference a letter, and the list route forwards the param to GeschichteService.list(). The loader already returns the active filter to the page (+page.server.ts returns it as documentFilter — the raw UUID) — but no UI consumes it. A reader following the drawer link lands on a silently filtered list (Nielsen #1, visibility of system status).

This issue was originally mis-referenced as #797 in the PR #792 description (round-3 review, Elicit blocker 2 follow-up); #797 is the reading-sheet surface issue. This is the issue that actually owns the filter UI.

Implementation notes

Title resolution (server-side, in the existing loader). frontend/src/routes/geschichten/+page.server.ts already fans out with Promise.all (it resolves personFilters the same way). Add a conditional api.GET('/api/documents/{id}') into that same Promise.all when a valid documentId is present — documentId is known up front, so the title fetch runs in parallel with the list + person fetches and adds zero wall-clock latency. Do not introduce a serial await after the list resolves. Return a minimal resolved shape — documentFilter: { id, title } | null — not the raw UUID string and not the whole Document entity. Data flows server-side via props; never client-side fetch/onMount. Reusing GET /api/documents/{id} (confirmed at DocumentController.java:136) keeps the Geschichte domain free of Document-title concerns (no new backend endpoint, no denormalization).

Breaking PageData shape change (not purely additive). documentFilter goes from string | null to { id, title: string | null } | null. hasFilters (!!data.documentFilter) still works (a non-null object is truthy), and the chip renders on {#if data.documentFilter} (single optional region — no {#each} keying). The existing page.svelte.spec.ts:26 / page.svelte.test.ts:32 makeData() factory references documentFilter: null and ends with as unknown as PageData (so the type checker won't flag a stale shape — npm run check stays green either way). Update it to return a real { id, title } object in the same PR anyway, so the new empty-state/chip tests run against faithful data — the reason is runtime test fidelity, not the type gate.

Display fallback chain for the chip label: doc.title || doc.originalFilename || id.slice(0, 8). This matches the document detail page (documents/[id]/+page.svelte:272, which uses doc.title || doc.originalFilename). The short-UUID fallback covers both "document is gone" and "document has no title". Resolve the label in a $derived chipLabel inside DocumentFilterChip.svelte, not inline in markup. Do not copy the detail page's 'Dokument' literal third fallback — here the short-UUID is the deliberate third fallback because it doubles as the "document gone" label. Always validate, then slice — never slice an unvalidated string.

Fail closed and quietly on the title fetch. If GET /api/documents/{id} returns non-ok, the loader must not call getErrorMessage(...) or throw error() — degrade to { id, title: null } so the chip shows the short UUID. Treat 403 and 404 identically (don't reveal exists-but-forbidden vs. missing). Do not log the failed title fetch at WARN/ERROR, and never log the raw documentId query string — a gone/forbidden-document filter link would otherwise generate attacker-triggerable error-log noise (avoidable Loki noise); degrade silently. The list request GET /api/geschichten?documentId=… carries the user's auth cookie via createApiClient(fetch), so filtered results already respect access rules — the authenticated fetch is the authorization boundary; do not add a bypass.

Note on the no-oracle guarantee (current model). GET /api/documents/{id} (DocumentController.java:136-138) has no @RequirePermission and there is no per-document ACL — reads are gated globally by anyRequest().authenticated(). So there is no "exists-but-forbidden" state for a document today: any authenticated user who can load the Geschichten list can also read any document's metadata, and a per-document 403 is effectively unreachable. The identical 403==404 fail-closed handling is still implemented exactly as specced — it future-proofs the day per-document authorization lands — but it closes a theoretical, not a live, enumeration oracle.

UUID validation. Add a strict, fully-anchored isUuid() guard as a local const in +page.server.ts (no existing helper; do not create a validation/ package for one regex): /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/. The chip renders only for a syntactically valid UUID, and the fallback label must reflect only the parsed/validated UUID — never echo url.searchParams.get('documentId') verbatim into the DOM or the title= attribute. (Svelte mustache escapes by default, so this is input hygiene, not an XSS fix.)

Invalid-UUID behavior — decided (option B). The backend binds documentId as a UUID (GeschichteController.java:36), so a syntactically invalid value raises MethodArgumentTypeMismatchException, which GlobalExceptionHandler.handleTypeMismatch (GlobalExceptionHandler.java:51) maps to 400 VALIDATION_ERROR (no stack trace, no raw message). The loader's existing throw error(status, getErrorMessage(extractErrorCode(listResult.error))) already maps VALIDATION_ERROR to a localized string (errors.ts:181) — so an invalid documentId renders a useful, localized error page that leaks nothing. We keep this; we do not strip-and-ignore. Distinct cases:

  • Valid UUID, document exists → chip with resolved title.
  • Valid UUID, document gone/forbidden → list comes back empty; chip renders with the short UUID; empty state shows the Brief-specific message (below).
  • Syntactically invalid documentId → useful localized error page (no chip).

Empty state — decided (add a new message + person-wins precedence). Today the empty branch (+page.svelte:119-128) is a two-way {#if personFilters.length > 0} / {:else} with only geschichten_empty_for_persons and geschichten_empty_no_filter; a Brief-only filter that matches nothing falls through to "no filter", contradicting the visible chip. Add a geschichten_empty_for_document key (de/en/es, below) and restructure the empty-state selection. Do not grow the inline {#if} chain — extract a single $derived emptyMessage in the script that resolves precedence once, and let the template read the one value (keeps business logic out of markup; makes precedence directly testable). Precedence (decided):

  1. person filter active → geschichten_empty_for_persons (person-wins, even when a document filter is also active — the more specific multi-entity query)
  2. else document filter active → geschichten_empty_for_document
  3. else → geschichten_empty_no_filter

The combined case (both filters) is reachable only by manual URL or adding a person after the document-only drawer link, but the $derived resolves it deterministically. (Wire the $derived to the existing plural geschichten_empty_for_persons; an orphan singular geschichten_empty_for_person key exists at de.json:1033 but is pre-existing and out of scope — leave it alone.)

Remove button — decided. Removes only the documentId, preserving any active person filters. rebuildUrl() (+page.svelte:16-22) already deletes documentId on every rebuild, so the handler is goto(rebuildUrl(selectedPersonIds)) — consistent with the existing removePerson behavior. Mirror removePerson exactly; do not write a bespoke removeDocument that re-implements the same delete.

Component extraction. Extract a DocumentFilterChip.svelte (a ~25-line nameable region taking title: string | null, id: string, and an onremove callback) so the component spec can render it in isolation. The existing inline person chips (+page.svelte:78-91) do not need to move.

UX / chip design (domain term: "Brief")

The existing person chips are uppercase tracking-wider text-xs on bg-ink — that works for short names but must not be copied verbatim for a document title (mixed-case, often long, e.g. "Brief an die Großmutter, Weihnachten 1923").

  • Two-zone chip, min-h-11. Static prefix label "Gefiltert nach Brief:" in chrome style (font-sans text-xs uppercase tracking-wider); the title in normal case, font-serif italic, not uppercased. Use min-h-11 (not a fixed h-11) so a single-line chip baseline matches the "Alle" and "+ Person wählen" pills in the same flex row on sm+, while the chip is allowed to grow to two lines on mobile (see truncation below).
  • Responsive truncation — decided (option 1, clamp-on-mobile). Use line-clamp-2 sm:truncate sm:max-w-[16rem]: the title wraps to at most two lines on small screens (so the full title is readable on a phone — the read-path audience), and truncates to a single 16rem-capped line on sm+ where the title= hover tooltip works. Keep the full title in a title= attribute for the desktop hover. Rationale (decided): title= is hover-only and never fires for touch users, so a fixed truncate everywhere would make a long Brief title unrecoverable in place on phones; clamping costs one responsive class and closes the gap.
  • Announce the filter to assistive tech. Give the chip region aria-live="polite" (or ensure the surrounding filter region announces) so a screen-reader user who triggered the Brief filter from the drawer hears that it is now active — visibility of system status for AT.
  • Separate remove target. The × is its own <button> with min-h-[44px] min-w-[44px] (≥44px touch target, WCAG 2.2 §2.5.8 — critical for the 60+ audience), aria-label via geschichten_filter_remove_document_chip({ title }), and focus-visible:ring-2 focus-visible:ring-focus-ring (established pattern on the existing pills at +page.svelte:60,73). Don't bundle label+remove into one full-width button (misclick risk on the long label).
  • Redundant cues / contrast. Meaning reads from prefix text + the × affordance, not color alone. Reuse text-primary-fg on bg-ink (same AA-passing token pair as the person chips); don't introduce a new color.
  • Italic glyphs. font-serif is Tinos, which ships a true italic — confirm during proofshot it's a real italic, not synthesized oblique; if the italic serif reads too light on bg-ink at small size, fall back to non-italic serif (do not bold — bolding changes the semantic weight).
  • 320px proofshot must confirm all three, not just overflow: (1) no horizontal scroll with the chip wrapping to ≤2 lines + "Alle" pill + "+ Person wählen" wrapping in the flex flex-wrap gap-2 container (+page.svelte:68); (2) the Tinos italic renders as a true italic at small size; (3) on sm+ the single-line prefix+title zone baseline aligns to the sibling pills (min-h-11); on mobile a two-line wrapped chip may be taller than 44px, which is expected.

i18n

Add keys in messages/{de,en,es}.json, Sie-free register matching the existing geschichten_filter_* / geschichten_empty_* keys (messages/de.json:1028-1035); {title} interpolation matches the existing geschichten_filter_remove_chip({name}):

  • Prefix label geschichten_filter_document_chip: de Gefiltert nach Brief: · en Filtered by letter: · es Filtrado por carta:
  • Remove aria-label geschichten_filter_remove_document_chip({ title }): de Brief {title} aus Filter entfernen · en Remove letter {title} from filter · es Quitar la carta {title} del filtro
  • Empty state geschichten_empty_for_document: de Noch keine Geschichten zu diesem Brief · en No stories reference this letter yet · es Aún no hay historias sobre esta carta

Test strategy

Loader unit tests — there is currently no +page.server.test.ts for geschichten; this AC creates the first one. Import load directly, mock only the api client at the $lib/shared/api.server boundary (the persona-filter resolution is a working template — same module, same fan-out). Don't spin up a browser. The riskiest logic lives here:

  • resolves document title when documentId is a valid UUID and the document exists
  • 404 on the title fetch → await expect(load(...)).resolves.toMatchObject({ documentFilter: { id, title: null } }) (assert it resolves, never rejects — explicit .resolves, not a loose "does not throw")
  • null document title → falls back to originalFilename
  • 403 on the title fetch treated identically to 404 (no error page, short-UUID fallback) — permanent regression test: assert the loader does not call getErrorMessage/throw on a forbidden title fetch, locking the no-oracle guarantee
  • syntactically invalid documentId → list request surfaces 400 VALIDATION_ERROR as a thrown localized error (option B)
  • mock the list call and the title call independently (two separate mockApi.GET resolutions keyed by path) so a title-fetch failure provably does not corrupt the geschichten list result — assert the list array is still populated when the title fetch returns 404

Empty-state precedence tests (component or page level) — the restructured $derived emptyMessage is new behavior; lock every branch. Prefer asserting the rendered message (getByText) over the $derived value directly:

  • document filter active, zero results → geschichten_empty_for_document
  • person filter active, zero results → geschichten_empty_for_persons
  • no filter, zero results → geschichten_empty_no_filter
  • both filters active, zero results → geschichten_empty_for_persons (person-wins)
  • non-empty case: valid UUID + list returns results → the chip renders and the cards render (the empty-state branch is not shown) — guards against the chip accidentally suppressing results

Component spec (DocumentFilterChip.svelte.spec.ts, vitest-browser-svelte, vi.mock('$app/navigation')):

  • renders the resolved title inside the chip
  • renders nothing when no document filter is set (documentFilter: null → chip absent)
  • calls onremove when the remove button is clicked — drive via getByRole('button', { name: <aria-label> }) + click; assert the spy fired once, not internal state
  • the rendered title= attribute equals the validated value, not a raw query string (reflected-input hygiene survives refactors)

Run targeted files locally only (--project=client for the .svelte.spec.ts, node for the loader test); leave the full sweep to CI. No E2E needed (presentation detail on an existing journey).

Acceptance criteria

  • When /geschichten?documentId=<uuid> is active and the document resolves, the list shows a visible filter chip above the results: "Gefiltert nach Brief: ", title resolved server-side via GET /api/documents/{id}.
  • Fallback chain for the label is title || originalFilename || short-UUID; a gone/forbidden document (404/403) degrades quietly to the short UUID — no thrown error, no leaked backend message, no WARN/ERROR log of the raw query string; 403 and 404 are indistinguishable in the UI.
  • The chip's remove button (its own ≥44px-touch-target <button>, with aria-label and visible focus ring) removes only documentId and preserves active person filters (goto(rebuildUrl(selectedPersonIds))).
  • The chip renders only for a syntactically valid, fully-anchored UUID; the label (and title= attribute) reflects only the parsed UUID, never the raw query string.
  • A syntactically invalid documentId renders an error page with a useful, localized message (option B — not stripped/ignored).
  • Empty-state message resolves via a single $derived with person-wins precedence: person filter (even if a document filter is also active) → geschichten_empty_for_persons; else document filter → geschichten_empty_for_document; else → geschichten_empty_no_filter. All four branches plus the non-empty case are test-locked.
  • Title shown in normal case (font-serif italic); responsive truncation line-clamp-2 sm:truncate sm:max-w-[16rem] (wraps to ≤2 lines on mobile so the full title is readable on a phone; single truncated line with title= hover on sm+); prefix label in chrome style; chip min-h-11 (single-line baseline aligns to sibling pills on sm+, may grow to two lines on mobile); chip region has aria-live="polite"; no horizontal overflow at 320px (verified by proofshot covering 2-line wrap + italic + baseline).
  • i18n keys geschichten_filter_document_chip, geschichten_filter_remove_document_chip, and geschichten_empty_for_document added in de/en/es, Sie-free register.
  • DocumentFilterChip.svelte extracted; component spec: chip renders with filter, absent without, remove fires onremove, and title= equals the validated value.
  • New +page.server.test.ts covers: resolve, 404 fallback (.resolves), null-title fallback, 403==404 no-oracle, invalid-UUID error, independent list/title mocks.
  • Existing page.svelte.spec.ts:26 / page.svelte.test.ts:32 makeData() updated to return a real documentFilter object shape (same PR) for runtime test fidelity.

Out of scope / non-impacts

  • No Flyway migration, no new entity, no new route (DocumentFilterChip.svelte is a component under the existing /geschichten route, not a new +page.svelte), no new ErrorCode/Permission, no backend controller/service change → no PlantUML or CLAUDE.md doc-currency triggers fire, no ADR. ("Brief" is an existing document-domain term — no new glossary entry.)
  • No infrastructure impact (frontend-only; one extra same-network GET /api/documents/{id} only on filtered loads, parallel in the existing Promise.all — zero added latency).

Refs

  • PR #792 (98e3d924 — backend + loader), frontend/src/routes/geschichten/+page.server.ts (active filter already in data as documentFilter), document-detail drawer link (DocumentMetadataDrawer.svelte:246).
  • Round-3 review: Elicit blocker-2 follow-up (comment 16259).
  • Six-persona review folded in (2026-06-12): architecture/security/test-strategy/infra confirmed sound with no open decisions; the one live tradeoff — mobile truncation — resolved to option 1 (clamp-on-mobile) above.
## Context PR #792 wired the headless `?documentId=<uuid>` filter on `GET /api/geschichten` (commit 98e3d924): the document-detail drawer links to `/geschichten?documentId=<uuid>` to surface Lesereisen that reference a letter, and the list route forwards the param to `GeschichteService.list()`. The loader already returns the active filter to the page (`+page.server.ts` returns it as **`documentFilter`** — the raw UUID) — **but no UI consumes it**. A reader following the drawer link lands on a silently filtered list (Nielsen #1, visibility of system status). This issue was originally mis-referenced as #797 in the PR #792 description (round-3 review, Elicit blocker 2 follow-up); #797 is the reading-sheet surface issue. This is the issue that actually owns the filter UI. ## Implementation notes **Title resolution (server-side, in the existing loader).** `frontend/src/routes/geschichten/+page.server.ts` already fans out with `Promise.all` (it resolves `personFilters` the same way). Add a conditional `api.GET('/api/documents/{id}')` into that **same** `Promise.all` when a valid `documentId` is present — `documentId` is known up front, so the title fetch runs in parallel with the list + person fetches and adds **zero wall-clock latency**. Do **not** introduce a serial `await` after the list resolves. Return a minimal resolved shape — `documentFilter: { id, title } | null` — **not** the raw UUID string and not the whole `Document` entity. Data flows server-side via props; never client-side `fetch`/`onMount`. Reusing `GET /api/documents/{id}` (confirmed at `DocumentController.java:136`) keeps the Geschichte domain free of Document-title concerns (no new backend endpoint, no denormalization). **Breaking `PageData` shape change (not purely additive).** `documentFilter` goes from `string | null` to `{ id, title: string | null } | null`. `hasFilters` (`!!data.documentFilter`) still works (a non-null object is truthy), and the chip renders on `{#if data.documentFilter}` (single optional region — no `{#each}` keying). The existing `page.svelte.spec.ts:26` / `page.svelte.test.ts:32` `makeData()` factory references `documentFilter: null` and ends with `as unknown as PageData` (so the type checker won't flag a stale shape — `npm run check` stays green either way). Update it to return a real `{ id, title }` object **in the same PR** anyway, so the new empty-state/chip tests run against faithful data — the reason is runtime test fidelity, not the type gate. **Display fallback chain** for the chip label: `doc.title || doc.originalFilename || id.slice(0, 8)`. This matches the document detail page (`documents/[id]/+page.svelte:272`, which uses `doc.title || doc.originalFilename`). The short-UUID fallback covers both "document is gone" and "document has no title". Resolve the label in a `$derived chipLabel` inside `DocumentFilterChip.svelte`, **not inline in markup**. Do **not** copy the detail page's `'Dokument'` literal third fallback — here the short-UUID is the deliberate third fallback because it doubles as the "document gone" label. Always validate, *then* slice — never slice an unvalidated string. **Fail closed and quietly on the title fetch.** If `GET /api/documents/{id}` returns non-ok, the loader must **not** call `getErrorMessage(...)` or throw `error()` — degrade to `{ id, title: null }` so the chip shows the short UUID. Treat **403 and 404 identically** (don't reveal exists-but-forbidden vs. missing). Do **not** log the failed title fetch at WARN/ERROR, and never log the raw `documentId` query string — a gone/forbidden-document filter link would otherwise generate attacker-triggerable error-log noise (avoidable Loki noise); degrade silently. The list request `GET /api/geschichten?documentId=…` carries the user's auth cookie via `createApiClient(fetch)`, so filtered results already respect access rules — the authenticated fetch is the authorization boundary; do not add a bypass. > **Note on the no-oracle guarantee (current model).** `GET /api/documents/{id}` (`DocumentController.java:136-138`) has **no** `@RequirePermission` and there is no per-document ACL — reads are gated globally by `anyRequest().authenticated()`. So there is no "exists-but-forbidden" state for a document today: any authenticated user who can load the Geschichten list can also read any document's metadata, and a per-document 403 is effectively unreachable. The identical 403==404 fail-closed handling is still implemented exactly as specced — it future-proofs the day per-document authorization lands — but it closes a theoretical, not a live, enumeration oracle. **UUID validation.** Add a strict, fully-anchored `isUuid()` guard as a local const in `+page.server.ts` (no existing helper; do **not** create a `validation/` package for one regex): `/^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/`. The chip renders **only** for a syntactically valid UUID, and the fallback label must reflect only the **parsed/validated** UUID — never echo `url.searchParams.get('documentId')` verbatim into the DOM or the `title=` attribute. (Svelte mustache escapes by default, so this is input hygiene, not an XSS fix.) **Invalid-UUID behavior — decided (option B).** The backend binds `documentId` as a `UUID` (`GeschichteController.java:36`), so a syntactically invalid value raises `MethodArgumentTypeMismatchException`, which `GlobalExceptionHandler.handleTypeMismatch` (`GlobalExceptionHandler.java:51`) maps to **400 `VALIDATION_ERROR`** (no stack trace, no raw message). The loader's existing `throw error(status, getErrorMessage(extractErrorCode(listResult.error)))` already maps `VALIDATION_ERROR` to a localized string (`errors.ts:181`) — so an invalid `documentId` renders a **useful, localized error page that leaks nothing**. We keep this; we do **not** strip-and-ignore. Distinct cases: - *Valid UUID, document exists* → chip with resolved title. - *Valid UUID, document gone/forbidden* → list comes back empty; chip renders with the short UUID; empty state shows the Brief-specific message (below). - *Syntactically invalid `documentId`* → useful localized error page (no chip). **Empty state — decided (add a new message + person-wins precedence).** Today the empty branch (`+page.svelte:119-128`) is a two-way `{#if personFilters.length > 0}` / `{:else}` with only `geschichten_empty_for_persons` and `geschichten_empty_no_filter`; a Brief-only filter that matches nothing falls through to "no filter", contradicting the visible chip. Add a `geschichten_empty_for_document` key (de/en/es, below) and restructure the empty-state selection. **Do not grow the inline `{#if}` chain** — extract a single `$derived emptyMessage` in the script that resolves precedence once, and let the template read the one value (keeps business logic out of markup; makes precedence directly testable). Precedence (decided): 1. person filter active → `geschichten_empty_for_persons` (**person-wins, even when a document filter is also active** — the more specific multi-entity query) 2. else document filter active → `geschichten_empty_for_document` 3. else → `geschichten_empty_no_filter` The combined case (both filters) is reachable only by manual URL or adding a person after the document-only drawer link, but the `$derived` resolves it deterministically. (Wire the `$derived` to the existing **plural** `geschichten_empty_for_persons`; an orphan singular `geschichten_empty_for_person` key exists at `de.json:1033` but is pre-existing and out of scope — leave it alone.) **Remove button — decided.** Removes **only** the `documentId`, preserving any active person filters. `rebuildUrl()` (`+page.svelte:16-22`) already deletes `documentId` on every rebuild, so the handler is `goto(rebuildUrl(selectedPersonIds))` — consistent with the existing `removePerson` behavior. Mirror `removePerson` exactly; do not write a bespoke `removeDocument` that re-implements the same delete. **Component extraction.** Extract a `DocumentFilterChip.svelte` (a ~25-line nameable region taking `title: string | null`, `id: string`, and an `onremove` callback) so the component spec can render it in isolation. The existing inline person chips (`+page.svelte:78-91`) do not need to move. ## UX / chip design (domain term: "Brief") The existing person chips are `uppercase tracking-wider text-xs` on `bg-ink` — that works for short names but **must not** be copied verbatim for a document title (mixed-case, often long, e.g. "Brief an die Großmutter, Weihnachten 1923"). - **Two-zone chip, `min-h-11`.** Static prefix label "Gefiltert nach Brief:" in chrome style (`font-sans text-xs uppercase tracking-wider`); the **title in normal case**, `font-serif italic`, **not** uppercased. Use `min-h-11` (not a fixed `h-11`) so a single-line chip baseline matches the "Alle" and "+ Person wählen" pills in the same flex row on `sm`+, while the chip is allowed to grow to two lines on mobile (see truncation below). - **Responsive truncation — decided (option 1, clamp-on-mobile).** Use `line-clamp-2 sm:truncate sm:max-w-[16rem]`: the title wraps to **at most two lines on small screens** (so the full title is readable on a phone — the read-path audience), and truncates to a single `16rem`-capped line on `sm`+ where the `title=` hover tooltip works. Keep the full title in a `title=` attribute for the desktop hover. **Rationale (decided):** `title=` is hover-only and never fires for touch users, so a fixed `truncate` everywhere would make a long Brief title unrecoverable in place on phones; clamping costs one responsive class and closes the gap. - **Announce the filter to assistive tech.** Give the chip region `aria-live="polite"` (or ensure the surrounding filter region announces) so a screen-reader user who triggered the Brief filter from the drawer hears that it is now active — visibility of system status for AT. - **Separate remove target.** The `×` is its **own** `<button>` with `min-h-[44px] min-w-[44px]` (≥44px touch target, WCAG 2.2 §2.5.8 — critical for the 60+ audience), `aria-label` via `geschichten_filter_remove_document_chip({ title })`, and `focus-visible:ring-2 focus-visible:ring-focus-ring` (established pattern on the existing pills at `+page.svelte:60,73`). Don't bundle label+remove into one full-width button (misclick risk on the long label). - **Redundant cues / contrast.** Meaning reads from prefix text + the × affordance, not color alone. Reuse `text-primary-fg` on `bg-ink` (same AA-passing token pair as the person chips); don't introduce a new color. - **Italic glyphs.** `font-serif` is Tinos, which ships a true italic — confirm during proofshot it's a real italic, not synthesized oblique; if the italic serif reads too light on `bg-ink` at small size, fall back to non-italic serif (do **not** bold — bolding changes the semantic weight). - **320px proofshot must confirm all three**, not just overflow: (1) no horizontal scroll with the chip wrapping to ≤2 lines + "Alle" pill + "+ Person wählen" wrapping in the `flex flex-wrap gap-2` container (`+page.svelte:68`); (2) the Tinos italic renders as a true italic at small size; (3) on `sm`+ the single-line prefix+title zone baseline aligns to the sibling pills (`min-h-11`); on mobile a two-line wrapped chip may be taller than 44px, which is expected. ## i18n Add keys in `messages/{de,en,es}.json`, Sie-free register matching the existing `geschichten_filter_*` / `geschichten_empty_*` keys (`messages/de.json:1028-1035`); `{title}` interpolation matches the existing `geschichten_filter_remove_chip({name})`: - Prefix label `geschichten_filter_document_chip`: de `Gefiltert nach Brief:` · en `Filtered by letter:` · es `Filtrado por carta:` - Remove aria-label `geschichten_filter_remove_document_chip({ title })`: de `Brief {title} aus Filter entfernen` · en `Remove letter {title} from filter` · es `Quitar la carta {title} del filtro` - Empty state `geschichten_empty_for_document`: de `Noch keine Geschichten zu diesem Brief` · en `No stories reference this letter yet` · es `Aún no hay historias sobre esta carta` ## Test strategy **Loader unit tests** — there is currently **no** `+page.server.test.ts` for `geschichten`; this AC creates the first one. Import `load` directly, mock only the api client at the `$lib/shared/api.server` boundary (the persona-filter resolution is a working template — same module, same fan-out). Don't spin up a browser. The riskiest logic lives here: - resolves document title when `documentId` is a valid UUID and the document exists - 404 on the title fetch → `await expect(load(...)).resolves.toMatchObject({ documentFilter: { id, title: null } })` (assert it **resolves**, never rejects — explicit `.resolves`, not a loose "does not throw") - null document title → falls back to `originalFilename` - 403 on the title fetch treated identically to 404 (no error page, short-UUID fallback) — **permanent regression test: assert the loader does not call `getErrorMessage`/`throw` on a forbidden title fetch**, locking the no-oracle guarantee - syntactically invalid `documentId` → list request surfaces 400 `VALIDATION_ERROR` as a thrown localized error (option B) - mock the list call and the title call **independently** (two separate `mockApi.GET` resolutions keyed by path) so a title-fetch failure provably does not corrupt the `geschichten` list result — assert the list array is still populated when the title fetch returns 404 **Empty-state precedence tests** (component or page level) — the restructured `$derived emptyMessage` is new behavior; lock every branch. Prefer asserting the **rendered message** (`getByText`) over the `$derived` value directly: - document filter active, zero results → `geschichten_empty_for_document` - person filter active, zero results → `geschichten_empty_for_persons` - no filter, zero results → `geschichten_empty_no_filter` - **both filters active, zero results → `geschichten_empty_for_persons` (person-wins)** - **non-empty case:** valid UUID + list returns results → the chip renders *and* the cards render (the empty-state branch is **not** shown) — guards against the chip accidentally suppressing results **Component spec** (`DocumentFilterChip.svelte.spec.ts`, `vitest-browser-svelte`, `vi.mock('$app/navigation')`): - renders the resolved title inside the chip - renders nothing when no document filter is set (`documentFilter: null` → chip absent) - calls `onremove` when the remove button is clicked — drive via `getByRole('button', { name: <aria-label> })` + click; assert the spy fired once, not internal state - **the rendered `title=` attribute equals the validated value, not a raw query string** (reflected-input hygiene survives refactors) Run targeted files locally only (`--project=client` for the `.svelte.spec.ts`, node for the loader test); leave the full sweep to CI. No E2E needed (presentation detail on an existing journey). ## Acceptance criteria - [ ] When `/geschichten?documentId=<uuid>` is active and the document resolves, the list shows a visible filter chip above the results: "Gefiltert nach Brief: _<Dokumenttitel>_", title resolved server-side via `GET /api/documents/{id}`. - [ ] Fallback chain for the label is `title || originalFilename || short-UUID`; a gone/forbidden document (404/403) degrades quietly to the short UUID — no thrown error, no leaked backend message, no WARN/ERROR log of the raw query string; 403 and 404 are indistinguishable in the UI. - [ ] The chip's remove button (its own ≥44px-touch-target `<button>`, with `aria-label` and visible focus ring) removes **only** `documentId` and preserves active person filters (`goto(rebuildUrl(selectedPersonIds))`). - [ ] The chip renders only for a syntactically valid, fully-anchored UUID; the label (and `title=` attribute) reflects only the parsed UUID, never the raw query string. - [ ] A syntactically invalid `documentId` renders an error page with a useful, localized message (option B — not stripped/ignored). - [ ] Empty-state message resolves via a single `$derived` with **person-wins precedence**: person filter (even if a document filter is also active) → `geschichten_empty_for_persons`; else document filter → `geschichten_empty_for_document`; else → `geschichten_empty_no_filter`. All four branches plus the non-empty case are test-locked. - [ ] Title shown in normal case (`font-serif italic`); **responsive truncation `line-clamp-2 sm:truncate sm:max-w-[16rem]`** (wraps to ≤2 lines on mobile so the full title is readable on a phone; single truncated line with `title=` hover on `sm`+); prefix label in chrome style; chip `min-h-11` (single-line baseline aligns to sibling pills on `sm`+, may grow to two lines on mobile); chip region has `aria-live="polite"`; no horizontal overflow at 320px (verified by proofshot covering 2-line wrap + italic + baseline). - [ ] i18n keys `geschichten_filter_document_chip`, `geschichten_filter_remove_document_chip`, and `geschichten_empty_for_document` added in de/en/es, Sie-free register. - [ ] `DocumentFilterChip.svelte` extracted; component spec: chip renders with filter, absent without, remove fires `onremove`, and `title=` equals the validated value. - [ ] New `+page.server.test.ts` covers: resolve, 404 fallback (`.resolves`), null-title fallback, 403==404 no-oracle, invalid-UUID error, independent list/title mocks. - [ ] Existing `page.svelte.spec.ts:26` / `page.svelte.test.ts:32` `makeData()` updated to return a real `documentFilter` object shape (same PR) for runtime test fidelity. ## Out of scope / non-impacts - No Flyway migration, no new entity, no new route (`DocumentFilterChip.svelte` is a component under the existing `/geschichten` route, not a new `+page.svelte`), no new `ErrorCode`/`Permission`, no backend controller/service change → **no PlantUML or `CLAUDE.md` doc-currency triggers fire**, no ADR. ("Brief" is an existing document-domain term — no new glossary entry.) - No infrastructure impact (frontend-only; one extra same-network `GET /api/documents/{id}` only on filtered loads, parallel in the existing `Promise.all` — zero added latency). ## Refs - PR #792 (`98e3d924` — backend + loader), `frontend/src/routes/geschichten/+page.server.ts` (active filter already in `data` as `documentFilter`), document-detail drawer link (`DocumentMetadataDrawer.svelte:246`). - Round-3 review: Elicit blocker-2 follow-up (comment 16259). - Six-persona review folded in (2026-06-12): architecture/security/test-strategy/infra confirmed sound with no open decisions; the one live tradeoff — mobile truncation — resolved to option 1 (clamp-on-mobile) above.
marcel added the P2-mediumfeatureui labels 2026-06-11 08:35:02 +02:00
marcel commented 2026-06-12 09:28:45 +02:00
Author
Owner
Copy Link

Implementation complete ✅

Branch: feat/issue-803-geschichten-document-filter-chip

Commits

  • 5a87e4d6 i18n keys — added geschichten_filter_document_chip, geschichten_filter_remove_document_chip, geschichten_empty_for_document to de/en/es
  • ba2481ae Loader — UUID-validates ?documentId=, fetches title from GET /api/documents/{id} in parallel with persons fetch, returns { id, title } | null; 403/404 fail-closed to { id, title: null }; invalid UUID still reaches the list API (→ backend 400, option B)
  • 4d8b8f15 DocumentFilterChip.svelte — two-zone chip with prefix label, font-serif italic title, line-clamp-2 sm:truncate, ≥44px touch target remove button, aria-live="polite"; 7 browser specs
  • b2b3eb0b +page.svelte integration — adds chip to filter bar, $derived.by() emptyMessage with person-wins precedence, removeDocument navigation helper; 16 browser specs + 11 server specs all green

Test coverage (34 tests, all passing)

  • page.server.test.ts (11): title resolution, null-title fallback, 403/404 fail-closed, invalid-UUID gating, option-B passthrough
  • DocumentFilterChip.svelte.spec.ts (7): rendering, fallback to short UUID, remove callback, aria-label, touch target
  • page.svelte.spec.ts (16): person chips, All-pill state, document chip show/hide, chip removal navigation, empty-state precedence (person-wins)
## Implementation complete ✅ Branch: `feat/issue-803-geschichten-document-filter-chip` ### Commits - `5a87e4d6` **i18n keys** — added `geschichten_filter_document_chip`, `geschichten_filter_remove_document_chip`, `geschichten_empty_for_document` to de/en/es - `ba2481ae` **Loader** — UUID-validates `?documentId=`, fetches title from `GET /api/documents/{id}` in parallel with persons fetch, returns `{ id, title } | null`; 403/404 fail-closed to `{ id, title: null }`; invalid UUID still reaches the list API (→ backend 400, option B) - `4d8b8f15` **DocumentFilterChip.svelte** — two-zone chip with prefix label, `font-serif italic` title, `line-clamp-2 sm:truncate`, `≥44px` touch target remove button, `aria-live="polite"`; 7 browser specs - `b2b3eb0b` **+page.svelte integration** — adds chip to filter bar, `$derived.by()` `emptyMessage` with person-wins precedence, `removeDocument` navigation helper; 16 browser specs + 11 server specs all green ### Test coverage (34 tests, all passing) - `page.server.test.ts` (11): title resolution, null-title fallback, 403/404 fail-closed, invalid-UUID gating, option-B passthrough - `DocumentFilterChip.svelte.spec.ts` (7): rendering, fallback to short UUID, remove callback, aria-label, touch target - `page.svelte.spec.ts` (16): person chips, All-pill state, document chip show/hide, chip removal navigation, empty-state precedence (person-wins)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
P0-critical
Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.
 P1-high
Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.
 P2-medium
Noticeable improvement; doesn't block anything; schedule opportunistically.
 P3-later
Cosmetic or parking-lot work; fine to stay open indefinitely.
 audit
Read-only audit / assessment work; produces a report rather than changing code
 bug
Something isn't working
 cleanup
Removal of dead code, vague comments, naming violations, and scratch artifacts
 collaboration
We want to extend the family archive to have more collaboration tools
 conversation
descoped
We will do that later
 devops
documentation
README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs
 epic
Marker for epic-level issues that group multiple child issues
 feature
file-upload
legibility
Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction
 needs-discussion
Has an open decision or design question that must be resolved before implementation can start.
 notification
person
phase-1: security
Security hygiene — must be done first
 phase-2: container-images
Production-ready multi-stage Docker images
 phase-3: prod-compose
Production compose overlay + Caddy reverse proxy
 phase-4: spring-prod-profile
Spring Boot production configuration profile
 phase-5: backups
Database and object storage backup strategy
 phase-6: deployment-docs
.env.example, DEPLOYMENT.md, runbook
 phase-7: monitoring
Prometheus, Loki, Grafana, Alertmanager
 refactor
Code restructuring without behaviour change
 security
test
ui
UI/UX and accessibility issues
 user
No Label P2-medium feature ui
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Jens marcel
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#803
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?