From 1b9a40f874e11f91610b75065278796d65832e00 Mon Sep 17 00:00:00 2001 From: Anso Date: Mon, 29 Jun 2026 18:19:18 -0400 Subject: [PATCH] fix: align notification unread badge with panel visibility rules (#1520) Stack success events were hidden from the panel but still counted unread on the bell and dashboard. Share one visibility helper across badge, panel, and Recent Alerts. Harden mark-all-read against partial API failures. Fixes #1513 --- .../EditorLayout/hooks/useNotifications.ts | 29 +++++++++- frontend/src/components/NotificationPanel.tsx | 28 ++++------ .../components/dashboard/HealthStatusBar.tsx | 3 +- .../src/components/dashboard/RecentAlerts.tsx | 5 +- .../__tests__/notificationVisibility.test.ts | 56 +++++++++++++++++++ frontend/src/lib/notificationVisibility.ts | 37 ++++++++++++ 6 files changed, 136 insertions(+), 22 deletions(-) create mode 100644 frontend/src/lib/__tests__/notificationVisibility.test.ts create mode 100644 frontend/src/lib/notificationVisibility.ts diff --git a/frontend/src/components/EditorLayout/hooks/useNotifications.ts b/frontend/src/components/EditorLayout/hooks/useNotifications.ts index ead74021..91a3d9c8 100644 --- a/frontend/src/components/EditorLayout/hooks/useNotifications.ts +++ b/frontend/src/components/EditorLayout/hooks/useNotifications.ts @@ -222,12 +222,37 @@ export function useNotifications({ nodes, onStateInvalidate, onImageUpdatesChang try { const localNode = nodesRef.current.find(n => n.type === 'local'); const unreadNodeIds = [...new Set(notifications.filter(n => !n.is_read && n.nodeId != null).map(n => n.nodeId as number))]; - await Promise.allSettled(unreadNodeIds.map(nodeId => + if (unreadNodeIds.length === 0) return; + + const results = await Promise.allSettled(unreadNodeIds.map(nodeId => nodeId === localNode?.id ? apiFetch('/notifications/read', { method: 'POST', localOnly: true } as Parameters[1]) : fetchForNode('/notifications/read', nodeId, { method: 'POST' }), )); - setNotifications(prev => prev.map(n => ({ ...n, is_read: 1 }))); + + const succeededNodeIds = new Set(); + let hadFailure = false; + for (let i = 0; i < results.length; i++) { + const result = results[i]; + const nodeId = unreadNodeIds[i]; + if (result.status === 'fulfilled' && result.value.ok) { + succeededNodeIds.add(nodeId); + } else { + hadFailure = true; + } + } + + if (succeededNodeIds.size > 0) { + setNotifications(prev => prev.map(n => + n.nodeId != null && succeededNodeIds.has(n.nodeId) ? { ...n, is_read: 1 } : n, + )); + } + + if (hadFailure) { + toast.error('Some notifications could not be marked as read'); + } + + void fetchNotificationsRef.current(); } catch (e: unknown) { const err = e as { message?: string; error?: string }; toast.error(err?.message || err?.error || 'Failed to mark notifications as read'); diff --git a/frontend/src/components/NotificationPanel.tsx b/frontend/src/components/NotificationPanel.tsx index b11a16c4..dc1187c3 100644 --- a/frontend/src/components/NotificationPanel.tsx +++ b/frontend/src/components/NotificationPanel.tsx @@ -25,6 +25,7 @@ import { cn } from '@/lib/utils'; import type { NotificationCategory, NotificationItem } from './dashboard/types'; import type { Node } from '@/context/NodeContext'; import { CATEGORY_LABELS } from '@/lib/notificationCategories'; +import { countVisibleUnread, filterPanelVisible } from '@/lib/notificationVisibility'; const NODE_FILTER_ALL = 'all' as const; const CATEGORY_FILTER_ALL = 'all' as const; @@ -94,25 +95,13 @@ function formatRelative(ms: number): string { return new Date(ms).toLocaleDateString(undefined, { month: 'short', day: 'numeric' }); } -const USER_OP_CATEGORIES = new Set([ - 'deploy_success', 'stack_started', 'stack_stopped', 'stack_restarted', 'image_update_applied', -]); - -function isUserInitiatedSuccess(n: NotificationItem): boolean { - return n.level === 'info' - && n.category !== undefined - && USER_OP_CATEGORIES.has(n.category) - && n.actor_username != null - && n.actor_username !== 'system'; -} - function applyFilter( items: NotificationItem[], filter: NotifFilter, nodeFilter: NodeFilter, categoryFilter: CategoryFilter, ): NotificationItem[] { - let result = items.filter(n => !isUserInitiatedSuccess(n)); + let result = filterPanelVisible(items); if (filter === 'unread') result = result.filter((n) => !n.is_read); else if (filter === 'alerts') result = result.filter((n) => n.level === 'warning' || n.level === 'error'); if (nodeFilter !== NODE_FILTER_ALL) result = result.filter((n) => n.nodeId === nodeFilter); @@ -149,7 +138,7 @@ export function NotificationPanel({ nodeFilter !== NODE_FILTER_ALL || categoryFilter !== CATEGORY_FILTER_ALL; const unreadCount = useMemo( - () => notifications.filter((n) => !n.is_read).length, + () => countVisibleUnread(notifications), [notifications], ); @@ -358,7 +347,11 @@ export function NotificationPanel({ {/* Stream */} {groups.length === 0 ? ( - 0} /> + 0} + hasVisibleUnread={unreadCount > 0} + /> ) : (
{groups.map((group) => ( @@ -493,13 +486,14 @@ function NotificationRow({ notif, showNodeName, onDelete, onNavigate, onNavigate interface EmptyStateProps { filter: NotifFilter; hasAny: boolean; + hasVisibleUnread: boolean; } -function EmptyState({ filter, hasAny }: EmptyStateProps) { +function EmptyState({ filter, hasAny, hasVisibleUnread }: EmptyStateProps) { let title = "You're all caught up"; let subtitle = 'New notifications appear here in real time.'; - if (hasAny && filter === 'unread') { + if (hasAny && filter === 'unread' && !hasVisibleUnread) { title = 'No unread notifications'; subtitle = 'Everything in your feed has been read.'; } else if (hasAny && filter === 'alerts') { diff --git a/frontend/src/components/dashboard/HealthStatusBar.tsx b/frontend/src/components/dashboard/HealthStatusBar.tsx index 2c9836f1..91402ea2 100644 --- a/frontend/src/components/dashboard/HealthStatusBar.tsx +++ b/frontend/src/components/dashboard/HealthStatusBar.tsx @@ -2,6 +2,7 @@ import { useEffect, useMemo, useState } from 'react'; import { Bell } from 'lucide-react'; import type { Stats, SystemStats, NotificationItem, HealthLevel } from './types'; import { deriveHealth } from './deriveHealth'; +import { countVisibleUnread } from '@/lib/notificationVisibility'; interface HealthStatusBarProps { stats: Stats; @@ -78,7 +79,7 @@ export function HealthStatusBar({ ); const config = healthConfig[level]; const now = useTicker(SYNC_LABEL_TICK_MS); - const unreadAlerts = notifications.filter(n => !n.is_read).length; + const unreadAlerts = countVisibleUnread(notifications); const running = `${stats.active}/${stats.total}`; const cpuLabel = systemStats ? `${parseFloat(systemStats.cpu.usage).toFixed(0)}%` : '--'; const memLabel = systemStats ? formatGib(systemStats.memory.used) : '--'; diff --git a/frontend/src/components/dashboard/RecentAlerts.tsx b/frontend/src/components/dashboard/RecentAlerts.tsx index 81bc1a1f..8a7e388b 100644 --- a/frontend/src/components/dashboard/RecentAlerts.tsx +++ b/frontend/src/components/dashboard/RecentAlerts.tsx @@ -5,6 +5,7 @@ import { Badge } from '@/components/ui/badge'; import { Info, AlertTriangle, AlertOctagon, CheckCircle2, Trash2, ChevronLeft, ChevronRight } from 'lucide-react'; import { toast } from '@/components/ui/toast-store'; import { formatRelativeTime } from '@/lib/utils'; +import { filterPanelVisible } from '@/lib/notificationVisibility'; import type { NotificationItem } from './types'; import type { Node } from '@/context/NodeContext'; @@ -26,7 +27,7 @@ export function RecentAlerts({ notifications, nodes, onCleared }: RecentAlertsPr const [clearing, setClearing] = useState(false); const [page, setPage] = useState(0); - const sorted = notifications + const sorted = filterPanelVisible(notifications) .slice() .sort((a, b) => b.timestamp - a.timestamp); @@ -93,7 +94,7 @@ export function RecentAlerts({ notifications, nodes, onCleared }: RecentAlertsPr const Icon = config.icon; return (
diff --git a/frontend/src/lib/__tests__/notificationVisibility.test.ts b/frontend/src/lib/__tests__/notificationVisibility.test.ts new file mode 100644 index 00000000..b380c667 --- /dev/null +++ b/frontend/src/lib/__tests__/notificationVisibility.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect } from 'vitest'; +import type { NotificationItem } from '@/components/dashboard/types'; +import { + countVisibleUnread, + filterPanelVisible, + isPanelHiddenNotification, + isVisibleUnread, +} from '@/lib/notificationVisibility'; + +function notif(overrides: Partial = {}): NotificationItem { + return { + id: 1, + level: 'info', + message: 'test', + timestamp: 1000, + is_read: 0, + ...overrides, + }; +} + +describe('notificationVisibility', () => { + it('hides unread deploy_success from a human actor', () => { + const n = notif({ category: 'deploy_success', actor_username: 'alice' }); + expect(isPanelHiddenNotification(n)).toBe(true); + expect(isVisibleUnread(n)).toBe(false); + expect(countVisibleUnread([n])).toBe(0); + expect(filterPanelVisible([n])).toEqual([]); + }); + + it('shows unread monitor_alert and node_update_available', () => { + const monitor = notif({ category: 'monitor_alert', level: 'warning' }); + const update = notif({ category: 'node_update_available' }); + expect(isPanelHiddenNotification(monitor)).toBe(false); + expect(isVisibleUnread(monitor)).toBe(true); + expect(isVisibleUnread(update)).toBe(true); + expect(countVisibleUnread([monitor, update])).toBe(2); + }); + + it('shows scheduler image_update_applied (system actor, not human)', () => { + const n = notif({ category: 'image_update_applied', actor_username: 'system:scheduler' }); + expect(isPanelHiddenNotification(n)).toBe(false); + expect(isVisibleUnread(n)).toBe(true); + }); + + it('does not count read panel-hidden notifications as visible unread', () => { + const n = notif({ category: 'stack_started', actor_username: 'bob', is_read: 1 }); + expect(isPanelHiddenNotification(n)).toBe(true); + expect(isVisibleUnread(n)).toBe(false); + }); + + it('badge count ignores hidden unread but includes visible unread', () => { + const hidden = notif({ id: 1, category: 'deploy_success', actor_username: 'alice' }); + const visible = notif({ id: 2, category: 'monitor_alert', level: 'error' }); + expect(countVisibleUnread([hidden, visible])).toBe(1); + }); +}); diff --git a/frontend/src/lib/notificationVisibility.ts b/frontend/src/lib/notificationVisibility.ts new file mode 100644 index 00000000..2f73be9e --- /dev/null +++ b/frontend/src/lib/notificationVisibility.ts @@ -0,0 +1,37 @@ +import type { NotificationCategory, NotificationItem } from '@/components/dashboard/types'; + +const PANEL_HIDDEN_CATEGORIES = new Set([ + 'deploy_success', + 'stack_started', + 'stack_stopped', + 'stack_restarted', + 'image_update_applied', +]); + +function isHumanActor(actor: string | null | undefined): boolean { + if (actor == null || actor === '') return false; + if (actor === 'system' || actor.startsWith('system:')) return false; + return true; +} + +/** + * User-initiated stack success events are surfaced in the activity timeline + * and sidebar ticker; hide them from the notification panel feed. + */ +export function isPanelHiddenNotification(n: NotificationItem): boolean { + if (n.level !== 'info' || n.category === undefined) return false; + if (!PANEL_HIDDEN_CATEGORIES.has(n.category as NotificationCategory)) return false; + return isHumanActor(n.actor_username); +} + +export function isVisibleUnread(n: NotificationItem): boolean { + return !n.is_read && !isPanelHiddenNotification(n); +} + +export function countVisibleUnread(notifications: NotificationItem[]): number { + return notifications.filter(isVisibleUnread).length; +} + +export function filterPanelVisible(notifications: NotificationItem[]): NotificationItem[] { + return notifications.filter((n) => !isPanelHiddenNotification(n)); +}