fix(notification): address review suggestions
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m23s
CI / OCR Service Tests (pull_request) Successful in 20s
CI / Backend Unit Tests (pull_request) Successful in 3m17s
CI / fail2ban Regex (pull_request) Successful in 40s
CI / Semgrep Security Scan (pull_request) Successful in 20s
CI / Compose Bucket Idempotency (pull_request) Successful in 58s

- 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 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-05-20 07:31:26 +02:00
parent f2bb58e294
commit b5239f515f
3 changed files with 20 additions and 3 deletions

View File

@@ -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 });
};
}}
>

View File

@@ -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, {

View File

@@ -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 } }