From b5239f515f081508d4e01fca24e3a60afe40a827 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 20 May 2026 07:31:26 +0200 Subject: [PATCH] fix(notification): address review suggestions - ChronikFuerDichBox: move update() inside the failure branch so success path skips it, matching NotificationDropdown's pattern - NotificationDropdown test: add role=alert assertion for mark-all-read failure to match existing dismiss-failure coverage in ChronikFuerDichBox - +page.server.ts: use getErrorMessage(undefined) instead of null so the missing-notificationId 400 goes through the same i18n pipeline as other errors Co-Authored-By: Claude Sonnet 4.6 --- .../src/lib/activity/ChronikFuerDichBox.svelte | 4 ++-- .../NotificationDropdown.svelte.test.ts | 17 +++++++++++++++++ .../src/routes/aktivitaeten/+page.server.ts | 2 +- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/frontend/src/lib/activity/ChronikFuerDichBox.svelte b/frontend/src/lib/activity/ChronikFuerDichBox.svelte index 18c2edff..ac893171 100644 --- a/frontend/src/lib/activity/ChronikFuerDichBox.svelte +++ b/frontend/src/lib/activity/ChronikFuerDichBox.svelte @@ -81,8 +81,8 @@ function href(n: NotificationItem): string { return async ({ result, update }) => { if (result.type === 'failure' || result.type === 'error') { errorMessage = m.notification_error_generic(); + await update({ reset: false, invalidateAll: false }); } - await update({ reset: false, invalidateAll: false }); }; }} > @@ -129,8 +129,8 @@ function href(n: NotificationItem): string { return async ({ result, update }) => { if (result.type === 'failure' || result.type === 'error') { errorMessage = m.notification_error_generic(); + await update({ reset: false, invalidateAll: false }); } - await update({ reset: false, invalidateAll: false }); }; }} > diff --git a/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts b/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts index 459bd72b..bdbda7eb 100644 --- a/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts +++ b/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts @@ -234,6 +234,23 @@ describe('NotificationDropdown', () => { expect(optimisticMarkAllRead).toHaveBeenCalledOnce(); }); + it('shows a role=alert error banner when mark-all-read returns a failure', async () => { + mockFormResult.type = 'failure'; + render(NotificationDropdown, { + props: { + notifications: [makeNotification()], + optimisticMarkRead: () => {}, + optimisticMarkAllRead: () => {}, + onClose: () => {} + } + }); + + await page.getByRole('button', { name: /alle gelesen/i }).click(); + + const alert = document.querySelector('[role="alert"]'); + expect(alert).not.toBeNull(); + }); + it('calls onClose when the view-all button is clicked', async () => { const onClose = vi.fn(); render(NotificationDropdown, { diff --git a/frontend/src/routes/aktivitaeten/+page.server.ts b/frontend/src/routes/aktivitaeten/+page.server.ts index 8d5e3fb1..368b7bc3 100644 --- a/frontend/src/routes/aktivitaeten/+page.server.ts +++ b/frontend/src/routes/aktivitaeten/+page.server.ts @@ -73,7 +73,7 @@ export const actions = { const data = await request.formData(); const raw = data.get('notificationId'); const notificationId = typeof raw === 'string' ? raw : null; - if (!notificationId) return fail(400, { error: null }); + if (!notificationId) return fail(400, { error: getErrorMessage(undefined) }); const api = createApiClient(fetch); const result = await api.PATCH('/api/notifications/{id}/read', { params: { path: { id: notificationId } }