admin-valentin/agent-ecosystem
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #36 from matt1398/feat/noti-tab-scope

Browse source 
feat: enhance notification handling with scoped actions
This commit is contained in:
matt 2026-02-20 12:48:30 +09:00 committed by GitHub
commit 12df279225
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 316 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
src/renderer/components/notifications/NotificationsView.tsx
View file

@ -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<void> => {
await markAllNotificationsRead();
await markAllNotificationsRead(activeFilter ?? undefined);
};
// Handle clear all with confirmation
// Handle clear all with confirmation (scoped to active filter)
const handleClearAll = async (): Promise<void> => {
if (showClearConfirm) {
await clearNotifications();
await clearNotifications(activeFilter ?? undefined);
setShowClearConfirm(false);
} else {
setShowClearConfirm(true);
@ -205,7 +211,13 @@ export const NotificationsView = (): React.JSX.Element => {
</span>
{notifications.length > 0 && (
<span className="text-xs" style={{ color: 'var(--color-text-muted)' }}>
{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`}
</span>
)}
</div>
@ -213,19 +225,21 @@ export const NotificationsView = (): React.JSX.Element => {
{/* Action Buttons */}
{notifications.length > 0 && (
<div className="flex items-center gap-1">
{/* Mark all read */}
{unreadCount > 0 && (
{/* Mark all/filtered read */}
{filteredUnreadCount > 0 && (
<button
onClick={handleMarkAllRead}
className="flex items-center gap-1.5 rounded-md px-2 py-1.5 text-xs transition-colors hover:opacity-80"
style={{ color: 'var(--color-text-muted)' }}
title="Mark all as read"
title={activeFilter !== null ? 'Mark filtered as read' : 'Mark all as read'}
>
<CheckCheck className="size-4" />
<span className="hidden sm:inline">Mark all read</span>
<span className="hidden sm:inline">
{activeFilter !== null ? 'Mark filtered read' : 'Mark all read'}
</span>
</button>
)}
{/* Clear all */}
{/* Clear all/filtered */}
<button
onClick={handleClearAll}
className={`flex items-center gap-1.5 rounded-md px-2 py-1.5 text-xs transition-colors ${
@ -234,11 +248,17 @@ export const NotificationsView = (): React.JSX.Element => {
: 'hover:opacity-80'
}`}
style={showClearConfirm ? undefined : { color: 'var(--color-text-muted)' }}
title="Clear all notifications"
title={
activeFilter !== null ? 'Clear filtered notifications' : 'Clear all notifications'
}
>
<Trash2 className="size-4" />
<span className="hidden sm:inline">
{showClearConfirm ? 'Click to confirm' : 'Clear all'}
{showClearConfirm
? 'Click to confirm'
: activeFilter !== null
? 'Clear filtered'
: 'Clear all'}
</span>
</button>
</div>

92
src/renderer/store/slices/notificationSlice.ts
View file

@ -29,9 +29,9 @@ export interface NotificationSlice {
// Actions
fetchNotifications: () => Promise<void>;
markNotificationRead: (id: string) => Promise<void>;
markAllNotificationsRead: () => Promise<void>;
markAllNotificationsRead: (triggerName?: string) => Promise<void>;
deleteNotification: (id: string) => Promise<void>;
clearNotifications: () => Promise<void>;
clearNotifications: (triggerName?: string) => Promise<void>;
navigateToError: (error: DetectedError) => void;
openNotificationsTab: () => void;
}
@ -99,19 +99,41 @@ export const createNotificationSlice: StateCreator<AppState, [], [], Notificatio
}
},
// Mark all notifications as read
markAllNotificationsRead: async () => {
// 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<AppState, [], [], Notificatio
}
},
// Clear all notifications
clearNotifications: async () => {
// 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);
}

215
test/renderer/store/notificationSlice.test.ts
View file

@ -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>): DetectedError => ({
id: 'error-1',