From 731cdc75ab78a543e0736ddcb76651bed637d9b8 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 12:43:40 +0200 Subject: [PATCH 01/19] refactor(frontend): delete dead conversations/ route (#193) Remove the old conversations page that was superseded by briefwechsel/. No navigation link pointed to /conversations; it was unreachable through the UI. Deletes 5 files, removes 14 orphaned i18n keys from de/en/es message bundles, and removes E2E tests that navigated to /conversations. Co-Authored-By: Claude Sonnet 4.6 --- frontend/e2e/auth.spec.ts | 2 +- frontend/e2e/persons.spec.ts | 129 -------------- frontend/messages/de.json | 14 -- frontend/messages/en.json | 14 -- frontend/messages/es.json | 14 -- .../src/routes/conversations/+page.server.ts | 64 ------- .../src/routes/conversations/+page.svelte | 104 ----------- .../ConversationFilterBar.svelte | 142 --------------- .../conversations/ConversationTimeline.svelte | 160 ----------------- .../routes/conversations/page.svelte.spec.ts | 164 ------------------ 10 files changed, 1 insertion(+), 806 deletions(-) delete mode 100644 frontend/src/routes/conversations/+page.server.ts delete mode 100644 frontend/src/routes/conversations/+page.svelte delete mode 100644 frontend/src/routes/conversations/ConversationFilterBar.svelte delete mode 100644 frontend/src/routes/conversations/ConversationTimeline.svelte delete mode 100644 frontend/src/routes/conversations/page.svelte.spec.ts diff --git a/frontend/e2e/auth.spec.ts b/frontend/e2e/auth.spec.ts index da5c4118..9d3315ef 100644 --- a/frontend/e2e/auth.spec.ts +++ b/frontend/e2e/auth.spec.ts @@ -24,7 +24,7 @@ test.describe('Authentication', () => { }); test('protected routes redirect to /login without session', async ({ page }) => { - for (const url of ['/documents/new', '/persons', '/conversations']) { + for (const url of ['/documents/new', '/persons', '/briefwechsel']) { await page.goto(url); await expect(page).toHaveURL(/\/login/); } diff --git a/frontend/e2e/persons.spec.ts b/frontend/e2e/persons.spec.ts index 1b5aed2d..e392d4e8 100644 --- a/frontend/e2e/persons.spec.ts +++ b/frontend/e2e/persons.spec.ts @@ -181,132 +181,3 @@ test.describe('Person detail — sent and received documents', () => { // If no person has dated documents, the test is a no-op (year range is optional) }); }); - -test.describe('Person detail — conversations link', () => { - test('co-correspondent chips link to conversations pre-filled with both persons', async ({ - page - }) => { - await page.goto('/persons'); - const firstLink = page.locator('a[href^="/persons/"]:not([href="/persons/new"])').first(); - const href = await firstLink.getAttribute('href'); - const personId = href!.split('/persons/')[1]; - await firstLink.click(); - await page.waitForSelector('[data-hydrated]'); - - // Co-correspondent chips link to /conversations?senderId=X&receiverId=Y - const chip = page.locator(`a[href^="/conversations?senderId=${personId}&receiverId="]`).first(); - if ((await chip.count()) > 0) { - const chipHref = await chip.getAttribute('href'); - expect(chipHref).toMatch(/\/conversations\?senderId=.+&receiverId=.+/); - } - }); -}); - -test.describe('Conversations', () => { - test('shows the empty state when no persons are selected', async ({ page }) => { - await page.goto('/conversations'); - await expect(page.getByText(/Wählen Sie zwei Personen aus/i)).toBeVisible(); - await page.screenshot({ path: 'test-results/e2e/conversations-empty.png' }); - }); - - test('nav link is active on the conversations page', async ({ page }) => { - await page.goto('/conversations'); - const navLink = page.getByRole('link', { name: 'Konversationen' }); - await expect(navLink).toHaveClass(/bg-nav-active/); - }); - - test('sort toggle changes the button label', async ({ page }) => { - await page.goto('/conversations'); - await page.waitForSelector('[data-hydrated]'); - const btn = page.getByRole('button', { name: /Sortierung/i }); - await expect(btn).toContainText('Neueste zuerst'); - await btn.click(); - await expect(page).toHaveURL(/dir=ASC/); - await expect(btn).toContainText('Älteste zuerst'); - await page.screenshot({ path: 'test-results/e2e/conversations-sort.png' }); - }); -}); - -test.describe('Conversations — enhancements', () => { - // Hans→Anna (1923) and Anna→Hans (1965) are seeded in DataInitializer - // Navigate directly by URL so the test doesn't rely on typeahead interaction - async function loadHansAnnaConversation(page: import('@playwright/test').Page) { - // Resolve person IDs from the persons list - await page.goto('/persons'); - const hansLink = page.getByRole('link', { name: /Hans Müller/ }).first(); - const hansHref = await hansLink.getAttribute('href'); - const hansId = hansHref!.split('/').pop()!; - - const annaLink = page.getByRole('link', { name: /Anna Schmidt/ }).first(); - const annaHref = await annaLink.getAttribute('href'); - const annaId = annaHref!.split('/').pop()!; - - await page.goto(`/conversations?senderId=${hansId}&receiverId=${annaId}`); - await page.waitForURL(/senderId=/); - } - - test('shows document count and year range summary when both persons are selected', async ({ - page - }) => { - await loadHansAnnaConversation(page); - // Hans→Anna (1923) + Anna→Hans (1965) = 2 documents, range 1923–1965 - await expect(page.getByTestId('conv-summary')).toContainText('2'); - await expect(page.getByTestId('conv-summary')).toContainText('1923'); - await expect(page.getByTestId('conv-summary')).toContainText('1965'); - await page.screenshot({ path: 'test-results/e2e/conversations-summary.png' }); - }); - - test('shows year dividers between documents from different years', async ({ page }) => { - await loadHansAnnaConversation(page); - // Expect at least two year dividers (1923 and 1965) - await expect(page.getByTestId('year-divider').first()).toBeVisible(); - const dividers = page.getByTestId('year-divider'); - const texts = await dividers.allTextContents(); - expect(texts.some((t) => t.includes('1923'))).toBe(true); - expect(texts.some((t) => t.includes('1965'))).toBe(true); - await page.screenshot({ path: 'test-results/e2e/conversations-year-dividers.png' }); - }); - - test('swap button switches sender and receiver and reloads', async ({ page }) => { - await loadHansAnnaConversation(page); - const url = new URL(page.url()); - const originalSenderId = url.searchParams.get('senderId')!; - const originalReceiverId = url.searchParams.get('receiverId')!; - - await page.getByTestId('conv-swap-btn').click(); - // Wait for the URL to reflect the swapped IDs (not just any URL with senderId=) - await page.waitForURL( - (url) => new URL(url).searchParams.get('senderId') === originalReceiverId - ); - - const swappedUrl = new URL(page.url()); - expect(swappedUrl.searchParams.get('senderId')).toBe(originalReceiverId); - expect(swappedUrl.searchParams.get('receiverId')).toBe(originalSenderId); - await page.screenshot({ path: 'test-results/e2e/conversations-swap.png' }); - }); - - test('shows "new document" link pre-filled with both persons when conversation is loaded', async ({ - page - }) => { - await loadHansAnnaConversation(page); - const url = new URL(page.url()); - const senderId = url.searchParams.get('senderId')!; - const receiverId = url.searchParams.get('receiverId')!; - - const link = page.getByTestId('conv-new-doc-link'); - await expect(link).toBeVisible(); - const href = await link.getAttribute('href'); - expect(href).toContain(`senderId=${senderId}`); - expect(href).toContain(`receiverId=${receiverId}`); - await page.screenshot({ path: 'test-results/e2e/conversations-new-doc-link.png' }); - }); - - test('does not show swap button or new document link when only one person is selected', async ({ - page - }) => { - await page.goto('/conversations'); - await page.waitForURL('/conversations'); - await expect(page.getByTestId('conv-swap-btn')).not.toBeVisible(); - await expect(page.getByTestId('conv-new-doc-link')).not.toBeVisible(); - }); -}); diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 3ba37d97..20dba446 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -136,8 +136,6 @@ "person_co_correspondents_heading": "Häufige Korrespondenten", "person_correspondents_hint": "klicken für Konversation", "person_show_more": "+ {count} weitere anzeigen", - "conv_heading": "Briefwechsel", - "conv_subtitle": "Briefwechsel einer Person durchsuchen — mit oder ohne Korrespondent.", "conv_label_person_a": "Person A (Absender)", "conv_label_person_b": "Korrespondent", "conv_label_from": "Zeitraum von", @@ -146,30 +144,18 @@ "conv_sort_newest": "Neueste zuerst", "conv_sort_oldest": "Älteste zuerst", "conv_empty_heading": "Wessen Briefe möchten Sie lesen?", - "conv_empty_text": "Wähle eine Person aus dem Archiv um deren Briefe zu sehen — mit oder ohne Korrespondent.", "conv_hero_crosslink": "Suchen Sie ein bestimmtes Dokument? → Zur Dokumentensuche", "conv_no_results_heading": "Keine Dokumente gefunden.", "conv_no_results_text": "Versuchen Sie, den Zeitraum anzupassen.", "conv_swap_btn": "Personen tauschen", - "conv_summary": "{count} Dokumente · {yearFrom}–{yearTo}", "conv_new_doc_link": "Neues Dokument in diesem Briefwechsel", - "conv_label_correspondent_optional": "Korrespondent", - "conv_hint_single_person": "Alle Briefe von {name} — wähle einen Korrespondenten oben um einzugrenzen", - "conv_hint_single_person_filtered": "Alle Briefe von {name} · {from}–{to} · {sortLabel}", - "conv_strip_period": "Zeitraum", - "conv_strip_from_placeholder": "Von…", - "conv_strip_to_placeholder": "Bis…", - "conv_strip_all_correspondents": "Alle Korrespondenten", "conv_strip_sort_newest": "Neueste", "conv_strip_sort_oldest": "Älteste", "conv_suggestions_heading": "Häufigste Korrespondenten", "conv_suggestions_all_label": "Alle Korrespondenten von {name}", "conv_letters_count": "{count} Briefe", - "conv_empty_search_placeholder": "Person suchen…", "conv_hero_divider": "oder", "conv_empty_recent_label": "Zuletzt geöffnet", - "conv_asym_sent": "{count} von {name} →", - "conv_asym_received": "{count} von {name} ←", "conv_no_party": "—", "admin_heading": "Admin Dashboard", "admin_tab_users": "Benutzer", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 9ffc814c..901adb29 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -136,8 +136,6 @@ "person_co_correspondents_heading": "Frequent correspondents", "person_correspondents_hint": "click to view conversation", "person_show_more": "+ {count} more", - "conv_heading": "Letters", - "conv_subtitle": "Browse a person's letters — with or without a correspondent.", "conv_label_person_a": "Person A (Sender)", "conv_label_person_b": "Correspondent", "conv_label_from": "Period from", @@ -146,30 +144,18 @@ "conv_sort_newest": "Newest first", "conv_sort_oldest": "Oldest first", "conv_empty_heading": "Whose letters would you like to read?", - "conv_empty_text": "Choose a person from the archive to see their letters — with or without a correspondent.", "conv_hero_crosslink": "Looking for a specific document? → Go to document search", "conv_no_results_heading": "No documents found.", "conv_no_results_text": "Try adjusting the time period.", "conv_swap_btn": "Swap persons", - "conv_summary": "{count} documents · {yearFrom}–{yearTo}", "conv_new_doc_link": "New document in this exchange", - "conv_label_correspondent_optional": "Correspondent", - "conv_hint_single_person": "All letters from {name} — choose a correspondent above to narrow down", - "conv_hint_single_person_filtered": "All letters from {name} · {from}–{to} · {sortLabel}", - "conv_strip_period": "Period", - "conv_strip_from_placeholder": "From…", - "conv_strip_to_placeholder": "To…", - "conv_strip_all_correspondents": "All correspondents", "conv_strip_sort_newest": "Newest", "conv_strip_sort_oldest": "Oldest", "conv_suggestions_heading": "Top correspondents", "conv_suggestions_all_label": "All correspondents of {name}", "conv_letters_count": "{count} letters", - "conv_empty_search_placeholder": "Search person…", "conv_hero_divider": "or", "conv_empty_recent_label": "Recently opened", - "conv_asym_sent": "{count} from {name} →", - "conv_asym_received": "{count} from {name} ←", "conv_no_party": "—", "admin_heading": "Admin Dashboard", "admin_tab_users": "Users", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 213bf53e..53d4f1a0 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -136,8 +136,6 @@ "person_co_correspondents_heading": "Corresponsales frecuentes", "person_correspondents_hint": "clic para ver conversación", "person_show_more": "+ {count} más", - "conv_heading": "Cartas", - "conv_subtitle": "Explore las cartas de una persona — con o sin corresponsal.", "conv_label_person_a": "Persona A (Remitente)", "conv_label_person_b": "Corresponsal", "conv_label_from": "Período desde", @@ -146,30 +144,18 @@ "conv_sort_newest": "Más reciente primero", "conv_sort_oldest": "Más antiguo primero", "conv_empty_heading": "¿De quién desea leer las cartas?", - "conv_empty_text": "Elige una persona del archivo para ver sus cartas — con o sin corresponsal.", "conv_hero_crosslink": "¿Busca un documento en particular? → Ir a la búsqueda", "conv_no_results_heading": "No se encontraron documentos.", "conv_no_results_text": "Intente ajustar el período de tiempo.", "conv_swap_btn": "Intercambiar personas", - "conv_summary": "{count} documentos · {yearFrom}–{yearTo}", "conv_new_doc_link": "Nuevo documento en este intercambio", - "conv_label_correspondent_optional": "Corresponsal", - "conv_hint_single_person": "Todas las cartas de {name} — elige un corresponsal arriba para filtrar", - "conv_hint_single_person_filtered": "Todas las cartas de {name} · {from}–{to} · {sortLabel}", - "conv_strip_period": "Período", - "conv_strip_from_placeholder": "Desde…", - "conv_strip_to_placeholder": "Hasta…", - "conv_strip_all_correspondents": "Todos los corresponsales", "conv_strip_sort_newest": "Más reciente", "conv_strip_sort_oldest": "Más antiguo", "conv_suggestions_heading": "Corresponsales frecuentes", "conv_suggestions_all_label": "Todos los corresponsales de {name}", "conv_letters_count": "{count} cartas", - "conv_empty_search_placeholder": "Buscar persona…", "conv_hero_divider": "o", "conv_empty_recent_label": "Recientemente abiertos", - "conv_asym_sent": "{count} de {name} →", - "conv_asym_received": "{count} de {name} ←", "conv_no_party": "—", "admin_heading": "Panel de administración", "admin_tab_users": "Usuarios", diff --git a/frontend/src/routes/conversations/+page.server.ts b/frontend/src/routes/conversations/+page.server.ts deleted file mode 100644 index d6f4cfd6..00000000 --- a/frontend/src/routes/conversations/+page.server.ts +++ /dev/null @@ -1,64 +0,0 @@ -import type { components } from '$lib/generated/api'; -import { createApiClient } from '$lib/api.server'; - -export async function load({ url, fetch }) { - const senderId = url.searchParams.get('senderId') || ''; - const receiverId = url.searchParams.get('receiverId') || ''; - const from = url.searchParams.get('from') || ''; - const to = url.searchParams.get('to') || ''; - const dir = url.searchParams.get('dir') || 'DESC'; - - const api = createApiClient(fetch); - - let documents: components['schemas']['Document'][] = []; - let senderName = ''; - let receiverName = ''; - - const requests: Promise[] = []; - - if (senderId && receiverId) { - requests.push( - api - .GET('/api/documents/conversation', { - params: { - query: { - senderId, - receiverId, - dir, - from: from || undefined, - to: to || undefined - } - } - }) - .then(({ data }) => { - documents = data ?? []; - }) - ); - } - - if (senderId) { - requests.push( - api.GET('/api/persons/{id}', { params: { path: { id: senderId } } }).then(({ data }) => { - const p = data as { displayName: string } | undefined; - if (p) senderName = p.displayName; - }) - ); - } - - if (receiverId) { - requests.push( - api.GET('/api/persons/{id}', { params: { path: { id: receiverId } } }).then(({ data }) => { - const p = data as { displayName: string } | undefined; - if (p) receiverName = p.displayName; - }) - ); - } - - await Promise.all(requests); - - return { - documents, - initialValues: { senderName, receiverName }, - filters: { senderId, receiverId, from, to, dir } - }; -} diff --git a/frontend/src/routes/conversations/+page.svelte b/frontend/src/routes/conversations/+page.svelte deleted file mode 100644 index 05b4b522..00000000 --- a/frontend/src/routes/conversations/+page.svelte +++ /dev/null @@ -1,104 +0,0 @@ - - -
- -
-

{m.conv_heading()}

-

- {m.conv_subtitle()} -

-
- - - - - {#if !senderId || !receiverId} -
-
- -
-

{m.conv_empty_heading()}

-

{m.conv_empty_text()}

-
- {:else if data.documents.length === 0} -
-

{m.conv_no_results_heading()}

-

{m.conv_no_results_text()}

-
- {:else} - - {/if} -
diff --git a/frontend/src/routes/conversations/ConversationFilterBar.svelte b/frontend/src/routes/conversations/ConversationFilterBar.svelte deleted file mode 100644 index da7b33c1..00000000 --- a/frontend/src/routes/conversations/ConversationFilterBar.svelte +++ /dev/null @@ -1,142 +0,0 @@ - - -
-
- -
- onapplyFilters()} - /> -
- - -
- -
- - -
- onapplyFilters()} - /> -
-
- -
- -
- - onapplyFilters()} - class="block w-full border-line py-2.5 text-sm shadow-sm focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring" - /> -
- - -
- - onapplyFilters()} - class="block w-full border-line py-2.5 text-sm shadow-sm focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring" - /> -
- - -
- -
-
-
diff --git a/frontend/src/routes/conversations/ConversationTimeline.svelte b/frontend/src/routes/conversations/ConversationTimeline.svelte deleted file mode 100644 index ecc9d14a..00000000 --- a/frontend/src/routes/conversations/ConversationTimeline.svelte +++ /dev/null @@ -1,160 +0,0 @@ - - - -
- {#if yearFrom !== null && yearTo !== null} -

- {m.conv_summary({ count: documents.length, yearFrom, yearTo })} -

- {:else} -

- {documents.length} -

- {/if} - {#if canWrite} - - - - - {m.conv_new_doc_link()} - - {/if} -
- - -
- - - -
-
- {#each documentGroups as group (group.label)} - {#if group.label} - - {/if} - {#each group.documents as doc (doc.id)} - {@const isRight = doc.sender?.id === senderId} - - -
- -
- - - - - - -
-

- {doc.title || doc.originalFilename} -

- - - - -
- - -
- - {doc.documentDate ? formatDate(doc.documentDate) : '—'} - - {#if doc.location} - - • {doc.location} - - {/if} -
- -
-
- {/each} - {/each} -
-
-
diff --git a/frontend/src/routes/conversations/page.svelte.spec.ts b/frontend/src/routes/conversations/page.svelte.spec.ts deleted file mode 100644 index 3b3ba356..00000000 --- a/frontend/src/routes/conversations/page.svelte.spec.ts +++ /dev/null @@ -1,164 +0,0 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { cleanup, render } from 'vitest-browser-svelte'; -import { page } from 'vitest/browser'; -import Page from './+page.svelte'; - -vi.mock('$app/navigation', () => ({ goto: vi.fn() })); - -afterEach(cleanup); - -// ─── Test data ──────────────────────────────────────────────────────────────── - -const baseData = { - user: undefined, - canWrite: true, - canAnnotate: false, - documents: [], - initialValues: { senderName: '', receiverName: '' }, - filters: { senderId: '', receiverId: '', from: '', to: '', dir: 'DESC' as const } -}; - -const withPersons = { - ...baseData, - filters: { ...baseData.filters, senderId: 'p1', receiverId: 'p2' } -}; - -const makeDoc = (overrides: Record = {}) => ({ - id: 'd1', - title: 'Testbrief', - originalFilename: 'testbrief.pdf', - status: 'UPLOADED' as const, - documentDate: '1923-04-12', - location: 'Berlin', - sender: { id: 'p1', firstName: 'Hans', lastName: 'Müller' }, - receivers: [{ id: 'p2', firstName: 'Anna', lastName: 'Schmidt' }], - tags: [], - transcription: undefined, - filePath: undefined, - createdAt: '1923-04-12T00:00:00Z', - updatedAt: '1923-04-12T00:00:00Z', - ...overrides -}); - -const withDocs = { - ...withPersons, - documents: [makeDoc()] -}; - -// ─── Empty state ────────────────────────────────────────────────────────────── - -describe('Conversations page – empty state', () => { - it('shows the empty-state heading when no persons are selected', async () => { - render(Page, { data: baseData }); - await expect.element(page.getByText(/Wessen Briefe möchten Sie lesen/i)).toBeInTheDocument(); - }); - - it('hides the swap button when no persons are selected', async () => { - render(Page, { data: baseData }); - // Button is always in the DOM (holds grid column width on desktop) but made invisible - await expect.element(page.getByTestId('conv-swap-btn')).toHaveClass('invisible'); - }); - - it('does not show the new document link when no persons are selected', async () => { - render(Page, { data: baseData }); - await expect.element(page.getByTestId('conv-new-doc-link')).not.toBeInTheDocument(); - }); -}); - -// ─── No results ─────────────────────────────────────────────────────────────── - -describe('Conversations page – no results', () => { - it('shows "no documents found" when both persons are selected but there are no documents', async () => { - render(Page, { data: withPersons }); - await expect.element(page.getByText(/Keine Dokumente gefunden/i)).toBeInTheDocument(); - }); -}); - -// ─── Swap button ────────────────────────────────────────────────────────────── - -describe('Conversations page – swap button', () => { - it('shows the swap button when both persons are selected', async () => { - render(Page, { data: withPersons }); - await expect.element(page.getByTestId('conv-swap-btn')).not.toHaveClass('invisible'); - }); - - it('calls goto with swapped sender and receiver when clicked', async () => { - const { goto } = await import('$app/navigation'); - vi.mocked(goto).mockClear(); - render(Page, { data: withPersons }); - document.querySelector('[data-testid="conv-swap-btn"]')!.click(); - expect(goto).toHaveBeenCalledWith(expect.stringContaining('senderId=p2'), expect.anything()); - expect(goto).toHaveBeenCalledWith(expect.stringContaining('receiverId=p1'), expect.anything()); - }); -}); - -// ─── Summary ────────────────────────────────────────────────────────────────── - -describe('Conversations page – summary', () => { - it('shows document count and year range when documents are loaded', async () => { - const data = { - ...withPersons, - documents: [ - makeDoc({ documentDate: '1923-04-12' }), - makeDoc({ id: 'd2', documentDate: '1965-08-03' }) - ] - }; - render(Page, { data }); - const summary = page.getByTestId('conv-summary'); - await expect.element(summary).toHaveTextContent('2'); - await expect.element(summary).toHaveTextContent('1923'); - await expect.element(summary).toHaveTextContent('1965'); - }); -}); - -// ─── Year dividers ──────────────────────────────────────────────────────────── - -describe('Conversations page – year dividers', () => { - it('renders a year divider for the first document', async () => { - render(Page, { data: withDocs }); - await expect.element(page.getByTestId('group-divider').first()).toHaveTextContent('1923'); - }); - - it('renders a divider for each new year in the document list', async () => { - const data = { - ...withPersons, - documents: [ - makeDoc({ documentDate: '1923-04-12' }), - makeDoc({ id: 'd2', documentDate: '1965-08-03' }) - ] - }; - render(Page, { data }); - await expect.element(page.getByTestId('group-divider').first()).toHaveTextContent('1923'); - await expect.element(page.getByTestId('group-divider').nth(1)).toHaveTextContent('1965'); - }); - - it('does not render a second divider for documents from the same year', async () => { - const data = { - ...withPersons, - documents: [ - makeDoc({ documentDate: '1923-04-12' }), - makeDoc({ id: 'd2', documentDate: '1923-09-01' }) - ] - }; - render(Page, { data }); - // Only one divider for 1923; 1965 divider should not appear - await expect.element(page.getByTestId('group-divider').first()).toHaveTextContent('1923'); - await expect.element(page.getByTestId('group-divider').nth(1)).not.toBeInTheDocument(); - }); -}); - -// ─── New document link ──────────────────────────────────────────────────────── - -describe('Conversations page – new document link', () => { - it('shows the link with correct href for a write user', async () => { - render(Page, { data: { ...withDocs, canWrite: true } }); - const link = page.getByTestId('conv-new-doc-link'); - await expect.element(link).toBeInTheDocument(); - await expect.element(link).toHaveAttribute('href', '/documents/new?senderId=p1&receiverId=p2'); - }); - - it('hides the link for a read-only user', async () => { - render(Page, { data: { ...withDocs, canWrite: false } }); - await expect.element(page.getByTestId('conv-new-doc-link')).not.toBeInTheDocument(); - }); -}); -- 2.49.1 From 4446e808753b0dcdee9bee73e1e533b65c70abab Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 12:46:04 +0200 Subject: [PATCH 02/19] test(actions): add defaultPrevented coverage for clickOutside (#195) The action already checks event.defaultPrevented before dispatching clickoutside, but that branch had no test. Add the missing case and add a one-line comment explaining why capture phase is used. Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/lib/actions/clickOutside.svelte.spec.ts | 12 ++++++++++++ frontend/src/lib/actions/clickOutside.ts | 1 + 2 files changed, 13 insertions(+) diff --git a/frontend/src/lib/actions/clickOutside.svelte.spec.ts b/frontend/src/lib/actions/clickOutside.svelte.spec.ts index 81917cb0..1616b463 100644 --- a/frontend/src/lib/actions/clickOutside.svelte.spec.ts +++ b/frontend/src/lib/actions/clickOutside.svelte.spec.ts @@ -51,6 +51,18 @@ describe('clickOutside action', () => { expect(fired).toBe(false); }); + it('does not dispatch clickoutside when event.defaultPrevented is true', () => { + const node = makeNode(); + const outside = makeNode(); + let fired = false; + node.addEventListener('clickoutside', () => (fired = true)); + clickOutside(node); + const event = new MouseEvent('click', { bubbles: true, cancelable: true }); + event.preventDefault(); + outside.dispatchEvent(event); + expect(fired).toBe(false); + }); + it('removes the listener on destroy', () => { const node = makeNode(); const outside = makeNode(); diff --git a/frontend/src/lib/actions/clickOutside.ts b/frontend/src/lib/actions/clickOutside.ts index ef7fc3a2..018b4736 100644 --- a/frontend/src/lib/actions/clickOutside.ts +++ b/frontend/src/lib/actions/clickOutside.ts @@ -5,6 +5,7 @@ export function clickOutside(node: HTMLElement): { destroy: () => void } { } } + // Capture phase (true) ensures this fires before any child stopPropagation() calls. document.addEventListener('click', handleClick, true); return { -- 2.49.1 From c50845bcfc619689aafbacdf0785a61ef81990e5 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 12:55:29 +0200 Subject: [PATCH 03/19] refactor(bell): migrate attachClickOutside to use:clickOutside action (#195) Replace the inline attachClickOutside attachment in NotificationBell with the shared use:clickOutside action from $lib/actions/clickOutside. The inline implementation was functionally identical to the existing action. Guard the onclickoutside handler so it only calls closeDropdown() when the notification panel is already open, preventing the bell button from stealing focus from other interactive elements (e.g. the user avatar menu). Co-Authored-By: Claude Sonnet 4.6 --- .../src/lib/components/NotificationBell.svelte | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/frontend/src/lib/components/NotificationBell.svelte b/frontend/src/lib/components/NotificationBell.svelte index d21ae303..a1bb7b04 100644 --- a/frontend/src/lib/components/NotificationBell.svelte +++ b/frontend/src/lib/components/NotificationBell.svelte @@ -2,6 +2,7 @@ import { onMount, onDestroy } from 'svelte'; import { goto } from '$app/navigation'; import { m } from '$lib/paraglide/messages.js'; +import { clickOutside } from '$lib/actions/clickOutside'; import { type NotificationItem, relativeTime, @@ -110,21 +111,6 @@ function attachFirstFocusable(node: HTMLButtonElement) { }; } -// Attachment: closes dropdown when clicking outside the wrapper element -function attachClickOutside(node: HTMLElement) { - const handleClick = (event: MouseEvent) => { - if (!node.contains(event.target as Node) && !event.defaultPrevented) { - if (open) { - open = false; - } - } - }; - document.addEventListener('click', handleClick, true); - return () => { - document.removeEventListener('click', handleClick, true); - }; -} - onMount(() => { fetchUnreadCount(); eventSource = new EventSource('/api/notifications/stream'); @@ -143,7 +129,7 @@ onDestroy(() => { -
+
{ if (open) closeDropdown(); }}>
{#if parsed.quote} diff --git a/frontend/src/lib/utils/notifications.spec.ts b/frontend/src/lib/utils/notifications.spec.ts index e1333d5a..294ea0ac 100644 --- a/frontend/src/lib/utils/notifications.spec.ts +++ b/frontend/src/lib/utils/notifications.spec.ts @@ -1,56 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { relativeTime, parseNotificationEvent } from '$lib/utils/notifications'; - -const rtf = new Intl.RelativeTimeFormat(undefined, { numeric: 'auto' }); - -function msAgo(ms: number, now: Date): string { - return new Date(now.getTime() - ms).toISOString(); -} - -describe('relativeTime', () => { - const now = new Date('2024-06-15T12:00:00.000Z'); - - it('should use minute bucket for timestamps under 60 seconds ago', () => { - const ts = msAgo(30_000, now); - expect(relativeTime(ts, now)).toBe(rtf.format(0, 'minute')); - }); - - it('should use minute bucket for exactly 59 minutes ago', () => { - const ts = msAgo(59 * 60_000, now); - expect(relativeTime(ts, now)).toBe(rtf.format(-59, 'minute')); - }); - - it('should use minute bucket for exactly 1 minute ago', () => { - const ts = msAgo(60_000, now); - expect(relativeTime(ts, now)).toBe(rtf.format(-1, 'minute')); - }); - - it('should use hour bucket for exactly 1 hour ago', () => { - const ts = msAgo(60 * 60_000, now); - expect(relativeTime(ts, now)).toBe(rtf.format(-1, 'hour')); - }); - - it('should use hour bucket for 23 hours ago', () => { - const ts = msAgo(23 * 60 * 60_000, now); - expect(relativeTime(ts, now)).toBe(rtf.format(-23, 'hour')); - }); - - it('should use day bucket for exactly 24 hours ago', () => { - const ts = msAgo(24 * 60 * 60_000, now); - expect(relativeTime(ts, now)).toBe(rtf.format(-1, 'day')); - }); - - it('should use day bucket for 6 days ago', () => { - const ts = msAgo(6 * 24 * 60 * 60_000, now); - expect(relativeTime(ts, now)).toBe(rtf.format(-6, 'day')); - }); - - it('should default now to current time when omitted', () => { - // Just verify it returns a non-empty string — exact value depends on runtime clock - const ts = new Date(Date.now() - 5 * 60_000).toISOString(); - expect(relativeTime(ts)).toBeTruthy(); - }); -}); +import { parseNotificationEvent } from '$lib/utils/notifications'; describe('parseNotificationEvent', () => { const valid = { diff --git a/frontend/src/lib/utils/notifications.ts b/frontend/src/lib/utils/notifications.ts index a58f1b11..a4feb005 100644 --- a/frontend/src/lib/utils/notifications.ts +++ b/frontend/src/lib/utils/notifications.ts @@ -10,18 +10,7 @@ export type NotificationItem = { documentTitle: string | null; }; -const rtf = new Intl.RelativeTimeFormat(undefined, { numeric: 'auto' }); - -export function relativeTime(isoString: string, now: Date = new Date()): string { - const diffMs = now.getTime() - new Date(isoString).getTime(); - const diffMin = Math.floor(diffMs / 60_000); - if (diffMin < 1) return rtf.format(0, 'minute'); - if (diffMin < 60) return rtf.format(-diffMin, 'minute'); - const diffH = Math.floor(diffMin / 60); - if (diffH < 24) return rtf.format(-diffH, 'hour'); - const diffD = Math.floor(diffH / 24); - return rtf.format(-diffD, 'day'); -} +export { relativeTime } from '$lib/utils/time'; export function parseNotificationEvent(raw: string): NotificationItem | null { try { diff --git a/frontend/src/lib/utils/time.spec.ts b/frontend/src/lib/utils/time.spec.ts new file mode 100644 index 00000000..f3d99a5a --- /dev/null +++ b/frontend/src/lib/utils/time.spec.ts @@ -0,0 +1,52 @@ +import { describe, it, expect } from 'vitest'; +import { m } from '$lib/paraglide/messages.js'; + +const { relativeTime } = await import('./time'); + +function msAgo(ms: number, now: Date): string { + return new Date(now.getTime() - ms).toISOString(); +} + +describe('relativeTime', () => { + const now = new Date('2024-06-15T12:00:00.000Z'); + + it('returns "just now" for timestamps under 60 seconds ago', () => { + const ts = msAgo(30_000, now); + expect(relativeTime(ts, now)).toBe(m.comment_time_just_now()); + }); + + it('returns 1-minute label for exactly 1 minute ago', () => { + const ts = msAgo(60_000, now); + expect(relativeTime(ts, now)).toBe(m.comment_time_minutes({ count: 1 })); + }); + + it('returns 59-minute label for exactly 59 minutes ago', () => { + const ts = msAgo(59 * 60_000, now); + expect(relativeTime(ts, now)).toBe(m.comment_time_minutes({ count: 59 })); + }); + + it('returns 1-hour label for exactly 1 hour ago', () => { + const ts = msAgo(60 * 60_000, now); + expect(relativeTime(ts, now)).toBe(m.comment_time_hours({ count: 1 })); + }); + + it('returns 23-hour label for 23 hours ago', () => { + const ts = msAgo(23 * 60 * 60_000, now); + expect(relativeTime(ts, now)).toBe(m.comment_time_hours({ count: 23 })); + }); + + it('returns 1-day label for exactly 24 hours ago', () => { + const ts = msAgo(24 * 60 * 60_000, now); + expect(relativeTime(ts, now)).toBe(m.comment_time_days({ count: 1 })); + }); + + it('returns 6-day label for 6 days ago', () => { + const ts = msAgo(6 * 24 * 60 * 60_000, now); + expect(relativeTime(ts, now)).toBe(m.comment_time_days({ count: 6 })); + }); + + it('defaults now to current time when omitted', () => { + const ts = new Date(Date.now() - 5 * 60_000).toISOString(); + expect(relativeTime(ts)).toBeTruthy(); + }); +}); diff --git a/frontend/src/lib/utils/time.ts b/frontend/src/lib/utils/time.ts new file mode 100644 index 00000000..bbe47ee6 --- /dev/null +++ b/frontend/src/lib/utils/time.ts @@ -0,0 +1,12 @@ +import { m } from '$lib/paraglide/messages.js'; + +export function relativeTime(isoString: string, now: Date = new Date()): string { + const diff = now.getTime() - new Date(isoString).getTime(); + const minutes = Math.floor(diff / 60_000); + if (minutes < 1) return m.comment_time_just_now(); + if (minutes < 60) return m.comment_time_minutes({ count: minutes }); + const hours = Math.floor(minutes / 60); + if (hours < 24) return m.comment_time_hours({ count: hours }); + const days = Math.floor(hours / 24); + return m.comment_time_days({ count: days }); +} -- 2.49.1 From 76d6f234b43a0e632ac81d14e6c02a7ddfee9c30 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 13:07:23 +0200 Subject: [PATCH 05/19] refactor(personFormat): replace getInitials(Person) with getInitials(name: string) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unify the initials-extraction logic: the new string-based getInitials() splits on whitespace, takes the first char of the first and last word uppercased — matching the pattern that was already inlined in CommentThread. Update PersonChip, DocumentMetadataDrawer, and CommentThread to use the shared function. Co-Authored-By: Claude Sonnet 4.6 --- .../src/lib/components/CommentThread.svelte | 9 +------ .../components/DocumentMetadataDrawer.svelte | 8 ++---- frontend/src/lib/components/PersonChip.svelte | 2 +- frontend/src/lib/utils/personFormat.spec.ts | 25 +++++++++++++++++++ frontend/src/lib/utils/personFormat.ts | 8 +++--- 5 files changed, 34 insertions(+), 18 deletions(-) diff --git a/frontend/src/lib/components/CommentThread.svelte b/frontend/src/lib/components/CommentThread.svelte index 6796cc3f..1d5ce5e5 100644 --- a/frontend/src/lib/components/CommentThread.svelte +++ b/frontend/src/lib/components/CommentThread.svelte @@ -5,6 +5,7 @@ import type { Comment } from '$lib/types'; import MentionEditor from '$lib/components/MentionEditor.svelte'; import { renderBody, extractContent } from '$lib/utils/mention'; import { relativeTime } from '$lib/utils/time'; +import { getInitials } from '$lib/utils/personFormat'; import type { MentionDTO } from '$lib/types'; type Props = { @@ -76,14 +77,6 @@ function isOwn(c: { authorId: string | null }): boolean { return currentUserId !== null && c.authorId === currentUserId; } -function getInitials(name: string): string { - return name - .split(/\s+/) - .slice(0, 2) - .map((w) => w.charAt(0).toUpperCase()) - .join(''); -} - function extractQuote(content: string): { quote: string | null; body: string } { const match = content.match(/^>\s*"(.+?)"\s*\n\n?([\s\S]*)$/); if (match) return { quote: match[1], body: match[2] }; diff --git a/frontend/src/lib/components/DocumentMetadataDrawer.svelte b/frontend/src/lib/components/DocumentMetadataDrawer.svelte index d057a993..a24a27d1 100644 --- a/frontend/src/lib/components/DocumentMetadataDrawer.svelte +++ b/frontend/src/lib/components/DocumentMetadataDrawer.svelte @@ -2,7 +2,7 @@ import { m } from '$lib/paraglide/messages.js'; import { formatDate } from '$lib/utils/date'; import { formatDocumentStatus } from '$lib/utils/documentStatusLabel'; -import { getInitials as calcInitials, personAvatarColor } from '$lib/utils/personFormat'; +import { getInitials, personAvatarColor } from '$lib/utils/personFormat'; type Person = { id: string; firstName?: string | null; lastName: string; displayName: string }; type Tag = { id: string; name: string }; @@ -32,10 +32,6 @@ let showAllReceivers = $state(false); const displayedReceivers = $derived(showAllReceivers ? receivers : visibleReceivers); -function getInitials(person: Person): string { - return calcInitials(person); -} - function getFullName(person: Person): string { return person.displayName; } @@ -51,7 +47,7 @@ function getFullName(person: Person): string { style="background-color: {personAvatarColor(person.id)}" aria-hidden="true" > - {getInitials(person)} + {getInitials(person.displayName)} {getFullName(person)} diff --git a/frontend/src/lib/components/PersonChip.svelte b/frontend/src/lib/components/PersonChip.svelte index 97796c00..89dd502a 100644 --- a/frontend/src/lib/components/PersonChip.svelte +++ b/frontend/src/lib/components/PersonChip.svelte @@ -12,7 +12,7 @@ let { person, abbreviated }: Props = $props(); const name = $derived(abbreviated ? abbreviateName(person) : person.displayName); const avatarColor = $derived(personAvatarColor(person.id)); -const initials = $derived(getInitials(person)); +const initials = $derived(getInitials(person.displayName)); { + it('returns first chars of first and last word uppercased', () => { + expect(getInitials('Marcel Raddatz')).toBe('MR'); + }); + + it('returns single char for a single-word name', () => { + expect(getInitials('Raddatz')).toBe('R'); + }); + + it('returns empty string for an empty name', () => { + expect(getInitials('')).toBe(''); + }); + + it('splits on whitespace only — hyphenated first word counts as one', () => { + expect(getInitials('Anna-Maria Raddatz')).toBe('AR'); + }); + + it('ignores extra whitespace between words', () => { + expect(getInitials(' Karl Raddatz ')).toBe('KR'); + }); +}); + // ─── abbreviateName ────────────────────────────────────────────────────────── describe('abbreviateName', () => { diff --git a/frontend/src/lib/utils/personFormat.ts b/frontend/src/lib/utils/personFormat.ts index 241339f3..d60023e0 100644 --- a/frontend/src/lib/utils/personFormat.ts +++ b/frontend/src/lib/utils/personFormat.ts @@ -18,9 +18,11 @@ function djb2(str: string): number { return Math.abs(hash); } -export function getInitials(person: Person): string { - if (person.firstName) return `${person.firstName[0]}${person.lastName[0]}`.toUpperCase(); - return person.lastName.substring(0, 2).toUpperCase(); +export function getInitials(name: string): string { + const words = name.trim().split(/\s+/).filter(Boolean); + if (words.length === 0) return ''; + if (words.length === 1) return words[0].charAt(0).toUpperCase(); + return (words[0].charAt(0) + words[words.length - 1].charAt(0)).toUpperCase(); } export function abbreviateName(person: Person): string { -- 2.49.1 From 8be876492cd71983b2c83bedf21c25a72c369285 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 13:10:44 +0200 Subject: [PATCH 06/19] refactor(date): consolidate formatDate in date.ts with optional format param Add format?: 'short'|'long' (default 'long') to date.ts formatDate and remove the duplicate from personFormat.ts. Update DocumentTopBar to import from date.ts directly. Move the formatDate tests from personFormat.spec to date.spec. Co-Authored-By: Claude Sonnet 4.6 --- .../src/lib/components/DocumentTopBar.svelte | 2 +- frontend/src/lib/utils/date.spec.ts | 22 ++++++++++++++++++- frontend/src/lib/utils/date.ts | 13 +++++++++-- frontend/src/lib/utils/personFormat.spec.ts | 2 +- frontend/src/lib/utils/personFormat.ts | 17 +------------- 5 files changed, 35 insertions(+), 21 deletions(-) diff --git a/frontend/src/lib/components/DocumentTopBar.svelte b/frontend/src/lib/components/DocumentTopBar.svelte index c7604e7c..995077a4 100644 --- a/frontend/src/lib/components/DocumentTopBar.svelte +++ b/frontend/src/lib/components/DocumentTopBar.svelte @@ -1,7 +1,7 @@ + +
+ {m.admin_unsaved_warning()} + +
diff --git a/frontend/src/lib/hooks/__tests__/useUnsavedWarning.svelte.test.ts b/frontend/src/lib/hooks/__tests__/useUnsavedWarning.svelte.test.ts new file mode 100644 index 00000000..9225a956 --- /dev/null +++ b/frontend/src/lib/hooks/__tests__/useUnsavedWarning.svelte.test.ts @@ -0,0 +1,95 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Capture the beforeNavigate callback so tests can simulate navigation events +let registeredBeforeNavigate: + | ((nav: { cancel: () => void; to: { url: { href: string } } | null }) => void) + | null = null; + +const mockGoto = vi.fn(); + +vi.mock('$app/navigation', () => ({ + beforeNavigate: vi.fn((fn: typeof registeredBeforeNavigate) => { + registeredBeforeNavigate = fn; + }), + goto: mockGoto +})); + +const { createUnsavedWarning } = await import('../useUnsavedWarning.svelte'); + +function simulateNavigate(href: string | null = '/somewhere') { + const cancel = vi.fn(); + registeredBeforeNavigate?.({ + cancel, + to: href ? { url: { href } } : null + }); + return cancel; +} + +beforeEach(() => { + registeredBeforeNavigate = null; + mockGoto.mockClear(); +}); + +describe('createUnsavedWarning', () => { + it('isDirty starts false', () => { + const w = createUnsavedWarning(); + expect(w.isDirty).toBe(false); + }); + + it('markDirty sets isDirty to true', () => { + const w = createUnsavedWarning(); + w.markDirty(); + expect(w.isDirty).toBe(true); + }); + + it('markDirty hides any existing warning banner', () => { + const w = createUnsavedWarning(); + // Simulate a navigation event that showed the banner + w.markDirty(); + simulateNavigate(); + expect(w.showUnsavedWarning).toBe(true); + // Typing again should hide the banner (form input re-triggers markDirty) + w.markDirty(); + expect(w.showUnsavedWarning).toBe(false); + }); + + it('beforeNavigate cancels and shows banner when dirty', () => { + const w = createUnsavedWarning(); + w.markDirty(); + const cancel = simulateNavigate('/admin/users'); + expect(cancel).toHaveBeenCalled(); + expect(w.showUnsavedWarning).toBe(true); + }); + + it('beforeNavigate stores the target URL', () => { + const w = createUnsavedWarning(); + w.markDirty(); + simulateNavigate('/admin/users'); + expect(w.discardTarget).toBe('/admin/users'); + }); + + it('beforeNavigate does not cancel when not dirty', () => { + createUnsavedWarning(); + const cancel = simulateNavigate('/admin/users'); + expect(cancel).not.toHaveBeenCalled(); + }); + + it('discard resets state and navigates to target', () => { + const w = createUnsavedWarning(); + w.markDirty(); + simulateNavigate('/admin/tags'); + w.discard(); + expect(w.isDirty).toBe(false); + expect(w.showUnsavedWarning).toBe(false); + expect(mockGoto).toHaveBeenCalledWith('/admin/tags'); + }); + + it('clearOnSuccess resets isDirty and warning', () => { + const w = createUnsavedWarning(); + w.markDirty(); + simulateNavigate('/somewhere'); + w.clearOnSuccess(); + expect(w.isDirty).toBe(false); + expect(w.showUnsavedWarning).toBe(false); + }); +}); diff --git a/frontend/src/lib/hooks/useUnsavedWarning.svelte.ts b/frontend/src/lib/hooks/useUnsavedWarning.svelte.ts new file mode 100644 index 00000000..b43bf7e5 --- /dev/null +++ b/frontend/src/lib/hooks/useUnsavedWarning.svelte.ts @@ -0,0 +1,46 @@ +import { beforeNavigate, goto } from '$app/navigation'; + +export function createUnsavedWarning() { + let isDirty = $state(false); + let showUnsavedWarning = $state(false); + let discardTarget: string | null = $state(null); + + beforeNavigate(({ cancel, to }) => { + if (isDirty) { + cancel(); + showUnsavedWarning = true; + discardTarget = to?.url.href ?? null; + } + }); + + function markDirty() { + isDirty = true; + showUnsavedWarning = false; + } + + function discard() { + isDirty = false; + showUnsavedWarning = false; + if (discardTarget) goto(discardTarget); + } + + function clearOnSuccess() { + isDirty = false; + showUnsavedWarning = false; + } + + return { + get isDirty() { + return isDirty; + }, + get showUnsavedWarning() { + return showUnsavedWarning; + }, + get discardTarget() { + return discardTarget; + }, + markDirty, + discard, + clearOnSuccess + }; +} diff --git a/frontend/src/routes/admin/groups/[id]/+page.svelte b/frontend/src/routes/admin/groups/[id]/+page.svelte index 898a8214..d17d102f 100644 --- a/frontend/src/routes/admin/groups/[id]/+page.svelte +++ b/frontend/src/routes/admin/groups/[id]/+page.svelte @@ -1,16 +1,15 @@ @@ -53,23 +41,8 @@ $effect(() => {
- {#if showUnsavedWarning} -
- {m.admin_unsaved_warning()} - -
+ {#if unsaved.showUnsavedWarning} + {/if} {#if form?.success}
@@ -88,10 +61,7 @@ $effect(() => { method="POST" action="?/update" use:enhance - oninput={() => { - isDirty = true; - showUnsavedWarning = false; - }} + oninput={unsaved.markDirty} class="mb-5" >
diff --git a/frontend/src/routes/admin/users/[id]/+page.svelte b/frontend/src/routes/admin/users/[id]/+page.svelte index c0e8f8db..161e63ad 100644 --- a/frontend/src/routes/admin/users/[id]/+page.svelte +++ b/frontend/src/routes/admin/users/[id]/+page.svelte @@ -1,21 +1,20 @@ @@ -76,23 +64,8 @@ $effect(() => {
- {#if showUnsavedWarning} -
- {m.admin_unsaved_warning()} - -
+ {#if unsaved.showUnsavedWarning} + {/if} {#if form?.success}
@@ -109,10 +82,7 @@ $effect(() => { id="edit-user-form" method="POST" use:enhance - oninput={() => { - isDirty = true; - showUnsavedWarning = false; - }} + oninput={unsaved.markDirty} class="space-y-5" > -- 2.49.1 From fe6c247882d5d1def0df26c19ca88b2bfb95fa7c Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 13:54:42 +0200 Subject: [PATCH 10/19] refactor(admin): extract EntityNavSection to eliminate nav markup repetition (#197) Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/routes/admin/EntityNav.svelte | 520 +++++------------- .../src/routes/admin/EntityNavSection.svelte | 90 +++ .../admin/EntityNavSection.svelte.spec.ts | 140 +++++ 3 files changed, 365 insertions(+), 385 deletions(-) create mode 100644 frontend/src/routes/admin/EntityNavSection.svelte create mode 100644 frontend/src/routes/admin/EntityNavSection.svelte.spec.ts diff --git a/frontend/src/routes/admin/EntityNav.svelte b/frontend/src/routes/admin/EntityNav.svelte index bffc5b76..a8db2d2c 100644 --- a/frontend/src/routes/admin/EntityNav.svelte +++ b/frontend/src/routes/admin/EntityNav.svelte @@ -3,6 +3,7 @@ import { tick } from 'svelte'; import { fly } from 'svelte/transition'; import { page } from '$app/state'; import { m } from '$lib/paraglide/messages.js'; +import EntityNavSection from './EntityNavSection.svelte'; let { userCount, @@ -51,6 +52,76 @@ function handleKeydown(event: KeyboardEvent) { } +{#snippet usersIcon()} + +{/snippet} + +{#snippet groupsIcon()} + +{/snippet} + +{#snippet tagsIcon()} + +{/snippet} + +{#snippet systemIcon()} + +{/snippet} + - - - + label={m.admin_tab_users()} + isActive={isActive('users')} + count={userCount} + onTabletTrigger={openFlyout} + icon={usersIcon} + /> {/if} {#if canManagePermissions} - - - - + label={m.admin_tab_groups()} + isActive={isActive('groups')} + count={groupCount} + onTabletTrigger={openFlyout} + icon={groupsIcon} + /> {/if} {#if canManageTags} - - - - + label={m.admin_tab_tags()} + isActive={isActive('tags')} + count={tagCount} + onTabletTrigger={openFlyout} + icon={tagsIcon} + /> {/if}
{#if canRunMaintenance} - - - - + label={m.admin_tab_system()} + isActive={isActive('system')} + topBorder={true} + onTabletTrigger={openFlyout} + icon={systemIcon} + /> {/if} @@ -360,156 +213,53 @@ function handleKeydown(event: KeyboardEvent) {
{#if canManageUsers} - - - - {userCount} - - - {m.admin_tab_users()} - - + label={m.admin_tab_users()} + isActive={isActive('users')} + count={userCount} + onFlyoutClick={closeFlyout} + icon={usersIcon} + /> {/if} {#if canManagePermissions} - - - - {groupCount} - - - {m.admin_tab_groups()} - - + label={m.admin_tab_groups()} + isActive={isActive('groups')} + count={groupCount} + onFlyoutClick={closeFlyout} + icon={groupsIcon} + /> {/if} {#if canManageTags} - - - - {tagCount} - - - {m.admin_tab_tags()} - - + label={m.admin_tab_tags()} + isActive={isActive('tags')} + count={tagCount} + onFlyoutClick={closeFlyout} + icon={tagsIcon} + /> {/if}
{#if canRunMaintenance} - - - - {m.admin_tab_system()} - - + label={m.admin_tab_system()} + isActive={isActive('system')} + topBorder={true} + onFlyoutClick={closeFlyout} + icon={systemIcon} + /> {/if}
{/if} diff --git a/frontend/src/routes/admin/EntityNavSection.svelte b/frontend/src/routes/admin/EntityNavSection.svelte new file mode 100644 index 00000000..c255403e --- /dev/null +++ b/frontend/src/routes/admin/EntityNavSection.svelte @@ -0,0 +1,90 @@ + + +{#if variant === 'sidebar'} + + + + + +{:else} + + + {@render icon()} + {#if count !== undefined} + {count} + {/if} + {label} + +{/if} diff --git a/frontend/src/routes/admin/EntityNavSection.svelte.spec.ts b/frontend/src/routes/admin/EntityNavSection.svelte.spec.ts new file mode 100644 index 00000000..0387d638 --- /dev/null +++ b/frontend/src/routes/admin/EntityNavSection.svelte.spec.ts @@ -0,0 +1,140 @@ +import { afterEach, describe, it, expect } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { page } from 'vitest/browser'; +import { createRawSnippet } from 'svelte'; +import EntityNavSection from './EntityNavSection.svelte'; + +afterEach(cleanup); + +const testIcon = createRawSnippet(() => ({ + render: () => ``, + setup: () => {} +})); + +const baseProps = { + href: '/admin/users', + label: 'Benutzer', + icon: testIcon +}; + +describe('EntityNavSection — sidebar variant (default)', () => { + it('tablet button has border-brand-mint class when isActive=true', async () => { + render(EntityNavSection, { ...baseProps, isActive: true }); + const button = document.querySelector('button[data-flyout-trigger]')!; + expect(button.className).toContain('border-brand-mint'); + }); + + it('tablet button has border-transparent class when isActive=false', async () => { + render(EntityNavSection, { ...baseProps, isActive: false }); + const button = document.querySelector('button[data-flyout-trigger]')!; + expect(button.className).toContain('border-transparent'); + }); + + it('renders count span when count is provided', async () => { + render(EntityNavSection, { ...baseProps, isActive: false, count: 42 }); + // Sidebar renders two elements (tablet button + desktop link), each with a count span + const countSpans = document.querySelectorAll('span'); + const countTexts = Array.from(countSpans).filter((s) => s.textContent?.trim() === '42'); + expect(countTexts.length).toBeGreaterThanOrEqual(1); + }); + + it('does not render count span when count is undefined', async () => { + render(EntityNavSection, { ...baseProps, isActive: false }); + // No numeric count element — the label text is present but no count span + const spans = document.querySelectorAll('button[data-flyout-trigger] span'); + expect(spans.length).toBe(0); + }); + + it('desktop link has hidden and lg:flex classes', async () => { + render(EntityNavSection, { ...baseProps, isActive: false }); + const link = document.querySelector('a[href="/admin/users"]')!; + expect(link.className).toContain('hidden'); + expect(link.className).toContain('lg:flex'); + }); + + it('desktop link has aria-current=page when isActive=true', async () => { + render(EntityNavSection, { ...baseProps, isActive: true }); + await expect + .element(page.getByRole('link', { name: 'Benutzer' })) + .toHaveAttribute('aria-current', 'page'); + }); + + it('desktop link does not have aria-current when isActive=false', async () => { + render(EntityNavSection, { ...baseProps, isActive: false }); + await expect + .element(page.getByRole('link', { name: 'Benutzer' })) + .not.toHaveAttribute('aria-current'); + }); + + it('renders the icon in the tablet button', async () => { + render(EntityNavSection, { ...baseProps, isActive: false }); + const button = document.querySelector('button[data-flyout-trigger]')!; + expect(button.querySelector('svg')).not.toBeNull(); + }); + + it('renders count in desktop link when count is provided', async () => { + render(EntityNavSection, { ...baseProps, isActive: false, count: 7 }); + const link = document.querySelector('a[href="/admin/users"]')!; + expect(link.textContent).toContain('7'); + }); +}); + +describe('EntityNavSection — topBorder prop', () => { + it('tablet button has border-l-transparent (not border-transparent) when topBorder=true and inactive', async () => { + render(EntityNavSection, { ...baseProps, isActive: false, topBorder: true }); + const button = document.querySelector('button[data-flyout-trigger]')!; + expect(button.className).toContain('border-l-transparent'); + expect(button.className).not.toContain('border-transparent hover:bg-white/5'); + }); + + it('tablet button still has border-brand-mint when topBorder=true and isActive=true', async () => { + render(EntityNavSection, { ...baseProps, isActive: true, topBorder: true }); + const button = document.querySelector('button[data-flyout-trigger]')!; + expect(button.className).toContain('border-brand-mint'); + }); +}); + +describe('EntityNavSection — flyout variant', () => { + it('renders a single anchor element (no button) in flyout variant', async () => { + render(EntityNavSection, { ...baseProps, isActive: false, variant: 'flyout' }); + expect(document.querySelector('button[data-flyout-trigger]')).toBeNull(); + expect(document.querySelector('a[href="/admin/users"]')).not.toBeNull(); + }); + + it('flyout link has border-brand-mint class when isActive=true', async () => { + render(EntityNavSection, { ...baseProps, isActive: true, variant: 'flyout' }); + const link = document.querySelector('a[href="/admin/users"]')!; + expect(link.className).toContain('border-brand-mint'); + }); + + it('flyout link has border-transparent class when isActive=false', async () => { + render(EntityNavSection, { ...baseProps, isActive: false, variant: 'flyout' }); + const link = document.querySelector('a[href="/admin/users"]')!; + expect(link.className).toContain('border-transparent'); + }); + + it('flyout link shows count when count=42', async () => { + render(EntityNavSection, { ...baseProps, isActive: false, variant: 'flyout', count: 42 }); + await expect.element(page.getByText('42')).toBeInTheDocument(); + }); + + it('flyout link has aria-current=page when isActive=true', async () => { + render(EntityNavSection, { ...baseProps, isActive: true, variant: 'flyout' }); + const link = document.querySelector('a[href="/admin/users"]')!; + expect(link.getAttribute('aria-current')).toBe('page'); + }); + + it('flyout link calls onFlyoutClick when clicked', async () => { + let called = false; + render(EntityNavSection, { + ...baseProps, + isActive: false, + variant: 'flyout', + onFlyoutClick: () => { + called = true; + } + }); + document.querySelector('a[href="/admin/users"]')!.click(); + expect(called).toBe(true); + }); +}); -- 2.49.1 From bc3fec11a9c9e1f7e95e69de2474b10643f666c2 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 14:23:25 +0200 Subject: [PATCH 11/19] refactor(comments): extract CommentMessage component from CommentThread (#198) Co-Authored-By: Claude Sonnet 4.6 --- .../src/lib/components/CommentMessage.svelte | 111 ++++++++++++++++++ .../components/CommentMessage.svelte.spec.ts | 85 ++++++++++++++ .../src/lib/components/CommentThread.svelte | 110 +++-------------- frontend/src/lib/types.ts | 10 ++ frontend/src/lib/utils/comment.spec.ts | 40 +++++++ frontend/src/lib/utils/comment.ts | 5 + 6 files changed, 265 insertions(+), 96 deletions(-) create mode 100644 frontend/src/lib/components/CommentMessage.svelte create mode 100644 frontend/src/lib/components/CommentMessage.svelte.spec.ts create mode 100644 frontend/src/lib/utils/comment.spec.ts create mode 100644 frontend/src/lib/utils/comment.ts diff --git a/frontend/src/lib/components/CommentMessage.svelte b/frontend/src/lib/components/CommentMessage.svelte new file mode 100644 index 00000000..f32e7d7d --- /dev/null +++ b/frontend/src/lib/components/CommentMessage.svelte @@ -0,0 +1,111 @@ + + +
+ +
+ {getInitials(message.authorName)} +
+ + +
+ +
+ {message.authorName} + {#if wasEdited} + {relativeTime(message.updatedAt)} {m.comment_edited_label()} + {:else} + {relativeTime(message.createdAt)} + {/if} +
+ + + {#if parsed.quote} +
+ “{parsed.quote}” +
+ {/if} + + + {#if isEditing} + +
Enter speichern · Esc abbrechen
+ {:else} + + +
{ if (isOwn) onEdit(); }}> +

+ + {@html renderBody(parsed.body, message.mentionDTOs ?? [])} +

+ {#if isOwn} + + {/if} +
+ {/if} +
+
diff --git a/frontend/src/lib/components/CommentMessage.svelte.spec.ts b/frontend/src/lib/components/CommentMessage.svelte.spec.ts new file mode 100644 index 00000000..9c1f7d75 --- /dev/null +++ b/frontend/src/lib/components/CommentMessage.svelte.spec.ts @@ -0,0 +1,85 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { page, userEvent } from 'vitest/browser'; +import CommentMessage from './CommentMessage.svelte'; +import type { FlatMessage } from '$lib/types'; + +afterEach(cleanup); + +const baseMsg: FlatMessage = { + id: 'msg-1', + authorId: 'user-1', + authorName: 'Anna Müller', + content: 'Hello world', + createdAt: new Date(Date.now() - 5 * 60_000).toISOString(), + updatedAt: new Date(Date.now() - 5 * 60_000).toISOString() +}; + +function defaultProps(overrides: Partial[1]> = {}) { + return { + message: baseMsg, + isOwn: false, + isEditing: false, + editText: '', + onEdit: vi.fn(), + onDelete: vi.fn(), + onEditTextChange: vi.fn(), + onEditKeydown: vi.fn(), + ...overrides + }; +} + +describe('CommentMessage', () => { + it('renders author name', async () => { + render(CommentMessage, defaultProps()); + await expect.element(page.getByText('Anna Müller')).toBeInTheDocument(); + }); + + it('renders initials in avatar', async () => { + render(CommentMessage, defaultProps()); + await expect.element(page.getByText('AM')).toBeInTheDocument(); + }); + + it('renders message body', async () => { + render(CommentMessage, defaultProps()); + await expect.element(page.getByText('Hello world')).toBeInTheDocument(); + }); + + it('renders quoted section when content contains a quote', async () => { + render( + CommentMessage, + defaultProps({ + message: { ...baseMsg, content: '> "Interesting passage"\n\nMy reply' } + }) + ); + await expect.element(page.getByText(/Interesting passage/)).toBeInTheDocument(); + await expect.element(page.getByText('My reply')).toBeInTheDocument(); + }); + + it('does not show delete button for messages not owned by current user', async () => { + render(CommentMessage, defaultProps({ isOwn: false })); + await expect.element(page.getByRole('button')).not.toBeInTheDocument(); + }); + + it('shows delete button for own messages', async () => { + render(CommentMessage, defaultProps({ isOwn: true })); + await expect.element(page.getByRole('button')).toBeInTheDocument(); + }); + + it('calls onDelete when delete button is clicked', async () => { + const onDelete = vi.fn(); + render(CommentMessage, defaultProps({ isOwn: true, onDelete })); + await userEvent.click(page.getByRole('button')); + expect(onDelete).toHaveBeenCalled(); + }); + + it('shows edit textarea when isEditing is true', async () => { + render( + CommentMessage, + defaultProps({ isOwn: true, isEditing: true, editText: 'current edit text' }) + ); + const textarea = page.getByRole('textbox'); + await expect.element(textarea).toBeInTheDocument(); + await expect.element(textarea).toHaveValue('current edit text'); + }); +}); diff --git a/frontend/src/lib/components/CommentThread.svelte b/frontend/src/lib/components/CommentThread.svelte index 1d5ce5e5..f943c671 100644 --- a/frontend/src/lib/components/CommentThread.svelte +++ b/frontend/src/lib/components/CommentThread.svelte @@ -1,13 +1,10 @@ + +
+ +
+ + + {#if totalPages > 0} + + {currentPage} / {totalPages} + + {/if} + + +
+ + +
+ + +
+ + + {#if annotationCount > 0} + + {/if} +
diff --git a/frontend/src/lib/components/PdfViewer.svelte b/frontend/src/lib/components/PdfViewer.svelte index 359c9700..87a651f3 100644 --- a/frontend/src/lib/components/PdfViewer.svelte +++ b/frontend/src/lib/components/PdfViewer.svelte @@ -1,6 +1,7 @@ {#if !url}

Keine Datei vorhanden

-{:else if error} +{:else if renderer.error}

Fehler beim Laden der PDF

{annotationUpdateError}
{/if} - -
- -
- - {#if totalPages > 0} - - {currentPage} / {totalPages} - - {/if} - - -
- - -
- - -
- - - {#if annotations.length > 0} - - {/if} -
+ renderer.prevPage()} + onNext={() => renderer.nextPage()} + onZoomIn={() => renderer.zoomIn()} + onZoomOut={() => renderer.zoomOut()} + onToggleAnnotations={() => (showAnnotations = !showAnnotations)} + />
- {#if loading} + {#if renderer.loading}
@@ -501,7 +254,9 @@ function zoomOut() { >
{#if showAnnotations} a.pageNumber === currentPage)} + annotations={visibleAnnotations.filter( + (a) => a.pageNumber === renderer.currentPage + )} canDraw={transcribeMode} color={TRANSCRIPTION_COLOR} blockNumbers={blockNumbers} diff --git a/frontend/src/lib/hooks/__tests__/usePdfRenderer.svelte.test.ts b/frontend/src/lib/hooks/__tests__/usePdfRenderer.svelte.test.ts new file mode 100644 index 00000000..d36b5c66 --- /dev/null +++ b/frontend/src/lib/hooks/__tests__/usePdfRenderer.svelte.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect } from 'vitest'; +import { createPdfRenderer } from '../usePdfRenderer.svelte'; + +// Note: init() and loadDocument() require pdfjsLib (browser module). +// These tests cover pure state logic only — bounds clamping and zoom limits. + +describe('createPdfRenderer', () => { + it('starts at page 1 with scale 1.5 and no error', () => { + const r = createPdfRenderer(); + expect(r.currentPage).toBe(1); + expect(r.scale).toBe(1.5); + expect(r.totalPages).toBe(0); + expect(r.loading).toBe(false); + expect(r.error).toBeNull(); + expect(r.isLoaded).toBe(false); + expect(r.pdfjsReady).toBe(false); + }); + + it('prevPage does not go below page 1', () => { + const r = createPdfRenderer(); + r.prevPage(); + expect(r.currentPage).toBe(1); + }); + + it('nextPage does not exceed totalPages', () => { + const r = createPdfRenderer(); + // totalPages = 0, so 1 < 0 is false → stays at 1 + r.nextPage(); + expect(r.currentPage).toBe(1); + }); + + it('goToPage does not navigate when n > totalPages', () => { + const r = createPdfRenderer(); + r.goToPage(5); + expect(r.currentPage).toBe(1); + }); + + it('goToPage does not navigate when n < 1', () => { + const r = createPdfRenderer(); + r.goToPage(0); + expect(r.currentPage).toBe(1); + }); + + it('zoomIn increases scale by 0.25', () => { + const r = createPdfRenderer(); + r.zoomIn(); + expect(r.scale).toBeCloseTo(1.75); + }); + + it('zoomOut decreases scale by 0.25', () => { + const r = createPdfRenderer(); + r.zoomOut(); + expect(r.scale).toBeCloseTo(1.25); + }); + + it('zoomOut does not go below 0.5', () => { + const r = createPdfRenderer(); + for (let i = 0; i < 20; i++) r.zoomOut(); + expect(r.scale).toBeCloseTo(0.5); + }); + + it('loadDocument is a no-op when pdfjsLib not initialized', async () => { + const r = createPdfRenderer(); + await r.loadDocument('/some/path'); + // No-op because pdfjsLib is null (init not called) + expect(r.error).toBeNull(); + expect(r.loading).toBe(false); + }); +}); diff --git a/frontend/src/lib/hooks/usePdfRenderer.svelte.ts b/frontend/src/lib/hooks/usePdfRenderer.svelte.ts new file mode 100644 index 00000000..87b37f8c --- /dev/null +++ b/frontend/src/lib/hooks/usePdfRenderer.svelte.ts @@ -0,0 +1,203 @@ +import type { PDFDocumentProxy, RenderTask } from 'pdfjs-dist'; + +export function createPdfRenderer() { + // Reactive state — exposed via getters + let currentPage = $state(1); + let totalPages = $state(0); + let scale = $state(1.5); + let loading = $state(false); + let error = $state(null); + let pdfjsReady = $state(false); + + // Internal mutable refs — NOT $state to avoid reactive loops + let pdfDoc: PDFDocumentProxy | null = null; + let canvasEl: HTMLCanvasElement | null = null; + let textLayerEl: HTMLDivElement | null = null; + let renderTask: RenderTask | null = null; + let textLayerInstance: { cancel: () => void } | null = null; + let pdfjsLib: typeof import('pdfjs-dist') | null = null; + + async function init(): Promise { + const [lib, { default: workerUrl }] = await Promise.all([ + import('pdfjs-dist'), + import('pdfjs-dist/build/pdf.worker.min.mjs?url') + ]); + lib.GlobalWorkerOptions.workerSrc = workerUrl; + pdfjsLib = lib; + pdfjsReady = true; + } + + function setElements(canvas: HTMLCanvasElement, textLayer: HTMLDivElement): void { + canvasEl = canvas; + textLayerEl = textLayer; + } + + async function loadDocument(src: string): Promise { + if (!pdfjsLib) return; + loading = true; + error = null; + pdfDoc = null; + currentPage = 1; + totalPages = 0; + + try { + const loadingTask = pdfjsLib.getDocument(src); + const doc = await loadingTask.promise; + pdfDoc = doc; + totalPages = doc.numPages; + } catch (e) { + error = e instanceof Error ? e.message : 'Failed to load PDF'; + } finally { + loading = false; + } + } + + async function renderCurrentPage(): Promise { + if (!pdfjsLib || !canvasEl || !textLayerEl || !pdfDoc) return; + + if (renderTask) { + renderTask.cancel(); + renderTask = null; + } + if (textLayerInstance) { + textLayerInstance.cancel(); + textLayerInstance = null; + } + + let page; + try { + page = await pdfDoc.getPage(currentPage); + } catch { + return; + } + + const dpr = window.devicePixelRatio || 1; + const viewport = page.getViewport({ scale: scale * dpr }); + + const canvas = canvasEl; + const ctx = canvas.getContext('2d'); + if (!ctx) return; + + canvas.width = viewport.width; + canvas.height = viewport.height; + canvas.style.width = `${viewport.width / dpr}px`; + canvas.style.height = `${viewport.height / dpr}px`; + + const task = page.render({ canvas, canvasContext: ctx, viewport }); + renderTask = task; + + try { + await task.promise; + } catch (e: unknown) { + if ( + typeof e === 'object' && + e !== null && + 'name' in e && + (e as { name: string }).name === 'RenderingCancelledException' + ) + return; + return; + } + renderTask = null; + + const textDiv = textLayerEl; + if (!textDiv) return; + textDiv.innerHTML = ''; + textDiv.style.width = `${viewport.width / dpr}px`; + textDiv.style.height = `${viewport.height / dpr}px`; + + const tl = new pdfjsLib.TextLayer({ + textContentSource: page.streamTextContent(), + container: textDiv, + viewport + }); + textLayerInstance = tl; + try { + await tl.render(); + } catch { + // cancelled + } + } + + async function prerender(): Promise { + if (!pdfDoc) return; + const neighbors = [currentPage - 1, currentPage + 1].filter( + (n) => n >= 1 && n <= (pdfDoc?.numPages ?? 0) + ); + for (const n of neighbors) { + try { + await pdfDoc.getPage(n); + } catch { + // ignore + } + } + } + + function prevPage(): void { + if (currentPage > 1) currentPage -= 1; + } + + function nextPage(): void { + if (currentPage < totalPages) currentPage += 1; + } + + function goToPage(n: number): void { + if (n >= 1 && n <= totalPages) currentPage = n; + } + + function zoomIn(): void { + scale += 0.25; + } + + function zoomOut(): void { + if (scale > 0.5) scale -= 0.25; + } + + function destroy(): void { + if (renderTask) { + renderTask.cancel(); + renderTask = null; + } + if (textLayerInstance) { + textLayerInstance.cancel(); + textLayerInstance = null; + } + pdfDoc?.destroy(); + pdfDoc = null; + } + + return { + get currentPage() { + return currentPage; + }, + get totalPages() { + return totalPages; + }, + get scale() { + return scale; + }, + get loading() { + return loading; + }, + get error() { + return error; + }, + get isLoaded() { + return pdfDoc !== null; + }, + get pdfjsReady() { + return pdfjsReady; + }, + setElements, + init, + loadDocument, + renderCurrentPage, + prerender, + prevPage, + nextPage, + goToPage, + zoomIn, + zoomOut, + destroy + }; +} -- 2.49.1 From 8898863a4839a5b828e2e12776d47a8b02002b70 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 14:45:03 +0200 Subject: [PATCH 13/19] refactor(transcription): extract useBlockAutoSave and useBlockDragDrop from TranscriptionEditView (#199) Co-Authored-By: Claude Sonnet 4.6 --- .../components/TranscriptionEditView.svelte | 228 ++++-------------- .../__tests__/useBlockAutoSave.svelte.test.ts | 84 +++++++ .../__tests__/useBlockDragDrop.svelte.test.ts | 79 ++++++ .../src/lib/hooks/useBlockAutoSave.svelte.ts | 127 ++++++++++ .../src/lib/hooks/useBlockDragDrop.svelte.ts | 88 +++++++ 5 files changed, 419 insertions(+), 187 deletions(-) create mode 100644 frontend/src/lib/hooks/__tests__/useBlockAutoSave.svelte.test.ts create mode 100644 frontend/src/lib/hooks/__tests__/useBlockDragDrop.svelte.test.ts create mode 100644 frontend/src/lib/hooks/useBlockAutoSave.svelte.ts create mode 100644 frontend/src/lib/hooks/useBlockDragDrop.svelte.ts diff --git a/frontend/src/lib/components/TranscriptionEditView.svelte b/frontend/src/lib/components/TranscriptionEditView.svelte index 774cebc5..85becbc2 100644 --- a/frontend/src/lib/components/TranscriptionEditView.svelte +++ b/frontend/src/lib/components/TranscriptionEditView.svelte @@ -1,11 +1,10 @@
@@ -309,20 +161,22 @@ $effect(() => {
{#each sortedBlocks as block, i (block.id)} - {#if dropTargetIdx === i} + {#if dragDrop.dropTargetIdx === i}
{/if}
handleGripDown(e, block.id)} - class="relative transition-all duration-150 {draggedBlockId === block.id ? 'z-10 rounded-lg shadow-xl ring-2 ring-turquoise/40' : ''}" - style={draggedBlockId === block.id ? `transform: translateY(${dragOffsetY}px) scale(1.02); opacity: 0.9;` : ''} + onblur={autoSave.handleBlur} + onpointerdown={(e) => dragDrop.handleGripDown(e, block.id)} + class="relative transition-all duration-150 {dragDrop.draggedBlockId === block.id ? 'z-10 rounded-lg shadow-xl ring-2 ring-turquoise/40' : ''}" + style={dragDrop.draggedBlockId === block.id + ? `transform: translateY(${dragDrop.dragOffsetY}px) scale(1.02); opacity: 0.9;` + : ''} > { label={block.label} active={activeBlockId === block.id} reviewed={block.reviewed ?? false} - saveState={getSaveState(block.id)} + saveState={autoSave.getSaveState(block.id)} canComment={canComment} currentUserId={currentUserId} - onTextChange={(text) => handleTextChange(block.id, text)} + onTextChange={(text) => autoSave.handleTextChange(block.id, text)} onFocus={() => handleFocus(block.id)} onDeleteClick={() => handleDelete(block.id)} - onRetry={() => handleRetry(block.id)} + onRetry={() => autoSave.handleRetry(block.id, block.text)} onReviewToggle={() => onReviewToggle(block.id)} onMoveUp={() => handleMoveUp(block.id)} onMoveDown={() => handleMoveDown(block.id)} @@ -349,7 +203,7 @@ $effect(() => {
{/each} - {#if dropTargetIdx === sortedBlocks.length} + {#if dragDrop.dropTargetIdx === sortedBlocks.length}
{/if} diff --git a/frontend/src/lib/hooks/__tests__/useBlockAutoSave.svelte.test.ts b/frontend/src/lib/hooks/__tests__/useBlockAutoSave.svelte.test.ts new file mode 100644 index 00000000..dd13a7a6 --- /dev/null +++ b/frontend/src/lib/hooks/__tests__/useBlockAutoSave.svelte.test.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +const mockSaveFn = vi.fn<(blockId: string, text: string) => Promise>(); + +const { createBlockAutoSave } = await import('../useBlockAutoSave.svelte'); + +describe('createBlockAutoSave', () => { + beforeEach(() => { + vi.useFakeTimers(); + mockSaveFn.mockClear(); + mockSaveFn.mockResolvedValue(undefined); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('getSaveState returns idle initially', () => { + const as = createBlockAutoSave({ saveFn: mockSaveFn, documentId: 'doc-1' }); + expect(as.getSaveState('block-1')).toBe('idle'); + }); + + it('debounce coalesces multiple changes — saves once after 1500ms', async () => { + const as = createBlockAutoSave({ saveFn: mockSaveFn, documentId: 'doc-1' }); + as.handleTextChange('block-1', 'text 1'); + as.handleTextChange('block-1', 'text 2'); + as.handleTextChange('block-1', 'text 3'); + await vi.advanceTimersByTimeAsync(1500); + expect(mockSaveFn).toHaveBeenCalledTimes(1); + expect(mockSaveFn).toHaveBeenCalledWith('block-1', 'text 3'); + }); + + it('handles concurrent blocks independently', async () => { + const as = createBlockAutoSave({ saveFn: mockSaveFn, documentId: 'doc-1' }); + as.handleTextChange('block-1', 'hello'); + as.handleTextChange('block-2', 'world'); + await vi.advanceTimersByTimeAsync(1500); + expect(mockSaveFn).toHaveBeenCalledTimes(2); + }); + + it('sets save state to saving then saved on success', async () => { + const as = createBlockAutoSave({ saveFn: mockSaveFn, documentId: 'doc-1' }); + as.handleTextChange('block-1', 'text'); + vi.advanceTimersByTime(1500); + expect(as.getSaveState('block-1')).toBe('saving'); + await Promise.resolve(); + expect(as.getSaveState('block-1')).toBe('saved'); + }); + + it('sets save state to error on save failure', async () => { + mockSaveFn.mockRejectedValue(new Error('save failed')); + const as = createBlockAutoSave({ saveFn: mockSaveFn, documentId: 'doc-1' }); + as.handleTextChange('block-1', 'text'); + await vi.advanceTimersByTimeAsync(1500); + expect(as.getSaveState('block-1')).toBe('error'); + }); + + it('handleRetry saves with provided current text', async () => { + mockSaveFn.mockRejectedValueOnce(new Error('first fails')); + mockSaveFn.mockResolvedValueOnce(undefined); + const as = createBlockAutoSave({ saveFn: mockSaveFn, documentId: 'doc-1' }); + as.handleTextChange('block-1', 'original'); + await vi.advanceTimersByTimeAsync(1500); + expect(as.getSaveState('block-1')).toBe('error'); + await as.handleRetry('block-1', 'original'); + expect(mockSaveFn).toHaveBeenCalledTimes(2); + expect(as.getSaveState('block-1')).toBe('saved'); + }); + + it('clearBlock removes all state for a block', () => { + const as = createBlockAutoSave({ saveFn: mockSaveFn, documentId: 'doc-1' }); + as.handleTextChange('block-1', 'text'); + as.clearBlock('block-1'); + expect(as.getSaveState('block-1')).toBe('idle'); + }); + + it('destroy clears all pending timers so no save occurs', async () => { + const as = createBlockAutoSave({ saveFn: mockSaveFn, documentId: 'doc-1' }); + as.handleTextChange('block-1', 'text'); + as.destroy(); + await vi.advanceTimersByTimeAsync(2000); + expect(mockSaveFn).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/lib/hooks/__tests__/useBlockDragDrop.svelte.test.ts b/frontend/src/lib/hooks/__tests__/useBlockDragDrop.svelte.test.ts new file mode 100644 index 00000000..7291e19f --- /dev/null +++ b/frontend/src/lib/hooks/__tests__/useBlockDragDrop.svelte.test.ts @@ -0,0 +1,79 @@ +import { describe, it, expect, vi } from 'vitest'; +import { createBlockDragDrop } from '../useBlockDragDrop.svelte'; +import type { TranscriptionBlockData } from '$lib/types'; + +function makeBlock(id: string, sortOrder: number): TranscriptionBlockData { + return { + id, + annotationId: `ann-${id}`, + documentId: 'doc-1', + text: '', + label: null, + sortOrder, + version: 1, + source: 'MANUAL', + reviewed: false + }; +} + +describe('createBlockDragDrop', () => { + it('initial state — no drag in progress', () => { + const dd = createBlockDragDrop({ getSortedBlocks: () => [], onReorder: vi.fn() }); + expect(dd.draggedBlockId).toBeNull(); + expect(dd.dropTargetIdx).toBeNull(); + expect(dd.dragOffsetY).toBe(0); + }); + + it('handleGripDown sets draggedBlockId when grip is hit', () => { + const dd = createBlockDragDrop({ getSortedBlocks: () => [], onReorder: vi.fn() }); + const grip = document.createElement('div'); + grip.setAttribute('data-drag-handle', ''); + const wrapper = document.createElement('div'); + wrapper.setAttribute('data-block-wrapper', ''); + wrapper.appendChild(grip); + document.body.appendChild(wrapper); + + const e = new PointerEvent('pointerdown', { clientY: 100, cancelable: true, bubbles: true }); + Object.defineProperty(e, 'target', { value: grip }); + wrapper.setPointerCapture = vi.fn(); + + dd.handleGripDown(e as PointerEvent, 'block-1'); + expect(dd.draggedBlockId).toBe('block-1'); + + document.body.removeChild(wrapper); + }); + + it('handlePointerUp without active drag is a no-op', () => { + const onReorder = vi.fn(); + const dd = createBlockDragDrop({ getSortedBlocks: () => [], onReorder }); + dd.handlePointerUp(); + expect(onReorder).not.toHaveBeenCalled(); + }); + + it('handlePointerUp with null dropTargetIdx does not call onReorder', () => { + const onReorder = vi.fn(); + const blocks = [makeBlock('b1', 0), makeBlock('b2', 1)]; + const dd = createBlockDragDrop({ getSortedBlocks: () => blocks, onReorder }); + + // Simulate a drag start without going through handleGripDown internals + // by checking that handlePointerUp without a drop target is a no-op for reorder + const grip = document.createElement('div'); + grip.setAttribute('data-drag-handle', ''); + const wrapper = document.createElement('div'); + wrapper.setAttribute('data-block-wrapper', ''); + wrapper.appendChild(grip); + document.body.appendChild(wrapper); + wrapper.setPointerCapture = vi.fn(); + + const downEvent = new PointerEvent('pointerdown', { clientY: 50, cancelable: true }); + Object.defineProperty(downEvent, 'target', { value: grip }); + dd.handleGripDown(downEvent as PointerEvent, 'b1'); + + // dropTargetIdx is still null (no pointer move happened) + dd.handlePointerUp(); + expect(onReorder).not.toHaveBeenCalled(); + expect(dd.draggedBlockId).toBeNull(); + + document.body.removeChild(wrapper); + }); +}); diff --git a/frontend/src/lib/hooks/useBlockAutoSave.svelte.ts b/frontend/src/lib/hooks/useBlockAutoSave.svelte.ts new file mode 100644 index 00000000..a627f50e --- /dev/null +++ b/frontend/src/lib/hooks/useBlockAutoSave.svelte.ts @@ -0,0 +1,127 @@ +import { SvelteMap } from 'svelte/reactivity'; + +export type SaveState = 'idle' | 'saving' | 'saved' | 'fading' | 'error'; + +type Options = { + saveFn: (blockId: string, text: string) => Promise; + documentId: string; +}; + +export function createBlockAutoSave({ saveFn, documentId }: Options) { + const saveStates = new SvelteMap(); + const debounceTimers = new SvelteMap>(); + const pendingTexts = new SvelteMap(); + const fadeTimers: ReturnType[] = []; + + function getSaveState(blockId: string): SaveState { + return saveStates.get(blockId) ?? 'idle'; + } + + function setSaveState(blockId: string, state: SaveState) { + saveStates.set(blockId, state); + } + + async function executeSave(blockId: string): Promise { + const text = pendingTexts.get(blockId); + if (text === undefined) return; + + pendingTexts.delete(blockId); + setSaveState(blockId, 'saving'); + + try { + await saveFn(blockId, text); + setSaveState(blockId, 'saved'); + scheduleSavedFade(blockId); + } catch { + setSaveState(blockId, 'error'); + } + } + + function scheduleSavedFade(blockId: string): void { + const t1 = setTimeout(() => { + if (getSaveState(blockId) === 'saved') { + setSaveState(blockId, 'fading'); + const t2 = setTimeout(() => { + if (getSaveState(blockId) === 'fading') { + setSaveState(blockId, 'idle'); + } + }, 300); + fadeTimers.push(t2); + } + }, 2000); + fadeTimers.push(t1); + } + + function scheduleDebounce(blockId: string): void { + clearDebounce(blockId); + const timer = setTimeout(() => { + debounceTimers.delete(blockId); + executeSave(blockId); + }, 1500); + debounceTimers.set(blockId, timer); + } + + function clearDebounce(blockId: string): void { + const existing = debounceTimers.get(blockId); + if (existing !== undefined) { + clearTimeout(existing); + debounceTimers.delete(blockId); + } + } + + function handleTextChange(blockId: string, text: string): void { + pendingTexts.set(blockId, text); + scheduleDebounce(blockId); + } + + function handleBlur(): void { + for (const [blockId] of [...debounceTimers]) { + clearDebounce(blockId); + executeSave(blockId); + } + } + + async function handleRetry(blockId: string, currentText: string): Promise { + const pending = pendingTexts.get(blockId); + const text = pending ?? currentText; + pendingTexts.set(blockId, text); + await executeSave(blockId); + } + + function clearBlock(blockId: string): void { + clearDebounce(blockId); + pendingTexts.delete(blockId); + saveStates.delete(blockId); + } + + function flushViaBeacon(): void { + for (const [blockId, text] of pendingTexts) { + clearDebounce(blockId); + const url = `/api/documents/${documentId}/transcription-blocks/${blockId}`; + const body = JSON.stringify({ text }); + navigator.sendBeacon(url, new Blob([body], { type: 'application/json' })); + pendingTexts.delete(blockId); + } + } + + function destroy(): void { + for (const timer of debounceTimers.values()) { + clearTimeout(timer); + } + debounceTimers.clear(); + for (const timer of fadeTimers) { + clearTimeout(timer); + } + fadeTimers.length = 0; + } + + return { + getSaveState, + handleTextChange, + handleBlur, + handleRetry, + clearBlock, + flushViaBeacon, + destroy + }; +} diff --git a/frontend/src/lib/hooks/useBlockDragDrop.svelte.ts b/frontend/src/lib/hooks/useBlockDragDrop.svelte.ts new file mode 100644 index 00000000..176bd467 --- /dev/null +++ b/frontend/src/lib/hooks/useBlockDragDrop.svelte.ts @@ -0,0 +1,88 @@ +import type { TranscriptionBlockData } from '$lib/types'; + +type Options = { + getSortedBlocks: () => TranscriptionBlockData[]; + onReorder: (blockIds: string[]) => void; +}; + +export function createBlockDragDrop({ getSortedBlocks, onReorder }: Options) { + let draggedBlockId = $state(null); + let dropTargetIdx = $state(null); + let dragOffsetY = $state(0); + + // Internal mutable refs — not reactive + let dragStartY = 0; + let capturedEl: HTMLElement | null = null; + let listEl: HTMLElement | null = null; + + function setListElement(el: HTMLElement | null): void { + listEl = el; + } + + function handleGripDown(e: PointerEvent, blockId: string): void { + if (!(e.target as HTMLElement).closest('[data-drag-handle]')) return; + e.preventDefault(); + draggedBlockId = blockId; + dragStartY = e.clientY; + dragOffsetY = 0; + capturedEl = (e.target as HTMLElement).closest('[data-block-wrapper]') as HTMLElement; + capturedEl?.setPointerCapture(e.pointerId); + } + + function handlePointerMove(e: PointerEvent): void { + if (!draggedBlockId || !listEl) return; + dragOffsetY = e.clientY - dragStartY; + + const sortedBlocks = getSortedBlocks(); + const wrappers = Array.from(listEl.querySelectorAll('[data-block-wrapper]')); + const dragIdx = sortedBlocks.findIndex((b) => b.id === draggedBlockId); + let target: number | null = null; + + for (let i = 0; i < wrappers.length; i++) { + const rect = wrappers[i].getBoundingClientRect(); + if (e.clientY < rect.top + rect.height / 2) { + target = i; + break; + } + } + if (target === null) target = wrappers.length; + if (target === dragIdx || target === dragIdx + 1) target = null; + dropTargetIdx = target; + } + + function handlePointerUp(): void { + if (!draggedBlockId) return; + + if (dropTargetIdx !== null) { + const sorted = [...getSortedBlocks()]; + const fromIdx = sorted.findIndex((b) => b.id === draggedBlockId); + if (fromIdx >= 0) { + const [moved] = sorted.splice(fromIdx, 1); + const insertAt = dropTargetIdx > fromIdx ? dropTargetIdx - 1 : dropTargetIdx; + sorted.splice(insertAt, 0, moved); + onReorder(sorted.map((b) => b.id)); + } + } + + draggedBlockId = null; + dropTargetIdx = null; + dragOffsetY = 0; + capturedEl = null; + } + + return { + get draggedBlockId() { + return draggedBlockId; + }, + get dropTargetIdx() { + return dropTargetIdx; + }, + get dragOffsetY() { + return dragOffsetY; + }, + setListElement, + handleGripDown, + handlePointerMove, + handlePointerUp + }; +} -- 2.49.1 From ff9ae198c45540a8f0c9df2b11c798342c2ab4b0 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 14:54:55 +0200 Subject: [PATCH 14/19] refactor(notifications): extract useNotificationStream and NotificationDropdown from NotificationBell (#200) Co-Authored-By: Claude Sonnet 4.6 --- .../lib/components/NotificationBell.svelte | 244 ++---------------- .../components/NotificationDropdown.svelte | 138 ++++++++++ .../useNotificationStream.svelte.test.ts | 109 ++++++++ .../lib/hooks/useNotificationStream.svelte.ts | 95 +++++++ 4 files changed, 370 insertions(+), 216 deletions(-) create mode 100644 frontend/src/lib/components/NotificationDropdown.svelte create mode 100644 frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts create mode 100644 frontend/src/lib/hooks/useNotificationStream.svelte.ts diff --git a/frontend/src/lib/components/NotificationBell.svelte b/frontend/src/lib/components/NotificationBell.svelte index a1bb7b04..9781e366 100644 --- a/frontend/src/lib/components/NotificationBell.svelte +++ b/frontend/src/lib/components/NotificationBell.svelte @@ -3,53 +3,23 @@ import { onMount, onDestroy } from 'svelte'; import { goto } from '$app/navigation'; import { m } from '$lib/paraglide/messages.js'; import { clickOutside } from '$lib/actions/clickOutside'; -import { - type NotificationItem, - relativeTime, - parseNotificationEvent -} from '$lib/utils/notifications'; +import { createNotificationStream } from '$lib/hooks/useNotificationStream.svelte'; +import NotificationDropdown from './NotificationDropdown.svelte'; -let notifications: NotificationItem[] = $state([]); -let unreadCount: number = $state(0); let open = $state(false); - -// DOM refs managed via attachments let bellButtonEl: HTMLButtonElement | null = null; -let firstFocusableEl: HTMLButtonElement | null = null; -let eventSource: EventSource | null = null; - -async function fetchNotifications() { - try { - const res = await fetch('/api/notifications?size=10'); - if (res.ok) { - const data = await res.json(); - notifications = data.content ?? []; - } - } catch (e) { - console.error('Failed to fetch notifications', e); - } -} - -async function fetchUnreadCount() { - try { - const res = await fetch('/api/notifications/unread-count'); - if (res.ok) { - const data = await res.json(); - unreadCount = data.count; - } - } catch (e) { - console.error('Failed to fetch unread count', e); - } -} +const stream = createNotificationStream(); async function toggleDropdown() { open = !open; if (open) { - await fetchNotifications(); - // defer focus until DOM updates + await stream.fetchNotifications(); setTimeout(() => { - firstFocusableEl?.focus(); + const firstBtn = document.querySelector( + '[role="dialog"] button, [role="dialog"] a' + ); + firstBtn?.focus(); }, 0); } } @@ -59,16 +29,8 @@ function closeDropdown() { bellButtonEl?.focus(); } -async function markRead(notification: NotificationItem) { - if (!notification.read) { - try { - await fetch(`/api/notifications/${notification.id}/read`, { method: 'PATCH' }); - notification.read = true; - unreadCount = Math.max(0, unreadCount - 1); - } catch (e) { - console.error('Failed to mark notification as read', e); - } - } +async function handleMarkRead(notification: Parameters[0]) { + await stream.markRead(notification); const url = notification.annotationId ? `/documents/${notification.documentId}?commentId=${notification.referenceId}&annotationId=${notification.annotationId}` : `/documents/${notification.documentId}?commentId=${notification.referenceId}`; @@ -76,18 +38,6 @@ async function markRead(notification: NotificationItem) { goto(url); } -async function markAllRead() { - try { - await fetch('/api/notifications/read-all', { method: 'POST' }); - for (const n of notifications) { - n.read = true; - } - unreadCount = 0; - } catch (e) { - console.error('Failed to mark all notifications as read', e); - } -} - function handleKeydown(event: KeyboardEvent) { if (event.key === 'Escape' && open) { event.stopPropagation(); @@ -95,7 +45,6 @@ function handleKeydown(event: KeyboardEvent) { } } -// Attachment: stores the element reference for the bell button function attachBellButton(node: HTMLButtonElement) { bellButtonEl = node; return () => { @@ -103,27 +52,12 @@ function attachBellButton(node: HTMLButtonElement) { }; } -// Attachment: stores the element reference for the first focusable element in the dropdown -function attachFirstFocusable(node: HTMLButtonElement) { - firstFocusableEl = node; - return () => { - firstFocusableEl = null; - }; -} - onMount(() => { - fetchUnreadCount(); - eventSource = new EventSource('/api/notifications/stream'); - eventSource.addEventListener('notification', (e) => { - const notification = parseNotificationEvent(e.data); - if (!notification) return; - notifications = [notification, ...notifications]; - if (!notification.read) unreadCount += 1; - }); + stream.init(); }); onDestroy(() => { - eventSource?.close(); + stream.destroy(); }); @@ -135,14 +69,13 @@ onDestroy(() => { {@attach attachBellButton} type="button" onclick={toggleDropdown} - aria-label={unreadCount > 0 - ? m.notification_bell_unread_label({ count: unreadCount }) + aria-label={stream.unreadCount > 0 + ? m.notification_bell_unread_label({ count: stream.unreadCount }) : m.notification_bell_label()} aria-expanded={open} aria-haspopup="true" class="relative rounded-sm p-2 text-white/65 transition-colors hover:bg-white/10 hover:text-white focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring" > - { /> - - {#if unreadCount > 0} - - {unreadCount} - - {/if} + + + {stream.unreadCount} + - {#if open} - + {/if}
diff --git a/frontend/src/lib/components/NotificationDropdown.svelte b/frontend/src/lib/components/NotificationDropdown.svelte new file mode 100644 index 00000000..ee46548c --- /dev/null +++ b/frontend/src/lib/components/NotificationDropdown.svelte @@ -0,0 +1,138 @@ + + + diff --git a/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts b/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts new file mode 100644 index 00000000..d195406d --- /dev/null +++ b/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts @@ -0,0 +1,109 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { NotificationItem } from '../useNotificationStream.svelte'; + +// Track the last created EventSource instance +let lastEventSource: { + close: ReturnType; + onopen: (() => void) | null; + onerror: (() => void) | null; +} | null = null; + +class MockEventSource { + onopen: (() => void) | null = null; + onerror: (() => void) | null = null; + close = vi.fn(); + private listeners: Record void)[]> = {}; + + constructor() { + // eslint-disable-next-line @typescript-eslint/no-this-alias + lastEventSource = this; + } + + addEventListener(type: string, fn: (e: MessageEvent) => void) { + if (!this.listeners[type]) this.listeners[type] = []; + this.listeners[type].push(fn); + } +} + +vi.stubGlobal('EventSource', MockEventSource); + +const mockFetch = vi.fn(); +vi.stubGlobal('fetch', mockFetch); + +// Import after stubs are set up +const { createNotificationStream } = await import('../useNotificationStream.svelte'); + +beforeEach(() => { + mockFetch.mockReset(); + lastEventSource = null; +}); + +function makeNotification(overrides: Partial = {}): NotificationItem { + return { + id: 'n1', + type: 'REPLY', + actorName: 'Hans', + documentId: 'doc-1', + referenceId: 'ref-1', + annotationId: null, + read: false, + createdAt: new Date().toISOString(), + ...overrides + }; +} + +describe('createNotificationStream', () => { + it('starts with empty notifications and zero unreadCount', () => { + const stream = createNotificationStream(); + expect(stream.notifications).toHaveLength(0); + expect(stream.unreadCount).toBe(0); + }); + + it('fetchUnreadCount updates unreadCount from API', async () => { + mockFetch.mockResolvedValueOnce(new Response(JSON.stringify({ count: 3 }), { status: 200 })); + const stream = createNotificationStream(); + await stream.fetchUnreadCount(); + expect(stream.unreadCount).toBe(3); + }); + + it('fetchNotifications populates notifications from API', async () => { + const items = [makeNotification()]; + mockFetch.mockResolvedValueOnce( + new Response(JSON.stringify({ content: items }), { status: 200 }) + ); + const stream = createNotificationStream(); + await stream.fetchNotifications(); + expect(stream.notifications).toHaveLength(1); + expect(stream.notifications[0].id).toBe('n1'); + }); + + it('markRead marks notification as read and decrements unreadCount', async () => { + mockFetch + .mockResolvedValueOnce(new Response(JSON.stringify({ count: 2 }), { status: 200 })) + .mockResolvedValueOnce(new Response(null, { status: 200 })); + const stream = createNotificationStream(); + await stream.fetchUnreadCount(); + + const notification = makeNotification({ read: false }); + await stream.markRead(notification); + expect(notification.read).toBe(true); + expect(stream.unreadCount).toBe(1); + }); + + it('markAllRead calls the API and resets unreadCount', async () => { + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); + const stream = createNotificationStream(); + await stream.markAllRead(); + expect(mockFetch).toHaveBeenCalledWith('/api/notifications/read-all', { method: 'POST' }); + expect(stream.unreadCount).toBe(0); + }); + + it('destroy closes the EventSource', async () => { + mockFetch.mockResolvedValue(new Response(JSON.stringify({ count: 0 }), { status: 200 })); + const stream = createNotificationStream(); + stream.init(); + expect(lastEventSource).not.toBeNull(); + stream.destroy(); + expect(lastEventSource!.close).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/lib/hooks/useNotificationStream.svelte.ts b/frontend/src/lib/hooks/useNotificationStream.svelte.ts new file mode 100644 index 00000000..0a03dada --- /dev/null +++ b/frontend/src/lib/hooks/useNotificationStream.svelte.ts @@ -0,0 +1,95 @@ +import { type NotificationItem, parseNotificationEvent } from '$lib/utils/notifications'; + +export type { NotificationItem }; + +export function createNotificationStream() { + let notifications = $state([]); + let unreadCount = $state(0); + let eventSource: EventSource | null = null; + + async function fetchNotifications(): Promise { + try { + const res = await fetch('/api/notifications?size=10'); + if (res.ok) { + const data = await res.json(); + notifications = data.content ?? []; + } + } catch (e) { + console.error('Failed to fetch notifications', e); + } + } + + async function fetchUnreadCount(): Promise { + try { + const res = await fetch('/api/notifications/unread-count'); + if (res.ok) { + const data = await res.json(); + unreadCount = data.count; + } + } catch (e) { + console.error('Failed to fetch unread count', e); + } + } + + async function markRead(notification: NotificationItem): Promise { + if (!notification.read) { + try { + await fetch(`/api/notifications/${notification.id}/read`, { method: 'PATCH' }); + notification.read = true; + unreadCount = Math.max(0, unreadCount - 1); + } catch (e) { + console.error('Failed to mark notification as read', e); + } + } + } + + async function markAllRead(): Promise { + try { + await fetch('/api/notifications/read-all', { method: 'POST' }); + for (const n of notifications) { + n.read = true; + } + unreadCount = 0; + } catch (e) { + console.error('Failed to mark all notifications as read', e); + } + } + + function init(): void { + fetchUnreadCount(); + eventSource = new EventSource('/api/notifications/stream'); + eventSource.addEventListener('notification', (e) => { + const notification = parseNotificationEvent(e.data); + if (!notification) return; + notifications = [notification, ...notifications]; + if (!notification.read) unreadCount += 1; + }); + eventSource.onopen = () => { + fetchUnreadCount(); + }; + eventSource.onerror = () => { + // Close on error to avoid repeated reconnect noise + eventSource?.close(); + }; + } + + function destroy(): void { + eventSource?.close(); + eventSource = null; + } + + return { + get notifications() { + return notifications; + }, + get unreadCount() { + return unreadCount; + }, + fetchNotifications, + fetchUnreadCount, + markRead, + markAllRead, + init, + destroy + }; +} -- 2.49.1 From 2b93ccf92d70d137a8e4cde3af42723973dec538 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 15:06:26 +0200 Subject: [PATCH 15/19] refactor(notifications): import relativeTime from canonical time.ts NotificationDropdown was importing relativeTime through notifications.ts, creating an accidental coupling to a module unrelated to timestamp formatting. Now imports directly from the canonical \$lib/utils/time module. Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/lib/components/NotificationDropdown.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/lib/components/NotificationDropdown.svelte b/frontend/src/lib/components/NotificationDropdown.svelte index ee46548c..b3b161fb 100644 --- a/frontend/src/lib/components/NotificationDropdown.svelte +++ b/frontend/src/lib/components/NotificationDropdown.svelte @@ -1,6 +1,6 @@