diff --git a/src/renderer/components/notifications/NotificationsView.tsx b/src/renderer/components/notifications/NotificationsView.tsx index e7303e0b..839cd45b 100644 --- a/src/renderer/components/notifications/NotificationsView.tsx +++ b/src/renderer/components/notifications/NotificationsView.tsx @@ -128,15 +128,21 @@ export const NotificationsView = (): React.JSX.Element => { rowVirtualizer.scrollToIndex(0); }, [activeFilter, rowVirtualizer]); - // Handle mark all read + // Derive filtered unread count for scoped button visibility + const filteredUnreadCount = useMemo(() => { + if (activeFilter === null) return unreadCount; + return filteredNotifications.filter((n) => !n.isRead).length; + }, [activeFilter, filteredNotifications, unreadCount]); + + // Handle mark all read (scoped to active filter) const handleMarkAllRead = async (): Promise => { - await markAllNotificationsRead(); + await markAllNotificationsRead(activeFilter ?? undefined); }; - // Handle clear all with confirmation + // Handle clear all with confirmation (scoped to active filter) const handleClearAll = async (): Promise => { if (showClearConfirm) { - await clearNotifications(); + await clearNotifications(activeFilter ?? undefined); setShowClearConfirm(false); } else { setShowClearConfirm(true); @@ -205,7 +211,13 @@ export const NotificationsView = (): React.JSX.Element => { {notifications.length > 0 && ( - {unreadCount > 0 ? `${unreadCount} unread` : `${notifications.length} total`} + {activeFilter !== null + ? filteredUnreadCount > 0 + ? `${filteredUnreadCount} unread in filter` + : `${filteredNotifications.length} in filter` + : unreadCount > 0 + ? `${unreadCount} unread` + : `${notifications.length} total`} )} @@ -213,19 +225,21 @@ export const NotificationsView = (): React.JSX.Element => { {/* Action Buttons */} {notifications.length > 0 && (
- {/* Mark all read */} - {unreadCount > 0 && ( + {/* Mark all/filtered read */} + {filteredUnreadCount > 0 && ( )} - {/* Clear all */} + {/* Clear all/filtered */}
diff --git a/src/renderer/store/slices/notificationSlice.ts b/src/renderer/store/slices/notificationSlice.ts index e4e00541..320c71a7 100644 --- a/src/renderer/store/slices/notificationSlice.ts +++ b/src/renderer/store/slices/notificationSlice.ts @@ -29,9 +29,9 @@ export interface NotificationSlice { // Actions fetchNotifications: () => Promise; markNotificationRead: (id: string) => Promise; - markAllNotificationsRead: () => Promise; + markAllNotificationsRead: (triggerName?: string) => Promise; deleteNotification: (id: string) => Promise; - clearNotifications: () => Promise; + clearNotifications: (triggerName?: string) => Promise; navigateToError: (error: DetectedError) => void; openNotificationsTab: () => void; } @@ -99,19 +99,41 @@ export const createNotificationSlice: StateCreator { + // Mark all notifications as read (optionally scoped to a trigger) + markAllNotificationsRead: async (triggerName?: string) => { try { - const success = await api.notifications.markAllRead(); - if (!success) { - await get().fetchNotifications(); - return; + if (triggerName !== undefined) { + // Scoped: mark only matching unread notifications + const { notifications } = get(); + const matching = notifications.filter((n) => { + const label = n.triggerName ?? 'Other'; + return label === triggerName && !n.isRead; + }); + if (matching.length === 0) return; + const results = await Promise.all(matching.map((n) => api.notifications.markRead(n.id))); + if (results.some((r) => !r)) { + await get().fetchNotifications(); + return; + } + const matchingIds = new Set(matching.map((n) => n.id)); + set((state) => { + const updated = state.notifications.map((n) => + matchingIds.has(n.id) ? { ...n, isRead: true } : n + ); + return { notifications: updated, unreadCount: updated.filter((n) => !n.isRead).length }; + }); + } else { + // Unscoped: mark all + const success = await api.notifications.markAllRead(); + if (!success) { + await get().fetchNotifications(); + return; + } + set((state) => ({ + notifications: state.notifications.map((n) => ({ ...n, isRead: true })), + unreadCount: 0, + })); } - // Optimistically update local state - set((state) => ({ - notifications: state.notifications.map((n) => ({ ...n, isRead: true })), - unreadCount: 0, - })); } catch (error) { logger.error('Failed to mark all notifications as read:', error); } @@ -136,18 +158,42 @@ export const createNotificationSlice: StateCreator { + // Clear all notifications (optionally scoped to a trigger) + clearNotifications: async (triggerName?: string) => { try { - const success = await api.notifications.clear(); - if (!success) { - await get().fetchNotifications(); - return; + if (triggerName !== undefined) { + // Scoped: delete only matching notifications + const { notifications } = get(); + const matching = notifications.filter((n) => { + const label = n.triggerName ?? 'Other'; + return label === triggerName; + }); + if (matching.length === 0) return; + const results = await Promise.all(matching.map((n) => api.notifications.delete(n.id))); + if (results.some((r) => !r)) { + await get().fetchNotifications(); + return; + } + const matchingIds = new Set(matching.map((n) => n.id)); + set((state) => { + const remaining = state.notifications.filter((n) => !matchingIds.has(n.id)); + return { + notifications: remaining, + unreadCount: remaining.filter((n) => !n.isRead).length, + }; + }); + } else { + // Unscoped: clear all + const success = await api.notifications.clear(); + if (!success) { + await get().fetchNotifications(); + return; + } + set({ + notifications: [], + unreadCount: 0, + }); } - set({ - notifications: [], - unreadCount: 0, - }); } catch (error) { logger.error('Failed to clear notifications:', error); } diff --git a/test/renderer/store/notificationSlice.test.ts b/test/renderer/store/notificationSlice.test.ts index f96f06e0..5ffb106d 100644 --- a/test/renderer/store/notificationSlice.test.ts +++ b/test/renderer/store/notificationSlice.test.ts @@ -70,6 +70,221 @@ describe('notificationSlice', () => { }); }); + describe('scoped markAllNotificationsRead', () => { + const makeNotification = ( + id: string, + triggerName: string | undefined, + isRead: boolean + ): DetectedError => ({ + id, + sessionId: 's1', + projectId: 'p1', + lineNumber: 1, + timestamp: Date.now(), + triggerName, + severity: 'error', + message: `msg-${id}`, + isRead, + }); + + it('marks only matching trigger notifications as read', async () => { + const n1 = makeNotification('n1', 'tool result error', false); + const n2 = makeNotification('n2', 'high token usage', false); + const n3 = makeNotification('n3', 'tool result error', false); + store.setState({ notifications: [n1, n2, n3] as never[], unreadCount: 3 }); + + await store.getState().markAllNotificationsRead('tool result error'); + + const state = store.getState(); + expect(state.notifications.find((n) => n.id === 'n1')!.isRead).toBe(true); + expect(state.notifications.find((n) => n.id === 'n2')!.isRead).toBe(false); + expect(state.notifications.find((n) => n.id === 'n3')!.isRead).toBe(true); + expect(state.unreadCount).toBe(1); + }); + + it('calls markRead individually for each matching notification', async () => { + const n1 = makeNotification('n1', 'trigger-a', false); + const n2 = makeNotification('n2', 'trigger-a', false); + store.setState({ notifications: [n1, n2] as never[], unreadCount: 2 }); + + await store.getState().markAllNotificationsRead('trigger-a'); + + expect(mockAPI.notifications.markRead).toHaveBeenCalledWith('n1'); + expect(mockAPI.notifications.markRead).toHaveBeenCalledWith('n2'); + expect(mockAPI.notifications.markAllRead).not.toHaveBeenCalled(); + }); + + it('uses markAllRead API when no triggerName is provided', async () => { + const n1 = makeNotification('n1', 'trigger-a', false); + store.setState({ notifications: [n1] as never[], unreadCount: 1 }); + + await store.getState().markAllNotificationsRead(); + + expect(mockAPI.notifications.markAllRead).toHaveBeenCalled(); + expect(mockAPI.notifications.markRead).not.toHaveBeenCalled(); + }); + + it('treats notifications without triggerName as "Other"', async () => { + const n1 = makeNotification('n1', undefined, false); + const n2 = makeNotification('n2', 'trigger-a', false); + store.setState({ notifications: [n1, n2] as never[], unreadCount: 2 }); + + await store.getState().markAllNotificationsRead('Other'); + + expect(store.getState().notifications.find((n) => n.id === 'n1')!.isRead).toBe(true); + expect(store.getState().notifications.find((n) => n.id === 'n2')!.isRead).toBe(false); + expect(store.getState().unreadCount).toBe(1); + }); + + it('skips already-read notifications in scoped mode', async () => { + const n1 = makeNotification('n1', 'trigger-a', true); + const n2 = makeNotification('n2', 'trigger-a', false); + store.setState({ notifications: [n1, n2] as never[], unreadCount: 1 }); + + await store.getState().markAllNotificationsRead('trigger-a'); + + // Only n2 should be sent to API (n1 already read) + expect(mockAPI.notifications.markRead).toHaveBeenCalledTimes(1); + expect(mockAPI.notifications.markRead).toHaveBeenCalledWith('n2'); + }); + + it('no-ops when no unread notifications match the trigger', async () => { + const n1 = makeNotification('n1', 'trigger-a', true); + store.setState({ notifications: [n1] as never[], unreadCount: 0 }); + + await store.getState().markAllNotificationsRead('trigger-a'); + + expect(mockAPI.notifications.markRead).not.toHaveBeenCalled(); + }); + + it('re-fetches when any scoped markRead call fails', async () => { + const n1 = makeNotification('n1', 'trigger-a', false); + const n2 = makeNotification('n2', 'trigger-a', false); + store.setState({ notifications: [n1, n2] as never[], unreadCount: 2 }); + + mockAPI.notifications.markRead.mockResolvedValueOnce(true).mockResolvedValueOnce(false); + mockAPI.notifications.get.mockResolvedValue({ notifications: [] }); + + await store.getState().markAllNotificationsRead('trigger-a'); + + expect(mockAPI.notifications.get).toHaveBeenCalled(); + }); + }); + + describe('scoped clearNotifications', () => { + const makeNotification = ( + id: string, + triggerName: string | undefined, + isRead: boolean + ): DetectedError => ({ + id, + sessionId: 's1', + projectId: 'p1', + lineNumber: 1, + timestamp: Date.now(), + triggerName, + severity: 'error', + message: `msg-${id}`, + isRead, + }); + + it('deletes only matching trigger notifications', async () => { + const n1 = makeNotification('n1', 'tool result error', false); + const n2 = makeNotification('n2', 'high token usage', false); + const n3 = makeNotification('n3', 'tool result error', true); + store.setState({ notifications: [n1, n2, n3] as never[], unreadCount: 2 }); + + await store.getState().clearNotifications('tool result error'); + + const state = store.getState(); + expect(state.notifications).toHaveLength(1); + expect(state.notifications[0].id).toBe('n2'); + expect(state.unreadCount).toBe(1); + }); + + it('calls delete individually for each matching notification', async () => { + const n1 = makeNotification('n1', 'trigger-a', false); + const n2 = makeNotification('n2', 'trigger-a', true); + store.setState({ notifications: [n1, n2] as never[], unreadCount: 1 }); + + await store.getState().clearNotifications('trigger-a'); + + expect(mockAPI.notifications.delete).toHaveBeenCalledWith('n1'); + expect(mockAPI.notifications.delete).toHaveBeenCalledWith('n2'); + expect(mockAPI.notifications.clear).not.toHaveBeenCalled(); + }); + + it('uses clear API when no triggerName is provided', async () => { + const n1 = makeNotification('n1', 'trigger-a', false); + store.setState({ notifications: [n1] as never[], unreadCount: 1 }); + + await store.getState().clearNotifications(); + + expect(mockAPI.notifications.clear).toHaveBeenCalled(); + expect(mockAPI.notifications.delete).not.toHaveBeenCalled(); + }); + + it('treats notifications without triggerName as "Other"', async () => { + const n1 = makeNotification('n1', undefined, false); + const n2 = makeNotification('n2', 'trigger-a', false); + store.setState({ notifications: [n1, n2] as never[], unreadCount: 2 }); + + await store.getState().clearNotifications('Other'); + + const state = store.getState(); + expect(state.notifications).toHaveLength(1); + expect(state.notifications[0].id).toBe('n2'); + expect(state.unreadCount).toBe(1); + }); + + it('clears both read and unread notifications for the trigger', async () => { + const n1 = makeNotification('n1', 'trigger-a', false); + const n2 = makeNotification('n2', 'trigger-a', true); + store.setState({ notifications: [n1, n2] as never[], unreadCount: 1 }); + + await store.getState().clearNotifications('trigger-a'); + + expect(store.getState().notifications).toHaveLength(0); + expect(store.getState().unreadCount).toBe(0); + }); + + it('no-ops when no notifications match the trigger', async () => { + const n1 = makeNotification('n1', 'trigger-b', false); + store.setState({ notifications: [n1] as never[], unreadCount: 1 }); + + await store.getState().clearNotifications('trigger-a'); + + expect(mockAPI.notifications.delete).not.toHaveBeenCalled(); + expect(store.getState().notifications).toHaveLength(1); + }); + + it('re-fetches when any scoped delete call fails', async () => { + const n1 = makeNotification('n1', 'trigger-a', false); + const n2 = makeNotification('n2', 'trigger-a', false); + store.setState({ notifications: [n1, n2] as never[], unreadCount: 2 }); + + mockAPI.notifications.delete.mockResolvedValueOnce(true).mockResolvedValueOnce(false); + mockAPI.notifications.get.mockResolvedValue({ notifications: [] }); + + await store.getState().clearNotifications('trigger-a'); + + expect(mockAPI.notifications.get).toHaveBeenCalled(); + }); + + it('correctly recalculates unreadCount after scoped clear', async () => { + const n1 = makeNotification('n1', 'trigger-a', false); + const n2 = makeNotification('n2', 'trigger-b', false); + const n3 = makeNotification('n3', 'trigger-b', true); + store.setState({ notifications: [n1, n2, n3] as never[], unreadCount: 2 }); + + await store.getState().clearNotifications('trigger-a'); + + // n1 removed (trigger-a, unread), n2+n3 remain + expect(store.getState().notifications).toHaveLength(2); + expect(store.getState().unreadCount).toBe(1); // only n2 is unread + }); + }); + describe('navigateToError', () => { const createMockError = (overrides?: Partial): DetectedError => ({ id: 'error-1',