fix(timeline): gate event delete behind the confirm dialog

The delete <form> combined onsubmit={confirmDelete} with use:enhance.
SvelteKit's enhance ignores event.defaultPrevented, so the DELETE fired on
the bare click — before the dialog was answered — and the post-DELETE
redirect ran regardless of the user's choice. Reading getConfirmService()
lazily inside the handler also threw (Svelte context is init-only), so the
dialog never appeared even with <ConfirmDialog> mounted.

Capture confirm at init and run the gate inside the enhance submit phase,
calling cancel() on "no". Clear dirty in the result callback so the
beforeNavigate guard no longer prompts "unsaved changes" on the post-delete
redirect.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-06-14 09:00:45 +02:00
parent b8c8fcb1fb
commit 5f2cf5f2c2
2 changed files with 85 additions and 29 deletions

View File

@@ -82,6 +82,10 @@ let selectedDocuments = $state<DocumentOption[]>(
const isEdit = $derived(event !== undefined); const isEdit = $derived(event !== undefined);
// Captured at init — Svelte context is init-only, so reading it lazily inside an
// event handler throws even when <ConfirmDialog> is mounted. Gates the delete.
const { confirm } = getConfirmService();
let titleTouched = $state(false); let titleTouched = $state(false);
let submitting = $state(false); let submitting = $state(false);
let dirty = $state(false); let dirty = $state(false);
@@ -109,18 +113,6 @@ beforeNavigate(({ cancel }) => {
function markDirty() { function markDirty() {
dirty = true; dirty = true;
} }
async function confirmDelete(e: SubmitEvent) {
e.preventDefault();
const { confirm } = getConfirmService();
const ok = await confirm({
title: m.event_editor_delete_confirm_title(),
body: m.event_editor_delete_confirm_body(),
destructive: true,
confirmLabel: m.event_editor_delete()
});
if (ok) (e.target as HTMLFormElement).requestSubmit();
}
</script> </script>
<div class="mx-auto max-w-5xl px-4 py-8"> <div class="mx-auto max-w-5xl px-4 py-8">
@@ -309,9 +301,32 @@ async function confirmDelete(e: SubmitEvent) {
{#if isEdit} {#if isEdit}
<!-- Delete lives in its own form so it posts to the dedicated ?/delete action. <!-- Delete lives in its own form so it posts to the dedicated ?/delete action.
getConfirmService() is read lazily inside the handler so the component The confirm gate runs inside the enhance submit phase: enhance ignores an
mounts cleanly outside a layout (tests) where no confirm context exists. --> onsubmit preventDefault(), so awaiting confirm() and calling cancel() is the
<form method="POST" action="?/delete" onsubmit={confirmDelete} use:enhance class="mt-4"> only thing that actually stops the destructive POST. -->
<form
method="POST"
action="?/delete"
use:enhance={async ({ cancel }) => {
const ok = await confirm({
title: m.event_editor_delete_confirm_title(),
body: m.event_editor_delete_confirm_body(),
destructive: true,
confirmLabel: m.event_editor_delete()
});
if (!ok) {
cancel();
return;
}
return async ({ update }) => {
// Clear dirtiness so beforeNavigate doesn't prompt "unsaved changes"
// on the post-delete redirect.
dirty = false;
await update();
};
}}
class="mt-4"
>
<input type="hidden" name="originPersonId" value={originPersonId} /> <input type="hidden" name="originPersonId" value={originPersonId} />
<button <button
type="submit" type="submit"

View File

@@ -2,6 +2,11 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
import { cleanup, render } from 'vitest-browser-svelte'; import { cleanup, render } from 'vitest-browser-svelte';
import { page } from 'vitest/browser'; import { page } from 'vitest/browser';
import EventForm from './EventForm.svelte'; import EventForm from './EventForm.svelte';
import {
createConfirmService,
CONFIRM_KEY,
type ConfirmService
} from '$lib/shared/services/confirm.svelte.js';
import type { components } from '$lib/generated/api'; import type { components } from '$lib/generated/api';
afterEach(() => { afterEach(() => {
@@ -11,6 +16,17 @@ afterEach(() => {
type TimelineEventView = components['schemas']['TimelineEventView']; type TimelineEventView = components['schemas']['TimelineEventView'];
/**
* EventForm captures the confirm service at init (`getConfirmService()`), so every
* render must provide a CONFIRM_KEY context — mirrors documents/[id]/edit's spec.
* The returned service is handed back so delete tests can drive `settle()`.
*/
function renderForm(props: Record<string, unknown> = {}): ConfirmService {
const service = createConfirmService();
render(EventForm, { props, context: new Map([[CONFIRM_KEY, service]]) });
return service;
}
/** /**
* Minimal TimelineEventView shape used to seed the edit form. Mirrors * Minimal TimelineEventView shape used to seed the edit form. Mirrors
* components['schemas']['TimelineEventView'] — all server-populated fields. * components['schemas']['TimelineEventView'] — all server-populated fields.
@@ -35,12 +51,12 @@ function makeEvent(overrides: Partial<TimelineEventView> = {}): TimelineEventVie
describe('EventForm — date precision RANGE reveal (headline AC, REQ-008/009)', () => { describe('EventForm — date precision RANGE reveal (headline AC, REQ-008/009)', () => {
it('reveals the end-date field when precision is RANGE', async () => { it('reveals the end-date field when precision is RANGE', async () => {
render(EventForm, { event: makeEvent({ precision: 'RANGE', eventDateEnd: '1925-05-01' }) }); renderForm({ event: makeEvent({ precision: 'RANGE', eventDateEnd: '1925-05-01' }) });
await expect.element(page.getByLabelText('Enddatum')).toBeVisible(); await expect.element(page.getByLabelText('Enddatum')).toBeVisible();
}); });
it('hides the end-date field when precision is YEAR', async () => { it('hides the end-date field when precision is YEAR', async () => {
render(EventForm, { event: makeEvent({ precision: 'YEAR' }) }); renderForm({ event: makeEvent({ precision: 'YEAR' }) });
await expect.element(page.getByTestId('end-date-region')).toBeInTheDocument(); await expect.element(page.getByTestId('end-date-region')).toBeInTheDocument();
await expect.element(page.getByLabelText('Enddatum')).not.toBeInTheDocument(); await expect.element(page.getByLabelText('Enddatum')).not.toBeInTheDocument();
}); });
@@ -48,16 +64,14 @@ describe('EventForm — date precision RANGE reveal (headline AC, REQ-008/009)',
describe('EventForm — picker preselect (REQ-014)', () => { describe('EventForm — picker preselect (REQ-014)', () => {
it('preselects a person when initialPersons is provided', async () => { it('preselects a person when initialPersons is provided', async () => {
render(EventForm, { renderForm({ initialPersons: [{ id: 'p1', displayName: 'Anna Müller' }] });
initialPersons: [{ id: 'p1', displayName: 'Anna Müller' }]
});
await expect.element(page.getByText('Anna Müller')).toBeInTheDocument(); await expect.element(page.getByText('Anna Müller')).toBeInTheDocument();
}); });
}); });
describe('EventForm — required-field error (REQ-010)', () => { describe('EventForm — required-field error (REQ-010)', () => {
it('shows a required-field error when title is blank and save is attempted', async () => { it('shows a required-field error when title is blank and save is attempted', async () => {
render(EventForm, {}); renderForm({});
await page.getByRole('button', { name: 'Speichern' }).click(); await page.getByRole('button', { name: 'Speichern' }).click();
await expect.element(page.getByText('Bitte einen Titel eingeben.')).toBeInTheDocument(); await expect.element(page.getByText('Bitte einen Titel eingeben.')).toBeInTheDocument();
}); });
@@ -68,14 +82,14 @@ describe('EventForm — required-field error (REQ-010)', () => {
// a blank-title Save still POSTs (and update()->applyAction crashes with no app). // a blank-title Save still POSTs (and update()->applyAction crashes with no app).
const fetchSpy = vi.fn(() => new Promise<Response>(() => {})); const fetchSpy = vi.fn(() => new Promise<Response>(() => {}));
vi.stubGlobal('fetch', fetchSpy); vi.stubGlobal('fetch', fetchSpy);
render(EventForm, {}); renderForm({});
await page.getByRole('button', { name: 'Speichern' }).click(); await page.getByRole('button', { name: 'Speichern' }).click();
await expect.element(page.getByText('Bitte einen Titel eingeben.')).toBeInTheDocument(); await expect.element(page.getByText('Bitte einen Titel eingeben.')).toBeInTheDocument();
expect(fetchSpy).not.toHaveBeenCalled(); expect(fetchSpy).not.toHaveBeenCalled();
}); });
it('rehydrates the pickers from the fail payload (Decision 6)', async () => { it('rehydrates the pickers from the fail payload (Decision 6)', async () => {
render(EventForm, { renderForm({
form: { form: {
titleError: 'Bitte einen Titel eingeben.', titleError: 'Bitte einen Titel eingeben.',
title: '', title: '',
@@ -88,9 +102,39 @@ describe('EventForm — required-field error (REQ-010)', () => {
}); });
}); });
describe('EventForm — delete is gated behind confirmation (REQ-006)', () => {
it('fires no DELETE until the user confirms, and not at all if they cancel', async () => {
// The DELETE must wait for the confirm dialog. enhance ignores onsubmit
// preventDefault(), so the only correct gate is awaiting confirm() inside the
// enhance submit phase and calling cancel() on a "no" — anything else POSTs
// the destructive DELETE on the bare click, before the dialog is answered.
const fetchSpy = vi.fn(() => new Promise<Response>(() => {}));
vi.stubGlobal('fetch', fetchSpy);
const service = renderForm({ event: makeEvent() });
await page.getByRole('button', { name: 'Löschen' }).click();
// Dialog is pending — nothing must have been POSTed yet.
expect(fetchSpy).not.toHaveBeenCalled();
service.settle(false); // user cancels
await new Promise((r) => setTimeout(r, 0));
expect(fetchSpy).not.toHaveBeenCalled();
});
it('fires the DELETE once the user confirms', async () => {
const fetchSpy = vi.fn(() => new Promise<Response>(() => {}));
vi.stubGlobal('fetch', fetchSpy);
const service = renderForm({ event: makeEvent() });
await page.getByRole('button', { name: 'Löschen' }).click();
service.settle(true); // user confirms
await vi.waitFor(() => expect(fetchSpy).toHaveBeenCalled());
});
});
describe('EventForm — server date error wired per-field (REQ-011)', () => { describe('EventForm — server date error wired per-field (REQ-011)', () => {
it('marks the date field aria-invalid and shows the message on a server date error', async () => { it('marks the date field aria-invalid and shows the message on a server date error', async () => {
render(EventForm, { form: { dateError: 'Bitte ein Datum eingeben.' } }); renderForm({ form: { dateError: 'Bitte ein Datum eingeben.' } });
await expect.element(page.getByText('Bitte ein Datum eingeben.')).toBeInTheDocument(); await expect.element(page.getByText('Bitte ein Datum eingeben.')).toBeInTheDocument();
const dateInput = document.querySelector('#eventDate') as HTMLInputElement; const dateInput = document.querySelector('#eventDate') as HTMLInputElement;
expect(dateInput.getAttribute('aria-invalid')).toBe('true'); expect(dateInput.getAttribute('aria-invalid')).toBe('true');
@@ -106,7 +150,7 @@ describe('EventForm — submitting state (named AC, Decision 8)', () => {
'fetch', 'fetch',
vi.fn(() => new Promise(() => {})) vi.fn(() => new Promise(() => {}))
); );
render(EventForm, { event: makeEvent() }); renderForm({ event: makeEvent() });
const btn = page.getByRole('button', { name: 'Speichern' }); const btn = page.getByRole('button', { name: 'Speichern' });
await expect.element(btn).not.toBeDisabled(); await expect.element(btn).not.toBeDisabled();
await btn.click(); await btn.click();
@@ -116,10 +160,7 @@ describe('EventForm — submitting state (named AC, Decision 8)', () => {
describe('EventForm — server error surfaced inline (REQ-007/013)', () => { describe('EventForm — server error surfaced inline (REQ-007/013)', () => {
it('renders the mapped error from the form prop', async () => { it('renders the mapped error from the form prop', async () => {
render(EventForm, { renderForm({ event: makeEvent(), form: { error: 'Etwas ist schiefgelaufen.' } });
event: makeEvent(),
form: { error: 'Etwas ist schiefgelaufen.' }
});
await expect.element(page.getByText('Etwas ist schiefgelaufen.')).toBeInTheDocument(); await expect.element(page.getByText('Etwas ist schiefgelaufen.')).toBeInTheDocument();
}); });
}); });