diff --git a/packages/core/inbox/queries.test.ts b/packages/core/inbox/queries.test.ts index e01e0534a..f14da54d4 100644 --- a/packages/core/inbox/queries.test.ts +++ b/packages/core/inbox/queries.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { InboxItem, InboxWorkspaceUnread } from "../types"; -import { deduplicateInboxItems, hasOtherWorkspaceUnread, inboxKeys } from "./queries"; +import { deduplicateInboxItems, hasOtherWorkspaceUnread, inboxKeys, unreadWorkspaceIds } from "./queries"; function item(overrides: Partial): InboxItem { return { @@ -129,6 +129,24 @@ describe("hasOtherWorkspaceUnread", () => { }); }); +describe("unreadWorkspaceIds", () => { + it("collects only workspaces with a non-zero count", () => { + const ids = unreadWorkspaceIds([ + { workspace_id: "ws-1", count: 0 }, + { workspace_id: "ws-2", count: 3 }, + { workspace_id: "ws-3", count: 1 }, + ]); + expect(ids.has("ws-1")).toBe(false); + expect(ids.has("ws-2")).toBe(true); + expect(ids.has("ws-3")).toBe(true); + expect(ids.size).toBe(2); + }); + + it("returns an empty set for an empty summary", () => { + expect(unreadWorkspaceIds([]).size).toBe(0); + }); +}); + describe("inboxKeys.unreadSummary", () => { it("is a stable account-level key independent of any workspace", () => { expect(inboxKeys.unreadSummary()).toEqual(["inbox", "unread-summary"]); diff --git a/packages/core/inbox/queries.ts b/packages/core/inbox/queries.ts index 90388a9cb..fc536fe1f 100644 --- a/packages/core/inbox/queries.ts +++ b/packages/core/inbox/queries.ts @@ -42,6 +42,16 @@ export function hasOtherWorkspaceUnread( return summary.some((s) => s.workspace_id !== currentWsId && s.count > 0); } +/** + * Set of workspace ids that have unread inbox items. Lets the workspace + * switcher dropdown mark WHICH workspace a pending message lives in (the + * aggregate switcher dot only says "somewhere else"). Workspaces with a zero + * count are excluded. + */ +export function unreadWorkspaceIds(summary: InboxWorkspaceUnread[]): Set { + return new Set(summary.filter((s) => s.count > 0).map((s) => s.workspace_id)); +} + /** * Unread inbox count for the given workspace, aligned with what the inbox * list UI renders: archived items excluded, then deduplicated by issue so a diff --git a/packages/views/layout/app-sidebar.test.tsx b/packages/views/layout/app-sidebar.test.tsx index 2fb520c31..76836e256 100644 --- a/packages/views/layout/app-sidebar.test.tsx +++ b/packages/views/layout/app-sidebar.test.tsx @@ -3,11 +3,14 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { ApiError } from "@multica/core/api"; import { AppSidebar } from "./app-sidebar"; -const { detail, deletePin, navigation, pins, summary } = vi.hoisted(() => ({ +const { detail, deletePin, navigation, pins, summary, workspaces } = vi.hoisted(() => ({ detail: { current: { isPending: false, isError: false, data: null as unknown, error: null as unknown } }, deletePin: vi.fn(), navigation: { current: { pathname: "/acme/issues" } }, summary: { current: [] as { workspace_id: string; count: number }[] }, + workspaces: { + current: [] as { id: string; name: string; slug: string; avatar_url: string | null }[], + }, pins: { current: [ { @@ -63,7 +66,7 @@ vi.mock("@multica/ui/components/ui/sidebar", () => ({ })); vi.mock("@multica/ui/components/ui/dropdown-menu", () => ({ DropdownMenu: ({ children }: { children: React.ReactNode }) => <>{children}, - DropdownMenuContent: () => null, + DropdownMenuContent: ({ children }: { children: React.ReactNode }) => <>{children}, DropdownMenuGroup: ({ children }: { children: React.ReactNode }) => <>{children}, DropdownMenuItem: ({ children }: { children: React.ReactNode }) => <>{children}, DropdownMenuLabel: ({ children }: { children: React.ReactNode }) => <>{children}, @@ -131,6 +134,8 @@ vi.mock("@multica/core/inbox/queries", () => ({ entries: { workspace_id: string; count: number }[], currentWsId: string | null, ) => entries.some((s) => s.workspace_id !== currentWsId && s.count > 0), + unreadWorkspaceIds: (entries: { workspace_id: string; count: number }[]) => + new Set(entries.filter((s) => s.count > 0).map((s) => s.workspace_id)), })); vi.mock("@multica/core/issues/queries", () => ({ issueDetailOptions: () => ({ queryKey: ["issue"] }) })); vi.mock("@multica/core/issues/stores/create-mode-store", () => ({ @@ -155,6 +160,7 @@ vi.mock("@tanstack/react-query", async (importOriginal) => ({ if (queryKey[0] === "pins") return { data: pins.current }; if (queryKey[0] === "issue") return detail.current; if (queryKey[0] === "inbox" && queryKey[1] === "unread-summary") return { data: summary.current }; + if (queryKey[0] === "workspaces") return { data: workspaces.current }; return { data: [] }; }, useQueryClient: () => ({ fetchQuery: vi.fn(), invalidateQueries: vi.fn() }), @@ -166,6 +172,7 @@ describe("PinRow", () => { navigation.current.pathname = "/acme/issues"; detail.current = { isPending: false, isError: false, data: null, error: null }; summary.current = []; + workspaces.current = []; }); it("unpins missing details", async () => { @@ -209,10 +216,11 @@ describe("PinRow", () => { describe("workspace-switcher unread dot", () => { beforeEach(() => { summary.current = []; + workspaces.current = []; }); - // The dot is the only `.ring-sidebar` span in the tree (DraftDot is null - // when there's no draft, and there are no pending invitations here). + // The aggregate switcher dot is the only `.ring-sidebar` span in the tree + // (DraftDot is null when there's no draft, and there are no invitations). const dot = (container: HTMLElement) => container.querySelector("span.bg-brand.ring-sidebar"); it("shows a dot when another workspace has unread inbox items", () => { @@ -234,3 +242,40 @@ describe("workspace-switcher unread dot", () => { expect(dot(container)).toBeNull(); }); }); + +describe("workspace-switcher dropdown per-workspace dot", () => { + beforeEach(() => { + summary.current = []; + // Active workspace is ws-1 (see useCurrentWorkspace mock); "Other" is ws-2. + workspaces.current = [ + { id: "ws-1", name: "Active WS", slug: "active", avatar_url: null }, + { id: "ws-2", name: "Other WS", slug: "other", avatar_url: null }, + ]; + }); + + // Row dots are brand dots WITHOUT the aggregate avatar dot's `ring-sidebar`. + const rowDots = (container: HTMLElement) => + container.querySelectorAll("span.bg-brand:not(.ring-sidebar)"); + + it("dots the specific other workspace that has unread", () => { + summary.current = [{ workspace_id: "ws-2", count: 3 }]; + const { container } = render(); + // Exactly one row dot, sitting right after the "Other WS" name; the active + // row shows the check, not a dot. + expect(rowDots(container)).toHaveLength(1); + expect(screen.getByText("Other WS").nextElementSibling?.className).toContain("bg-brand"); + expect(screen.getByText("Active WS").nextElementSibling?.className ?? "").not.toContain("bg-brand"); + }); + + it("does not dot a workspace whose unread count is zero", () => { + summary.current = [{ workspace_id: "ws-2", count: 0 }]; + const { container } = render(); + expect(rowDots(container)).toHaveLength(0); + }); + + it("never dots the active workspace even when it has unread", () => { + summary.current = [{ workspace_id: "ws-1", count: 5 }]; + const { container } = render(); + expect(rowDots(container)).toHaveLength(0); + }); +}); diff --git a/packages/views/layout/app-sidebar.tsx b/packages/views/layout/app-sidebar.tsx index d4d451fa5..6953a99d9 100644 --- a/packages/views/layout/app-sidebar.tsx +++ b/packages/views/layout/app-sidebar.tsx @@ -70,7 +70,7 @@ import { useCurrentWorkspace, useWorkspacePaths, paths } from "@multica/core/pat import { workspaceListOptions, myInvitationListOptions, workspaceKeys } from "@multica/core/workspace/queries"; import { resolvePublicFileUrl } from "@multica/core/workspace/avatar-url"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; -import { inboxKeys, deduplicateInboxItems, inboxUnreadSummaryOptions, hasOtherWorkspaceUnread } from "@multica/core/inbox/queries"; +import { inboxKeys, deduplicateInboxItems, inboxUnreadSummaryOptions, hasOtherWorkspaceUnread, unreadWorkspaceIds } from "@multica/core/inbox/queries"; import { api, ApiError } from "@multica/core/api"; import { useModalStore } from "@multica/core/modals"; import { useConfigStore } from "@multica/core/config"; @@ -376,6 +376,9 @@ export function AppSidebar({ topSlot, searchSlot, headerClassName, headerStyle } () => hasOtherWorkspaceUnread(unreadSummary, wsId), [unreadSummary, wsId], ); + // Which workspaces have unread, so the switcher dropdown can point at the + // specific one(s) rather than just the aggregate avatar dot. + const unreadWsIds = React.useMemo(() => unreadWorkspaceIds(unreadSummary), [unreadSummary]); const hasRuntimeUpdates = useMyRuntimesNeedUpdate(wsId); const { data: pinnedItems = EMPTY_PINS } = useQuery({ ...pinListOptions(wsId ?? "", userId ?? ""), @@ -549,6 +552,14 @@ export function AppSidebar({ topSlot, searchSlot, headerClassName, headerStyle } > {ws.name} + {/* Points at the specific workspace holding unread + inbox items. Sits in the same right-edge slot as the + active-workspace check; the active workspace is + excluded (its unread is the Inbox nav count), so dot + and check never collide on one row. */} + {ws.id !== workspace?.id && unreadWsIds.has(ws.id) && ( + + )} {ws.id === workspace?.id && ( )} diff --git a/server/cmd/server/integration_test.go b/server/cmd/server/integration_test.go index 827e12dc4..ad2098255 100644 --- a/server/cmd/server/integration_test.go +++ b/server/cmd/server/integration_test.go @@ -848,6 +848,72 @@ func TestInboxUnreadSummaryThroughRouter(t *testing.T) { } } +// An issue's inbox notifications are deduplicated per issue: opening the issue +// marks only the NEWEST item read, leaving older siblings unread. The summary +// must mirror the inbox UI (issue is read when its newest item is read), so a +// read-newest / unread-older issue must NOT light the switcher dot (MUL-3695). +func TestInboxUnreadSummaryDedupesByIssue(t *testing.T) { + ctx := context.Background() + + var issueID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO issue (workspace_id, title, creator_type, creator_id) + VALUES ($1, 'Dedup fixture', 'member', $2) + RETURNING id + `, testWorkspaceID, testUserID).Scan(&issueID); err != nil { + t.Fatalf("failed to seed issue: %v", err) + } + // Deleting the issue cascades to its inbox_item rows (FK ON DELETE CASCADE). + t.Cleanup(func() { + testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID) + }) + + // Older sibling stays unread; newer sibling is read (the one "opening the + // issue" would have marked read). + if _, err := testPool.Exec(ctx, ` + INSERT INTO inbox_item (workspace_id, recipient_type, recipient_id, type, title, issue_id, read, created_at) + VALUES + ($1, 'member', $2, 'new_comment', 'older', $3, false, now() - interval '1 hour'), + ($1, 'member', $2, 'status_changed', 'newer', $3, true, now()) + `, testWorkspaceID, testUserID, issueID); err != nil { + t.Fatalf("failed to seed inbox items: %v", err) + } + + resp := authRequest(t, "GET", "/api/inbox/unread-summary", nil) + if resp.StatusCode != 200 { + t.Fatalf("UnreadInboxSummary: expected 200, got %d", resp.StatusCode) + } + var summary []struct { + WorkspaceID string `json:"workspace_id"` + Count int64 `json:"count"` + } + readJSON(t, resp, &summary) + for _, s := range summary { + if s.WorkspaceID == testWorkspaceID && s.Count > 0 { + t.Fatalf("issue whose newest item is read must not count as unread, got count %d", s.Count) + } + } + + // Now mark the newest item unread again → the issue becomes unread and the + // workspace reappears in the summary. + if _, err := testPool.Exec(ctx, ` + UPDATE inbox_item SET read = false WHERE issue_id = $1 AND title = 'newer' + `, issueID); err != nil { + t.Fatalf("failed to flip newest item unread: %v", err) + } + resp = authRequest(t, "GET", "/api/inbox/unread-summary", nil) + readJSON(t, resp, &summary) + var found bool + for _, s := range summary { + if s.WorkspaceID == testWorkspaceID && s.Count >= 1 { + found = true + } + } + if !found { + t.Fatalf("expected workspace in summary once newest item is unread, got %+v", summary) + } +} + // ---- 404 for non-existent resources ---- func TestNonExistentResources(t *testing.T) { diff --git a/server/pkg/db/generated/inbox.sql.go b/server/pkg/db/generated/inbox.sql.go index acbd94682..bafd32ca6 100644 --- a/server/pkg/db/generated/inbox.sql.go +++ b/server/pkg/db/generated/inbox.sql.go @@ -176,14 +176,19 @@ func (q *Queries) CountUnreadInbox(ctx context.Context, arg CountUnreadInboxPara } const countUnreadInboxByWorkspace = `-- name: CountUnreadInboxByWorkspace :many -SELECT i.workspace_id, count(*) AS count -FROM inbox_item i -JOIN member m ON m.workspace_id = i.workspace_id AND m.user_id = i.recipient_id -WHERE i.recipient_type = 'member' - AND i.recipient_id = $1 - AND i.read = false - AND i.archived = false -GROUP BY i.workspace_id +SELECT newest.workspace_id, count(*) AS count +FROM ( + SELECT DISTINCT ON (i.workspace_id, COALESCE(i.issue_id, i.id)) + i.workspace_id, i.read + FROM inbox_item i + JOIN member m ON m.workspace_id = i.workspace_id AND m.user_id = i.recipient_id + WHERE i.recipient_type = 'member' + AND i.recipient_id = $1 + AND i.archived = false + ORDER BY i.workspace_id, COALESCE(i.issue_id, i.id), i.created_at DESC +) newest +WHERE newest.read = false +GROUP BY newest.workspace_id ` type CountUnreadInboxByWorkspaceRow struct { @@ -191,12 +196,16 @@ type CountUnreadInboxByWorkspaceRow struct { Count int64 `json:"count"` } -// Per-workspace unread (non-archived) inbox counts for a recipient member, -// across every workspace they currently belong to. Powers the sidebar -// "other workspaces have unread" dot without fetching each workspace's full -// inbox list. The member join keeps counts scoped to workspaces the user is -// still a member of, so a stale item left behind in a workspace the user -// has since left cannot light the dot. +// Per-workspace unread inbox counts for a recipient member, matching the +// inbox UI's deduplicated view: notifications are grouped per issue +// (Linear-style, one row per issue) and an issue counts as unread only when +// its NEWEST non-archived item is unread. Opening an issue marks just that +// newest item read, so counting raw unread rows would keep older siblings +// alive and light the switcher dot for a workspace whose inbox the user sees +// as empty (MUL-3695). Items without an issue group on their own id. The +// member join keeps counts scoped to workspaces the user still belongs to, +// so a stale item left behind in a workspace the user has since left cannot +// light the dot. func (q *Queries) CountUnreadInboxByWorkspace(ctx context.Context, recipientID pgtype.UUID) ([]CountUnreadInboxByWorkspaceRow, error) { rows, err := q.db.Query(ctx, countUnreadInboxByWorkspace, recipientID) if err != nil { diff --git a/server/pkg/db/queries/inbox.sql b/server/pkg/db/queries/inbox.sql index 3dc269634..bc7a5161e 100644 --- a/server/pkg/db/queries/inbox.sql +++ b/server/pkg/db/queries/inbox.sql @@ -46,20 +46,29 @@ SELECT count(*) FROM inbox_item WHERE workspace_id = $1 AND recipient_type = $2 AND recipient_id = $3 AND read = false AND archived = false; -- name: CountUnreadInboxByWorkspace :many --- Per-workspace unread (non-archived) inbox counts for a recipient member, --- across every workspace they currently belong to. Powers the sidebar --- "other workspaces have unread" dot without fetching each workspace's full --- inbox list. The member join keeps counts scoped to workspaces the user is --- still a member of, so a stale item left behind in a workspace the user --- has since left cannot light the dot. -SELECT i.workspace_id, count(*) AS count -FROM inbox_item i -JOIN member m ON m.workspace_id = i.workspace_id AND m.user_id = i.recipient_id -WHERE i.recipient_type = 'member' - AND i.recipient_id = $1 - AND i.read = false - AND i.archived = false -GROUP BY i.workspace_id; +-- Per-workspace unread inbox counts for a recipient member, matching the +-- inbox UI's deduplicated view: notifications are grouped per issue +-- (Linear-style, one row per issue) and an issue counts as unread only when +-- its NEWEST non-archived item is unread. Opening an issue marks just that +-- newest item read, so counting raw unread rows would keep older siblings +-- alive and light the switcher dot for a workspace whose inbox the user sees +-- as empty (MUL-3695). Items without an issue group on their own id. The +-- member join keeps counts scoped to workspaces the user still belongs to, +-- so a stale item left behind in a workspace the user has since left cannot +-- light the dot. +SELECT newest.workspace_id, count(*) AS count +FROM ( + SELECT DISTINCT ON (i.workspace_id, COALESCE(i.issue_id, i.id)) + i.workspace_id, i.read + FROM inbox_item i + JOIN member m ON m.workspace_id = i.workspace_id AND m.user_id = i.recipient_id + WHERE i.recipient_type = 'member' + AND i.recipient_id = $1 + AND i.archived = false + ORDER BY i.workspace_id, COALESCE(i.issue_id, i.id), i.created_at DESC +) newest +WHERE newest.read = false +GROUP BY newest.workspace_id; -- name: MarkAllInboxRead :execrows UPDATE inbox_item SET read = true