diff --git a/frontend/src/lib/timeline/EventForm.svelte b/frontend/src/lib/timeline/EventForm.svelte index 48c78100..eb323232 100644 --- a/frontend/src/lib/timeline/EventForm.svelte +++ b/frontend/src/lib/timeline/EventForm.svelte @@ -30,6 +30,10 @@ interface FormResult { type?: string; personIds?: string[]; documentIds?: string[]; + // Rehydrated chip data (id + label) so the pickers re-render after a fail(400) + // even on a no-JS full reload — bare ids alone can't rebuild a chip (REQ-010). + persons?: PersonOption[]; + documents?: DocumentOption[]; } let { @@ -46,8 +50,10 @@ let { form?: FormResult | null; } = $props(); -// Initial-state snapshot from incoming props (event > preserved fail payload). -// The form owns these after mount; re-mount with a different `event` to reset. +// Initial-state snapshot from incoming props, preferring a preserved fail payload +// over the seeded `event`. This component is intentionally single-shot: props are +// snapshotted into $state once, so a parent re-render with a different `event` +// won't update the form — the two dedicated routes always remount, which is fine. let title = $state(form?.title ?? event?.title ?? ''); let description = $state(form?.description ?? event?.description ?? ''); let type = $state(form?.type ?? event?.type ?? 'PERSONAL'); @@ -55,19 +61,23 @@ let dateIso = $state(event?.eventDate ?? ''); let precision = $state((event?.precision as DatePrecision) ?? 'DAY'); let endDateIso = $state(event?.eventDateEnd ?? ''); +// On a fail(400) the server returns rehydrated chip data (form.persons/documents) +// so the pickers survive the round-trip — even without JS — ahead of the seeded +// `event` or the prefill initials (REQ-010 / Decision 6). let selectedPersons = $state( - event?.persons ? event.persons.map(toPersonOption) : initialPersons + form?.persons ?? (event?.persons ? event.persons.map(toPersonOption) : initialPersons) ); let selectedDocuments = $state( - event?.documents - ? event.documents.map((d) => ({ - // Graceful degradation: DocumentRef has no precision fields. formatDocumentOption - // defaults a missing precision to DAY, so the chip shows the full documentDate. - id: d.id, - title: d.title, - documentDate: d.documentDate - })) - : initialDocuments + form?.documents ?? + (event?.documents + ? event.documents.map((d) => ({ + // Graceful degradation: DocumentRef has no precision fields. formatDocumentOption + // defaults a missing precision to DAY, so the chip shows the full documentDate. + id: d.id, + title: d.title, + documentDate: d.documentDate + })) + : initialDocuments) ); const isEdit = $derived(event !== undefined); diff --git a/frontend/src/lib/timeline/EventForm.svelte.spec.ts b/frontend/src/lib/timeline/EventForm.svelte.spec.ts index 5803483c..05610533 100644 --- a/frontend/src/lib/timeline/EventForm.svelte.spec.ts +++ b/frontend/src/lib/timeline/EventForm.svelte.spec.ts @@ -58,6 +58,19 @@ describe('EventForm — required-field error (REQ-010)', () => { await page.getByRole('button', { name: 'Speichern' }).click(); await expect.element(page.getByText('Bitte einen Titel eingeben.')).toBeInTheDocument(); }); + + it('rehydrates the pickers from the fail payload (Decision 6)', async () => { + render(EventForm, { + form: { + titleError: 'Bitte einen Titel eingeben.', + title: '', + persons: [{ id: 'p1', displayName: 'Anna Müller' }], + documents: [{ id: 'd1', title: 'Brief A', documentDate: '1925-04-01' }] + } + }); + await expect.element(page.getByText('Anna Müller')).toBeInTheDocument(); + await expect.element(page.getByText(/Brief A/)).toBeInTheDocument(); + }); }); describe('EventForm — submitting state (named AC)', () => { diff --git a/frontend/src/lib/timeline/eventFormServer.ts b/frontend/src/lib/timeline/eventFormServer.ts index 7d31b89d..aa7334a5 100644 --- a/frontend/src/lib/timeline/eventFormServer.ts +++ b/frontend/src/lib/timeline/eventFormServer.ts @@ -1,9 +1,12 @@ -import { fail } from '@sveltejs/kit'; import { m } from '$lib/paraglide/messages.js'; +import { createApiClient } from '$lib/shared/api.server'; import type { components } from '$lib/generated/api'; import type { DatePrecision } from '$lib/shared/utils/documentDate'; +import type { PersonOption } from '$lib/person/personOption'; +import type { DocumentOption } from '$lib/document/documentTypeahead'; type TimelineEventRequest = components['schemas']['TimelineEventRequest']; +type ApiClient = ReturnType; // Prevents open redirect: validate before constructing /persons/{id}. See OWASP CWE-601. const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; @@ -68,24 +71,60 @@ export function parseEventForm(formData: FormData): ParsedEventForm { } /** - * Surfaces all failing required-field errors simultaneously (title + date) via a - * fail(400) payload that also preserves every entered value — including the - * picker arrays — so the form re-renders without losing state. Returns null when - * the form is valid. + * Returns both failing required-field errors (title + date) simultaneously, or + * null when the form is valid. The route owns the `fail(400)` so it can enrich + * the payload with the preserved field values and rehydrated picker selections. */ -export function validateEventForm(parsed: ParsedEventForm) { +export function validateEventForm( + parsed: ParsedEventForm +): { titleError: string; dateError: string } | null { const titleError = parsed.title.length === 0 ? m.event_editor_title_required() : ''; const dateError = parsed.eventDate.length === 0 ? m.event_editor_date_required() : ''; if (!titleError && !dateError) return null; - return fail(400, { - titleError, - dateError, + return { titleError, dateError }; +} + +/** The entered field values echoed back in every `fail(...)` so the form re-renders without loss. */ +export function preservedFormFields(parsed: ParsedEventForm) { + return { title: parsed.title, description: parsed.description, type: parsed.type, personIds: parsed.personIds, documentIds: parsed.documentIds - }); + }; +} + +/** + * Re-fetches the selected persons/documents by id so a `fail(400)` can re-render + * the pickers with full chip labels — the form only resubmits bare ids, which + * cannot rebuild a chip on their own (Decision 6 / REQ-010). Non-ok lookups are + * swallowed: a since-deleted id silently drops from the picker rather than + * leaking existence, mirroring the prefill path in the new-route load. + */ +export async function lookupSelections( + api: ApiClient, + personIds: string[], + documentIds: string[] +): Promise<{ persons: PersonOption[]; documents: DocumentOption[] }> { + const [personResults, documentResults] = await Promise.all([ + Promise.all(personIds.map((id) => api.GET('/api/persons/{id}', { params: { path: { id } } }))), + Promise.all( + documentIds.map((id) => api.GET('/api/documents/{id}', { params: { path: { id } } })) + ) + ]); + return { + persons: personResults.filter((r) => r.response.ok && r.data).map((r) => r.data!), + documents: documentResults + .filter((r) => r.response.ok && r.data) + .map((r) => ({ + id: r.data!.id, + title: r.data!.title, + documentDate: r.data!.documentDate, + metaDatePrecision: r.data!.metaDatePrecision, + metaDateEnd: r.data!.metaDateEnd + })) + }; } /** Builds the TimelineEventRequest write body from parsed form fields. */ diff --git a/frontend/src/routes/zeitstrahl/events/[id]/edit/+page.server.ts b/frontend/src/routes/zeitstrahl/events/[id]/edit/+page.server.ts index fd1f5931..1fe3913c 100644 --- a/frontend/src/routes/zeitstrahl/events/[id]/edit/+page.server.ts +++ b/frontend/src/routes/zeitstrahl/events/[id]/edit/+page.server.ts @@ -5,6 +5,8 @@ import { requireWriteAll } from '$lib/shared/server/permissions'; import { parseEventForm, validateEventForm, + preservedFormFields, + lookupSelections, toEventRequest, resolveNavTarget } from '$lib/timeline/eventFormServer'; @@ -47,14 +49,21 @@ export const actions = { }) => { const formData = await request.formData(); const parsed = parseEventForm(formData); + const api = createApiClient(fetch); - const invalid = validateEventForm(parsed); - if (invalid) return invalid; + const errors = validateEventForm(parsed); + if (errors) { + const { persons, documents } = await lookupSelections( + api, + parsed.personIds, + parsed.documentIds + ); + return fail(400, { ...errors, ...preservedFormFields(parsed), persons, documents }); + } const versionRaw = formData.get('version')?.toString(); const version = versionRaw ? Number(versionRaw) : undefined; - const api = createApiClient(fetch); const result = await api.PUT('/api/timeline/events/{id}', { params: { path: { id: params.id } }, body: toEventRequest(parsed, version) @@ -63,11 +72,7 @@ export const actions = { if (!result.response.ok) { return fail(result.response.status, { error: getErrorMessage(extractErrorCode(result.error)), - title: parsed.title, - description: parsed.description, - type: parsed.type, - personIds: parsed.personIds, - documentIds: parsed.documentIds + ...preservedFormFields(parsed) }); } diff --git a/frontend/src/routes/zeitstrahl/events/new/+page.server.ts b/frontend/src/routes/zeitstrahl/events/new/+page.server.ts index 28f14a45..6b0b3a32 100644 --- a/frontend/src/routes/zeitstrahl/events/new/+page.server.ts +++ b/frontend/src/routes/zeitstrahl/events/new/+page.server.ts @@ -5,6 +5,8 @@ import { requireWriteAll } from '$lib/shared/server/permissions'; import { parseEventForm, validateEventForm, + preservedFormFields, + lookupSelections, toEventRequest, resolveNavTarget } from '$lib/timeline/eventFormServer'; @@ -53,21 +55,24 @@ export async function load({ export const actions = { save: async ({ request, fetch }: { request: Request; fetch: typeof globalThis.fetch }) => { const parsed = parseEventForm(await request.formData()); - - const invalid = validateEventForm(parsed); - if (invalid) return invalid; - const api = createApiClient(fetch); + + const errors = validateEventForm(parsed); + if (errors) { + const { persons, documents } = await lookupSelections( + api, + parsed.personIds, + parsed.documentIds + ); + return fail(400, { ...errors, ...preservedFormFields(parsed), persons, documents }); + } + const result = await api.POST('/api/timeline/events', { body: toEventRequest(parsed) }); if (!result.response.ok) { return fail(result.response.status, { error: getErrorMessage(extractErrorCode(result.error)), - title: parsed.title, - description: parsed.description, - type: parsed.type, - personIds: parsed.personIds, - documentIds: parsed.documentIds + ...preservedFormFields(parsed) }); } diff --git a/frontend/src/routes/zeitstrahl/events/new/page.server.spec.ts b/frontend/src/routes/zeitstrahl/events/new/page.server.spec.ts index cf45f266..83a6482d 100644 --- a/frontend/src/routes/zeitstrahl/events/new/page.server.spec.ts +++ b/frontend/src/routes/zeitstrahl/events/new/page.server.spec.ts @@ -150,9 +150,23 @@ describe('zeitstrahl/events/new save action (REQ-004/009/010/015)', () => { expect(post.mock.calls[0][1].body.precision).toBe('DAY'); }); - it('returns fail(400) with preserved picker arrays on blank title', async () => { + it('returns fail(400) with preserved + rehydrated pickers on blank title', async () => { const post = vi.fn(); - vi.mocked(createApiClient).mockReturnValue({ POST: post } as never); + // On validation failure the action re-fetches the selected persons/documents + // by id so the fail payload can rebuild full chips (Decision 6 / REQ-010). + const get = vi.fn((path: string, opts: { params: { path: { id: string } } }) => { + const id = opts.params.path.id; + if (path === '/api/persons/{id}') + return Promise.resolve({ + response: { ok: true }, + data: { id, displayName: `Person ${id}` } + }); + return Promise.resolve({ + response: { ok: true }, + data: { id, title: `Doc ${id}`, documentDate: '1925-04-01' } + }); + }); + vi.mocked(createApiClient).mockReturnValue({ POST: post, GET: get } as never); const result = await actions.save( saveEvent({ @@ -169,6 +183,12 @@ describe('zeitstrahl/events/new save action (REQ-004/009/010/015)', () => { expect(failData(result).personIds).toEqual(['p1', 'p2']); expect(failData(result).documentIds).toEqual(['d1']); expect(failData(result).titleError).toBeTruthy(); + // Rehydrated chips carry labels, not just ids. + expect(failData(result).persons).toEqual([ + { id: 'p1', displayName: 'Person p1' }, + { id: 'p2', displayName: 'Person p2' } + ]); + expect((failData(result).documents as { id: string }[])[0].id).toBe('d1'); }); it('surfaces both title and date errors when both blank (REQ-011)', async () => {