diff --git a/packages/core/inbox/filter-store.test.ts b/packages/core/inbox/filter-store.test.ts index 3128074a5..0013fa956 100644 --- a/packages/core/inbox/filter-store.test.ts +++ b/packages/core/inbox/filter-store.test.ts @@ -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", () => { diff --git a/packages/core/inbox/filter-store.ts b/packages/core/inbox/filter-store.ts index 0ce31b8cf..145df956f 100644 --- a/packages/core/inbox/filter-store.ts +++ b/packages/core/inbox/filter-store.ts @@ -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) ); }); } diff --git a/packages/views/inbox/components/inbox-page-query.test.tsx b/packages/views/inbox/components/inbox-page-query.test.tsx new file mode 100644 index 000000000..768c8b3cb --- /dev/null +++ b/packages/views/inbox/components/inbox-page-query.test.tsx @@ -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
{issueId}
; + }, + 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 }) =>
{children}
, + ResizablePanel: ({ children }: { children: ReactNode }) =>
{children}
, + ResizableHandle: () => null, +})); +vi.mock("./inbox-list", () => ({ + InboxList: ({ items, onSelect }: { items: InboxItem[]; onSelect: (item: InboxItem) => void }) => ( +
{items.map((row) => ( + + ))}
+ ), +})); +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 { + 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(); +} + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason: Error) => void; + const promise = new Promise((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(); + const lookup = deferred(); + 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, 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"); +}); diff --git a/packages/views/inbox/components/inbox-page.test.tsx b/packages/views/inbox/components/inbox-page.test.tsx index b3c07adc2..1321e1371 100644 --- a/packages/views/inbox/components/inbox-page.test.tsx +++ b/packages/views/inbox/components/inbox-page.test.tsx @@ -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(); + fireEvent.click(screen.getByTestId("row")); + rerender(); + + 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 diff --git a/packages/views/inbox/components/inbox-page.tsx b/packages/views/inbox/components/inbox-page.tsx index e02e72b38..2985ed1ce 100644 --- a/packages/views/inbox/components/inbox-page.tsx +++ b/packages/views/inbox/components/inbox-page.tsx @@ -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(() => 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(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