mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[w9-1] Wire read/ack into the notification bell (#527)
The bell showed a transient WebSocket-stream buffer count with no read/ack actions; the persisted unread/ack state and the mark-read / acknowledge / mark-all-read mutation hooks already existed (use-notifications.ts) and powered the notifications page, but the bell ignored them. The bell now derives its badge from useNotifications().unread_count (the real DB count, not the stream buffer), renders the recent items with per-item Mark Read + Acknowledge + a header Mark all read, shows the pending-ack count, and keeps the WS stream only for the connection indicator (the NotificationAlerts sibling still owns the toast/chime). The stream buffer is cleared on popover close so it can't grow unbounded now that it's no longer displayed. The mutations self-invalidate notificationKeys.all on success, so the badge + popover refresh immediately after each action; useNotifications also refetches every 30s. Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -1,21 +1,78 @@
|
|||||||
import { describe, it, expect, vi } from "vitest";
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||||
import { render, screen } from "@testing-library/react";
|
import { render, screen, waitFor } from "@testing-library/react";
|
||||||
|
import userEvent from "@testing-library/user-event";
|
||||||
import { NotificationBell } from "../notification-bell";
|
import { NotificationBell } from "../notification-bell";
|
||||||
|
import { NotificationType, NotificationPriority, type Notification } from "@/types";
|
||||||
|
|
||||||
// tooltip-aria-label-spec.md §1a: the bell button's only visible content is
|
// tooltip-aria-label-spec.md §1a: the bell button's only visible content is
|
||||||
// an icon — it needs a mandatory aria-label, plus a matching visible
|
// an icon — it needs a mandatory aria-label, plus a matching visible Tooltip
|
||||||
// Tooltip using the identical string per §2.
|
// using the identical string per §2.
|
||||||
|
|
||||||
|
const { useNotificationStream, useNotifications, useMarkNotificationRead, useAcknowledgeNotification, useMarkAllNotificationsRead } =
|
||||||
|
vi.hoisted(() => ({
|
||||||
|
useNotificationStream: vi.fn(),
|
||||||
|
useNotifications: vi.fn(),
|
||||||
|
useMarkNotificationRead: vi.fn(),
|
||||||
|
useAcknowledgeNotification: vi.fn(),
|
||||||
|
useMarkAllNotificationsRead: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
vi.mock("@/hooks/use-websocket", () => ({
|
vi.mock("@/hooks/use-websocket", () => ({
|
||||||
useNotificationStream: () => ({
|
useNotificationStream,
|
||||||
notifications: [],
|
|
||||||
isConnected: false,
|
|
||||||
isConnecting: false,
|
|
||||||
clearMessages: () => {},
|
|
||||||
}),
|
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
vi.mock("@/hooks/use-notifications", () => ({
|
||||||
|
useNotifications,
|
||||||
|
useMarkNotificationRead,
|
||||||
|
useAcknowledgeNotification,
|
||||||
|
useMarkAllNotificationsRead,
|
||||||
|
notificationKeys: { all: ["notifications"], list: (f: unknown) => ["notifications", "list", f], detail: (id: string) => ["notifications", "detail", id] },
|
||||||
|
}));
|
||||||
|
|
||||||
|
function buildNotification(overrides: Partial<Notification> = {}): Notification {
|
||||||
|
return {
|
||||||
|
id: "notif-1",
|
||||||
|
type: NotificationType.TASK_ASSIGNMENT,
|
||||||
|
priority: NotificationPriority.NORMAL,
|
||||||
|
from_agent: "fe-pm-00000000",
|
||||||
|
to_agents: ["fe-dev-1"],
|
||||||
|
subject: "New task assigned",
|
||||||
|
body: "You have been assigned a new task.",
|
||||||
|
requires_ack: false,
|
||||||
|
is_acknowledged: false,
|
||||||
|
is_fully_acknowledged: false,
|
||||||
|
is_read: false,
|
||||||
|
related_task_id: null,
|
||||||
|
related_message_ids: [],
|
||||||
|
timestamp: "2026-07-11T09:00:00Z",
|
||||||
|
expires_at: null,
|
||||||
|
acked_by: [],
|
||||||
|
acked_at: {},
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function streamMock() {
|
||||||
|
useNotificationStream.mockReturnValue({
|
||||||
|
notifications: [],
|
||||||
|
lastMessage: undefined,
|
||||||
|
allMessages: [],
|
||||||
|
clearMessages: vi.fn(),
|
||||||
|
isConnected: true,
|
||||||
|
isConnecting: false,
|
||||||
|
state: "open",
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
describe("NotificationBell — aria-label + tooltip (tooltip-aria-label-spec §1a)", () => {
|
describe("NotificationBell — aria-label + tooltip (tooltip-aria-label-spec §1a)", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
streamMock();
|
||||||
|
useNotifications.mockReturnValue({ data: undefined });
|
||||||
|
useMarkNotificationRead.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
useAcknowledgeNotification.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
useMarkAllNotificationsRead.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
});
|
||||||
|
|
||||||
it("exposes 'View notifications' as the bell button's accessible name and title", () => {
|
it("exposes 'View notifications' as the bell button's accessible name and title", () => {
|
||||||
render(<NotificationBell />);
|
render(<NotificationBell />);
|
||||||
const button = screen.getByRole("button", { name: "View notifications" });
|
const button = screen.getByRole("button", { name: "View notifications" });
|
||||||
@@ -23,16 +80,111 @@ describe("NotificationBell — aria-label + tooltip (tooltip-aria-label-spec §1
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("shows a matching visible tooltip once hovered", async () => {
|
it("shows a matching visible tooltip once hovered", async () => {
|
||||||
const { default: userEvent } = await import("@testing-library/user-event");
|
|
||||||
const user = userEvent.setup();
|
const user = userEvent.setup();
|
||||||
render(<NotificationBell />);
|
render(<NotificationBell />);
|
||||||
|
await user.hover(screen.getByRole("button", { name: "View notifications" }));
|
||||||
|
expect(await screen.findByRole("tooltip")).toHaveTextContent("View notifications");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
await user.hover(
|
describe("NotificationBell — read/ack integration (W9-1)", () => {
|
||||||
screen.getByRole("button", { name: "View notifications" }),
|
beforeEach(() => {
|
||||||
);
|
streamMock();
|
||||||
|
useMarkNotificationRead.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
useAcknowledgeNotification.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
useMarkAllNotificationsRead.mockReturnValue({ mutateAsync: vi.fn() });
|
||||||
|
});
|
||||||
|
|
||||||
expect(await screen.findByRole("tooltip")).toHaveTextContent(
|
it("shows the persisted unread_count as the badge (not the stream buffer)", () => {
|
||||||
"View notifications",
|
useNotifications.mockReturnValue({
|
||||||
);
|
data: {
|
||||||
|
items: [buildNotification()],
|
||||||
|
total: 1,
|
||||||
|
unread_count: 3,
|
||||||
|
pending_ack_count: 0,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
render(<NotificationBell />);
|
||||||
|
// The badge is the only "3" in the closed popover.
|
||||||
|
expect(screen.getByText("3")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("caps the badge at 9+ for large counts", () => {
|
||||||
|
useNotifications.mockReturnValue({
|
||||||
|
data: { items: [], total: 42, unread_count: 42, pending_ack_count: 0 },
|
||||||
|
});
|
||||||
|
render(<NotificationBell />);
|
||||||
|
expect(screen.getByText("9+")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders no badge when there is nothing unread", () => {
|
||||||
|
useNotifications.mockReturnValue({
|
||||||
|
data: { items: [], total: 0, unread_count: 0, pending_ack_count: 0 },
|
||||||
|
});
|
||||||
|
render(<NotificationBell />);
|
||||||
|
expect(screen.queryByText(/^[0-9]/)).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("marks a notification read when its 'Mark Read' button is clicked", async () => {
|
||||||
|
const markRead = vi.fn().mockResolvedValue(undefined);
|
||||||
|
useMarkNotificationRead.mockReturnValue({ mutateAsync: markRead });
|
||||||
|
useNotifications.mockReturnValue({
|
||||||
|
data: {
|
||||||
|
items: [buildNotification({ id: "notif-1", is_read: false })],
|
||||||
|
total: 1,
|
||||||
|
unread_count: 1,
|
||||||
|
pending_ack_count: 0,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(<NotificationBell />);
|
||||||
|
await user.click(screen.getByRole("button", { name: "View notifications" }));
|
||||||
|
const markReadBtn = await screen.findByRole("button", { name: /Mark Read/ });
|
||||||
|
await user.click(markReadBtn);
|
||||||
|
await waitFor(() => expect(markRead).toHaveBeenCalledWith("notif-1"));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("acknowledges a notification when its 'Acknowledge' button is clicked", async () => {
|
||||||
|
const ack = vi.fn().mockResolvedValue(buildNotification({ is_acknowledged: true }));
|
||||||
|
useAcknowledgeNotification.mockReturnValue({ mutateAsync: ack });
|
||||||
|
useNotifications.mockReturnValue({
|
||||||
|
data: {
|
||||||
|
items: [
|
||||||
|
buildNotification({
|
||||||
|
id: "notif-2",
|
||||||
|
requires_ack: true,
|
||||||
|
is_acknowledged: false,
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
total: 1,
|
||||||
|
unread_count: 1,
|
||||||
|
pending_ack_count: 1,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(<NotificationBell />);
|
||||||
|
await user.click(screen.getByRole("button", { name: "View notifications" }));
|
||||||
|
const ackBtn = await screen.findByRole("button", { name: /Acknowledge/ });
|
||||||
|
await user.click(ackBtn);
|
||||||
|
await waitFor(() => expect(ack).toHaveBeenCalledWith("notif-2"));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("marks all notifications read via the header action", async () => {
|
||||||
|
const markAllRead = vi.fn().mockResolvedValue(undefined);
|
||||||
|
useMarkAllNotificationsRead.mockReturnValue({ mutateAsync: markAllRead });
|
||||||
|
useNotifications.mockReturnValue({
|
||||||
|
data: {
|
||||||
|
items: [buildNotification({ is_read: false })],
|
||||||
|
total: 1,
|
||||||
|
unread_count: 1,
|
||||||
|
pending_ack_count: 0,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(<NotificationBell />);
|
||||||
|
await user.click(screen.getByRole("button", { name: "View notifications" }));
|
||||||
|
const allBtn = await screen.findByRole("button", { name: /Mark all read/ });
|
||||||
|
await user.click(allBtn);
|
||||||
|
await waitFor(() => expect(markAllRead).toHaveBeenCalled());
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
@@ -3,6 +3,13 @@
|
|||||||
import { useState } from "react";
|
import { useState } from "react";
|
||||||
import Link from "next/link";
|
import Link from "next/link";
|
||||||
import { useNotificationStream } from "@/hooks/use-websocket";
|
import { useNotificationStream } from "@/hooks/use-websocket";
|
||||||
|
import {
|
||||||
|
useNotifications,
|
||||||
|
useMarkNotificationRead,
|
||||||
|
useAcknowledgeNotification,
|
||||||
|
useMarkAllNotificationsRead,
|
||||||
|
} from "@/hooks/use-notifications";
|
||||||
|
import type { Notification } from "@/types";
|
||||||
import { Button } from "@/components/ui/button";
|
import { Button } from "@/components/ui/button";
|
||||||
import { Badge } from "@/components/ui/badge";
|
import { Badge } from "@/components/ui/badge";
|
||||||
import {
|
import {
|
||||||
@@ -16,18 +23,43 @@ import {
|
|||||||
TooltipProvider,
|
TooltipProvider,
|
||||||
TooltipTrigger,
|
TooltipTrigger,
|
||||||
} from "@/components/ui/tooltip";
|
} from "@/components/ui/tooltip";
|
||||||
import { Bell, Wifi, WifiOff } from "lucide-react";
|
import { Bell, Wifi, WifiOff, CheckCheck, MailOpen, Check } from "lucide-react";
|
||||||
|
|
||||||
const BELL_LABEL = "View notifications";
|
const BELL_LABEL = "View notifications";
|
||||||
|
const PREVIEW_LIMIT = 10;
|
||||||
|
|
||||||
export function NotificationBell() {
|
export function NotificationBell() {
|
||||||
const [open, setOpen] = useState(false);
|
const [open, setOpen] = useState(false);
|
||||||
const { notifications, isConnected, clearMessages } = useNotificationStream();
|
const { isConnected, clearMessages } = useNotificationStream();
|
||||||
|
const { data } = useNotifications();
|
||||||
|
const markRead = useMarkNotificationRead();
|
||||||
|
const acknowledge = useAcknowledgeNotification();
|
||||||
|
const markAllRead = useMarkAllNotificationsRead();
|
||||||
|
|
||||||
const unreadCount = notifications.length;
|
const unreadCount = data?.unread_count ?? 0;
|
||||||
|
const pendingAckCount = data?.pending_ack_count ?? 0;
|
||||||
|
const items = (data?.items ?? []).slice(0, PREVIEW_LIMIT);
|
||||||
|
|
||||||
|
const handleMarkRead = (id: string) => {
|
||||||
|
void markRead.mutateAsync(id);
|
||||||
|
};
|
||||||
|
const handleAcknowledge = (id: string) => {
|
||||||
|
void acknowledge.mutateAsync(id);
|
||||||
|
};
|
||||||
|
const handleMarkAllRead = () => {
|
||||||
|
void markAllRead.mutateAsync();
|
||||||
|
};
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Popover open={open} onOpenChange={setOpen}>
|
<Popover
|
||||||
|
open={open}
|
||||||
|
onOpenChange={(o) => {
|
||||||
|
setOpen(o);
|
||||||
|
// The stream buffer is no longer displayed here (the alerts sibling
|
||||||
|
// owns the toast); clear it on close so it can't grow unbounded.
|
||||||
|
if (!o) clearMessages();
|
||||||
|
}}
|
||||||
|
>
|
||||||
<TooltipProvider>
|
<TooltipProvider>
|
||||||
<Tooltip>
|
<Tooltip>
|
||||||
<TooltipTrigger asChild>
|
<TooltipTrigger asChild>
|
||||||
@@ -41,7 +73,7 @@ export function NotificationBell() {
|
|||||||
>
|
>
|
||||||
<Bell className="h-5 w-5" />
|
<Bell className="h-5 w-5" />
|
||||||
{unreadCount > 0 && (
|
{unreadCount > 0 && (
|
||||||
<Badge className="absolute -top-1 -right-1 h-5 w-5 flex items-center justify-center p-0 text-xs bg-red-500">
|
<Badge className="absolute -top-1 -right-1 h-5 min-w-5 flex items-center justify-center p-0 text-xs bg-red-500">
|
||||||
{unreadCount > 9 ? "9+" : unreadCount}
|
{unreadCount > 9 ? "9+" : unreadCount}
|
||||||
</Badge>
|
</Badge>
|
||||||
)}
|
)}
|
||||||
@@ -52,49 +84,49 @@ export function NotificationBell() {
|
|||||||
</Tooltip>
|
</Tooltip>
|
||||||
</TooltipProvider>
|
</TooltipProvider>
|
||||||
<PopoverContent className="w-80" align="end">
|
<PopoverContent className="w-80" align="end">
|
||||||
<div className="space-y-4">
|
<div className="space-y-3">
|
||||||
<div className="flex items-center justify-between">
|
<div className="flex items-center justify-between">
|
||||||
<h4 className="font-semibold">Notifications</h4>
|
|
||||||
<div className="flex items-center gap-2">
|
<div className="flex items-center gap-2">
|
||||||
|
<h4 className="font-semibold">Notifications</h4>
|
||||||
{isConnected ? (
|
{isConnected ? (
|
||||||
<Wifi className="h-4 w-4 text-green-500" />
|
<Wifi className="h-4 w-4 text-green-500" aria-label="connected" />
|
||||||
) : (
|
) : (
|
||||||
<WifiOff className="h-4 w-4 text-gray-400" />
|
<WifiOff className="h-4 w-4 text-gray-400" aria-label="disconnected" />
|
||||||
)}
|
)}
|
||||||
|
</div>
|
||||||
{unreadCount > 0 && (
|
{unreadCount > 0 && (
|
||||||
<Button variant="ghost" size="sm" onClick={clearMessages}>
|
<Button
|
||||||
Clear
|
variant="ghost"
|
||||||
|
size="sm"
|
||||||
|
onClick={handleMarkAllRead}
|
||||||
|
className="h-7 px-2 text-xs"
|
||||||
|
>
|
||||||
|
<CheckCheck className="h-3.5 w-3.5 mr-1" />
|
||||||
|
Mark all read
|
||||||
</Button>
|
</Button>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
</div>
|
|
||||||
|
|
||||||
{notifications.length === 0 ? (
|
{pendingAckCount > 0 && (
|
||||||
|
<p className="text-xs text-amber-600 dark:text-amber-400">
|
||||||
|
{pendingAckCount} pending acknowledgement
|
||||||
|
{pendingAckCount > 1 ? "s" : ""}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
|
|
||||||
|
{items.length === 0 ? (
|
||||||
<p className="text-sm text-muted-foreground text-center py-4">
|
<p className="text-sm text-muted-foreground text-center py-4">
|
||||||
No new notifications
|
No new notifications
|
||||||
</p>
|
</p>
|
||||||
) : (
|
) : (
|
||||||
<div className="space-y-2 max-h-64 overflow-y-auto">
|
<div className="space-y-1.5 max-h-64 overflow-y-auto">
|
||||||
{notifications
|
{items.map((notification) => (
|
||||||
.slice(-10)
|
<BellRow
|
||||||
.reverse()
|
key={notification.id}
|
||||||
.map((notification, i) => (
|
notification={notification}
|
||||||
<div
|
onMarkRead={handleMarkRead}
|
||||||
key={notification.notification_id ?? `anon-${i}`}
|
onAcknowledge={handleAcknowledge}
|
||||||
className="p-2 rounded bg-muted hover:bg-muted/80 cursor-pointer"
|
/>
|
||||||
>
|
|
||||||
<div className="flex items-center justify-between">
|
|
||||||
<span className="font-medium text-sm">
|
|
||||||
{notification.subject}
|
|
||||||
</span>
|
|
||||||
<Badge variant="outline" className="text-xs">
|
|
||||||
{notification.priority}
|
|
||||||
</Badge>
|
|
||||||
</div>
|
|
||||||
<p className="text-xs text-muted-foreground mt-1">
|
|
||||||
{notification.notification_type}
|
|
||||||
</p>
|
|
||||||
</div>
|
|
||||||
))}
|
))}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
@@ -115,3 +147,66 @@ export function NotificationBell() {
|
|||||||
</Popover>
|
</Popover>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface BellRowProps {
|
||||||
|
notification: Notification;
|
||||||
|
onMarkRead: (id: string) => void;
|
||||||
|
onAcknowledge: (id: string) => void;
|
||||||
|
}
|
||||||
|
|
||||||
|
function BellRow({ notification, onMarkRead, onAcknowledge }: BellRowProps) {
|
||||||
|
const needsAck = notification.requires_ack && !notification.is_acknowledged;
|
||||||
|
return (
|
||||||
|
<div
|
||||||
|
className={
|
||||||
|
"p-2 rounded bg-muted/60 " +
|
||||||
|
(notification.is_read ? "opacity-60" : "border-l-2 border-l-primary")
|
||||||
|
}
|
||||||
|
>
|
||||||
|
<div className="flex items-start justify-between gap-2">
|
||||||
|
<span className="font-medium text-sm line-clamp-1 flex-1 min-w-0">
|
||||||
|
{notification.subject}
|
||||||
|
</span>
|
||||||
|
<Badge variant="outline" className="text-[10px] shrink-0">
|
||||||
|
{notification.priority}
|
||||||
|
</Badge>
|
||||||
|
</div>
|
||||||
|
<div className="flex flex-wrap items-center gap-1.5 mt-1.5">
|
||||||
|
{!notification.is_read && (
|
||||||
|
<Badge variant="secondary" className="text-[10px]">
|
||||||
|
New
|
||||||
|
</Badge>
|
||||||
|
)}
|
||||||
|
{needsAck && (
|
||||||
|
<Badge variant="destructive" className="text-[10px]">
|
||||||
|
Needs Ack
|
||||||
|
</Badge>
|
||||||
|
)}
|
||||||
|
<div className="ml-auto flex items-center gap-1">
|
||||||
|
{!notification.is_read && (
|
||||||
|
<Button
|
||||||
|
variant="ghost"
|
||||||
|
size="sm"
|
||||||
|
className="h-6 px-2 text-xs"
|
||||||
|
onClick={() => onMarkRead(notification.id)}
|
||||||
|
>
|
||||||
|
<MailOpen className="h-3 w-3 mr-1" />
|
||||||
|
Mark Read
|
||||||
|
</Button>
|
||||||
|
)}
|
||||||
|
{needsAck && (
|
||||||
|
<Button
|
||||||
|
variant="default"
|
||||||
|
size="sm"
|
||||||
|
className="h-6 px-2 text-xs"
|
||||||
|
onClick={() => onAcknowledge(notification.id)}
|
||||||
|
>
|
||||||
|
<Check className="h-3 w-3 mr-1" />
|
||||||
|
Acknowledge
|
||||||
|
</Button>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user