From ff9ae198c45540a8f0c9df2b11c798342c2ab4b0 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 15 Apr 2026 14:54:55 +0200 Subject: [PATCH] refactor(notifications): extract useNotificationStream and NotificationDropdown from NotificationBell (#200) Co-Authored-By: Claude Sonnet 4.6 --- .../lib/components/NotificationBell.svelte | 244 ++---------------- .../components/NotificationDropdown.svelte | 138 ++++++++++ .../useNotificationStream.svelte.test.ts | 109 ++++++++ .../lib/hooks/useNotificationStream.svelte.ts | 95 +++++++ 4 files changed, 370 insertions(+), 216 deletions(-) create mode 100644 frontend/src/lib/components/NotificationDropdown.svelte create mode 100644 frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts create mode 100644 frontend/src/lib/hooks/useNotificationStream.svelte.ts diff --git a/frontend/src/lib/components/NotificationBell.svelte b/frontend/src/lib/components/NotificationBell.svelte index a1bb7b04..9781e366 100644 --- a/frontend/src/lib/components/NotificationBell.svelte +++ b/frontend/src/lib/components/NotificationBell.svelte @@ -3,53 +3,23 @@ import { onMount, onDestroy } from 'svelte'; import { goto } from '$app/navigation'; import { m } from '$lib/paraglide/messages.js'; import { clickOutside } from '$lib/actions/clickOutside'; -import { - type NotificationItem, - relativeTime, - parseNotificationEvent -} from '$lib/utils/notifications'; +import { createNotificationStream } from '$lib/hooks/useNotificationStream.svelte'; +import NotificationDropdown from './NotificationDropdown.svelte'; -let notifications: NotificationItem[] = $state([]); -let unreadCount: number = $state(0); let open = $state(false); - -// DOM refs managed via attachments let bellButtonEl: HTMLButtonElement | null = null; -let firstFocusableEl: HTMLButtonElement | null = null; -let eventSource: EventSource | null = null; - -async function fetchNotifications() { - try { - const res = await fetch('/api/notifications?size=10'); - if (res.ok) { - const data = await res.json(); - notifications = data.content ?? []; - } - } catch (e) { - console.error('Failed to fetch notifications', e); - } -} - -async function fetchUnreadCount() { - try { - const res = await fetch('/api/notifications/unread-count'); - if (res.ok) { - const data = await res.json(); - unreadCount = data.count; - } - } catch (e) { - console.error('Failed to fetch unread count', e); - } -} +const stream = createNotificationStream(); async function toggleDropdown() { open = !open; if (open) { - await fetchNotifications(); - // defer focus until DOM updates + await stream.fetchNotifications(); setTimeout(() => { - firstFocusableEl?.focus(); + const firstBtn = document.querySelector( + '[role="dialog"] button, [role="dialog"] a' + ); + firstBtn?.focus(); }, 0); } } @@ -59,16 +29,8 @@ function closeDropdown() { bellButtonEl?.focus(); } -async function markRead(notification: NotificationItem) { - if (!notification.read) { - try { - await fetch(`/api/notifications/${notification.id}/read`, { method: 'PATCH' }); - notification.read = true; - unreadCount = Math.max(0, unreadCount - 1); - } catch (e) { - console.error('Failed to mark notification as read', e); - } - } +async function handleMarkRead(notification: Parameters[0]) { + await stream.markRead(notification); const url = notification.annotationId ? `/documents/${notification.documentId}?commentId=${notification.referenceId}&annotationId=${notification.annotationId}` : `/documents/${notification.documentId}?commentId=${notification.referenceId}`; @@ -76,18 +38,6 @@ async function markRead(notification: NotificationItem) { goto(url); } -async function markAllRead() { - try { - await fetch('/api/notifications/read-all', { method: 'POST' }); - for (const n of notifications) { - n.read = true; - } - unreadCount = 0; - } catch (e) { - console.error('Failed to mark all notifications as read', e); - } -} - function handleKeydown(event: KeyboardEvent) { if (event.key === 'Escape' && open) { event.stopPropagation(); @@ -95,7 +45,6 @@ function handleKeydown(event: KeyboardEvent) { } } -// Attachment: stores the element reference for the bell button function attachBellButton(node: HTMLButtonElement) { bellButtonEl = node; return () => { @@ -103,27 +52,12 @@ function attachBellButton(node: HTMLButtonElement) { }; } -// Attachment: stores the element reference for the first focusable element in the dropdown -function attachFirstFocusable(node: HTMLButtonElement) { - firstFocusableEl = node; - return () => { - firstFocusableEl = null; - }; -} - onMount(() => { - fetchUnreadCount(); - eventSource = new EventSource('/api/notifications/stream'); - eventSource.addEventListener('notification', (e) => { - const notification = parseNotificationEvent(e.data); - if (!notification) return; - notifications = [notification, ...notifications]; - if (!notification.read) unreadCount += 1; - }); + stream.init(); }); onDestroy(() => { - eventSource?.close(); + stream.destroy(); }); @@ -135,14 +69,13 @@ onDestroy(() => { {@attach attachBellButton} type="button" onclick={toggleDropdown} - aria-label={unreadCount > 0 - ? m.notification_bell_unread_label({ count: unreadCount }) + aria-label={stream.unreadCount > 0 + ? m.notification_bell_unread_label({ count: stream.unreadCount }) : m.notification_bell_label()} aria-expanded={open} aria-haspopup="true" class="relative rounded-sm p-2 text-white/65 transition-colors hover:bg-white/10 hover:text-white focus:outline-none focus-visible:ring-2 focus-visible:ring-focus-ring" > - { /> - - {#if unreadCount > 0} - - {unreadCount} - - {/if} + + + {stream.unreadCount} + - {#if open} - + {/if} diff --git a/frontend/src/lib/components/NotificationDropdown.svelte b/frontend/src/lib/components/NotificationDropdown.svelte new file mode 100644 index 00000000..ee46548c --- /dev/null +++ b/frontend/src/lib/components/NotificationDropdown.svelte @@ -0,0 +1,138 @@ + + + diff --git a/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts b/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts new file mode 100644 index 00000000..d195406d --- /dev/null +++ b/frontend/src/lib/hooks/__tests__/useNotificationStream.svelte.test.ts @@ -0,0 +1,109 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { NotificationItem } from '../useNotificationStream.svelte'; + +// Track the last created EventSource instance +let lastEventSource: { + close: ReturnType; + onopen: (() => void) | null; + onerror: (() => void) | null; +} | null = null; + +class MockEventSource { + onopen: (() => void) | null = null; + onerror: (() => void) | null = null; + close = vi.fn(); + private listeners: Record void)[]> = {}; + + constructor() { + // eslint-disable-next-line @typescript-eslint/no-this-alias + lastEventSource = this; + } + + addEventListener(type: string, fn: (e: MessageEvent) => void) { + if (!this.listeners[type]) this.listeners[type] = []; + this.listeners[type].push(fn); + } +} + +vi.stubGlobal('EventSource', MockEventSource); + +const mockFetch = vi.fn(); +vi.stubGlobal('fetch', mockFetch); + +// Import after stubs are set up +const { createNotificationStream } = await import('../useNotificationStream.svelte'); + +beforeEach(() => { + mockFetch.mockReset(); + lastEventSource = null; +}); + +function makeNotification(overrides: Partial = {}): NotificationItem { + return { + id: 'n1', + type: 'REPLY', + actorName: 'Hans', + documentId: 'doc-1', + referenceId: 'ref-1', + annotationId: null, + read: false, + createdAt: new Date().toISOString(), + ...overrides + }; +} + +describe('createNotificationStream', () => { + it('starts with empty notifications and zero unreadCount', () => { + const stream = createNotificationStream(); + expect(stream.notifications).toHaveLength(0); + expect(stream.unreadCount).toBe(0); + }); + + it('fetchUnreadCount updates unreadCount from API', async () => { + mockFetch.mockResolvedValueOnce(new Response(JSON.stringify({ count: 3 }), { status: 200 })); + const stream = createNotificationStream(); + await stream.fetchUnreadCount(); + expect(stream.unreadCount).toBe(3); + }); + + it('fetchNotifications populates notifications from API', async () => { + const items = [makeNotification()]; + mockFetch.mockResolvedValueOnce( + new Response(JSON.stringify({ content: items }), { status: 200 }) + ); + const stream = createNotificationStream(); + await stream.fetchNotifications(); + expect(stream.notifications).toHaveLength(1); + expect(stream.notifications[0].id).toBe('n1'); + }); + + it('markRead marks notification as read and decrements unreadCount', async () => { + mockFetch + .mockResolvedValueOnce(new Response(JSON.stringify({ count: 2 }), { status: 200 })) + .mockResolvedValueOnce(new Response(null, { status: 200 })); + const stream = createNotificationStream(); + await stream.fetchUnreadCount(); + + const notification = makeNotification({ read: false }); + await stream.markRead(notification); + expect(notification.read).toBe(true); + expect(stream.unreadCount).toBe(1); + }); + + it('markAllRead calls the API and resets unreadCount', async () => { + mockFetch.mockResolvedValueOnce(new Response(null, { status: 200 })); + const stream = createNotificationStream(); + await stream.markAllRead(); + expect(mockFetch).toHaveBeenCalledWith('/api/notifications/read-all', { method: 'POST' }); + expect(stream.unreadCount).toBe(0); + }); + + it('destroy closes the EventSource', async () => { + mockFetch.mockResolvedValue(new Response(JSON.stringify({ count: 0 }), { status: 200 })); + const stream = createNotificationStream(); + stream.init(); + expect(lastEventSource).not.toBeNull(); + stream.destroy(); + expect(lastEventSource!.close).toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/lib/hooks/useNotificationStream.svelte.ts b/frontend/src/lib/hooks/useNotificationStream.svelte.ts new file mode 100644 index 00000000..0a03dada --- /dev/null +++ b/frontend/src/lib/hooks/useNotificationStream.svelte.ts @@ -0,0 +1,95 @@ +import { type NotificationItem, parseNotificationEvent } from '$lib/utils/notifications'; + +export type { NotificationItem }; + +export function createNotificationStream() { + let notifications = $state([]); + let unreadCount = $state(0); + let eventSource: EventSource | null = null; + + async function fetchNotifications(): Promise { + try { + const res = await fetch('/api/notifications?size=10'); + if (res.ok) { + const data = await res.json(); + notifications = data.content ?? []; + } + } catch (e) { + console.error('Failed to fetch notifications', e); + } + } + + async function fetchUnreadCount(): Promise { + try { + const res = await fetch('/api/notifications/unread-count'); + if (res.ok) { + const data = await res.json(); + unreadCount = data.count; + } + } catch (e) { + console.error('Failed to fetch unread count', e); + } + } + + async function markRead(notification: NotificationItem): Promise { + if (!notification.read) { + try { + await fetch(`/api/notifications/${notification.id}/read`, { method: 'PATCH' }); + notification.read = true; + unreadCount = Math.max(0, unreadCount - 1); + } catch (e) { + console.error('Failed to mark notification as read', e); + } + } + } + + async function markAllRead(): Promise { + try { + await fetch('/api/notifications/read-all', { method: 'POST' }); + for (const n of notifications) { + n.read = true; + } + unreadCount = 0; + } catch (e) { + console.error('Failed to mark all notifications as read', e); + } + } + + function init(): void { + fetchUnreadCount(); + eventSource = new EventSource('/api/notifications/stream'); + eventSource.addEventListener('notification', (e) => { + const notification = parseNotificationEvent(e.data); + if (!notification) return; + notifications = [notification, ...notifications]; + if (!notification.read) unreadCount += 1; + }); + eventSource.onopen = () => { + fetchUnreadCount(); + }; + eventSource.onerror = () => { + // Close on error to avoid repeated reconnect noise + eventSource?.close(); + }; + } + + function destroy(): void { + eventSource?.close(); + eventSource = null; + } + + return { + get notifications() { + return notifications; + }, + get unreadCount() { + return unreadCount; + }, + fetchNotifications, + fetchUnreadCount, + markRead, + markAllRead, + init, + destroy + }; +}