fix(geschichten): correct three bugs found in PR review
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 4m36s
CI / OCR Service Tests (pull_request) Successful in 23s
CI / Backend Unit Tests (pull_request) Successful in 5m21s
CI / fail2ban Regex (pull_request) Successful in 52s
CI / Semgrep Security Scan (pull_request) Successful in 26s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m5s

- Pass validated documentId (not raw) to /api/geschichten — Spring backend
  declares @RequestParam UUID documentId; invalid strings cause HTTP 400
- rebuildUrl no longer deletes documentId unconditionally; clearAll and
  removeDocument handle their own documentId deletion explicitly, so
  removePerson/addPerson now preserve an active document filter
- Use ?? instead of || when falling back from doc.title to originalFilename,
  preserving an intentionally-empty title rather than overwriting it

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit was merged in pull request #811.
This commit is contained in:
Marcel
2026-06-12 10:27:19 +02:00
parent 1de10986c3
commit 8eb4a0ffde
4 changed files with 77 additions and 7 deletions

View File

@@ -20,7 +20,7 @@ export const load: PageServerLoad = async ({ url, fetch }) => {
query: { query: {
status: 'PUBLISHED', status: 'PUBLISHED',
personId: personIds.length ? personIds : undefined, personId: personIds.length ? personIds : undefined,
documentId: rawDocumentId ?? undefined documentId: documentId ?? undefined
} }
} }
}), }),
@@ -44,7 +44,7 @@ export const load: PageServerLoad = async ({ url, fetch }) => {
const doc = docResult.data; const doc = docResult.data;
documentFilter = { documentFilter = {
id: documentId, id: documentId,
title: doc.title || doc.originalFilename || null title: doc.title ?? doc.originalFilename ?? null
}; };
} else { } else {
documentFilter = { id: documentId, title: null }; documentFilter = { id: documentId, title: null };

View File

@@ -28,13 +28,15 @@ const emptyMessage = $derived.by(() => {
function rebuildUrl(personIds: string[]) { function rebuildUrl(personIds: string[]) {
const url = new URL(window.location.href); const url = new URL(window.location.href);
url.searchParams.delete('personId'); url.searchParams.delete('personId');
url.searchParams.delete('documentId');
for (const id of personIds) url.searchParams.append('personId', id); for (const id of personIds) url.searchParams.append('personId', id);
return url.pathname + url.search; return url.pathname + url.search;
} }
function clearAll() { function clearAll() {
goto(rebuildUrl([]), { replaceState: true }); const url = new URL(window.location.href);
url.searchParams.delete('personId');
url.searchParams.delete('documentId');
goto(url.pathname + url.search, { replaceState: true });
} }
function addPerson(personId: string) { function addPerson(personId: string) {
@@ -51,7 +53,9 @@ function removePerson(personId: string) {
} }
function removeDocument() { function removeDocument() {
goto(rebuildUrl(selectedPersonIds)); const url = new URL(window.location.href);
url.searchParams.delete('documentId');
goto(url.pathname + url.search);
} }
</script> </script>

View File

@@ -81,6 +81,14 @@ describe('geschichten page load — documentFilter title resolution', () => {
expect(result.documentFilter).toEqual({ id: VALID_UUID, title: 'scan_001.jpg' }); expect(result.documentFilter).toEqual({ id: VALID_UUID, title: 'scan_001.jpg' });
}); });
it('preserves an empty-string title rather than falling back to filename', async () => {
mockApi({ docData: { id: VALID_UUID, title: '', originalFilename: 'scan_001.jpg' } });
const result = await callLoad(makeUrl({ documentId: VALID_UUID }));
expect(result.documentFilter).toEqual({ id: VALID_UUID, title: '' });
});
it('degrades to {id, title: null} on 404 without throwing (resolves, never rejects)', async () => { it('degrades to {id, title: null} on 404 without throwing (resolves, never rejects)', async () => {
// Explicit .resolves locks the no-throw guarantee — if error() were called, this would reject // Explicit .resolves locks the no-throw guarantee — if error() were called, this would reject
mockApi({ docOk: false }); mockApi({ docOk: false });
@@ -159,13 +167,13 @@ describe('geschichten page load — documentFilter title resolution', () => {
); );
}); });
it('passes invalid documentId to the list API without stripping (option B)', async () => { it('omits documentId from the list API when the value is not a valid UUID', async () => {
const mockGet = mockApi(); const mockGet = mockApi();
await callLoad(makeUrl({ documentId: 'not-a-uuid' })); await callLoad(makeUrl({ documentId: 'not-a-uuid' }));
const listCall = mockGet.mock.calls.find((c) => c[0] === '/api/geschichten'); const listCall = mockGet.mock.calls.find((c) => c[0] === '/api/geschichten');
expect(listCall?.[1]?.params?.query?.documentId).toBe('not-a-uuid'); expect(listCall?.[1]?.params?.query?.documentId).toBeUndefined();
}); });
it('keeps forwarding personId filters alongside documentId', async () => { it('keeps forwarding personId filters alongside documentId', async () => {

View File

@@ -206,6 +206,64 @@ describe('geschichten page — multi-person filter chips', () => {
}); });
}); });
it('removing a person chip preserves an active document filter in the URL', async () => {
const { goto } = await import('$app/navigation');
vi.mocked(goto).mockClear();
window.history.replaceState(
{},
'',
'/geschichten?personId=a&documentId=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
);
render(Page, {
data: makeData({
personFilters: [person('a', 'Anna A')] as PageData['personFilters'],
documentFilter: makeDocumentFilter() as PageData['documentFilter']
})
});
const chipBtn = (await page
.getByRole('button', { name: /Anna A aus Filter entfernen/ })
.element()) as HTMLElement;
chipBtn.click();
await vi.waitFor(() => expect(goto).toHaveBeenCalledOnce());
const url = vi.mocked(goto).mock.calls[0][0] as string;
expect(url).not.toContain('personId=a');
expect(url).toContain('documentId=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee');
window.history.replaceState({}, '', '/');
});
it('clearAll removes both person and document filters from the URL', async () => {
const { goto } = await import('$app/navigation');
vi.mocked(goto).mockClear();
window.history.replaceState(
{},
'',
'/geschichten?personId=a&documentId=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
);
render(Page, {
data: makeData({
personFilters: [person('a', 'Anna A')] as PageData['personFilters'],
documentFilter: makeDocumentFilter() as PageData['documentFilter']
})
});
const allBtn = (await page.getByRole('button', { name: 'Alle' }).element()) as HTMLElement;
allBtn.click();
await vi.waitFor(() => expect(goto).toHaveBeenCalledOnce());
const url = vi.mocked(goto).mock.calls[0][0] as string;
expect(url).not.toContain('personId');
expect(url).not.toContain('documentId');
window.history.replaceState({}, '', '/');
});
describe('empty state precedence', () => { describe('empty state precedence', () => {
it('shows geschichten_empty_for_document when only document filter is active', async () => { it('shows geschichten_empty_for_document when only document filter is active', async () => {
render(Page, { render(Page, {