Compare commits

..

16 Commits

Author SHA1 Message Date
Marcel
3a56cc4e87 fix(document): add trainingLabels to Document.full entity graph (#642)
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m30s
CI / OCR Service Tests (pull_request) Successful in 19s
CI / Backend Unit Tests (pull_request) Successful in 3m22s
CI / fail2ban Regex (pull_request) Successful in 52s
CI / Semgrep Security Scan (pull_request) Successful in 20s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m0s
trainingLabels was switched to LAZY fetch in #467 but not added to the
Document.full @NamedEntityGraph. DocumentRepository.findById() uses
Document.full to eagerly load sender/receivers/tags, but the Hibernate
session closes before Jackson serializes the response. Accessing
trainingLabels outside the session throws LazyInitializationException,
causing GET /api/documents/{id} to return HTTP 500.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-20 08:16:19 +02:00
Marcel
0c0a4830cd ux(transcription): bump dismiss button icon from red-500 to red-600
All checks were successful
nightly / deploy-staging (push) Successful in 4m32s
CI / Unit & Component Tests (pull_request) Successful in 3m19s
CI / OCR Service Tests (pull_request) Successful in 21s
CI / Backend Unit Tests (pull_request) Successful in 3m27s
CI / fail2ban Regex (pull_request) Successful in 41s
CI / Semgrep Security Scan (pull_request) Successful in 20s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
CI / Unit & Component Tests (push) Successful in 3m30s
CI / OCR Service Tests (push) Successful in 19s
CI / Backend Unit Tests (push) Successful in 3m20s
CI / fail2ban Regex (push) Successful in 41s
CI / Semgrep Security Scan (push) Successful in 18s
CI / Compose Bucket Idempotency (push) Successful in 58s
text-red-500 on bg-red-50 gives ~3.8:1 contrast (passes AA for UI
components at 3:1 but leaves no margin). text-red-600 gives ~5.0:1,
comfortably above the AA threshold with no visual downgrade.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 21:32:47 +02:00
Marcel
dd843d76c2 a11y(transcription): remove redundant aria-live="polite" from alert div
role="alert" already implies aria-live="assertive". The polite override
caused screen readers to wait for the current announcement to finish
before reading the error — too gentle for a failure state the user just
triggered. Dropping the attribute restores the implicit assertive
behaviour.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 21:31:57 +02:00
Marcel
9601974db0 ux(transcription): bump error banner font size to text-sm for readability
text-xs (12px) is at the lower bound for the 60+ transcriber cohort.
text-sm (14px) matches the visual weight of the progress counter label
above and is more comfortable to read under stress (failed operation).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 21:30:54 +02:00
Marcel
1782526c99 test(transcription): gate second click on button re-enabled to fix race
Adds an await for the button to become non-disabled between the two
dispatchEvent calls in 'clears error on next successful call'. This
ensures the first async rejection has fully settled and Svelte has
flushed markingAllReviewed before the second click fires.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 21:29:31 +02:00
Marcel
76ef54e064 test(transcription): cover non-JSON fallback in markAllReviewed error path
Adds a test for when the server returns a non-JSON body (e.g. an nginx
502 HTML page). Confirms the res.json().catch(() => ({})) fallback
produces 'INTERNAL_ERROR' as the thrown message and leaves blocks intact.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 21:28:39 +02:00
Marcel
f1d1ac3f1a test(transcription): assert error banner shows domain-specific message
Adds toHaveTextContent(m.transcription_mark_all_reviewed_error()) to the
error-present test. The previous check only asserted presence via
role="alert", which would not have caught the dead key bug — the banner
was showing the generic fallback rather than the operation-specific copy.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 21:27:29 +02:00
Marcel
0f48ffede5 fix(transcription): use domain-specific message in markAllReviewed catch
Removes the getErrorMessage() indirection and calls
m.transcription_mark_all_reviewed_error() directly in the catch block.
The previous implementation routed through getErrorMessage(code) which
mapped any error code to the generic m.error_internal_error() fallback,
leaving the domain-specific key unreachable.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 21:23:59 +02:00
Marcel
3e72157ee1 test(transcription): update markAllReviewed non-OK test to expect throw
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m14s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m22s
CI / fail2ban Regex (pull_request) Successful in 41s
CI / Semgrep Security Scan (pull_request) Successful in 18s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s
The function now throws instead of silently returning on failure.
Update the test name and assertion to match the new behaviour, and
verify blocks remain unchanged after the error.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 20:43:21 +02:00
Marcel
e2d3975524 test(transcription): replace hardcoded regex with m.* calls in mark-all spec
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 20:40:28 +02:00
Marcel
59e99f862a fix(i18n): wire TranscriptionEditView mark-all button through Paraglide
Replace hardcoded German strings with m.transcription_mark_all_reviewed()
and m.transcription_mark_all_reviewed_disabled().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 20:39:39 +02:00
Marcel
bb39ca59ec feat(i18n): add transcription_mark_all_reviewed and _disabled message keys
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 20:39:06 +02:00
Marcel
6b53cbfc5b feat(transcription): show dismissible error banner when markAllReviewed fails
Adds markAllError state and catch block to handleMarkAllReviewed.
Error banner renders below the review progress bar with role="alert"
and aria-live="polite" for screen reader announcement. Dismiss button
clears the error; next successful call also clears it automatically.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 20:38:28 +02:00
Marcel
e3e8373526 fix(transcription): throw error from markAllReviewed() on non-2xx response
Previously the function silently returned on failure, leaving no way
for callers to detect or surface the error to the user.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 20:37:21 +02:00
Marcel
907a6a6b53 feat(i18n): add transcription_mark_all_reviewed_error message key
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 20:36:44 +02:00
Marcel
f27e2d33a5 test(transcription): add failing tests for markAllReviewed error display
RED phase: 4 new Vitest browser tests that fail because the error
banner and catch block don't exist yet.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-19 20:35:56 +02:00
21 changed files with 203 additions and 1007 deletions

View File

@@ -25,7 +25,8 @@ import java.util.UUID;
@NamedEntityGraph(name = "Document.full", attributeNodes = {
@NamedAttributeNode("sender"),
@NamedAttributeNode("receivers"),
@NamedAttributeNode("tags")
@NamedAttributeNode("tags"),
@NamedAttributeNode("trainingLabels")
})
@NamedEntityGraph(name = "Document.list", attributeNodes = {
@NamedAttributeNode("sender"),

View File

@@ -106,31 +106,6 @@ export default defineConfig(
]
}
},
{
// Forbid test fixtures (*.test-fixture.svelte) from being imported by
// production code. Tree-shaking keeps them out of the production bundle
// today (no route reaches them), but a lint rule makes the boundary
// explicit so an accidental autocomplete import in a route or component
// fails fast. Test files (*.spec.ts / *.test.ts) and the fixtures
// themselves are exempt — see the next block. Nora #2 on PR #629
// round 3.
files: ['**/*.svelte', '**/*.svelte.ts', '**/*.svelte.js', '**/*.ts'],
ignores: ['**/*.spec.ts', '**/*.test.ts', '**/*.test-fixture.svelte'],
rules: {
'no-restricted-imports': [
'error',
{
patterns: [
{
group: ['**/*.test-fixture.svelte'],
message:
'Test fixtures (*.test-fixture.svelte) are test-only — do not import from production code. Tracked by #637.'
}
]
}
]
}
},
{
plugins: { boundaries },
settings: {

View File

@@ -445,12 +445,8 @@
"person_mention_load_error": "Person konnte nicht geladen werden.",
"person_mention_loading": "Lade Person…",
"person_mention_popup_empty": "Keine Personen gefunden",
"person_mention_search_label": "Person suchen",
"person_mention_search_prompt": "Namen eingeben…",
"person_mention_btn_label": "Person verlinken",
"person_mention_create_new": "Neue Person anlegen",
"person_mention_results_count_singular": "1 Person gefunden",
"person_mention_results_count_plural": "{count} Personen gefunden",
"transcription_editor_aria_label": "Transkriptionstext",
"person_born_name_prefix": "geb.",
"page_title_home": "Archiv",
@@ -637,6 +633,9 @@
"transcription_block_review": "Als geprüft markieren",
"transcription_block_unreview": "Markierung aufheben",
"transcription_reviewed_count": "{reviewed} von {total} geprüft",
"transcription_mark_all_reviewed": "Alle als fertig markieren",
"transcription_mark_all_reviewed_disabled": "Alle Blöcke sind bereits als fertig markiert",
"transcription_mark_all_reviewed_error": "Markierung fehlgeschlagen. Bitte versuchen Sie es erneut.",
"training_ocr_heading": "Kurrent-Erkennung trainieren",
"training_ocr_description": "Starte ein neues Training mit den bisher geprüften OCR-Blöcken, um die Erkennungsgenauigkeit für Kurrentschrift zu verbessern.",
"training_ocr_blocks_ready": "{blocks} geprüfte Blöcke bereit / {docs} Dokumente",

View File

@@ -445,12 +445,8 @@
"person_mention_load_error": "Could not load person.",
"person_mention_loading": "Loading person…",
"person_mention_popup_empty": "No persons found",
"person_mention_search_label": "Search for a person",
"person_mention_search_prompt": "Enter a name…",
"person_mention_btn_label": "Link person",
"person_mention_create_new": "Create new person",
"person_mention_results_count_singular": "1 person found",
"person_mention_results_count_plural": "{count} persons found",
"transcription_editor_aria_label": "Transcription text",
"person_born_name_prefix": "née",
"page_title_home": "Archive",
@@ -637,6 +633,9 @@
"transcription_block_review": "Mark as reviewed",
"transcription_block_unreview": "Unmark as reviewed",
"transcription_reviewed_count": "{reviewed} of {total} reviewed",
"transcription_mark_all_reviewed": "Mark all as reviewed",
"transcription_mark_all_reviewed_disabled": "All blocks are already marked as reviewed",
"transcription_mark_all_reviewed_error": "Failed to mark all as reviewed. Please try again.",
"training_ocr_heading": "Train Kurrent recognition",
"training_ocr_description": "Start a new training run using the reviewed OCR blocks to improve recognition accuracy for Kurrent script.",
"training_ocr_blocks_ready": "{blocks} reviewed blocks ready / {docs} documents",

View File

@@ -445,12 +445,8 @@
"person_mention_load_error": "No se pudo cargar la persona.",
"person_mention_loading": "Cargando persona…",
"person_mention_popup_empty": "No se encontraron personas",
"person_mention_search_label": "Buscar persona",
"person_mention_search_prompt": "Escribe un nombre…",
"person_mention_btn_label": "Vincular persona",
"person_mention_create_new": "Crear nueva persona",
"person_mention_results_count_singular": "1 persona encontrada",
"person_mention_results_count_plural": "{count} personas encontradas",
"transcription_editor_aria_label": "Texto de transcripción",
"person_born_name_prefix": "n.",
"page_title_home": "Archivo",
@@ -637,6 +633,9 @@
"transcription_block_review": "Marcar como revisado",
"transcription_block_unreview": "Desmarcar como revisado",
"transcription_reviewed_count": "{reviewed} de {total} revisados",
"transcription_mark_all_reviewed": "Marcar todo como revisado",
"transcription_mark_all_reviewed_disabled": "Todos los bloques ya están marcados como revisados",
"transcription_mark_all_reviewed_error": "Error al marcar como revisado. Intente de nuevo.",
"training_ocr_heading": "Entrenar reconocimiento Kurrent",
"training_ocr_description": "Inicia un nuevo entrenamiento con los bloques OCR revisados para mejorar la precisión de reconocimiento del script Kurrent.",
"training_ocr_blocks_ready": "{blocks} bloques revisados listos / {docs} documentos",

View File

@@ -1,7 +1,7 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte';
import { page } from 'vitest/browser';
import TranscriptionBlockHost from './TranscriptionBlock.test-fixture.svelte';
import TranscriptionBlockHost from './TranscriptionBlock.test-host.svelte';
import type { ConfirmService } from '$lib/shared/services/confirm.svelte.js';
afterEach(cleanup);

View File

@@ -49,6 +49,7 @@ let activeBlockId: string | null = $state(null);
let localLabels: string[] = $derived.by(() => [...trainingLabels]);
let listEl: HTMLElement | null = $state(null);
let markingAllReviewed = $state(false);
let markAllError = $state<string | null>(null);
const sortedBlocks = $derived([...blocks].sort((a, b) => a.sortOrder - b.sortOrder));
const hasBlocks = $derived(blocks.length > 0);
@@ -67,8 +68,11 @@ $effect(() => {
async function handleMarkAllReviewed() {
if (!onMarkAllReviewed) return;
markingAllReviewed = true;
markAllError = null;
try {
await onMarkAllReviewed();
} catch {
markAllError = m.transcription_mark_all_reviewed_error();
} finally {
markingAllReviewed = false;
}
@@ -169,7 +173,7 @@ async function handleLabelToggle(label: string) {
<button
onclick={handleMarkAllReviewed}
disabled={allReviewed || markingAllReviewed}
title={allReviewed ? 'Alle Blöcke sind bereits als fertig markiert' : undefined}
title={allReviewed ? m.transcription_mark_all_reviewed_disabled() : undefined}
class="flex min-h-[44px] items-center gap-1.5 rounded-sm px-3 font-sans text-xs font-medium text-brand-navy/80 transition-colors hover:text-brand-navy focus-visible:ring-2 focus-visible:ring-brand-navy disabled:opacity-40"
>
{#if markingAllReviewed}
@@ -207,7 +211,7 @@ async function handleLabelToggle(label: string) {
<path stroke-linecap="round" stroke-linejoin="round" d="M5 13l4 4L19 7" />
</svg>
{/if}
Alle als fertig markieren
{m.transcription_mark_all_reviewed()}
</button>
{/if}
</div>
@@ -217,6 +221,31 @@ async function handleLabelToggle(label: string) {
style="width: {reviewProgress}%"
></div>
</div>
{#if markAllError}
<div
role="alert"
class="mt-1.5 flex items-center gap-2 rounded-sm border border-red-200 bg-red-50 px-3 py-2 font-sans text-sm text-red-700"
>
<span class="flex-1">{markAllError}</span>
<button
onclick={() => (markAllError = null)}
aria-label={m.comp_dismiss()}
class="flex min-h-[44px] min-w-[44px] items-center justify-center rounded text-red-600 hover:text-red-700 focus-visible:ring-2 focus-visible:ring-red-500"
>
<svg
class="h-4 w-4"
fill="none"
stroke="currentColor"
stroke-width="2"
viewBox="0 0 24 24"
xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
>
<path stroke-linecap="round" stroke-linejoin="round" d="M6 18L18 6M6 6l12 12" />
</svg>
</button>
</div>
{/if}
</div>
<div class="p-4">
<!-- svelte-ignore a11y_no_static_element_interactions -->

View File

@@ -3,6 +3,7 @@ import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser';
import TranscriptionEditView from './TranscriptionEditView.svelte';
import { createConfirmService, CONFIRM_KEY } from '$lib/shared/services/confirm.svelte.js';
import { m } from '$lib/paraglide/messages.js';
afterEach(cleanup);
@@ -312,14 +313,14 @@ describe('TranscriptionEditView — mark all reviewed', () => {
onMarkAllReviewed: vi.fn().mockResolvedValue(undefined)
});
await expect
.element(page.getByRole('button', { name: /Alle als fertig markieren/ }))
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() }))
.toBeInTheDocument();
});
it('does not show "Alle als fertig markieren" button when onMarkAllReviewed is not provided', async () => {
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2] });
await expect
.element(page.getByRole('button', { name: /Alle als fertig markieren/ }))
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() }))
.not.toBeInTheDocument();
});
@@ -329,7 +330,7 @@ describe('TranscriptionEditView — mark all reviewed', () => {
onMarkAllReviewed: vi.fn().mockResolvedValue(undefined)
});
await expect
.element(page.getByRole('button', { name: /Alle als fertig markieren/ }))
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() }))
.toBeDisabled();
});
@@ -343,7 +344,7 @@ describe('TranscriptionEditView — mark all reviewed', () => {
// userEvent.click() via Playwright CDP doesn't reliably trigger Svelte 5 onclick
// handlers when a TipTap editor is mounted in the same component tree.
const btn = (await page
.getByRole('button', { name: /Alle als fertig markieren/ })
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btn.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await vi.waitFor(() => expect(onMarkAllReviewed).toHaveBeenCalledTimes(1));
@@ -361,12 +362,83 @@ describe('TranscriptionEditView — mark all reviewed', () => {
// Same CDP click workaround: dispatch from browser JS to reliably fire Svelte 5 onclick
const btnEl = (await page
.getByRole('button', { name: /Alle als fertig markieren/ })
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect
.element(page.getByRole('button', { name: /Alle als fertig markieren/ }))
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() }))
.toBeDisabled();
resolveMarkAll();
});
it('shows error message when onMarkAllReviewed callback rejects', async () => {
const onMarkAllReviewed = vi.fn().mockRejectedValue(new Error('INTERNAL_ERROR'));
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2], onMarkAllReviewed });
const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).toBeInTheDocument();
await expect
.element(page.getByRole('alert'))
.toHaveTextContent(m.transcription_mark_all_reviewed_error());
});
it('clears error when dismiss button is clicked', async () => {
const onMarkAllReviewed = vi.fn().mockRejectedValue(new Error('INTERNAL_ERROR'));
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2], onMarkAllReviewed });
const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).toBeInTheDocument();
const dismissEl = (await page
.getByRole('button', { name: m.comp_dismiss() })
.element()) as HTMLButtonElement;
dismissEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).not.toBeInTheDocument();
});
it('clears error on next successful markAllReviewed call', async () => {
const onMarkAllReviewed = vi
.fn()
.mockRejectedValueOnce(new Error('INTERNAL_ERROR'))
.mockResolvedValue(undefined);
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2], onMarkAllReviewed });
const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).toBeInTheDocument();
// Wait for the button to be re-enabled before the second click — ensures the first
// async rejection has fully settled and Svelte has flushed state changes
await expect
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() }))
.not.toBeDisabled();
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).not.toBeInTheDocument();
});
it('re-enables button after markAllReviewed failure', async () => {
const onMarkAllReviewed = vi.fn().mockRejectedValue(new Error('INTERNAL_ERROR'));
renderView({ blocks: [unreviewedBlock1, unreviewedBlock2], onMarkAllReviewed });
const btnEl = (await page
.getByRole('button', { name: m.transcription_mark_all_reviewed() })
.element()) as HTMLButtonElement;
btnEl.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
await expect.element(page.getByRole('alert')).toBeInTheDocument();
await expect
.element(page.getByRole('button', { name: m.transcription_mark_all_reviewed() }))
.not.toBeDisabled();
});
});

