From 154f859efc276ef3a217358e08a325d3aa6eebb7 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 30 Mar 2026 19:57:48 +0200 Subject: [PATCH] =?UTF-8?q?feat(korrespondenz):=20address=20PR=20#164=20re?= =?UTF-8?q?view=20=E2=80=93=20blockers=20and=20suggestions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blockers (14): - B1: fix senderName/receiverName to use $derived instead of $state + sync $effect - B2: migrate all korrespondenz components from messages-extra shim to paraglide m.* - B3: i18n CorrespondenzEmptyState (heading, subtext, search placeholder) - B4: add response.ok checks to admin layout server load - B5: add response.ok checks to korrespondenz page server load - B6: add page.server.spec.ts with 5 test suites for korrespondenz load function - B7: add axe-core accessibility checks to all e2e korrespondenz tests - B8: add Testcontainers JPQL tests for findSinglePersonCorrespondence (DISTINCT + sender) - B9: hide auth reset-token endpoint from OpenAPI spec; remove from generated api.ts - B11: replace amber hardcoded hex colors in SinglePersonHintBar with brand tokens - B12: replace clipboard emoji with Heroicons SVG in SinglePersonHintBar - B13: create DateInput component (German dd.mm.yyyy); use it in CorrespondenzFilterControls - B14: add Paraglide compile step to CI workflow before lint/test Suggestions (11): - S1: make CorrespondentSuggestionsDropdown a pure display component; lift fetch to PersonBar - S2: fix leftover messages-extra import in ConversationTimeline; use brand tokens for status dots - S3: add intent comment to EntityNav openFlyout behavior - S4: rename canManageGroups → canManagePermissions throughout admin - S6: remove domFlush helper from DateInput spec; use expect.poll instead - S7: replace test.skip with throw new Error in bilateral e2e tests - S8: add inverse aria-disabled test for filter strip - S9: remove sm:min-h-0 from sort button to preserve 44px touch target - S10: add title attributes to tablet trigger buttons in EntityNav - S11: delete messages-extra.ts shim entirely Also: fix admin pages revealing blank strip at bottom (-mb-6 on admin layout) Co-Authored-By: Claude Sonnet 4.6 --- .gitea/workflows/ci.yml | 4 + .../controller/AuthE2EController.java | 5 + .../repository/DocumentRepositoryTest.java | 64 ++++++++ frontend/e2e/korrespondenz.spec.ts | 17 +- frontend/src/lib/components/DateInput.svelte | 75 +++++++++ .../lib/components/DateInput.svelte.spec.ts | 6 +- frontend/src/lib/generated/api.ts | 41 +---- frontend/src/lib/messages-extra.ts | 37 ----- frontend/src/routes/admin/+layout.server.ts | 18 ++- frontend/src/routes/admin/+layout.svelte | 4 +- frontend/src/routes/admin/+page.svelte | 2 +- frontend/src/routes/admin/EntityNav.svelte | 15 +- .../routes/admin/entity-nav.svelte.spec.ts | 2 +- .../src/routes/admin/layout.server.spec.ts | 2 +- .../src/routes/admin/layout.svelte.spec.ts | 2 +- frontend/src/routes/admin/page.svelte.spec.ts | 2 +- .../src/routes/korrespondenz/+page.server.ts | 26 +++- .../src/routes/korrespondenz/+page.svelte | 18 +-- .../korrespondenz/ConversationTimeline.svelte | 17 +- .../CorrespondentSuggestionsDropdown.svelte | 32 +--- .../CorrespondenzEmptyState.svelte | 12 +- .../CorrespondenzFilterControls.svelte | 40 ++--- .../CorrespondenzPersonBar.svelte | 25 ++- .../korrespondenz/SinglePersonHintBar.svelte | 25 ++- .../routes/korrespondenz/page.server.spec.ts | 146 ++++++++++++++++++ .../routes/korrespondenz/page.svelte.spec.ts | 6 + 26 files changed, 459 insertions(+), 184 deletions(-) create mode 100644 frontend/src/lib/components/DateInput.svelte delete mode 100644 frontend/src/lib/messages-extra.ts create mode 100644 frontend/src/routes/korrespondenz/page.server.spec.ts diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index d92bfd5d..f50e8e80 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -28,6 +28,10 @@ jobs: run: npm ci working-directory: frontend + - name: Compile Paraglide i18n + run: npx @inlang/paraglide-js compile --project ./project.inlang --outdir ./src/lib/paraglide + working-directory: frontend + - name: Lint run: npm run lint working-directory: frontend diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/AuthE2EController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/AuthE2EController.java index 312666e4..7d9be97b 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/AuthE2EController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/AuthE2EController.java @@ -10,6 +10,8 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; +import io.swagger.v3.oas.annotations.Operation; + import lombok.RequiredArgsConstructor; /** @@ -24,6 +26,9 @@ public class AuthE2EController { private final PasswordResetTokenRepository tokenRepository; + // Hidden from the OpenAPI spec — this endpoint must never appear in the generated api.ts + // even when the e2e profile is active alongside the dev profile during spec generation. + @Operation(hidden = true) @GetMapping("/reset-token-for-test") public ResponseEntity getResetTokenForTest(@RequestParam String email) { return tokenRepository.findLatestActiveTokenByEmail(email, LocalDateTime.now()) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/repository/DocumentRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/repository/DocumentRepositoryTest.java index 845646b7..51ab05b4 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/repository/DocumentRepositoryTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/repository/DocumentRepositoryTest.java @@ -15,8 +15,11 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; +import java.time.LocalDate; +import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; @@ -189,4 +192,65 @@ class DocumentRepositoryTest { assertThat(result.getTotalElements()).isEqualTo(5); assertThat(result.getContent()).allMatch(d -> !d.isMetadataComplete()); } + + // ─── findSinglePersonCorrespondence — DISTINCT / multi-receiver safety ──── + + @Test + void findSinglePersonCorrespondence_returnsExactlyOneResult_whenDocumentHasThreeReceiversAndOneMatchesPersonId() { + Person sender = personRepository.save(Person.builder() + .firstName("Hans").lastName("Müller").build()); + Person receiver1 = personRepository.save(Person.builder() + .firstName("Anna").lastName("Schmidt").build()); + Person receiver2 = personRepository.save(Person.builder() + .firstName("Bertha").lastName("Wagner").build()); + Person receiver3 = personRepository.save(Person.builder() + .firstName("Clara").lastName("Koch").build()); + + // Document addressed to all three receivers + Document doc = documentRepository.save(Document.builder() + .title("Rundschreiben") + .originalFilename("rundschreiben.pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .receivers(new HashSet<>(Set.of(receiver1, receiver2, receiver3))) + .documentDate(LocalDate.of(1950, 6, 1)) + .build()); + + Sort sort = Sort.by(Sort.Direction.DESC, "documentDate"); + LocalDate from = LocalDate.of(1900, 1, 1); + LocalDate to = LocalDate.of(2000, 1, 1); + + // Query for receiver1 — the DISTINCT must collapse the 3 JOIN rows into 1 result + List results = documentRepository.findSinglePersonCorrespondence( + receiver1.getId(), from, to, sort); + + assertThat(results).hasSize(1); + assertThat(results.get(0).getId()).isEqualTo(doc.getId()); + } + + @Test + void findSinglePersonCorrespondence_includesDocumentsWherePerson_isSender() { + Person sender = personRepository.save(Person.builder() + .firstName("Hans").lastName("Müller").build()); + Person receiver = personRepository.save(Person.builder() + .firstName("Anna").lastName("Schmidt").build()); + + documentRepository.save(Document.builder() + .title("Brief als Absender") + .originalFilename("brief_absender.pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .receivers(new HashSet<>(Set.of(receiver))) + .documentDate(LocalDate.of(1950, 6, 1)) + .build()); + + Sort sort = Sort.by(Sort.Direction.DESC, "documentDate"); + LocalDate from = LocalDate.of(1900, 1, 1); + LocalDate to = LocalDate.of(2000, 1, 1); + + List results = documentRepository.findSinglePersonCorrespondence( + sender.getId(), from, to, sort); + + assertThat(results).hasSize(1); + } } diff --git a/frontend/e2e/korrespondenz.spec.ts b/frontend/e2e/korrespondenz.spec.ts index 225847cd..6fcf0680 100644 --- a/frontend/e2e/korrespondenz.spec.ts +++ b/frontend/e2e/korrespondenz.spec.ts @@ -1,9 +1,16 @@ import { test, expect } from '@playwright/test'; +import AxeBuilder from '@axe-core/playwright'; + +function buildAxe(page: Parameters[0]['page']) { + return new AxeBuilder({ page }).withTags(['wcag2a', 'wcag2aa']); +} test.describe('Korrespondenz – empty state', () => { test('shows the search heading when no person is selected', async ({ page }) => { await page.goto('/korrespondenz'); await expect(page.getByText(/Korrespondenz durchsuchen/i)).toBeVisible(); + const a11y = await buildAxe(page).analyze(); + expect(a11y.violations, JSON.stringify(a11y.violations, null, 2)).toHaveLength(0); await page.screenshot({ path: 'test-results/e2e/korrespondenz-empty.png' }); }); @@ -37,6 +44,8 @@ test.describe('Korrespondenz – single-person mode', () => { const filterStrip = page.locator('[aria-disabled="false"]').first(); await expect(filterStrip).toBeAttached(); + const a11y = await buildAxe(page).analyze(); + expect(a11y.violations, JSON.stringify(a11y.violations, null, 2)).toHaveLength(0); await page.screenshot({ path: 'test-results/e2e/korrespondenz-single-person.png' }); }); @@ -75,10 +84,14 @@ test.describe('Korrespondenz – bilateral mode', () => { // Hint bar should NOT be shown in bilateral mode await expect(page.getByText(/Alle Briefe von/i)).not.toBeVisible(); + const a11y = await buildAxe(page).analyze(); + expect(a11y.violations, JSON.stringify(a11y.violations, null, 2)).toHaveLength(0); await page.screenshot({ path: 'test-results/e2e/korrespondenz-bilateral.png' }); } else { - // No bilateral data available for this person — skip with a note - test.skip(true, `No bilateral correspondent links found for person ${senderId}`); + // E2E seed must include bilateral correspondents — a missing link is a test failure. + throw new Error( + `No bilateral correspondent links found for person ${senderId}. Ensure the E2E seed contains at least one bilateral correspondence pair.` + ); } }); diff --git a/frontend/src/lib/components/DateInput.svelte b/frontend/src/lib/components/DateInput.svelte new file mode 100644 index 00000000..3314105a --- /dev/null +++ b/frontend/src/lib/components/DateInput.svelte @@ -0,0 +1,75 @@ + + + +{#if name} + +{/if} diff --git a/frontend/src/lib/components/DateInput.svelte.spec.ts b/frontend/src/lib/components/DateInput.svelte.spec.ts index 5c682812..6ba990d1 100644 --- a/frontend/src/lib/components/DateInput.svelte.spec.ts +++ b/frontend/src/lib/components/DateInput.svelte.spec.ts @@ -3,9 +3,6 @@ import { cleanup, render } from 'vitest-browser-svelte'; import { page } from 'vitest/browser'; import DateInput from './DateInput.svelte'; -/** Wait one macrotask so Svelte can flush reactive DOM updates. */ -const domFlush = () => new Promise((r) => setTimeout(r, 50)); - afterEach(() => cleanup()); // ─── Rendering ──────────────────────────────────────────────────────────────── @@ -197,10 +194,9 @@ describe('DateInput – hidden input for form submission', () => { render(DateInput, { name: 'documentDate', value: '' }); const input = page.getByRole('textbox'); await input.fill('20122024'); - await domFlush(); const hidden = document.querySelector( 'input[type="hidden"][name="documentDate"]' ); - expect(hidden?.value).toBe('2024-12-20'); + await expect.poll(() => hidden?.value).toBe('2024-12-20'); }); }); diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index d376129c..52be38ff 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -724,22 +724,8 @@ export interface paths { patch?: never; trace?: never; }; - "/api/auth/reset-token-for-test": { - parameters: { - query?: never; - header?: never; - path?: never; - cookie?: never; - }; - get: operations["getResetTokenForTest"]; - put?: never; - post?: never; - delete?: never; - options?: never; - head?: never; - patch?: never; - trace?: never; - }; + // "/api/auth/reset-token-for-test" removed — @Operation(hidden=true) on AuthE2EController. + // Regenerate with `npm run generate:api` after the next backend build to keep in sync. "/api/admin/import-status": { parameters: { query?: never; @@ -2514,28 +2500,7 @@ export interface operations { }; }; }; - getResetTokenForTest: { - parameters: { - query: { - email: string; - }; - header?: never; - path?: never; - cookie?: never; - }; - requestBody?: never; - responses: { - /** @description OK */ - 200: { - headers: { - [name: string]: unknown; - }; - content: { - "*/*": string; - }; - }; - }; - }; + // getResetTokenForTest removed — @Operation(hidden=true) on AuthE2EController. importStatus: { parameters: { query?: never; diff --git a/frontend/src/lib/messages-extra.ts b/frontend/src/lib/messages-extra.ts deleted file mode 100644 index 799999b6..00000000 --- a/frontend/src/lib/messages-extra.ts +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Extra message functions for i18n keys added after the last paraglide compile. - * - * TODO: Remove this file once the root-owned paraglide files in src/lib/paraglide/ - * are regenerated (run `npm run dev` or the paraglide compile step as the owning user). - * At that point, these functions will be generated into _index.js and the components - * that import from here should switch back to importing from $lib/paraglide/messages.js. - * - * Note: these fall back to German only — locale switching is handled by the generated - * paraglide files, not this shim. - */ - -// Svelte auto-escapes interpolated values — do not use {@html} with these strings. - -export const conv_hint_single_person = (inputs: { name: string }) => - `Alle Briefe von ${inputs.name} — wähle einen Korrespondenten oben um einzugrenzen`; - -export const conv_hint_single_person_filtered = (inputs: { - name: string; - from: string; - to: string; - sortLabel: string; -}) => `Alle Briefe von ${inputs.name} · ${inputs.from}–${inputs.to} · ${inputs.sortLabel}`; - -export const conv_strip_period = () => 'Zeitraum'; -export const conv_strip_from_placeholder = () => 'Von…'; -export const conv_strip_to_placeholder = () => 'Bis…'; -export const conv_strip_all_correspondents = () => 'Alle Korrespondenten'; -export const conv_strip_sort_newest = () => 'Neueste'; -export const conv_strip_sort_oldest = () => 'Älteste'; -export const conv_suggestions_heading = () => 'Häufigste Korrespondenten'; -export const conv_suggestions_all_label = (inputs: { name: string }) => - `Alle Korrespondenten von ${inputs.name}`; -export const conv_letters_count = (inputs: { count: number }) => `${inputs.count} Briefe`; -export const conv_empty_search_placeholder = () => 'Person suchen…'; -export const conv_empty_recent_label = () => 'Zuletzt geöffnet'; -export const conv_no_party = () => '—'; diff --git a/frontend/src/routes/admin/+layout.server.ts b/frontend/src/routes/admin/+layout.server.ts index f21bbff4..875c403a 100644 --- a/frontend/src/routes/admin/+layout.server.ts +++ b/frontend/src/routes/admin/+layout.server.ts @@ -23,19 +23,35 @@ export async function load({ fetch, locals }) { if (!hasAnyAdminPerm(user)) throw error(403, getErrorMessage('FORBIDDEN')); const api = createApiClient(fetch); + + // TODO: replace with a dedicated /api/admin/stats endpoint that returns counts only, + // so the System page does not load full entity lists it does not render. const [usersResult, groupsResult, tagsResult] = await Promise.all([ api.GET('/api/users'), api.GET('/api/groups'), api.GET('/api/tags') ]); + if (!usersResult.response.ok) { + const code = (usersResult.error as unknown as { code?: string })?.code; + throw error(usersResult.response.status, getErrorMessage(code)); + } + if (!groupsResult.response.ok) { + const code = (groupsResult.error as unknown as { code?: string })?.code; + throw error(groupsResult.response.status, getErrorMessage(code)); + } + if (!tagsResult.response.ok) { + const code = (tagsResult.error as unknown as { code?: string })?.code; + throw error(tagsResult.response.status, getErrorMessage(code)); + } + return { userCount: (usersResult.data ?? []).length, groupCount: (groupsResult.data ?? []).length, tagCount: (tagsResult.data ?? []).length, canManageUsers: hasPerm(user, 'ADMIN_USER'), canManageTags: hasPerm(user, 'ADMIN_TAG'), - canManageGroups: hasPerm(user, 'ADMIN_PERMISSION'), + canManagePermissions: hasPerm(user, 'ADMIN_PERMISSION'), canRunMaintenance: hasPerm(user, 'ADMIN') }; } diff --git a/frontend/src/routes/admin/+layout.svelte b/frontend/src/routes/admin/+layout.svelte index ba3e1849..abe0106f 100644 --- a/frontend/src/routes/admin/+layout.svelte +++ b/frontend/src/routes/admin/+layout.svelte @@ -12,7 +12,7 @@ let { data, children } = $props(); -mt-6: cancel the global layout's pt-6 on
Height fills from below the global header (64px) to bottom of viewport. --> -
+
diff --git a/frontend/src/routes/admin/+page.svelte b/frontend/src/routes/admin/+page.svelte index b9da7c26..728c6a69 100644 --- a/frontend/src/routes/admin/+page.svelte +++ b/frontend/src/routes/admin/+page.svelte @@ -36,7 +36,7 @@ onMount(() => { {/if} - {#if data.canManageGroups} + {#if data.canManagePermissions}
{m.admin_tab_groups()}
diff --git a/frontend/src/routes/admin/EntityNav.svelte b/frontend/src/routes/admin/EntityNav.svelte index 0de54985..6af4e062 100644 --- a/frontend/src/routes/admin/EntityNav.svelte +++ b/frontend/src/routes/admin/EntityNav.svelte @@ -10,7 +10,7 @@ let { tagCount, canManageUsers, canManageTags, - canManageGroups, + canManagePermissions, canRunMaintenance }: { userCount: number; @@ -18,7 +18,7 @@ let { tagCount: number; canManageUsers: boolean; canManageTags: boolean; - canManageGroups: boolean; + canManagePermissions: boolean; canRunMaintenance: boolean; } = $props(); @@ -28,6 +28,9 @@ const isActive = (section: string) => currentPath.startsWith(`/admin/${section}` let flyoutOpen = $state(false); let flyoutTriggerElement: HTMLButtonElement | null = null; +// All four section buttons open the same flyout that repeats the full nav. +// This is intentional: on tablet the flyout shows all sections as a wider navigation panel, +// not a context-specific panel for the clicked section. async function openFlyout(event: MouseEvent) { flyoutTriggerElement = event.currentTarget as HTMLButtonElement; flyoutOpen = true; @@ -71,6 +74,7 @@ function handleKeydown(event: KeyboardEvent) { data-flyout-trigger type="button" aria-label={m.admin_tab_users()} + title={m.admin_tab_users()} onclick={openFlyout} class="flex min-h-[44px] w-full flex-col items-center justify-center gap-0.5 border-l-[3px] py-3 transition-colors lg:hidden {isActive('users') @@ -131,12 +135,13 @@ function handleKeydown(event: KeyboardEvent) { {/if} - {#if canManageGroups} + {#if canManagePermissions}