fix(notifications): Mark all read empties a band of read rows, and a read landing mid-write cannot refill it

This commit is contained in:
2026-10-28 21:54:00 +01:00
parent ee658b8bfc
commit 0469812499
2 changed files with 74 additions and 9 deletions
@@ -97,7 +97,7 @@ function mount() {
</QueryClientProvider> </QueryClientProvider>
); );
const { rerender } = render(tree()); const { rerender } = render(tree());
return { rerender: () => rerender(tree()) }; return { qc, rerender: () => rerender(tree()) };
} }
// Every test has a "Ready 1" on screen once the server list has landed. // Every test has a "Ready 1" on screen once the server list has landed.
const loaded = () => screen.findByText("Ready 1"); const loaded = () => screen.findByText("Ready 1");
@@ -211,4 +211,59 @@ describe("NotificationsPanel · order", () => {
expect(row.isConnected).toBe(true); expect(row.isConnected).toBe(true);
expect(document.activeElement).toBe(row); expect(document.activeElement).toBe(row);
}); });
it("a band of rows already read still offers Mark all read, which empties it", async () => {
// Opening the center marked this browser's notices read: the band holds them, and nothing on
// the page is unread any more.
h.server = [server(1, 1 * H, true)];
h.client = [client(2, 2 * H, { read: false })];
const { rerender } = mount();
await loaded();
h.client = [client(2, 2 * H, { read: true })]; // what the mount's markAllRead leaves
await act(async () => rerender());
expect(listUnder("inbox.sectionNeedsYou:1")).toEqual(["Activity 2"]);
const button = screen.getByRole("button", { name: "inbox.markAllRead" });
expect(button.hasAttribute("disabled")).toBe(false);
fireEvent.click(button);
await act(async () => rerender());
expect(screen.queryByText(/inbox\.sectionNeedsYou/)).toBeNull();
});
it("a read that lands before Mark all read commits does not put the rows back", async () => {
let commit!: () => void;
vi.spyOn(api, "markAllNotificationsRead").mockImplementation(
() => new Promise((resolve) => (commit = () => resolve({ ok: true } as never))),
);
h.server = [server(1, 1 * H, false)];
h.client = [];
const { qc } = mount();
await loaded();
fireEvent.click(screen.getByRole("button", { name: "inbox.markAllRead" }));
await waitFor(() => expect(api.markAllNotificationsRead).toHaveBeenCalled());
// The poll (or another row's write) reads the list before the server has committed the write.
void qc.invalidateQueries({ queryKey: ["notifications"] });
await waitFor(() =>
expect(qc.getQueryData<{ items: AppNotification[] }>(["notifications"])?.items[0]?.read).toBe(
false,
),
);
h.server = [server(1, 1 * H, true)];
await act(async () => commit());
await waitFor(() => expect(screen.queryByText(/inbox\.sectionNeedsYou/)).toBeNull());
expect(listUnder("inbox.sectionRecent")).toEqual(["Ready 1"]);
});
it("a refused Mark all read brings the rows back at once, not at the next poll", async () => {
vi.spyOn(api, "markAllNotificationsRead").mockRejectedValue(new Error("offline"));
h.server = [server(1, 1 * H, false)];
h.client = [];
mount();
await loaded();
const reads = vi.mocked(api.notifications).mock.calls.length;
fireEvent.click(screen.getByRole("button", { name: "inbox.markAllRead" }));
await waitFor(() =>
expect(vi.mocked(api.notifications).mock.calls.length).toBeGreaterThan(reads),
);
await waitFor(() => expect(listUnder("inbox.sectionNeedsYou:1")).toEqual(["Ready 1"]));
});
}); });
@@ -108,9 +108,6 @@ export default function NotificationsPanel() {
); );
const [openedAt] = useState(() => Date.now()); const [openedAt] = useState(() => Date.now());
const [earlierOpen, setEarlierOpen] = useState(false); const [earlierOpen, setEarlierOpen] = useState(false);
const order = orderInbox(serverItems, clientItems, { now: openedAt, held });
const asking = order.needsYou.map(entryKey).filter((k) => !held.has(k));
if (asking.length > 0) setHeld(new Set([...held, ...asking]));
// Opening the center marks the (ephemeral) client items read, matching the old bell. Server // Opening the center marks the (ephemeral) client items read, matching the old bell. Server
// items stay unread until acted on individually. // items stay unread until acted on individually.
@@ -129,7 +126,9 @@ export default function NotificationsPanel() {
}); });
const readAllMut = useMutation({ const readAllMut = useMutation({
mutationFn: api.markAllNotificationsRead, mutationFn: api.markAllNotificationsRead,
onSuccess: refresh, // Settled, not only succeeded: a refused write brings the rows back unread at once, rather than
// leaving them read until the next poll moves them under the reader.
onSettled: refresh,
meta: { failTitle: "errors.writeFailed.notifReadAll" }, meta: { failTitle: "errors.writeFailed.notifReadAll" },
}); });
const dismissMut = useMutation({ const dismissMut = useMutation({
@@ -143,14 +142,25 @@ export default function NotificationsPanel() {
meta: { failTitle: "errors.writeFailed.notifClear" }, meta: { failTitle: "errors.writeFailed.notifClear" },
}); });
// While Mark all read is on its way, a read that lands before the server commits it (the poll, or
// another row's write refreshing the list) still says "unread": those rows are read already, or
// the band would take them back and hold them for the rest of the visit.
const shownServer = readAllMut.isPending
? serverItems.map((n) => (n.read ? n : { ...n, read: true }))
: serverItems;
const order = orderInbox(shownServer, clientItems, { now: openedAt, held });
const asking = order.needsYou.map(entryKey).filter((k) => !held.has(k));
if (asking.length > 0) setHeld(new Set([...held, ...asking]));
const serverUnread = serverItems.some((n) => !n.read); const serverUnread = serverItems.some((n) => !n.read);
const hasAny = serverItems.length > 0 || clientItems.length > 0; const hasAny = serverItems.length > 0 || clientItems.length > 0;
const hasUnread = serverUnread || clientUnread > 0; // A band of rows already read still offers the gesture that empties it.
const canMarkAll = serverUnread || clientUnread > 0 || order.needsYou.length > 0;
function markAll() { function markAll() {
markAllClientRead(); markAllClientRead();
// Read at once, not when the write lands: until then the rows still read unread, and the band // Read at once, not when the write lands: until then the rows still read unread, and the band
// would take them straight back. A failed write refetches the truth (refresh, then the poll). // would take them straight back. A failed write refetches the truth when it settles.
void qc.cancelQueries({ queryKey: qk.notifications() }); void qc.cancelQueries({ queryKey: qk.notifications() });
qc.setQueryData<{ items: AppNotification[]; total: number }>(qk.notifications(), (d) => qc.setQueryData<{ items: AppNotification[]; total: number }>(qk.notifications(), (d) =>
d ? { ...d, items: d.items.map((n) => (n.read ? n : { ...n, read: true })) } : d, d ? { ...d, items: d.items.map((n) => (n.read ? n : { ...n, read: true })) } : d,
@@ -404,7 +414,7 @@ export default function NotificationsPanel() {
id: "read-all", id: "read-all",
label: t("inbox.markAllRead"), label: t("inbox.markAllRead"),
icon: icon(CheckCheck), icon: icon(CheckCheck),
disabled: !hasUnread || readAllMut.isPending, disabled: !canMarkAll || readAllMut.isPending,
onSelect: markAll, onSelect: markAll,
}, },
{ {
@@ -438,7 +448,7 @@ export default function NotificationsPanel() {
variant="ghost" variant="ghost"
size="sm" size="sm"
onClick={markAll} onClick={markAll}
disabled={!hasUnread || readAllMut.isPending} disabled={!canMarkAll || readAllMut.isPending}
icon={<CheckCheck />} icon={<CheckCheck />}
> >
{t("inbox.markAllRead")} {t("inbox.markAllRead")}