View File

@@ -259,12 +259,15 @@ describe('createTranscriptionBlocks.markAllReviewed', () => {
expect(ctrl.blocks.every((b) => b.reviewed)).toBe(true);
});
it('is a no-op when PUT returns non-OK', async () => {
it('throws and leaves blocks unchanged when PUT returns non-OK', async () => {
const fetchImpl = vi.fn(async (url: RequestInfo | URL, init?: RequestInit) => {
const u = url.toString();
const method = init?.method ?? 'GET';
if (u.includes('/review-all') && method === 'PUT') {
return new Response('', { status: 500 });
return new Response(JSON.stringify({ code: 'INTERNAL_ERROR' }), {
status: 500,
headers: { 'Content-Type': 'application/json' }
});
}
return new Response(JSON.stringify([baseBlock({ id: 'b-1', reviewed: false })]), {
status: 200,
@@ -274,7 +277,26 @@ describe('createTranscriptionBlocks.markAllReviewed', () => {
const ctrl = createTranscriptionBlocks({ documentId: () => 'doc-1', fetchImpl });
await ctrl.load();
await ctrl.markAllReviewed();
await expect(ctrl.markAllReviewed()).rejects.toThrow('INTERNAL_ERROR');
expect(ctrl.blocks[0].reviewed).toBe(false);
});
it('throws INTERNAL_ERROR when PUT returns non-JSON body (e.g. nginx 502)', async () => {
const fetchImpl = vi.fn(async (url: RequestInfo | URL, init?: RequestInit) => {
const u = url.toString();
const method = init?.method ?? 'GET';
if (u.includes('/review-all') && method === 'PUT') {
return new Response('Bad Gateway', { status: 502 });
}
return new Response(JSON.stringify([baseBlock({ id: 'b-1', reviewed: false })]), {
status: 200,
headers: { 'Content-Type': 'application/json' }
});
});
const ctrl = createTranscriptionBlocks({ documentId: () => 'doc-1', fetchImpl });
await ctrl.load();
await expect(ctrl.markAllReviewed()).rejects.toThrow('INTERNAL_ERROR');
expect(ctrl.blocks[0].reviewed).toBe(false);
});
});

View File

@@ -119,7 +119,11 @@ export function createTranscriptionBlocks(
const res = await fetchImpl(`/api/documents/${documentId()}/transcription-blocks/review-all`, {
method: 'PUT'
});
if (!res.ok) return;
if (!res.ok) {
const body = await res.json().catch(() => ({}));
// Never render body.message — route through getErrorMessage() to prevent leaking backend internals
throw new Error((body as { code?: string })?.code ?? 'INTERNAL_ERROR');
}
const updated = (await res.json()) as { id: string; reviewed: boolean }[];
for (const b of updated) {
const existing = blocks.find((x) => x.id === b.id);

View File

@@ -2,18 +2,7 @@
import type { components } from '$lib/generated/api';
// eslint-disable-next-line boundaries/dependencies -- mention dropdown needs person date formatting; extract to shared if it becomes reusable
import { formatLifeDateRange } from '$lib/person/personLifeDates';
import { untrack } from 'svelte';
import { m } from '$lib/paraglide/messages.js';
// Layered defence cap on the @mention search query length (CWE-400
// amplification). The <input maxlength> attribute below caps direct
// user edits, but the editor-mirror path (Tiptap contenteditable -> mirror
// $effect -> searchQuery) is not covered by `maxlength` since the
// contenteditable has no such enforcement. Clipping at the mirror keeps
// the cap honest from both paths. Tracked server-side separately.
// Nora #1 on PR #629. Hoisted to mentionConstants.ts so the host editor
// (PersonMentionEditor) can clip the inserted displayName to the same cap
// — see Felix #3 on PR #629.
import { MAX_QUERY_LENGTH } from './mentionConstants';
type Person = components['schemas']['Person'];
@@ -28,46 +17,7 @@ type DropdownState = {
clientRect: (() => DOMRect | null) | null;
};
let {
model,
editorQuery = '',
onSearch = () => {}
}: {
model: DropdownState;
/** Text typed after `@` in the host editor. Mirrors into the search input
* until the user takes manual ownership by typing into the input itself. */
editorQuery?: string;
onSearch?: (query: string) => void;
} = $props();
let searchQuery = $state(untrack(() => editorQuery.slice(0, MAX_QUERY_LENGTH)));
let userHasEdited = $state(false);
// Intent-revealing alias used by both the persistent aria-live announcer and
// the visible empty-state copy. Folding the duplicated rule into one $derived
// keeps the two branches in lockstep. Felix #3 on PR #629 round 4.
const isQueryEmpty = $derived(searchQuery.trim() === '');
// Mirror the editor's typed text until the user takes ownership.
//
// Why `$state + $effect` (not `$derived`): `searchQuery` is also written by
// `bind:value` on the <input> below, so it needs to be a mutable `$state`.
// A `$derived` would be read-only and would clobber direct user edits on
// every editor keystroke. The `userHasEdited` latch pins ownership once the
// user types into the input. Felix #1 on PR #629.
$effect(() => {
if (!userHasEdited) {
searchQuery = editorQuery.slice(0, MAX_QUERY_LENGTH);
}
});
// Fire onSearch whenever the effective query changes — covers both the
// editor mirror and direct input edits. This is the only place onSearch
// fires; when the dropdown is unmounted, the effect is disposed and no
// further fetches occur.
$effect(() => {
onSearch(searchQuery);
});
let { model }: { model: DropdownState } = $props();
// highlightedIndex must be both writable (keyboard handler mutates it) and
// reset when `items` changes (so it never points past the end of a new list).
@@ -162,70 +112,16 @@ function selectItem(item: Person) {
unauthenticated users.
-->
<div
class="fixed z-50 w-72 max-w-[calc(100vw-1rem)] overflow-hidden rounded-sm border border-line bg-surface shadow-lg"
class="fixed z-50 w-72 overflow-hidden rounded-sm border border-line bg-surface shadow-lg"
role="listbox"
aria-label={m.person_mention_btn_label()}
style:top={position.top}
style:bottom={position.bottom}
style:left={position.left}
>
<div class="border-b border-line px-3 py-2">
<label class="sr-only" for="mention-search">{m.person_mention_search_label()}</label>
<div class="flex items-center gap-2">
<svg
aria-hidden="true"
viewBox="0 0 24 24"
fill="none"
stroke="currentColor"
stroke-width="2"
class="h-5 w-5 shrink-0 text-ink-2"
>
<circle cx="11" cy="11" r="7" />
<path d="m20 20-3.5-3.5" stroke-linecap="round" />
</svg>
<input
id="mention-search"
type="search"
data-test-search-input
maxlength={MAX_QUERY_LENGTH}
class="min-h-[44px] w-full bg-transparent font-sans text-base text-ink placeholder:text-ink-3 focus:outline-none focus-visible:ring-2 focus-visible:ring-brand-navy focus-visible:ring-inset"
placeholder={m.person_mention_search_prompt()}
bind:value={searchQuery}
oninput={() => {
userHasEdited = true;
}}
onmousedown={(e) => e.stopPropagation()}
/>
</div>
</div>
<!--
Persistent aria-live region — lives ABOVE the conditional branches so the
element never unmounts when items transition between empty and populated.
VoiceOver in particular swallows announcements from freshly-mounted live
regions, and the previous (conditional-inside) markup silently dropped
the "N persons found" announcement when results populated. Leonie #3 on
PR #629 round 3.
-->
<p class="sr-only" aria-live="polite">
{#if model.items.length === 0}
{isQueryEmpty ? m.person_mention_search_prompt() : m.person_mention_popup_empty()}
{:else if model.items.length === 1}
{m.person_mention_results_count_singular()}
{:else}
{m.person_mention_results_count_plural({ count: model.items.length })}
{/if}
</p>
{#if model.items.length === 0}
<!--
Visible empty-state copy — visual-only. The persistent sr-only <p>
above is the sole AT announcer; this one is hidden from screen readers
via aria-hidden="true" so VoiceOver does not double-announce
(NVDA de-dups, VoiceOver does not). Leonie S-2 on PR #629 round 4.
Do NOT add an aria-live attribute here — that would re-introduce
the duplicate announcement.
-->
<p aria-hidden="true" class="px-3 py-2.5 font-sans text-sm text-ink-3">
{isQueryEmpty ? m.person_mention_search_prompt() : m.person_mention_popup_empty()}
<p class="px-3 py-2.5 font-sans text-sm text-ink-3">
{m.person_mention_popup_empty()}
</p>
<!--
Empty-state escape hatch — without it the transcriber has to close
@@ -236,7 +132,7 @@ function selectItem(item: Person) {
<a
href="/persons/new"
target="_blank"
rel="noopener noreferrer"
rel="noopener"
class="flex min-h-[44px] items-center gap-2 border-t border-line px-3 py-2.5 font-sans text-sm font-medium text-brand-navy hover:bg-canvas focus:bg-canvas focus:outline-none"
onmousedown={(e) => e.preventDefault()}
>

View File

@@ -1,37 +1,22 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser';
import { flushSync, mount, tick, unmount } from 'svelte';
import { page } from 'vitest/browser';
import MentionDropdown from './MentionDropdown.svelte';
import MentionDropdownFixture from './MentionDropdown.test-fixture.svelte';
import { m } from '$lib/paraglide/messages.js';
import type { components } from '$lib/generated/api';
type Person = components['schemas']['Person'];
afterEach(cleanup);
const makePerson = (id: string, name: string, overrides: Partial<Person> = {}): Person => {
const parts = name.split(' ');
return {
id,
firstName: parts[0],
lastName: parts.slice(1).join(' ') || name,
displayName: name,
personType: 'PERSON',
familyMember: false,
...overrides
};
};
const makePerson = (id: string, name: string, overrides: Record<string, unknown> = {}) => ({
id,
firstName: name.split(' ')[0] ?? null,
lastName: name.split(' ').slice(1).join(' ') || name,
displayName: name,
birthYear: null as number | null,
deathYear: null as number | null,
...overrides
});
type DropdownState = {
items: Person[];
command: (item: Person) => void;
clientRect: (() => DOMRect | null) | null;
};
const baseModel = (overrides: Partial<DropdownState> = {}): DropdownState => ({
items: [],
const baseModel = (overrides: Record<string, unknown> = {}) => ({
items: [] as ReturnType<typeof makePerson>[],
command: vi.fn(),
clientRect: () => new DOMRect(100, 100, 0, 24),
...overrides
@@ -44,32 +29,14 @@ describe('MentionDropdown', () => {
await expect.element(page.getByRole('listbox', { name: /person verlinken/i })).toBeVisible();
});
it('shows the "enter a name" prompt when the search field is empty', async () => {
it('renders the empty placeholder when items is empty', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
// Scope to the visible empty-state <p> (text-ink-3) — the persistent
// sr-only aria-live region above also contains the same prompt copy.
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_search_prompt());
expect(visibleEmptyP!.textContent ?? '').not.toContain(m.person_mention_popup_empty());
});
it('shows "no persons found" when the search has a query but the list is empty', async () => {
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'WdG' } });
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
expect(visibleEmptyP!.textContent ?? '').not.toContain(m.person_mention_search_prompt());
await expect.element(page.getByText('Keine Personen gefunden')).toBeVisible();
});
it('shows the create-new escape hatch link in the empty state', async () => {
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'unknown' } });
render(MentionDropdown, { props: { model: baseModel() } });
const link = (await page
.getByRole('link', { name: /neue person anlegen/i })
@@ -77,7 +44,6 @@ describe('MentionDropdown', () => {
expect(link.href).toContain('/persons/new');
expect(link.target).toBe('_blank');
expect(link.rel).toContain('noopener');
expect(link.rel).toContain('noreferrer');
});
it('renders one option per item when populated', async () => {
@@ -138,315 +104,3 @@ describe('MentionDropdown', () => {
expect(dropdown.style.left).toBe('123px');
});
});
// ─── Search input — Issue #380 ────────────────────────────────────────────────
describe('MentionDropdown — search input', () => {
it('renders a search input pre-filled with the editorQuery prop', async () => {
render(MentionDropdown, {
props: { model: baseModel(), editorQuery: 'WdG' }
});
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
});
it('exposes a data-test-search-input attribute for E2E selectors', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const input = document.querySelector('[data-test-search-input]');
expect(input).not.toBeNull();
expect((input as HTMLInputElement).type).toBe('search');
});
it('search input wrapper meets the 44px touch target (WCAG 2.2 AA)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const input = document.querySelector('[data-test-search-input]') as HTMLElement;
expect(input).not.toBeNull();
expect(input.className).toContain('min-h-[44px]');
});
it('renders a persistent aria-live="polite" region (does not remount on items transition; Leonie #3 on PR #629)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const listbox = document.querySelector('[role="listbox"]');
expect(listbox).not.toBeNull();
const live = listbox!.querySelector('p[aria-live="polite"]');
expect(live).not.toBeNull();
// Empty + empty-query → "Namen eingeben…" prompt
expect(live!.textContent ?? '').toContain(m.person_mention_search_prompt());
});
it('announces the result count in the persistent live region when items populate (Leonie #3 on PR #629)', async () => {
render(MentionDropdown, {
props: {
model: baseModel({
items: [
makePerson('p1', 'Anna Schmidt'),
makePerson('p2', 'Bert Meier'),
makePerson('p3', 'Carl Vogel')
]
})
}
});
const listbox = document.querySelector('[role="listbox"]');
expect(listbox).not.toBeNull();
const live = listbox!.querySelector('p[aria-live="polite"]');
expect(live).not.toBeNull();
// Populated → "3 Personen gefunden" (plural)
expect(live!.textContent ?? '').toContain('3');
});
it('announces the singular form when exactly one item is present (Sara #4 on PR #629)', async () => {
render(MentionDropdown, {
props: {
model: baseModel({
items: [makePerson('p1', 'Anna Schmidt')]
})
}
});
const listbox = document.querySelector('[role="listbox"]');
expect(listbox).not.toBeNull();
const live = listbox!.querySelector('p[aria-live="polite"]');
expect(live).not.toBeNull();
// Singular branch — "1 Person gefunden" / "1 person found" / "1 persona encontrada"
// (locale-dependent; resolved via the Paraglide message helper).
expect(live!.textContent ?? '').toContain(m.person_mention_results_count_singular());
});
it('keeps the visible empty-state copy without its own aria-live and hides it from AT (Leonie #3 on PR #629 round 3; Leonie S-2 round 4)', async () => {
render(MentionDropdown, { props: { model: baseModel(), editorQuery: 'WdG' } });
// Visible empty-state <p> exists with the empty-result copy ...
const empty = document.querySelector('p.text-ink-3') as HTMLElement | null;
expect(empty).not.toBeNull();
expect(empty!.textContent ?? '').toContain(m.person_mention_popup_empty());
// ... but it must NOT carry its own aria-live (the persistent sr-only
// region above the conditional is the announcer now).
expect(empty!.hasAttribute('aria-live')).toBe(false);
// ... and it MUST be hidden from screen readers via aria-hidden="true"
// so VoiceOver does not double-announce (the persistent sr-only region
// is the sole AT source of truth). Leonie S-2 on PR #629 round 4.
expect(empty!.getAttribute('aria-hidden')).toBe('true');
});
it('renders the magnifier icon at h-5 w-5 with text-ink-2 (Leonie BLOCKER on PR #629)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const icon = document.querySelector('[data-test-search-input]')
?.previousElementSibling as SVGElement | null;
expect(icon).not.toBeNull();
expect(icon!.tagName.toLowerCase()).toBe('svg');
expect(icon!.getAttribute('class') ?? '').toContain('h-5');
expect(icon!.getAttribute('class') ?? '').toContain('w-5');
expect(icon!.getAttribute('class') ?? '').toContain('text-ink-2');
});
it('caps the search input at maxlength=100 (CWE-400 amplification — Nora on PR #629)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
expect(input).not.toBeNull();
expect(input.maxLength).toBe(100);
});
it('clips a long editorQuery mirror to 100 chars (CWE-400 layered — Nora #1 on PR #629)', async () => {
const longQuery = 'A'.repeat(200);
render(MentionDropdown, { props: { model: baseModel(), editorQuery: longQuery } });
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
expect(input).not.toBeNull();
expect(input.value.length).toBe(100);
expect(input.value).toBe('A'.repeat(100));
});
it('caps the listbox width to the viewport (320 px reflow guard — Leonie FINDING-MENTION-005)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const listbox = document.querySelector('[role="listbox"]') as HTMLElement;
expect(listbox).not.toBeNull();
expect(listbox.className).toContain('max-w-[calc(100vw-1rem)]');
});
it('renders the @mention search input at text-base (16 px senior-audience floor — Leonie FINDING-MENTION-006)', async () => {
render(MentionDropdown, { props: { model: baseModel() } });
const input = document.querySelector('[data-test-search-input]') as HTMLInputElement;
expect(input).not.toBeNull();
expect(input.className).toContain('text-base');
expect(input.className).not.toContain('text-sm');
});
it('invokes onSearch with the current value whenever the user types', async () => {
const onSearch = vi.fn();
render(MentionDropdown, { props: { model: baseModel(), onSearch } });
await userEvent.type(page.getByRole('searchbox'), 'Walter');
await vi.waitFor(() => {
expect(onSearch).toHaveBeenCalled();
expect(onSearch).toHaveBeenLastCalledWith('Walter');
});
});
it('keeps the user-edited search value when editorQuery changes after the takeover (Felix on PR #629)', async () => {
let setEditorQuery!: (q: string) => void;
render(MentionDropdownFixture, {
model: baseModel(),
initialEditorQuery: 'WdG',
onReady: (s: (q: string) => void) => {
setEditorQuery = s;
}
});
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
await page.getByRole('searchbox').fill('Walter');
await expect.element(page.getByRole('searchbox')).toHaveValue('Walter');
setEditorQuery('WdGruyter');
// Flush pending Svelte reactivity so any (non-)update from the mirror
// $effect has landed before we assert. expect.element already polls, so
// no fixed-timeout fallback is needed. Sara on PR #629 round 3.
await tick();
await expect.element(page.getByRole('searchbox')).toHaveValue('Walter');
});
});
// ─── ArrowDown via exported onKeyDown (Sara #3 on PR #629) ──────────────────
//
// In production, Tiptap intercepts ArrowDown/ArrowUp/Enter at the editor level
// and forwards them to the dropdown via its exported onKeyDown(event) function
// — the dropdown itself has no DOM keydown listener. This test exercises the
// same export so a regression in highlightedIndex/selection logic is caught
// at the unit level. The full E2E focus-chain test is deferred to a separate
// issue (Playwright).
//
// These unit tests directly invoke the exported `onKeyDown` to pin its
// behaviour in isolation. They do NOT exercise the Tiptap forwarding
// chain (PersonMentionEditor.suggestion.render() returning { onKeyDown })
// — that integration is covered by the 'ArrowDown moves the highlight'
// test in PersonMentionEditor.svelte.spec.ts. Sara on PR #629 round 3.
describe('MentionDropdown — onKeyDown forwarding', () => {
// flushSync ensures Svelte reactivity propagation completes before
// asserting (uniform across all four key tests so the next reader
// doesn't have to figure out why some are wrapped and others aren't).
// Felix #1 suggestion on PR #629 round 3.
it('ArrowDown advances aria-selected to the next option in the listbox', async () => {
const container = document.createElement('div');
document.body.appendChild(container);
const instance = mount(MentionDropdown, {
target: container,
props: {
model: baseModel({
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')]
})
}
});
try {
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
// First option starts highlighted.
const first = container.querySelector('[data-test-person-id="p1"]') as HTMLElement;
const second = container.querySelector('[data-test-person-id="p2"]') as HTMLElement;
expect(first.getAttribute('aria-selected')).toBe('true');
expect(second.getAttribute('aria-selected')).toBe('false');
let consumed = false;
flushSync(() => {
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'ArrowDown' }));
});
expect(consumed).toBe(true);
expect(first.getAttribute('aria-selected')).toBe('false');
expect(second.getAttribute('aria-selected')).toBe('true');
} finally {
unmount(instance);
container.remove();
}
});
it('ArrowUp wraps from the first option to the last', async () => {
const container = document.createElement('div');
document.body.appendChild(container);
const instance = mount(MentionDropdown, {
target: container,
props: {
model: baseModel({
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')]
})
}
});
try {
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
let consumed = false;
flushSync(() => {
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'ArrowUp' }));
});
expect(consumed).toBe(true);
const second = container.querySelector('[data-test-person-id="p2"]') as HTMLElement;
expect(second.getAttribute('aria-selected')).toBe('true');
} finally {
unmount(instance);
container.remove();
}
});
it('Enter invokes model.command with the currently highlighted item', async () => {
const command = vi.fn();
const container = document.createElement('div');
document.body.appendChild(container);
const instance = mount(MentionDropdown, {
target: container,
props: {
model: baseModel({
items: [makePerson('p1', 'Anna Schmidt'), makePerson('p2', 'Bert Meier')],
command
})
}
});
try {
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
let consumed = false;
flushSync(() => {
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'Enter' }));
});
expect(consumed).toBe(true);
expect(command).toHaveBeenCalledTimes(1);
expect(command.mock.calls[0][0].id).toBe('p1');
} finally {
unmount(instance);
container.remove();
}
});
it('Escape returns false so the suggestion plugin can handle it', async () => {
const container = document.createElement('div');
document.body.appendChild(container);
const instance = mount(MentionDropdown, {
target: container,
props: {
model: baseModel({ items: [makePerson('p1', 'Anna Schmidt')] })
}
});
try {
const exports = instance as unknown as { onKeyDown: (e: KeyboardEvent) => boolean };
let consumed = true;
flushSync(() => {
consumed = exports.onKeyDown(new KeyboardEvent('keydown', { key: 'Escape' }));
});
expect(consumed).toBe(false);
} finally {
unmount(instance);
container.remove();
}
});
});

