From 6c3552dc6af8e636d0f25d0d366bdbd3b04eaf05 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 27 May 2026 13:56:00 +0200 Subject: [PATCH] refactor(persons): update all callers for the paged /api/persons response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/persons now returns PersonSearchResult { items, … } instead of a bare list. Update every caller: the dashboard top-persons path reads .items; the unused full-list fetches in documents/new and documents/[id]/edit are dropped (both pages use the self-fetching PersonTypeahead); the raw-fetch consumers (PersonTypeahead, PersonMultiSelect, PersonMentionEditor) read body.items and pass review=true so search still spans the whole directory. Specs updated to the new envelope shape. Refs #667 Co-Authored-By: Claude Opus 4.7 --- .../src/lib/person/PersonMultiSelect.svelte | 5 +-- .../person/PersonMultiSelect.svelte.spec.ts | 3 +- .../src/lib/person/PersonTypeahead.svelte | 8 +++-- .../lib/person/PersonTypeahead.svelte.spec.ts | 18 +++++++--- .../discussion/PersonMentionEditor.svelte | 6 ++-- .../PersonMentionEditor.svelte.spec.ts | 36 +++++++++++-------- frontend/src/routes/+page.server.ts | 2 +- .../documents/[id]/edit/+page.server.ts | 11 ++---- .../src/routes/documents/new/+page.server.ts | 6 ++-- 9 files changed, 56 insertions(+), 39 deletions(-) diff --git a/frontend/src/lib/person/PersonMultiSelect.svelte b/frontend/src/lib/person/PersonMultiSelect.svelte index 80a4e97b..41b392a0 100644 --- a/frontend/src/lib/person/PersonMultiSelect.svelte +++ b/frontend/src/lib/person/PersonMultiSelect.svelte @@ -34,9 +34,10 @@ function handleInput() { } loading = true; try { - const res = await fetch(`/api/persons?q=${encodeURIComponent(searchTerm)}`); + const res = await fetch(`/api/persons?review=true&q=${encodeURIComponent(searchTerm)}`); if (res.ok) { - const all: Person[] = await res.json(); + const body = await res.json(); + const all: Person[] = body.items ?? []; results = all.filter((p) => !selectedPersons.some((s) => s.id === p.id)); } } catch { diff --git a/frontend/src/lib/person/PersonMultiSelect.svelte.spec.ts b/frontend/src/lib/person/PersonMultiSelect.svelte.spec.ts index f188ae74..86c26274 100644 --- a/frontend/src/lib/person/PersonMultiSelect.svelte.spec.ts +++ b/frontend/src/lib/person/PersonMultiSelect.svelte.spec.ts @@ -34,11 +34,12 @@ const PERSONS = [ ]; function mockFetch(persons = PERSONS) { + // /api/persons now returns a paged { items } envelope. vi.stubGlobal( 'fetch', vi.fn().mockResolvedValue({ ok: true, - json: vi.fn().mockResolvedValue(persons) + json: vi.fn().mockResolvedValue({ items: persons }) }) ); } diff --git a/frontend/src/lib/person/PersonTypeahead.svelte b/frontend/src/lib/person/PersonTypeahead.svelte index 7622fb84..1a650757 100644 --- a/frontend/src/lib/person/PersonTypeahead.svelte +++ b/frontend/src/lib/person/PersonTypeahead.svelte @@ -79,8 +79,12 @@ const typeahead = createTypeahead({ return res.ok ? filter(await res.json()) : []; } if (term.length < 1) return []; - const res = await fetch(`/api/persons?q=${encodeURIComponent(term)}`); - return res.ok ? filter(await res.json()) : []; + // review=true so the typeahead searches the whole directory (incl. provisional / + // zero-document persons), not just the clean reader subset. + const res = await fetch(`/api/persons?review=true&q=${encodeURIComponent(term)}`); + if (!res.ok) return []; + const body = await res.json(); + return filter(body.items ?? []); }, debounceMs: 300 }); diff --git a/frontend/src/lib/person/PersonTypeahead.svelte.spec.ts b/frontend/src/lib/person/PersonTypeahead.svelte.spec.ts index 81761078..7ae14ff2 100644 --- a/frontend/src/lib/person/PersonTypeahead.svelte.spec.ts +++ b/frontend/src/lib/person/PersonTypeahead.svelte.spec.ts @@ -24,12 +24,18 @@ const PERSONS = [ ]; function mockFetchWithPersons(persons = PERSONS) { + // The directory endpoint (/api/persons?…) now returns a paged { items } envelope; the + // correspondents endpoint still returns a bare array. Branch the mock on the URL. vi.stubGlobal( 'fetch', - vi.fn().mockResolvedValue({ - ok: true, - json: vi.fn().mockResolvedValue(persons) - }) + vi.fn().mockImplementation((url: string) => + Promise.resolve({ + ok: true, + json: vi + .fn() + .mockResolvedValue(url.includes('/correspondents') ? persons : { items: persons }) + }) + ) ); } @@ -266,7 +272,9 @@ describe('PersonTypeahead – correspondent mode', () => { await waitForDebounce(); const fetchMock = globalThis.fetch as ReturnType; - expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Anna')); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining('/api/persons?review=true&q=Anna') + ); }); }); diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte index 466c00ed..39261606 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte @@ -197,16 +197,16 @@ onMount(() => { // Defensive client-side cap — server-side enforcement is tracked // separately. Markus on PR #629. const res = await fetch( - `/api/persons?q=${encodeURIComponent(query)}&limit=${SEARCH_RESULT_LIMIT}` + `/api/persons?review=true&q=${encodeURIComponent(query)}&size=${SEARCH_RESULT_LIMIT}` ); if (id !== requestId) return; if (!res.ok) { dropdownState.items = []; return; } - const data = (await res.json()) as Person[]; + const body = (await res.json()) as { items?: Person[] }; if (id !== requestId) return; - dropdownState.items = data.slice(0, SEARCH_RESULT_LIMIT); + dropdownState.items = (body.items ?? []).slice(0, SEARCH_RESULT_LIMIT); } catch { if (id !== requestId) return; dropdownState.items = []; diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts index 9ce23358..1dfdd053 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts @@ -53,14 +53,14 @@ const ANNA: Person = { function mockFetchWithPersons(persons: Person[] = [AUGUSTE, ANNA]) { vi.stubGlobal( 'fetch', - vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue(persons) }) + vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: persons }) }) ); } function mockFetchEmpty() { vi.stubGlobal( 'fetch', - vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([]) }) + vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [] }) }) ); } @@ -132,28 +132,30 @@ describe('PersonMentionEditor — typeahead', () => { it('hits /api/persons?q= with the typed query', async () => { const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); await userEvent.type(page.getByRole('textbox'), '@Aug'); await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Aug')); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining('/api/persons?review=true&q=Aug') + ); }); }); it('appends &limit=5 to the fetch URL (defensive client-side cap, Markus on PR #629)', async () => { const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); await userEvent.type(page.getByRole('textbox'), '@Aug'); await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('limit=5')); + expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('size=5')); }); }); @@ -206,7 +208,7 @@ describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => { it('editing the search input fires a debounced fetch with the new query', async () => { const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); @@ -243,7 +245,7 @@ describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => { // Sara on PR #629 round 3. const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); @@ -270,7 +272,7 @@ describe('PersonMentionEditor — AC-2/3: search input drives fetch', () => { it('clearing the search input clears the list without firing a fetch', async () => { const fetchMock = vi .fn() - .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue([AUGUSTE]) }); + .mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ items: [AUGUSTE] }) }); vi.stubGlobal('fetch', fetchMock); renderHost(); @@ -323,10 +325,12 @@ describe('PersonMentionEditor — whitespace-only query', () => { describe('PersonMentionEditor — stale-response race', () => { it('discards a stale response that resolves after the search has been cleared', async () => { - let resolveFetch!: (v: { ok: boolean; json: () => Promise }) => void; - const pendingResponse = new Promise<{ ok: boolean; json: () => Promise }>((r) => { - resolveFetch = r; - }); + let resolveFetch!: (v: { ok: boolean; json: () => Promise<{ items: Person[] }> }) => void; + const pendingResponse = new Promise<{ ok: boolean; json: () => Promise<{ items: Person[] }> }>( + (r) => { + resolveFetch = r; + } + ); const fetchMock = vi.fn().mockReturnValue(pendingResponse); vi.stubGlobal('fetch', fetchMock); renderHost(); @@ -334,7 +338,9 @@ describe('PersonMentionEditor — stale-response race', () => { // Open the dropdown and let the debounce fire so a fetch is in flight. await userEvent.type(page.getByRole('textbox'), '@Aug'); await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith(expect.stringContaining('/api/persons?q=Aug')); + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining('/api/persons?review=true&q=Aug') + ); }); // Clear the search input *before* the fetch resolves. @@ -342,7 +348,7 @@ describe('PersonMentionEditor — stale-response race', () => { await expect.element(page.getByRole('searchbox')).toHaveValue(''); // The stale fetch now resolves with persons. The dropdown must stay empty. - resolveFetch({ ok: true, json: () => Promise.resolve([AUGUSTE]) }); + resolveFetch({ ok: true, json: () => Promise.resolve({ items: [AUGUSTE] }) }); // Flush pending Svelte reactivity so any (non-)update from the stale // fetch resolution has landed before we assert. expect.element already // polls, so no fixed-timeout fallback is needed. Sara on PR #629 round 4. diff --git a/frontend/src/routes/+page.server.ts b/frontend/src/routes/+page.server.ts index b217452f..7f7daa6e 100644 --- a/frontend/src/routes/+page.server.ts +++ b/frontend/src/routes/+page.server.ts @@ -52,7 +52,7 @@ export async function load({ fetch, parent }) { await Promise.allSettled(readerFetches); const readerStats = settled(statsRes); - const topPersons = settled(topPersonsRes) ?? []; + const topPersons = settled<{ items: PersonSummaryDTO[] }>(topPersonsRes)?.items ?? []; const searchData = settled<{ items: { document: Document }[] }>(recentDocsRes); const recentDocs = searchData?.items.map((i) => i.document) ?? []; const recentStories = settled(recentStoriesRes) ?? []; diff --git a/frontend/src/routes/documents/[id]/edit/+page.server.ts b/frontend/src/routes/documents/[id]/edit/+page.server.ts index 4cd24a81..5043e963 100644 --- a/frontend/src/routes/documents/[id]/edit/+page.server.ts +++ b/frontend/src/routes/documents/[id]/edit/+page.server.ts @@ -24,21 +24,16 @@ export async function load({ const { id } = params; const api = createApiClient(fetch); - const [docResult, personsResult] = await Promise.all([ - api.GET('/api/documents/{id}', { params: { path: { id } } }), - api.GET('/api/persons') - ]); + const docResult = await api.GET('/api/documents/{id}', { params: { path: { id } } }); if (!docResult.response.ok) { throw error(docResult.response.status, getErrorMessage(extractErrorCode(docResult.error))); } - if (!personsResult.response.ok) { - throw error(personsResult.response.status, getErrorMessage('INTERNAL_ERROR')); - } return { document: docResult.data!, - persons: personsResult.data + // Sender/receiver editing uses PersonTypeahead (self-fetching); no full list is consumed. + persons: [] as never[] }; } diff --git a/frontend/src/routes/documents/new/+page.server.ts b/frontend/src/routes/documents/new/+page.server.ts index b54496c3..c393ca98 100644 --- a/frontend/src/routes/documents/new/+page.server.ts +++ b/frontend/src/routes/documents/new/+page.server.ts @@ -57,10 +57,12 @@ export async function load({ ); } - const [personsResult] = await Promise.all([api.GET('/api/persons'), ...requests]); + await Promise.all(requests); return { - persons: personsResult.response.ok ? personsResult.data : [], + // Sender/receiver selection uses PersonTypeahead, which fetches its own results on + // demand — the page never consumes a pre-loaded full person list, so none is fetched. + persons: [] as never[], initialSenderId: senderId, initialSenderName, initialReceivers