feat(persons): clean filterable paginated directory with crash fix

Rewrite /persons: server-side filter chips (type, family-only, has-documents) that AND within the clean reader default (familyMember OR documentCount > 0), a writer-only show-all/Zu-prüfen toggle, and reused Pagination. Extract PersonCard (fixes the null-lastName render crash and never shows a "?" initial — provisional/UNKNOWN/"?" entries get a neutral placeholder avatar + a text+icon "unbestätigt" badge, WCAG 1.4.1) and PersonFilterBar (44px aria-pressed chips, role=switch toggle with the count in its accessible name). The loader applies the reader restriction unless review=1 and surfaces a cheap needsReviewCount. i18n keys added for de/en/es. Refs #667 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-27 13:55:18 +02:00
parent 67272178a9
commit 888adcb185
12 changed files with 802 additions and 87 deletions
--- a/frontend/src/routes/persons/+page.server.ts
+++ b/frontend/src/routes/persons/+page.server.ts
@@ -2,8 +2,23 @@ import { error } from '@sveltejs/kit';
 import { createApiClient } from '$lib/shared/api.server';
 import { getErrorMessage } from '$lib/shared/errors';

+const PAGE_SIZE = 50;
+
+type PersonType = 'PERSON' | 'INSTITUTION' | 'GROUP';
+
+function parseType(raw: string | null): PersonType | undefined {
+	return raw === 'PERSON' || raw === 'INSTITUTION' || raw === 'GROUP' ? raw : undefined;
+}
+
 export async function load({ url, fetch, locals }) {
 	const q = url.searchParams.get('q') || '';
+	const page = Math.max(0, Number.parseInt(url.searchParams.get('page') ?? '0', 10) || 0);
+	const review =
+		url.searchParams.get('review') === '1' || url.searchParams.get('review') === 'true';
+	const type = parseType(url.searchParams.get('type'));
+	const familyOnly = url.searchParams.get('familyOnly') === 'true';
+	const hasDocuments = url.searchParams.get('hasDocuments') === 'true';
+
 	const api = createApiClient(fetch);

 	const canWrite =
@@ -11,15 +26,32 @@ export async function load({ url, fetch, locals }) {
 			g.permissions.includes('WRITE_ALL')
 		) ?? false;

-	const [personsResult, statsResult] = await Promise.all([
-		api.GET('/api/persons', { params: { query: { q: q || undefined } } }),
-		api.GET('/api/stats', {})
+	const filters = {
+		q: q || undefined,
+		type,
+		familyOnly: familyOnly || undefined,
+		hasDocuments: hasDocuments || undefined,
+		review: review || undefined,
+		page,
+		size: PAGE_SIZE
+	};
+
+	// The "Zu prüfen (N)" link count is the totalElements of a provisional-only query. A size=1
+	// page keeps the extra request cheap — we only need the count, not the rows.
+	const [personsResult, statsResult, reviewCountResult] = await Promise.all([
+		api.GET('/api/persons', { params: { query: filters } }),
+		api.GET('/api/stats', {}),
+		canWrite
+			? api.GET('/api/persons', { params: { query: { provisional: true, review: true, size: 1 } } })
+			: Promise.resolve(null)
 	]);

 	if (!personsResult.response.ok) {
 		throw error(personsResult.response.status, getErrorMessage(undefined));
 	}

+	const result = personsResult.data!;
+
 	const stats = statsResult.response.ok
 		? {
 				totalPersons: statsResult.data!.totalPersons ?? 0,
@@ -27,5 +59,21 @@ export async function load({ url, fetch, locals }) {
 			}
 		: { totalPersons: 0, totalDocuments: 0 };

-	return { persons: personsResult.data!, stats, q, canWrite };
+	const needsReviewCount =
+		reviewCountResult && reviewCountResult.response.ok
+			? (reviewCountResult.data!.totalElements ?? 0)
+			: 0;
+
+	return {
+		persons: result.items,
+		totalElements: result.totalElements,
+		totalPages: result.totalPages,
+		pageNumber: result.pageNumber,
+		pageSize: result.pageSize,
+		filters: { type, familyOnly, hasDocuments, review },
+		needsReviewCount,
+		stats,
+		q,
+		canWrite
+	};
 }
--- a/frontend/src/routes/persons/+page.svelte
+++ b/frontend/src/routes/persons/+page.svelte
@@ -1,9 +1,12 @@
 <script lang="ts">
 import { goto } from '$app/navigation';
+import { page } from '$app/state';
 import { untrack } from 'svelte';
+import { SvelteURLSearchParams } from 'svelte/reactivity';
 import { m } from '$lib/paraglide/messages.js';
-import { formatLifeDateRange } from '$lib/person/personLifeDates';
-import PersonTypeBadge from '$lib/person/PersonTypeBadge.svelte';
+import PersonCard from '$lib/person/PersonCard.svelte';
+import PersonFilterBar from '$lib/person/PersonFilterBar.svelte';
+import Pagination from '$lib/shared/primitives/Pagination.svelte';
 import PersonsStatsBar from './PersonsStatsBar.svelte';
 import PersonsEmptyState from './PersonsEmptyState.svelte';

@@ -18,12 +21,31 @@ $effect(() => {

 let searchTimeout: ReturnType<typeof setTimeout>;

+// Debounced search must preserve the active filters/review flag — compose over the live URL,
+// never a bare `?q=` that would wipe page/type/review.
 function handleSearch() {
 	clearTimeout(searchTimeout);
 	searchTimeout = setTimeout(() => {
-		goto(`/persons?q=${q}`, { keepFocus: true });
+		const params = new SvelteURLSearchParams(page.url.searchParams);
+		params.delete('page');
+		if (q) params.set('q', q);
+		else params.delete('q');
+		const qs = params.toString();
+		goto(qs ? `/persons?${qs}` : '/persons', { keepFocus: true });
 	}, 300);
 }
+
+// Pagination links preserve every active param and only change the page index.
+function buildPageHref(targetPage: number): string {
+	const params = new SvelteURLSearchParams(page.url.searchParams);
+	params.set('page', String(targetPage));
+	return `/persons?${params.toString()}`;
+}
+
+const hasResults = $derived(data.persons.length > 0);
+const noFiltersActive = $derived(
+	!data.q && !data.filters.type && !data.filters.familyOnly && !data.filters.hasDocuments
+);
 </script>

 <svelte:head>
@@ -32,7 +54,7 @@ function handleSearch() {

 <div class="mx-auto max-w-7xl px-4 py-12 sm:px-6 lg:px-8">
 	<!-- Header: title+stats on left, search+CTA on right -->
-	<div class="mb-10 flex flex-wrap items-end justify-between gap-4 border-b border-ink/10 pb-6">
+	<div class="mb-6 flex flex-wrap items-end justify-between gap-4 border-b border-ink/10 pb-6">
 		<div>
 			<h1 class="font-serif text-3xl font-medium text-ink">{m.page_title_persons()}</h1>
 			<div class="mt-2">
@@ -46,7 +68,7 @@ function handleSearch() {
 		<div class="flex items-center gap-3">
 			<!-- Search -->
 			<div class="relative">
-				<label for="search" class="sr-only">Suche</label>
+				<label for="search" class="sr-only">{m.persons_search_placeholder()}</label>
 				<input
 					id="search"
 					type="text"
@@ -69,11 +91,21 @@ function handleSearch() {
 				</div>
 			</div>

+			<!-- Triage link (transcriber only) -->
+			{#if data.canWrite}
+				<a
+					href="/persons/review"
+					class="inline-flex min-h-[44px] items-center gap-1.5 rounded-sm border border-line bg-surface px-4 py-2 font-sans text-sm font-semibold text-ink transition-colors hover:bg-muted"
+				>
+					{m.persons_toggle_needs_review({ count: data.needsReviewCount })}
+				</a>
+			{/if}
+
 			<!-- New person CTA -->
 			{#if data.canWrite}
 				<a
 					href="/persons/new"
-					class="inline-flex items-center gap-1.5 rounded-sm bg-primary px-4 py-2.5 font-sans text-sm font-bold tracking-wide text-primary-fg transition-colors hover:bg-primary/80"
+					class="inline-flex min-h-[44px] items-center gap-1.5 rounded-sm bg-primary px-4 py-2.5 font-sans text-sm font-bold tracking-wide text-primary-fg transition-colors hover:bg-primary/80"
 				>
 					<img
 						src="/degruyter-icons/Simple/Medium-24px/SVG/Action/Add/Add-General-MD.svg"
@@ -87,86 +119,35 @@ function handleSearch() {
 		</div>
 	</div>

-	{#if data.persons.length === 0}
-		<PersonsEmptyState />
+	<!-- Filter chips + show-all toggle -->
+	<div class="mb-8">
+		<PersonFilterBar
+			type={data.filters.type}
+			familyOnly={data.filters.familyOnly}
+			hasDocuments={data.filters.hasDocuments}
+			review={data.filters.review}
+			needsReviewCount={data.needsReviewCount}
+			canWrite={data.canWrite}
+		/>
+	</div>
+
+	{#if !hasResults}
+		{#if noFiltersActive}
+			<PersonsEmptyState />
+		{:else}
+			<div
+				class="flex flex-col items-center justify-center rounded-lg border border-dashed border-line bg-surface py-16 text-center"
+			>
+				<p class="font-serif text-lg text-ink">{m.persons_empty_filtered()}</p>
+			</div>
+		{/if}
 	{:else}
 		<div class="grid grid-cols-1 gap-4 sm:grid-cols-2 lg:grid-cols-4">
 			{#each data.persons as person (person.id)}
-				<a href="/persons/{person.id}" class="group block">
-					<div
-						class="flex h-full flex-col items-center gap-2 rounded border border-line bg-surface px-4 py-6 text-center shadow-sm transition-all duration-200 hover:border-l-4 hover:border-accent hover:shadow-md"
-					>
-						<!-- Avatar -->
-						<div
-							class="flex h-12 w-12 flex-shrink-0 items-center justify-center rounded-full bg-primary font-serif text-base font-bold text-primary-fg transition-colors"
-						>
-							{#if person.personType && person.personType !== 'PERSON'}
-								<svg
-									class="h-5 w-5"
-									fill="none"
-									viewBox="0 0 24 24"
-									stroke="currentColor"
-									stroke-width="2"
-								>
-									{#if person.personType === 'INSTITUTION'}
-										<path
-											stroke-linecap="round"
-											stroke-linejoin="round"
-											d="M19 21V5a2 2 0 00-2-2H7a2 2 0 00-2 2v16m14 0H5m14 0h2m-2 0v-2M5 21H3m2 0v-2m4-12h2m-2 4h2m4-4h2m-2 4h2"
-										/>
-									{:else if person.personType === 'GROUP'}
-										<path
-											stroke-linecap="round"
-											stroke-linejoin="round"
-											d="M17 20h5v-2a3 3 0 00-5.356-1.857M17 20H7m10 0v-2c0-.656-.126-1.283-.356-1.857M7 20H2v-2a3 3 0 015.356-1.857M7 20v-2c0-.656.126-1.283.356-1.857m0 0a5.002 5.002 0 019.288 0M15 7a3 3 0 11-6 0 3 3 0 016 0zm6 3a2 2 0 11-4 0 2 2 0 014 0zM7 10a2 2 0 11-4 0 2 2 0 014 0z"
-										/>
-									{:else}
-										<path
-											stroke-linecap="round"
-											stroke-linejoin="round"
-											d="M8.228 9c.549-1.165 2.03-2 3.772-2 2.21 0 4 1.343 4 3 0 1.4-1.278 2.575-3.006 2.907-.542.104-.994.54-.994 1.093m0 3h.01"
-										/>
-									{/if}
-								</svg>
-							{:else}
-								{person.firstName ? person.firstName[0] : person.lastName[0]}{person.lastName[0]}
-							{/if}
-						</div>
-
-						<!-- Name -->
-						<p class="font-serif text-sm font-bold text-ink group-hover:underline">
-							{person.displayName}
-						</p>
-
-						{#if person.personType && person.personType !== 'PERSON'}
-							<PersonTypeBadge personType={person.personType} />
-						{/if}
-
-						<!-- Alias -->
-						{#if person.alias}
-							<p class="font-sans text-xs text-ink-2 italic">„{person.alias}"</p>
-						{/if}
-
-						<!-- Life dates -->
-						{#if person.birthYear || person.deathYear}
-							<p class="font-sans text-[11px] text-ink-3">
-								{formatLifeDateRange(person.birthYear, person.deathYear)}
-							</p>
-						{/if}
-
-						<!-- Doc count chip -->
-						{#if (person.documentCount ?? 0) > 0}
-							<span
-								class="mt-1 inline-flex items-center rounded-full border border-line bg-muted px-2.5 py-0.5 font-sans text-[11px] font-semibold text-ink-2"
-							>
-								{person.documentCount === 1
-									? m.person_card_doc_count_one()
-									: m.person_card_doc_count_many({ count: person.documentCount ?? 0 })}
-							</span>
-						{/if}
-					</div>
-				</a>
+				<PersonCard person={person} />
 			{/each}
 		</div>
+
+		<Pagination page={data.pageNumber} totalPages={data.totalPages} makeHref={buildPageHref} />
 	{/if}
 </div>
--- a/frontend/src/routes/persons/page.server.spec.ts
+++ b/frontend/src/routes/persons/page.server.spec.ts
@@ -0,0 +1,137 @@
+import { describe, expect, it, vi, beforeEach } from 'vitest';
+
+vi.mock('$lib/shared/api.server', () => ({
+	createApiClient: vi.fn(),
+	extractErrorCode: (e: unknown) => (e as { code?: string } | undefined)?.code
+}));
+
+import { load } from './+page.server';
+import { createApiClient } from '$lib/shared/api.server';
+
+beforeEach(() => vi.clearAllMocks());
+
+function makeUrl(params: Record<string, string> = {}) {
+	const url = new URL('http://localhost/persons');
+	for (const [key, value] of Object.entries(params)) url.searchParams.set(key, value);
+	return url;
+}
+
+/** Invokes the loader with a minimal event; the partial event is cast to satisfy the type. */
+function runLoad(url: URL, user: unknown) {
+	return load({
+		url,
+		fetch: vi.fn() as unknown as typeof fetch,
+		request: new Request('http://localhost/persons'),
+		locals: { user } as App.Locals
+	} as unknown as Parameters<typeof load>[0]);
+}
+
+/** Mock the typed client. /api/persons returns a paged envelope; /api/stats returns counts. */
+function mockApi() {
+	const personsResult = {
+		response: { ok: true, status: 200 },
+		data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 }
+	};
+	// Loose `...args` signature (matching the documents loader spec) so call tuples aren't
+	// narrowed to length 1 — the test inspects calls[i][1].params.query.
+	const get = vi.fn((...args: unknown[]) => {
+		if (args[0] === '/api/stats') {
+			return Promise.resolve({
+				response: { ok: true, status: 200 },
+				data: { totalPersons: 7, totalDocuments: 3 }
+			});
+		}
+		return Promise.resolve(personsResult);
+	});
+	vi.mocked(createApiClient).mockReturnValue({ GET: get } as unknown as ReturnType<
+		typeof createApiClient
+	>);
+	return get;
+}
+
+const writer = { groups: [{ permissions: ['READ_ALL', 'WRITE_ALL'] }] };
+const reader = { groups: [{ permissions: ['READ_ALL'] }] };
+
+type GetCall = [string, { params: { query: Record<string, unknown> } }];
+
+/** Find the GET call to a path, optionally narrowing by a query predicate. */
+function findCall(
+	get: ReturnType<typeof vi.fn>,
+	path: string,
+	matchQuery?: (q: Record<string, unknown>) => boolean
+): GetCall | undefined {
+	return (get.mock.calls as unknown as GetCall[]).find(
+		(c) => c[0] === path && (!matchQuery || matchQuery(c[1].params.query))
+	);
+}
+
+describe('persons page load — reader default', () => {
+	it('does NOT pass review when no review param is present (clean reader default)', async () => {
+		const get = mockApi();
+
+		await runLoad(makeUrl(), reader);
+
+		const personsCall = findCall(get, '/api/persons');
+		expect(personsCall?.[1].params.query.review).toBeUndefined();
+	});
+
+	it('passes review=true when review=1 is in the URL', async () => {
+		const get = mockApi();
+
+		await runLoad(makeUrl({ review: '1' }), reader);
+
+		const personsCall = findCall(get, '/api/persons');
+		expect(personsCall?.[1].params.query.review).toBe(true);
+	});
+});
+
+describe('persons page load — filter forwarding', () => {
+	it('forwards type, familyOnly, hasDocuments and page to the API', async () => {
+		const get = mockApi();
+
+		await runLoad(
+			makeUrl({ type: 'INSTITUTION', familyOnly: 'true', hasDocuments: 'true', page: '2' }),
+			reader
+		);
+
+		const personsCall = findCall(get, '/api/persons');
+		expect(personsCall?.[1].params.query).toMatchObject({
+			type: 'INSTITUTION',
+			familyOnly: true,
+			hasDocuments: true,
+			page: 2,
+			size: 50
+		});
+	});
+
+	it('clamps a negative page to 0', async () => {
+		const get = mockApi();
+
+		await runLoad(makeUrl({ page: '-5' }), reader);
+
+		const personsCall = findCall(get, '/api/persons');
+		expect(personsCall?.[1].params.query.page).toBe(0);
+	});
+});
+
+describe('persons page load — needsReviewCount', () => {
+	it('fires a provisional count request for writers', async () => {
+		const get = mockApi();
+
+		await runLoad(makeUrl(), writer);
+
+		const provisionalCall = findCall(get, '/api/persons', (query) => query.provisional === true);
+		expect(provisionalCall).toBeDefined();
+	});
+
+	it('does not fire a provisional count request for read-only users', async () => {
+		const get = mockApi();
+
+		const result = await runLoad(makeUrl(), reader);
+
+		const provisionalCall = findCall(get, '/api/persons', (query) => query.provisional === true);
+		expect(provisionalCall).toBeUndefined();
+		expect(result.needsReviewCount).toBe(0);
+		expect(result.canWrite).toBe(false);
+	});
+});
--- a/frontend/src/routes/persons/page.svelte.spec.ts
+++ b/frontend/src/routes/persons/page.svelte.spec.ts
@@ -6,6 +6,7 @@ import Page from './+page.svelte';
 const tick = () => new Promise((r) => setTimeout(r, 0));

 vi.mock('$app/navigation', () => ({ goto: vi.fn() }));
+vi.mock('$app/state', () => ({ page: { url: new URL('http://localhost/persons') } }));

 const makePerson = (overrides = {}) => ({
 	id: '1',
@@ -13,6 +14,8 @@ const makePerson = (overrides = {}) => ({
 	lastName: 'Mustermann',
 	displayName: 'Max Mustermann',
 	documentCount: 0,
+	provisional: false,
+	personType: 'PERSON',
 	...overrides
 });

@@ -24,7 +27,13 @@ const emptyData = {
 	canBlogWrite: false,
 	q: '',
 	persons: [],
-	stats: defaultStats
+	stats: defaultStats,
+	totalElements: 0,
+	totalPages: 0,
+	pageNumber: 0,
+	pageSize: 50,
+	filters: { type: undefined, familyOnly: false, hasDocuments: false, review: false },
+	needsReviewCount: 0
 };
 const dataWithPersons = {
 	...emptyData,
--- a/frontend/src/routes/persons/page.svelte.test.ts
+++ b/frontend/src/routes/persons/page.svelte.test.ts
@@ -16,6 +16,8 @@ vi.mock('$app/navigation', () => ({
 	onNavigate: () => () => {}
 }));

+vi.mock('$app/state', () => ({ page: { url: new URL('http://localhost/persons') } }));
+
 const { default: PersonsListPage } = await import('./+page.svelte');

 afterEach(cleanup);
@@ -31,8 +33,15 @@ const baseData = (overrides: Record<string, unknown> = {}) => ({
 		birthYear?: number;
 		deathYear?: number;
 		documentCount?: number;
+		provisional?: boolean;
 	}>,
 	stats: { totalPersons: 0, totalDocuments: 0 },
+	totalElements: 0,
+	totalPages: 0,
+	pageNumber: 0,
+	pageSize: 50,
+	filters: { type: undefined, familyOnly: false, hasDocuments: false, review: false },
+	needsReviewCount: 0,
 	canWrite: false,
 	q: '',
 	...overrides