Keep routine events out of notifications

This commit is contained in:
Jamie Brunton committed 2026-08-14 09:13:36 +01:00
1 parent 79c508be2b
commit 3bb1dda6bb
2 files changed
+41 -68

No files matched your search

+16 -62
View File
@@ -89,16 +89,7 @@ export function notificationForEvent(event: DashboardEvent): NotificationDraft |
href: `/conversations/${encodeURIComponent(event.payload.conversationId)}`, href: `/conversations/${encodeURIComponent(event.payload.conversationId)}`,
}; };
case 'message.sent': case 'message.sent':
return { return null;
id: event.id,
occurredAt: event.occurredAt,
title: `Message sent to u/${event.payload.recipient}`,
message: 'The message was delivered successfully.',
tone: 'success',
href: event.payload.conversationId
? `/conversations/${encodeURIComponent(event.payload.conversationId)}`
: '/conversations',
};
case 'message.failed': case 'message.failed':
return { return {
id: event.id, id: event.id,
@@ -111,23 +102,17 @@ export function notificationForEvent(event: DashboardEvent): NotificationDraft |
: '/operations', : '/operations',
}; };
case 'bot.status.changed': case 'bot.status.changed':
if (event.payload.status !== 'stopped') return null;
return { return {
id: event.id, id: event.id,
occurredAt: event.occurredAt, occurredAt: event.occurredAt,
title: `Bot ${event.payload.status}`, title: `Bot ${event.payload.status}`,
message: event.payload.reason ?? `The bot is now ${event.payload.status}.`, message: event.payload.reason ?? `The bot is now ${event.payload.status}.`,
tone: event.payload.status === 'running' ? 'success' : event.payload.status === 'stopped' ? 'warning' : 'info', tone: 'warning',
href: '/operations', href: '/operations',
}; };
case 'sync.completed': case 'sync.completed':
return { return null;
id: event.id,
occurredAt: event.occurredAt,
title: `${titleCase(event.payload.kind)} sync complete`,
message: `${event.payload.processed.toLocaleString()} items processed.`,
tone: 'success',
href: '/operations',
};
case 'sync.failed': case 'sync.failed':
return { return {
id: event.id, id: event.id,
@@ -138,26 +123,25 @@ export function notificationForEvent(event: DashboardEvent): NotificationDraft |
href: '/operations', href: '/operations',
}; };
case 'rate-limit.changed': case 'rate-limit.changed':
if (!event.payload.limited) return null;
return { return {
id: event.id, id: event.id,
occurredAt: event.occurredAt, occurredAt: event.occurredAt,
title: event.payload.limited ? 'Rate limit reached' : 'Rate limit cleared', title: 'Rate limit reached',
message: event.payload.reason ?? (event.payload.limited message: event.payload.reason ?? 'Sending will resume when the limit resets.',
? 'Sending will resume when the limit resets.' tone: 'warning',
: 'The bot can send messages again.'),
tone: event.payload.limited ? 'warning' : 'success',
href: '/operations', href: '/operations',
}; };
case 'reddit.session.changed': case 'reddit.session.changed':
if (!['authenticated', 'expired', 'logged-out', 'error'].includes(event.payload.status)) return null; if (!['expired', 'logged-out', 'error'].includes(event.payload.status)) return null;
return { return {
id: event.id, id: event.id,
occurredAt: event.occurredAt, occurredAt: event.occurredAt,
title: event.payload.status === 'authenticated' ? 'Reddit account connected' : 'Reddit account needs attention', title: 'Reddit account needs attention',
message: event.payload.reason ?? (event.payload.username message: event.payload.reason ?? (event.payload.username
? `Signed in as u/${event.payload.username}.` ? `The session for u/${event.payload.username} changed to ${event.payload.status}.`
: `Session status changed to ${event.payload.status}.`), : `Session status changed to ${event.payload.status}.`),
tone: event.payload.status === 'authenticated' ? 'success' : 'error', tone: 'error',
href: '/accounts', href: '/accounts',
}; };
case 'conversation.updated': case 'conversation.updated':
@@ -168,34 +152,6 @@ export function notificationForEvent(event: DashboardEvent): NotificationDraft |
} }
} }
/**
* Toasts interrupt the operator, so reserve them for inbound activity and
* states that need attention. Routine lifecycle and successful sync events
* remain available in the notification centre without producing a pop-up.
*/
export function shouldToastForEvent(event: DashboardEvent): boolean {
switch (event.type) {
case 'reply.received':
case 'media.received':
case 'message.failed':
case 'sync.failed':
return true;
case 'bot.status.changed':
return event.payload.status === 'stopped';
case 'rate-limit.changed':
return event.payload.limited;
case 'reddit.session.changed':
return ['expired', 'logged-out', 'error'].includes(event.payload.status);
case 'message.sent':
case 'conversation.updated':
case 'sync.started':
case 'sync.progress':
case 'sync.completed':
case 'log.created':
return false;
}
}
function loadNotifications(): NotificationItem[] { function loadNotifications(): NotificationItem[] {
const storage = notificationStorage(); const storage = notificationStorage();
if (!storage) return []; if (!storage) return [];
@@ -249,11 +205,9 @@ export function NotificationProvider({ children, realtime: realtimeOverride }: N
setNotifications((current) => current.some((entry) => entry.id === item.id) setNotifications((current) => current.some((entry) => entry.id === item.id)
? current ? current
: [item, ...current].slice(0, MAX_NOTIFICATIONS)); : [item, ...current].slice(0, MAX_NOTIFICATIONS));
if (shouldToastForEvent(event)) { setToasts((current) => current.some((entry) => entry.id === item.id)
setToasts((current) => current.some((entry) => entry.id === item.id) ? current
? current : [...current, item]);
: [...current, item]);
}
}), [realtime]); }), [realtime]);
useEffect(() => { useEffect(() => {
@@ -359,7 +313,7 @@ export function NotificationViewport() {
<div className="notification-empty"> <div className="notification-empty">
<span aria-hidden="true"><BellIcon /></span> <span aria-hidden="true"><BellIcon /></span>
<strong>All quiet</strong> <strong>All quiet</strong>
<p>Replies, delivery updates, syncs, and account alerts will appear here.</p> <p>New replies, received images, and important account alerts will appear here.</p>
</div> </div>
)} )}
</aside> </aside>
+25 -6
View File
@@ -11,7 +11,6 @@ import {
NotificationProvider, NotificationProvider,
NotificationViewport, NotificationViewport,
notificationForEvent, notificationForEvent,
shouldToastForEvent,
} from '../../apps/dashboard/src/app/notifications.js'; } from '../../apps/dashboard/src/app/notifications.js';
class FakeRealtime { class FakeRealtime {
@@ -54,6 +53,27 @@ describe('dashboard notifications', () => {
type: 'log.created', type: 'log.created',
payload: { level: 'info', message: 'Routine detail' }, payload: { level: 'info', message: 'Routine detail' },
})).toBeNull(); })).toBeNull();
expect(notificationForEvent({
id: 'sent-1',
occurredAt: '2026-08-09T09:30:00.000Z',
type: 'message.sent',
payload: { messageId: 'message-2', recipient: 'alice', conversationId: 'room/one' },
})).toBeNull();
expect(notificationForEvent({
id: 'running-1',
occurredAt: '2026-08-09T09:30:00.000Z',
type: 'bot.status.changed',
payload: { status: 'running', dryRun: false },
})).toBeNull();
expect(notificationForEvent({
id: 'rate-clear-1',
occurredAt: '2026-08-09T09:30:00.000Z',
type: 'rate-limit.changed',
payload: { limited: false },
})).toBeNull();
}); });
it('shows new events as toasts and keeps them in a readable notification centre', async () => { it('shows new events as toasts and keeps them in a readable notification centre', async () => {
@@ -94,9 +114,8 @@ describe('dashboard notifications', () => {
expect(within(centre).getByText('All quiet')).toBeInTheDocument(); expect(within(centre).getByText('All quiet')).toBeInTheDocument();
}); });
it('keeps routine sync completions in the notification centre without showing a toast', async () => { it('ignores routine sync completions entirely', () => {
const realtime = new FakeRealtime(); const realtime = new FakeRealtime();
const user = userEvent.setup();
render( render(
<MemoryRouter> <MemoryRouter>
<NotificationProvider realtime={realtime}> <NotificationProvider realtime={realtime}>
@@ -112,12 +131,12 @@ describe('dashboard notifications', () => {
type: 'sync.completed', type: 'sync.completed',
payload: { syncId: 'conversation-sync-1', kind: 'conversations', processed: 12 }, payload: { syncId: 'conversation-sync-1', kind: 'conversations', processed: 12 },
}; };
expect(shouldToastForEvent(syncEvent)).toBe(false); expect(notificationForEvent(syncEvent)).toBeNull();
act(() => realtime.emit(syncEvent)); act(() => realtime.emit(syncEvent));
expect(screen.queryByRole('status')).not.toBeInTheDocument(); expect(screen.queryByRole('status')).not.toBeInTheDocument();
await user.click(screen.getByRole('button', { name: 'Notifications, 1 unread' })); expect(screen.getByRole('button', { name: 'Notifications' })).toBeInTheDocument();
expect(within(screen.getByLabelText('Notifications', { selector: 'aside' })).getByText('Conversations sync complete')) expect(within(screen.getByLabelText('Notifications', { selector: 'aside' })).getByText('All quiet'))
.toBeInTheDocument(); .toBeInTheDocument();
}); });