View File

@@ -1,32 +0,0 @@
<script lang="ts">
import { untrack } from 'svelte';
import MentionDropdown from './MentionDropdown.svelte';
import type { components } from '$lib/generated/api';
type Person = components['schemas']['Person'];
type DropdownState = {
items: Person[];
command: (item: Person) => void;
clientRect: (() => DOMRect | null) | null;
};
type Props = {
model: DropdownState;
initialEditorQuery: string;
/** Test hook: receives a setter for editorQuery so the test can mutate it. */
onReady?: (setEditorQuery: (q: string) => void) => void;
onSearch?: (q: string) => void;
};
let { model, initialEditorQuery, onReady, onSearch = () => {} }: Props = $props();
let editorQuery = $state(untrack(() => initialEditorQuery));
$effect(() => {
onReady?.((q) => {
editorQuery = q;
});
});
</script>
<MentionDropdown model={model} editorQuery={editorQuery} onSearch={onSearch} />

View File

@@ -7,9 +7,7 @@ import { m } from '$lib/paraglide/messages.js';
import type { components } from '$lib/generated/api';
import type { PersonMention } from '$lib/shared/types';
import { deserialize, serialize } from '$lib/shared/discussion/mentionSerializer';
import { debounce } from '$lib/shared/utils/debounce';
import MentionDropdown from './MentionDropdown.svelte';
import { MAX_QUERY_LENGTH, SEARCH_DEBOUNCE_MS, SEARCH_RESULT_LIMIT } from './mentionConstants';
type Person = components['schemas']['Person'];
@@ -35,13 +33,6 @@ let {
let editorEl: HTMLDivElement;
let editor: Editor | null = null;
// Hoisted so onDestroy can guarantee the imperatively-mounted dropdown is
// torn down even if Tiptap's suggestion plugin onExit didn't fire (e.g. when
// the host component is unmounted while the dropdown is still open).
let mountedDropdown: object | null = null;
// Hoisted so onDestroy can cancel any pending fetch — otherwise a trailing
// debounced search can fire after the editor is gone and pollute later tests.
let cancelPendingSearch: (() => void) | null = null;
// Single reactive state object shared with MentionDropdown. Mutating these
// fields propagates to the mounted dropdown via Svelte's $state proxy —
@@ -51,12 +42,10 @@ let dropdownState = $state<{
items: Person[];
command: (item: Person) => void;
clientRect: (() => DOMRect | null) | null;
editorQuery: string;
}>({
items: [],
command: () => {},
clientRect: null,
editorQuery: ''
clientRect: null
});
type DropdownExports = {
@@ -149,13 +138,16 @@ onMount(() => {
// Nora #5618 #3 — separate issue tracks the GET /api/persons
// response-shape audit (PersonSummaryDTO leaks `notes`).
// ─────────────────────────────────────────────────────────────
// Tiptap's suggestion plugin requires an `items()` callback to keep
// the dropdown alive, but the actual fetch is owned by `runSearch`
// below — routed through the dropdown's search input via the
// debounced `onSearch` channel. Returning `[]` here keeps Tiptap
// happy without firing a duplicate per-keystroke fetch.
// Markus #5616 / Felix / Nora / Sara on PR #629.
items: async () => [],
items: async ({ query }: { query: string }) => {
if (!query) return [];
try {
const res = await fetch(`/api/persons?q=${encodeURIComponent(query)}`);
if (!res.ok) return [];
return ((await res.json()) as Person[]).slice(0, 5);
} catch {
return [];
}
},
// AC-1 fix: insert the typed query as displayName, not person.displayName.
command({ editor: ed, range, props }) {
const p = props as unknown as { personId: string; displayName: string };
@@ -173,6 +165,7 @@ onMount(() => {
.run();
},
render() {
let component: object | null = null;
let exports: DropdownExports | null = null;
// Tiptap's SuggestionProps types `command` against the default
@@ -185,84 +178,25 @@ onMount(() => {
clientRect?: (() => DOMRect | null) | null;
};
// Request-token guard: every onSearch invocation bumps `requestId`;
// runSearch captures the id active when its fetch starts and discards
// the response if a newer onSearch has fired since. Without this, a
// late response can repopulate the dropdown after the user cleared
// the search input. Sara on PR #629.
let requestId = 0;
const runSearch = async (query: string) => {
const id = requestId;
try {
// Defensive client-side cap — server-side enforcement is tracked
// separately. Markus on PR #629.
const res = await fetch(
`/api/persons?q=${encodeURIComponent(query)}&limit=${SEARCH_RESULT_LIMIT}`
);
if (id !== requestId) return;
if (!res.ok) {
dropdownState.items = [];
return;
}
const data = (await res.json()) as Person[];
if (id !== requestId) return;
dropdownState.items = data.slice(0, SEARCH_RESULT_LIMIT);
} catch {
if (id !== requestId) return;
dropdownState.items = [];
}
};
const debouncedSearch = debounce(runSearch, SEARCH_DEBOUNCE_MS);
cancelPendingSearch = () => debouncedSearch.cancel();
const onSearch = (query: string) => {
requestId++;
if (query.trim() === '') {
debouncedSearch.cancel();
dropdownState.items = [];
return;
}
debouncedSearch(query);
};
const updateState = (renderProps: LooseRenderProps) => {
// Clip once here so both the inserted displayName and the
// dropdown's editor-mirror see the same value. The dropdown
// already clips the mirror (Nora #1 CWE-400), but without
// clipping at the command boundary an unclipped query would
// still flow through as the inserted displayName — visible
// UI divergence between "what I searched" and "what was
// inserted". Felix #3 on PR #629.
const clippedQuery = renderProps.query.slice(0, MAX_QUERY_LENGTH);
dropdownState.items = renderProps.items as Person[];
// AC-1: pass typed query as displayName, not person.displayName
dropdownState.command = (item: Person) =>
renderProps.command({
personId: item.id,
displayName: clippedQuery
displayName: renderProps.query
});
dropdownState.clientRect = renderProps.clientRect ?? null;
dropdownState.editorQuery = clippedQuery;
};
return {
onStart(renderProps) {
const loose = renderProps as unknown as LooseRenderProps;
updateState(loose);
// MentionDropdown reads `editorQuery` off the shared state
// proxy via its `editorQuery` prop binding below — this is
// the same pattern as `model.items`. We do not pass it as a
// separate prop because Svelte 5's mount() does not expose
// settable prop accessors, so we route through the proxy.
updateState(renderProps as unknown as LooseRenderProps);
const mounted = mount(MentionDropdown, {
target: document.body,
props: {
model: dropdownState,
get editorQuery() {
return dropdownState.editorQuery;
},
onSearch
}
props: { model: dropdownState }
});
mountedDropdown = mounted as object;
component = mounted as object;
exports = mounted as unknown as DropdownExports;
},
onUpdate(renderProps) {
@@ -274,16 +208,9 @@ onMount(() => {
return exports?.onKeyDown(event) ?? false;
},
onExit() {
// Cancel any pending debounce so a closed dropdown's trailing
// runSearch cannot fire against the *next* dropdown's state.
// The hoisted `cancelPendingSearch` would be overwritten by
// the next render()'s onStart before the trailing call fires,
// so we cancel locally via the closure-scoped debouncedSearch.
// Felix #1 on PR #629.
debouncedSearch.cancel();
if (mountedDropdown) {
unmount(mountedDropdown);
mountedDropdown = null;
if (component) {
unmount(component);
component = null;
exports = null;
}
}
@@ -326,15 +253,7 @@ onMount(() => {
});
onDestroy(() => {
cancelPendingSearch?.();
editor?.destroy();
// Tiptap suggestion onExit usually unmounts the dropdown, but if the host
// component is destroyed while a suggestion is active the dropdown can
// outlive the editor — clean it up explicitly.
if (mountedDropdown) {
unmount(mountedDropdown);
mountedDropdown = null;
}
});
// Keep the data-placeholder attribute in sync with actual emptiness so the

View File

@@ -8,45 +8,29 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser';
import { tick } from 'svelte';
import PersonMentionEditorHost from './PersonMentionEditor.test-fixture.svelte';
import PersonMentionEditorHost from './PersonMentionEditor.test-host.svelte';
import type { components } from '$lib/generated/api';
import { m } from '$lib/paraglide/messages.js';
// Single source of truth for the debounce window — imported from the shared
// module so the test cannot drift from production. Sara on PR #629 round 3.
import { SEARCH_DEBOUNCE_MS } from './mentionConstants';
type Person = components['schemas']['Person'];
type PersonMention = components['schemas']['PersonMention'];
/**
* Headroom above SEARCH_DEBOUNCE_MS for the debounce-window wait
* assertions in this file. 350 ms is calibrated against CI-runner jitter
* we observed pre-#629; dropping it below ~200 ms reintroduces flake.
* See PR #629 round-2 review comment #10935 (Sara).
*/
const POST_DEBOUNCE_SLACK_MS = 350;
const AUGUSTE: Person = {
id: 'p-aug',
firstName: 'Auguste',
lastName: 'Raddatz',
displayName: 'Auguste Raddatz',
personType: 'PERSON',
familyMember: false,
birthYear: 1882,
deathYear: 1944
};
} as unknown as Person;
const ANNA: Person = {
id: 'p-anna',
firstName: 'Anna',
lastName: 'Schmidt',
displayName: 'Anna Schmidt',
personType: 'PERSON',
familyMember: false,
birthYear: 1860
};
} as unknown as Person;
function mockFetchWithPersons(persons: Person[] = [AUGUSTE, ANNA]) {
vi.stubGlobal(
@@ -141,20 +125,6 @@ describe('PersonMentionEditor — typeahead', () => {
});
});
it('appends &limit=5 to the fetch URL (defensive client-side cap, Markus on PR #629)', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@Aug');
await vi.waitFor(() => {
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('limit=5'));
});
});
it('shows life dates next to the name in the dropdown', async () => {
mockFetchWithPersons();
renderHost();
@@ -172,15 +142,8 @@ describe('PersonMentionEditor — typeahead', () => {
await userEvent.type(page.getByRole('textbox'), '@xyz');
// The visible empty-state <p> (text-ink-3) shows the copy. The persistent
// sr-only aria-live region also contains the same copy, so we scope to the
// visible element to avoid a multi-match resolution in expect.element.
await vi.waitFor(() => {
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain('Keine Personen gefunden');
await vi.waitFor(async () => {
await expect.element(page.getByText('Keine Personen gefunden')).toBeInTheDocument();
});
});
@@ -198,254 +161,6 @@ describe('PersonMentionEditor — typeahead', () => {
});
});
// ─── AC-2/3: search input drives the person fetch (debounced) ───────────────
describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => {
it('editing the search input fires a debounced fetch with the new query', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
// Open the dropdown so the search input is reachable.
await userEvent.type(page.getByRole('textbox'), '@');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toBeVisible();
});
const fetchesBeforeSearch = fetchMock.mock.calls.length;
// `fill` simulates a single input event with the final value — sidesteps
// per-keystroke timing of userEvent.type so the test can deterministically
// assert that one input event collapses into one debounced fetch.
await page.getByRole('searchbox').fill('Walter');
await vi.waitFor(
() => {
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('q=Walter'));
},
{ timeout: 1000 }
);
const fetchesAfterSearch = fetchMock.mock.calls.length - fetchesBeforeSearch;
expect(fetchesAfterSearch).toBe(1);
});
it('fires exactly one /api/persons fetch when the user searches for Walter (regression guard)', async () => {
// Regression guard: a previous version of PersonMentionEditor had a
// duplicated `items()` callback in the Tiptap suggestion config that
// fetched per-keystroke in addition to the debounced search-input fetch
// (Markus & Felix round-1). To catch that regression, we must NOT
// subtract any baseline — every fetch from render onwards counts.
// Sara on PR #629 round 3.
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
// Open the dropdown, then drive the search input via fill() — sidesteps
// per-keystroke timing of userEvent.type that Sara flagged round 2.
await userEvent.type(page.getByRole('textbox'), '@');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toBeVisible();
});
await page.getByRole('searchbox').fill('Walter');
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
// No baseline subtraction — count ALL /api/persons fetches since render.
// If the legacy per-keystroke items() callback returns, typing `@` alone
// would already produce one fetch and `fill('Walter')` another, breaking
// this assertion.
const personsFetches = fetchMock.mock.calls.filter(
([url]) => typeof url === 'string' && url.startsWith('/api/persons')
);
expect(personsFetches.length).toBe(1);
});
it('clearing the search input clears the list without firing a fetch', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@Aug');
await vi.waitFor(async () => {
await expect.element(page.getByText('Auguste Raddatz')).toBeInTheDocument();
});
const fetchesBeforeClear = fetchMock.mock.calls.length;
await userEvent.clear(page.getByRole('searchbox'));
// Negative assertion: wait past the debounce window to confirm no
// trailing fetch was scheduled. Removing this wait would mask a
// re-introduction of the keystroke-driven items() fetch.
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
expect(fetchMock.mock.calls.length).toBe(fetchesBeforeClear);
await expect.element(page.getByText('Auguste Raddatz')).not.toBeInTheDocument();
});
});
// ─── Whitespace-only query (Elicit AC-4 ambiguity on PR #629) ───────────────
describe('PersonMentionEditor — whitespace-only query', () => {
it('keeps the "Namen eingeben…" prompt and fires no fetch when @ is followed only by spaces', async () => {
const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@ ');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toBeVisible();
});
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
// Scope to the visible empty-state <p> (text-ink-3) — the persistent
// sr-only aria-live region above contains the same copy.
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_search_prompt());
expect(fetchMock).not.toHaveBeenCalled();
});
});
// ─── Stale-response race (Sara on PR #629) ───────────────────────────────────
describe('PersonMentionEditor — stale-response race', () => {
it('discards a stale response that resolves after the search has been cleared', async () => {
let resolveFetch!: (v: { ok: boolean; json: () => Promise<Person[]> }) => void;
const pendingResponse = new Promise<{ ok: boolean; json: () => Promise<Person[]> }>((r) => {
resolveFetch = r;
});
const fetchMock = vi.fn().mockReturnValue(pendingResponse);
vi.stubGlobal('fetch', fetchMock);
renderHost();
// Open the dropdown and let the debounce fire so a fetch is in flight.
await userEvent.type(page.getByRole('textbox'), '@Aug');
await vi.waitFor(() => {
expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Aug'));
});
// Clear the search input *before* the fetch resolves.
await userEvent.clear(page.getByRole('searchbox'));
await expect.element(page.getByRole('searchbox')).toHaveValue('');
// The stale fetch now resolves with persons. The dropdown must stay empty.
resolveFetch({ ok: true, json: () => Promise.resolve([AUGUSTE]) });
// Flush pending Svelte reactivity so any (non-)update from the stale
// fetch resolution has landed before we assert. expect.element already
// polls, so no fixed-timeout fallback is needed. Sara on PR #629 round 4.
await tick();
await expect.element(page.getByText('Auguste Raddatz')).not.toBeInTheDocument();
});
});
// ─── Server failure characterization (Sara #2 on PR #629) ───────────────────
describe('PersonMentionEditor — server failure', () => {
it('on 500 response keeps the dropdown open with the empty-state copy (silent failure pinned; distinct error UX tracked separately)', async () => {
const fetchMock = vi
.fn()
.mockResolvedValue({ ok: false, status: 500, json: vi.fn().mockResolvedValue({}) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@Aug');
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
// Pins current silent-failure behaviour. The day someone implements a
// distinct error UX (toast / "Suche fehlgeschlagen" copy), this test
// goes red and forces them to update the assertion. Scope to the
// visible <p> (text-ink-3) — the persistent sr-only live region
// above contains the same copy.
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
});
it('on a fetch reject (network failure) keeps the dropdown open with the empty-state copy', async () => {
const fetchMock = vi.fn().mockRejectedValue(new TypeError('NetworkError'));
vi.stubGlobal('fetch', fetchMock);
renderHost();
await userEvent.type(page.getByRole('textbox'), '@Aug');
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
const visibleEmptyP = document.querySelector(
'[role="listbox"] p.text-ink-3'
) as HTMLElement | null;
expect(visibleEmptyP).not.toBeNull();
expect(visibleEmptyP!.textContent ?? '').toContain(m.person_mention_popup_empty());
});
});
// ─── onExit cancels pending debounce (Felix #1 on PR #629) ───────────────────
describe('PersonMentionEditor — onExit cancels pending debounce', () => {
it('cancels the pending debounced fetch when Escape closes the dropdown before the debounce fires', async () => {
const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([]) });
vi.stubGlobal('fetch', fetchMock);
renderHost();
// Open the dropdown by typing @ + a query in the editor.
await userEvent.type(page.getByRole('textbox'), '@A');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toBeVisible();
});
// Wait for any in-flight fetch from opening the dropdown to settle.
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
const fetchesBeforeEscape = fetchMock.mock.calls.length;
// Trigger a new debounced search (queues runSearch after 150 ms), then
// immediately Escape *while focus is back in the editor* so Tiptap's
// suggestion-plugin Escape handler fires onExit before the debounce.
// Without onExit cancelling the pending debounce, runSearch executes
// against the now-unmounted dropdown's state.
await page.getByRole('searchbox').fill('Walter');
// Focus the editor so the Escape lands on Tiptap's suggestion handler.
(page.getByRole('textbox').element() as HTMLElement).focus();
await userEvent.keyboard('{Escape}');
// Wait past the debounce window. If onExit did not cancel the pending
// debounce, a fetch with q=Walter would still fire here.
await new Promise((r) => setTimeout(r, SEARCH_DEBOUNCE_MS + POST_DEBOUNCE_SLACK_MS));
const newFetches = fetchMock.mock.calls.slice(fetchesBeforeEscape);
const walterFetches = newFetches.filter(
([url]) => typeof url === 'string' && url.includes('q=Walter')
);
expect(walterFetches.length).toBe(0);
});
});
// ─── AC-1: search input prefilled with text typed after @ ───────────────────
describe('PersonMentionEditor — AC-1: search input prefill', () => {
it('prefills the dropdown search input with the text typed after @', async () => {
mockFetchEmpty();
renderHost();
await userEvent.type(page.getByRole('textbox'), '@WdG');
await vi.waitFor(async () => {
await expect.element(page.getByRole('searchbox')).toHaveValue('WdG');
});
});
});
// ─── AC-1: typed text becomes displayName, not DB name ───────────────────────
describe('PersonMentionEditor — AC-1: typed text as displayName', () => {
@@ -514,39 +229,6 @@ describe('PersonMentionEditor — AC-1: typed text as displayName', () => {
});
});
it('clips the inserted displayName to MAX_QUERY_LENGTH=100 chars (Felix #3 on PR #629)', async () => {
// CWE-400 amplification: the dropdown clips its search input + mirror at
// 100 chars (Nora #1), but the host editor was passing the unclipped
// renderProps.query straight through to displayName — so a 105-char
// @-suffix in the editor could insert a 105-char displayName into the
// sidecar even though the dropdown only searched the first 100.
mockFetchWithPersons();
const host = renderHost();
// Type @ + 105 'A' chars in the contenteditable. The renderProps.query
// fed into the command callback derives from the editor text after `@`,
// not the dropdown's searchbox — so we must drive the editor.
await userEvent.type(page.getByRole('textbox'), '@' + 'A'.repeat(105));
// The mocked /api/persons returns AUGUSTE for any query — wait for it.
await vi.waitFor(async () => {
await expect.element(page.getByRole('option', { name: /Auguste Raddatz/ })).toBeVisible();
});
const option = (await page
.getByRole('option', { name: /Auguste Raddatz/ })
.element()) as HTMLElement;
option.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, cancelable: true }));
await vi.waitFor(() => {
expect(host.snapshot.mentionedPersons).toHaveLength(1);
// Tight assertion: input is 105 chars, cap is exactly 100. Using
// `toHaveLength(100)` discriminates "clip works" from "clip works
// AND nothing weakened it to e.g. 95". Sara on PR #629 round 4.
expect(host.snapshot.mentionedPersons[0].displayName).toHaveLength(100);
});
});
it('does not duplicate the sidecar entry when the same person is selected twice', async () => {
mockFetchWithPersons();
const host = renderHost({

View File

@@ -1,10 +0,0 @@
/** Shared knobs for the @mention typeahead. Single source of truth for
* the dropdown component and the host editor — keeps the layered length
* cap and the debounce window consistent across both files. */
export const MAX_QUERY_LENGTH = 100;
export const SEARCH_DEBOUNCE_MS = 150;
/** Defensive client-side cap on the result list. Single consumer today
* (PersonMentionEditor), kept here for symmetry with the other limit
* knobs so the @mention configuration lives in one place. Felix #1 on
* PR #629 round 4. */
export const SEARCH_RESULT_LIMIT = 5;

View File

@@ -1,7 +1,7 @@
import { describe, it, expect, afterEach } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte';
import { page, userEvent } from 'vitest/browser';
import TestHost from './confirm.test-fixture.svelte';
import TestHost from './confirm.test-host.svelte';
import type { ConfirmService } from './confirm.svelte.js';
afterEach(cleanup);

View File

@@ -1,25 +1,12 @@
/**
* Returns a debounced version of fn that delays invocation until after
* `delay` ms have elapsed since the last call. The returned function
* exposes a `cancel()` method that DROPS (does not flush) the pending
* trailing invocation — essential when the host context (a destroyed
* component, an unmounted editor) shouldn't fire the trailing call.
* `delay` ms have elapsed since the last call.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function debounce<T extends (...args: any[]) => void>(
fn: T,
delay: number
): T & { cancel: () => void } {
let timer: ReturnType<typeof setTimeout> | undefined;
const wrapped = ((...args: Parameters<T>) => {
if (timer !== undefined) clearTimeout(timer);
export function debounce<T extends (...args: any[]) => void>(fn: T, delay: number): T {
let timer: ReturnType<typeof setTimeout>;
return ((...args: Parameters<T>) => {
clearTimeout(timer);
timer = setTimeout(() => fn(...args), delay);
}) as T & { cancel: () => void };
wrapped.cancel = () => {
if (timer !== undefined) {
clearTimeout(timer);
timer = undefined;
}
};
return wrapped;
}) as T;
}