fix(inbox): keep unread-filtered notification open (MUL-8073)

* fix(inbox): keep unread-filtered selection open

Opening an unread notification marks it read, which removed the row from the active unread filter and cleared the selection before the detail could render. Pin the open row across the unread dimension only, releasing it when the selection moves, so the notification actually opens without changing the other filter dimensions.

* fix(inbox): retain archived unread selection through refetch (MUL-8073)

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: Jean Wood <[email protected]>
Co-authored-by: Bohan-J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
This commit is contained in:
Jean Wood
2026-10-19 01:37:39 -04:00
committed by GitHub
co-authored by multica-agent Jean Wood Bohan-J
parent c4a7724afe
commit a5aba00dac
5 changed files with 335 additions and 12 deletions
+18
View File
@@ -170,6 +170,24 @@ describe("unread filtering", () => {
filterInboxItems(rows, { ...EMPTY_INBOX_FILTERS, unreadOnly: true }),
).toEqual(rows);
});
it("keeps the open notification visible after opening marks it read", () => {
const filters = { ...EMPTY_INBOX_FILTERS, unreadOnly: true };
const opened = item("opened", "todo", "high", { read: true });
const read = item("read", "todo", "high", { read: true });
expect(
filterInboxItems([opened, read], filters, "issue-opened").map(
(candidate) => candidate.id,
),
).toEqual(["opened"]);
// Pinning only crosses the unread dimension: an explicit issue filter
// still removes the open row, and the page clears its selection.
expect(
filterInboxItems([opened], { ...filters, statuses: ["done"] }, "issue-opened"),
).toEqual([]);
});
});
describe("inboxPriorityFilterSupport", () => {
+15 -3
View File
@@ -193,10 +193,18 @@ export function inboxFiltersForPrioritySupport(
* (`useInboxUnreadCount`) counts the same thing from the server's summary
* endpoint, which applies that newest-per-issue rule in SQL — so "only unread"
* and the number next to Inbox still cannot disagree.
*
* `pinnedKey` is the notification whose detail is open. Opening an unread
* notification marks it read, which would otherwise remove the row the user
* just clicked from under the detail pane. The pinned row therefore survives
* the unread dimension only; the status, priority, and actor dimensions still
* apply, because changing one of those is an explicit request to filter the
* notification itself out.
*/
export function filterInboxItems(
items: InboxItem[],
filters: InboxFilters,
pinnedKey?: string | null,
): InboxItem[] {
if (isEmptyInboxFilters(filters)) return items;
@@ -205,13 +213,17 @@ export function filterInboxItems(
const actors = new Set(filters.actors);
return items.filter((item) => {
const actor = inboxActorKey(item);
return (
const matchesOtherDimensions =
(statuses.size === 0 ||
(item.issue_status != null && statuses.has(item.issue_status))) &&
(priorities.size === 0 ||
(item.issue_priority != null && priorities.has(item.issue_priority))) &&
(actors.size === 0 || (actor != null && actors.has(actor))) &&
(!filters.unreadOnly || item.read !== true)
(actors.size === 0 || (actor != null && actors.has(actor)));
if (!matchesOtherDimensions) return false;
return (
!filters.unreadOnly ||
item.read !== true ||
(pinnedKey != null && (item.issue_id ?? item.id) === pinnedKey)
);
});
}
@@ -0,0 +1,238 @@
import { useEffect, type ReactNode } from "react";
import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react";
import { afterEach, beforeEach, expect, it, vi } from "vitest";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { setApiInstance } from "@multica/core/api";
import type { ApiClient } from "@multica/core/api/client";
import { useInboxFilterStore, type InboxFilters } from "@multica/core/inbox/filter-store";
import { archivedInboxLookupOptions, inboxKeys } from "@multica/core/inbox/queries";
import { onInboxIssueDeleted, patchInboxIssueProjection } from "@multica/core/inbox/ws-updaters";
import type { ArchivedInboxPage } from "@multica/core/types/inbox";
import type { InboxItem } from "@multica/core/types";
import { InboxPage } from "./inbox-page";
vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
vi.mock("react-resizable-panels", () => ({
useDefaultLayout: () => ({ defaultLayout: undefined, onLayoutChanged: vi.fn() }),
}));
vi.mock("@multica/core/hooks", () => ({ useWorkspaceId: () => "workspace-1" }));
vi.mock("@multica/core/paths", () => ({
useWorkspacePaths: () => ({
inbox: () => "/acme/inbox",
issueDetail: (id: string) => `/acme/issues/${id}`,
}),
}));
vi.mock("../../modals/use-issue-limit-upgrade-prompt", () => ({
useIssueLimitUpgradePrompt: () => vi.fn(),
}));
const detailUnmounted = vi.hoisted(() => vi.fn());
vi.mock("../../issues/components/issue-detail", () => ({
IssueDetail: ({ issueId }: { issueId: string }) => {
useEffect(() => () => detailUnmounted(issueId), [issueId]);
return <div data-testid="detail">{issueId}</div>;
},
issueHighlightMementoKey: (issueId: string) => `highlight:${issueId}`,
}));
const replace = vi.fn();
let searchParams = new URLSearchParams();
vi.mock("../../navigation", () => ({
useNavigation: () => ({ searchParams, replace }),
useReportNavigating: () => {},
}));
vi.mock("@multica/ui/hooks/use-mobile", () => ({ useIsCompact: () => false }));
vi.mock("@multica/ui/components/ui/resizable", () => ({
ResizablePanelGroup: ({ children }: { children: ReactNode }) => <div>{children}</div>,
ResizablePanel: ({ children }: { children: ReactNode }) => <div>{children}</div>,
ResizableHandle: () => null,
}));
vi.mock("./inbox-list", () => ({
InboxList: ({ items, onSelect }: { items: InboxItem[]; onSelect: (item: InboxItem) => void }) => (
<div>{items.map((row) => (
<button key={row.id} data-read={row.read} onClick={() => onSelect(row)}>{row.id}</button>
))}</div>
),
}));
vi.mock("./inbox-filter-menu", () => ({ InboxFilterMenu: () => null }));
vi.mock("./inbox-list-item", () => ({ useTimeAgo: () => vi.fn() }));
let rowActions: { onAction: (id: string) => void };
vi.mock("./inbox-context-menu", () => ({
InboxContextMenuProvider: ({ actions, children }: { actions: typeof rowActions; children: ReactNode }) => {
rowActions = actions;
return children;
},
}));
vi.mock("./inbox-detail-label", () => ({ useTypeLabels: () => ({}) }));
vi.mock("../../i18n", () => {
const t = () => "Inbox";
return { useT: () => ({ t }) };
});
function item(overrides: Partial<InboxItem> = {}): InboxItem {
return {
id: "inbox-a", workspace_id: "workspace-1", recipient_type: "member",
recipient_id: "member-1", actor_type: "agent", actor_id: "agent-1",
type: "new_comment", severity: "info", issue_id: "issue-a",
title: "Issue title", body: null, issue_status: "todo", issue_priority: "high",
read: false, archived: false, created_at: "2026-06-15T08:00:00Z", details: null,
...overrides,
};
}
let qc: QueryClient;
beforeEach(() => {
qc = new QueryClient({ defaultOptions: {
queries: { retry: false, staleTime: Infinity, gcTime: 0 },
mutations: { retry: false },
} });
searchParams = new URLSearchParams();
replace.mockClear();
detailUnmounted.mockClear();
useInboxFilterStore.setState({ filtersByWorkspace: {} });
useInboxFilterStore.getState().toggleUnreadOnly("workspace-1");
});
afterEach(() => { cleanup(); qc.clear(); });
function renderPage() {
return render(<QueryClientProvider client={qc}><InboxPage /></QueryClientProvider>);
}
function deferred<T>() {
let resolve!: (value: T) => void;
let reject!: (reason: Error) => void;
const promise = new Promise<T>((yes, no) => { resolve = yes; reject = no; });
return { promise, resolve, reject };
}
const page = (items: InboxItem[]): ArchivedInboxPage => ({ items, hasMore: false, nextCursor: null });
function archivedFixture() {
searchParams = new URLSearchParams("view=archived");
const readGate = deferred<void>();
const lookup = deferred<ArchivedInboxPage>();
const a = item({ archived: true });
let rows = [a, item({ id: "inbox-b", issue_id: "issue-b", archived: true })];
const listArchivedInboxPage = vi.fn(async (filters: InboxFilters, options: { groupId?: string }) => {
if (options.groupId === "issue-a") return lookup.promise;
if (options.groupId) return page(rows.filter((row) => row.issue_id === options.groupId));
return page(rows.filter((row) => !filters.unreadOnly || !row.read));
});
const markInboxRead = vi.fn(async (id: string) => {
if (id === a.id) await readGate.promise;
rows = rows.map((row) => row.id === id ? { ...row, read: true } : row);
return rows.find((row) => row.id === id)!;
});
const unarchiveInbox = vi.fn(async () => {
rows = rows.filter((row) => row.id !== a.id);
return { ...a, read: true, archived: false };
});
setApiInstance({ listArchivedInboxPage, markInboxRead, unarchiveInbox,
getInboxUnreadSummary: async () => [],
} as unknown as ApiClient);
return { a, readGate, lookup, listArchivedInboxPage, unarchiveInbox,
remove: () => { rows = rows.filter((row) => row.id !== a.id); },
};
}
async function openArchived(fixture: ReturnType<typeof archivedFixture>, viaUrl = false) {
if (viaUrl) searchParams.set("issue", "issue-a");
renderPage();
const row = await screen.findByText("inbox-a");
if (!viaUrl) fireEvent.click(row);
const detail = await screen.findByTestId("detail");
await waitFor(() => expect(screen.getByText("inbox-a")).toHaveAttribute("data-read", "true"));
await act(async () => fixture.readGate.resolve());
await waitFor(() => expect(fixture.listArchivedInboxPage.mock.calls.some((call) => call[1].groupId === "issue-a")).toBe(true));
return detail;
}
it("keeps the active read selection until moving to another notification", async () => {
let rows = [item(), item({ id: "inbox-b", issue_id: "issue-b" })];
const listInbox = vi.fn(async () => rows);
const markInboxRead = vi.fn(async (id: string) => {
rows = rows.map((row) => row.id === id ? { ...row, read: true } : row);
return rows.find((row) => row.id === id)!;
});
setApiInstance({ listInbox, markInboxRead, getInboxUnreadSummary: async () => [] } as unknown as ApiClient);
renderPage();
fireEvent.click(await screen.findByText("inbox-a"));
await waitFor(() => expect(listInbox).toHaveBeenCalledTimes(2));
expect(screen.getByText("inbox-a")).toHaveAttribute("data-read", "true");
expect(screen.getByTestId("detail")).toHaveTextContent("issue-a");
fireEvent.click(screen.getByText("inbox-b"));
await waitFor(() => expect(listInbox).toHaveBeenCalledTimes(3));
expect(screen.getByTestId("detail")).toHaveTextContent("issue-b");
expect(screen.queryByText("inbox-a")).not.toBeInTheDocument();
expect(screen.getByText("inbox-b")).toHaveAttribute("data-read", "true");
});
it.each(["click", "url"])("keeps the archived row and detail mounted through refetch and slow lookup after %s selection, then releases the pin", async (selection) => {
const fixture = archivedFixture();
const detail = await openArchived(fixture, selection === "url");
expect(screen.getByText("inbox-a")).toHaveAttribute("data-read", "true");
expect(screen.getByTestId("detail")).toBe(detail);
expect(detailUnmounted).not.toHaveBeenCalled();
await act(async () => fixture.lookup.resolve(page([{ ...fixture.a, read: true }])));
expect(screen.getByText("inbox-a")).toHaveAttribute("data-read", "true");
expect(screen.getByTestId("detail")).toBe(detail);
expect(detailUnmounted).not.toHaveBeenCalled();
expect(replace).not.toHaveBeenCalledWith("/acme/inbox?view=archived");
fireEvent.click(screen.getByText("inbox-b"));
await waitFor(() => expect(screen.getByTestId("detail")).toHaveTextContent("issue-b"));
expect(screen.queryByText("inbox-a")).not.toBeInTheDocument();
});
it("keeps reading through lookup failure and clears a confirmed missing notification", async () => {
const fixture = archivedFixture();
const detail = await openArchived(fixture);
await act(async () => fixture.lookup.reject(new Error("offline")));
await waitFor(() => expect(qc.getQueryState(archivedInboxLookupOptions("workspace-1", "issue-a").queryKey)?.status).toBe("error"));
expect(screen.getByText("inbox-a")).toBeInTheDocument();
expect(screen.getByTestId("detail")).toBe(detail);
expect(detailUnmounted).not.toHaveBeenCalled();
fixture.listArchivedInboxPage.mockImplementation(async () => page([]));
await act(async () => { await qc.invalidateQueries({ queryKey: inboxKeys.archived("workspace-1") }); });
await waitFor(() => expect(screen.queryByTestId("detail")).not.toBeInTheDocument());
expect(screen.queryByText("inbox-a")).not.toBeInTheDocument();
expect(replace).toHaveBeenLastCalledWith("/acme/inbox?view=archived");
});
it("removes a pinned archived notification when its issue is deleted", async () => {
const fixture = archivedFixture();
await openArchived(fixture);
fixture.remove();
fixture.listArchivedInboxPage.mockImplementation(async () => page([]));
await act(async () => { await onInboxIssueDeleted(qc, "workspace-1", "issue-a"); });
await waitFor(() => expect(screen.queryByTestId("detail")).not.toBeInTheDocument());
expect(screen.queryByText("inbox-a")).not.toBeInTheDocument();
expect(replace).toHaveBeenLastCalledWith("/acme/inbox?view=archived");
});
it("restores a pinned notification without keeping it in the archived list", async () => {
const fixture = archivedFixture();
await openArchived(fixture);
await act(async () => fixture.lookup.resolve(page([{ ...fixture.a, read: true }])));
act(() => rowActions.onAction("inbox-a"));
await waitFor(() => expect(fixture.unarchiveInbox).toHaveBeenCalledOnce());
await waitFor(() => expect(screen.getByTestId("detail")).toHaveTextContent("issue-b"));
expect(screen.queryByText("inbox-a")).not.toBeInTheDocument();
});
it.each(["status", "priority", "actor"] as const)("still applies the %s filter to a pinned archived row", async (filter) => {
const fixture = archivedFixture();
await openArchived(fixture);
act(() => {
const store = useInboxFilterStore.getState();
if (filter === "status") {
store.toggleStatusFilter("workspace-1", "todo");
patchInboxIssueProjection(qc, "workspace-1", "issue-a", { status: "done" });
} else if (filter === "priority") {
store.togglePriorityFilter("workspace-1", "high");
patchInboxIssueProjection(qc, "workspace-1", "issue-a", { priority: "low" });
} else {
store.toggleActorFilter("workspace-1", "agent:someone-else");
}
});
await waitFor(() => expect(screen.queryByTestId("detail")).not.toBeInTheDocument());
expect(screen.queryByText("inbox-a")).not.toBeInTheDocument();
expect(replace).toHaveBeenLastCalledWith("/acme/inbox?view=archived");
});
@@ -24,7 +24,9 @@ const listData: { active: InboxItem[]; archived: InboxItem[]; lookup?: InboxItem
const queryCalls: Array<{ queryKey: readonly unknown[]; enabled?: boolean }> = [];
const lookupState = { isLoading: false, isError: false, refetch: vi.fn() };
const queryClient = vi.hoisted(() => ({ setQueryData: vi.fn() }));
vi.mock("@tanstack/react-query", () => ({
useQueryClient: () => queryClient,
useQuery: (options: { queryKey: readonly unknown[]; enabled?: boolean }) => {
queryCalls.push(options);
return ({
@@ -277,7 +279,7 @@ function reset() {
queryCalls.length = 0;
searchParams = new URLSearchParams();
replace.mockClear();
markReadMutate.mockClear();
markReadMutate.mockReset();
markUnreadMutate.mockClear();
archiveMutate.mockClear();
unarchiveMutate.mockClear();
@@ -686,6 +688,30 @@ describe("InboxPage", () => {
expect(markReadMutate).toHaveBeenCalledWith("inbox-a", expect.anything());
});
it("keeps an unread-filtered notification open after it is marked read", () => {
// Opening a row marks it read. With the unread filter on, that would drop
// the row before its detail renders and clear the selection, so the click
// looked like it only made the notification disappear.
reset();
layout.width = DESKTOP;
listData.active = [
item({ id: "inbox-a", issue_id: "issue-a", read: false }),
];
useInboxFilterStore.getState().toggleUnreadOnly("workspace-1");
markReadMutate.mockImplementation((id: string) => {
listData.active = listData.active.map((current) =>
current.id === id ? { ...current, read: true } : current,
);
});
const { rerender } = render(<InboxPage />);
fireEvent.click(screen.getByTestId("row"));
rerender(<InboxPage />);
expect(screen.getByTestId("row")).toHaveTextContent("inbox-a");
expect(replace).not.toHaveBeenCalledWith("/acme/inbox");
});
it("keeps an explicitly unread row unread while it stays open", () => {
// Without the guard the auto-read effect fires on the very next commit and
// silently undoes the user's "mark as unread" — the action looks like a
+37 -8
View File
@@ -9,7 +9,7 @@ import {
useRef,
} from "react";
import { useDefaultLayout } from "react-resizable-panels";
import { useInfiniteQuery, useQuery } from "@tanstack/react-query";
import { useInfiniteQuery, useQuery, useQueryClient } from "@tanstack/react-query";
import { useWorkspaceId } from "@multica/core/hooks";
import { ApiError, errorCode } from "@multica/core/api";
import { useWorkspacePaths } from "@multica/core/paths";
@@ -107,6 +107,7 @@ const INBOX_LIST_MIN_SIZE = 240;
const INBOX_LIST_MAX_SIZE = 400;
export function InboxPage() {
const queryClient = useQueryClient();
const { t } = useT("inbox");
const showIssueLimitUpgradePrompt = useIssueLimitUpgradePrompt();
const showAutopilotQuotaRecoveryPrompt = useIssueLimitUpgradePrompt(
@@ -120,6 +121,10 @@ export function InboxPage() {
const [selectedKey, setSelectedKeyState] = useState(() => urlIssue);
const [view, setViewState] = useState<InboxView>(() => urlView);
// The row the user opened under the unread filter. Opening it marks it read,
// so the filter would otherwise drop the row before the detail finishes
// mounting. Scoped to the selection: moving away releases it.
const pinnedUnreadKeyRef = useRef<string | null>(null);
// Sync from URL when searchParams change (e.g. navigation)
useEffect(() => {
@@ -151,7 +156,14 @@ export function InboxPage() {
// The paginated endpoint guarantees the projection, including on empty pages.
const priorityFilterSupport = isArchivedView ? "supported" : inboxPriorityFilterSupport(rawItems);
const effectiveFilters = useMemo(() => inboxFiltersForPrioritySupport(filters, priorityFilterSupport), [filters, priorityFilterSupport]);
const visibleItems = useMemo(() => filterInboxItems(viewItems, effectiveFilters), [viewItems, effectiveFilters]);
const pinnedUnreadKey =
effectiveFilters.unreadOnly && pinnedUnreadKeyRef.current === selectedKey
? selectedKey
: null;
const filteredItems = useMemo(
() => filterInboxItems(viewItems, effectiveFilters, pinnedUnreadKey),
[viewItems, effectiveFilters, pinnedUnreadKey],
);
const hasActiveFilters = inboxFilterCount(effectiveFilters) > 0;
const selectedOnPage = viewItems.find((i) => (i.issue_id ?? i.id) === selectedKey);
// A deep link can point beyond every loaded page. Resolve its group directly
@@ -162,9 +174,13 @@ export function InboxPage() {
});
const lookupItems = useMemo(() => deduplicateArchivedInboxItems(lookup.data?.items ?? []), [lookup.data]);
const selectionItems = useMemo(() => {
if (!isArchivedView || selectedOnPage) return visibleItems;
return [...visibleItems, ...filterInboxItems(lookupItems, effectiveFilters)];
}, [isArchivedView, selectedOnPage, visibleItems, lookupItems, effectiveFilters]);
if (!isArchivedView || selectedOnPage) return filteredItems;
return deduplicateArchivedInboxItems([
...filteredItems,
...filterInboxItems(lookupItems, effectiveFilters, pinnedUnreadKey),
]);
}, [isArchivedView, selectedOnPage, filteredItems, lookupItems, effectiveFilters, pinnedUnreadKey]);
const visibleItems = isArchivedView && pinnedUnreadKey ? selectionItems : filteredItems;
const selected = selectionItems.find((i) => (i.issue_id ?? i.id) === selectedKey) ?? null;
const selectedInView = selectedOnPage ?? (isArchivedView ? lookupItems.find((i) => (i.issue_id ?? i.id) === selectedKey) : null);
const selectionFilteredOut = !!selectedKey && !!selectedInView && !selected;
@@ -311,6 +327,16 @@ export function InboxPage() {
useEffect(() => {
if (!selectedId || selectedRead) return;
if (manualUnreadIdRef.current === selectedId) return;
if (filters.unreadOnly) {
pinnedUnreadKeyRef.current = selectedKey;
if (isArchivedView && selected) {
// The unread page drops this group on refetch. Seed its lookup before
// the mutation so cache patches, rollback and deletion apply to it too.
queryClient.setQueryData(archivedInboxLookupOptions(wsId, selectedKey).queryKey, {
items: [selected], hasMore: false, nextCursor: null,
});
}
}
markReadMutate(selectedId, {
onError: (err) =>
toast.error(
@@ -319,14 +345,17 @@ export function InboxPage() {
: t(($) => $.errors.mark_read_failed),
),
});
}, [selectedId, selectedRead, markReadMutate, t]);
}, [selectedId, selectedRead, selectedKey, selected, filters.unreadOnly, isArchivedView, queryClient, wsId, markReadMutate, t]);
// Release the guard as soon as the selection moves off the parked row.
// Release the guards as soon as the selection moves off their row.
useEffect(() => {
if (manualUnreadIdRef.current && manualUnreadIdRef.current !== selectedId) {
manualUnreadIdRef.current = null;
}
}, [selectedId]);
if (pinnedUnreadKeyRef.current && pinnedUnreadKeyRef.current !== selectedKey) {
pinnedUnreadKeyRef.current = null;
}
}, [selectedId, selectedKey]);
const writeViewState = useViewStateWriter();
// Bumped when the already-open notification row is re-clicked; threaded to