From 33a526dc929cd56d74e646f07ff921452766c54a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 14:47:07 +0000 Subject: [PATCH 1/3] fix(app-shell): opening a notification from Home's My-work card marks it read Both notification doors (the top-bar bell's popover row and Home's "My work -> Notifications" row) now call one open action, openInboxNotification: mark the row read first, then follow its resolved link, or open the inbox when it has none. Home hands the action the bell's own write (useInboxBell().markRead), so the request it sends is the bell's POST /api/v1/notifications/read. Claude-Session: https://claude.ai/code/session_01Utp6CbDAucNyQ4UtzCJKyh Co-authored-by: Claude --- .../app-shell/src/console/home/HomePage.tsx | 39 +-- .../HomePage.aiStudioDisabled.test.tsx | 4 + .../HomePage.approvalsTarget.test.tsx | 4 + .../HomePage.authoringCapabilityGate.test.tsx | 4 + .../HomePage.draftsReadGate-25894.test.tsx | 4 + .../HomePage.greetingAndApps-12601.test.tsx | 4 + .../HomePage.greetingLocale-11689.test.tsx | 4 + .../HomePage.inboxLinksTarget.test.tsx | 4 + ...HomePage.launcherHiddenApps-26838.test.tsx | 4 + .../HomePage.marketplaceDisabled.test.tsx | 4 + .../__tests__/HomePage.myWork-12602.test.tsx | 4 + .../HomePage.noAgentsBuildChip-26614.test.tsx | 4 + .../HomePage.notificationDeepLink.test.tsx | 4 + .../__tests__/HomePage.openMarksRead.test.tsx | 260 ++++++++++++++++++ ...ge.pendingDraftsPackageName-13136.test.tsx | 4 + .../HomePage.recentlyAccessed-12604.test.tsx | 4 + .../app-shell/src/layout/InboxPopover.tsx | 80 +++--- .../__tests__/openInboxNotification.test.ts | 57 ++++ .../src/layout/openInboxNotification.ts | 80 ++++++ 19 files changed, 513 insertions(+), 59 deletions(-) create mode 100644 ui/packages/app-shell/src/console/home/__tests__/HomePage.openMarksRead.test.tsx create mode 100644 ui/packages/app-shell/src/layout/__tests__/openInboxNotification.test.ts create mode 100644 ui/packages/app-shell/src/layout/openInboxNotification.ts diff --git a/ui/packages/app-shell/src/console/home/HomePage.tsx b/ui/packages/app-shell/src/console/home/HomePage.tsx index 231e4b759b1..062c332fc67 100644 --- a/ui/packages/app-shell/src/console/home/HomePage.tsx +++ b/ui/packages/app-shell/src/console/home/HomePage.tsx @@ -46,8 +46,9 @@ import { matchAppBySegment, resolveHostAppSegment, resolveKeyedI18nLabel, - resolveNotificationTarget, } from '../../utils/index.js'; +import { useInboxBell } from '../../hooks/useInboxBell.js'; +import { openInboxNotification } from '../../layout/openInboxNotification.js'; import { resolveSetupAppPath } from '../ConsoleShell.js'; import { Empty, @@ -369,6 +370,11 @@ export function HomePage() { notificationsReadable, activity, } = useHomeInbox(); + // Opening a notification from the My-work card marks it read through the + // bell's own write — `useInboxBell` is the one wiring of mark-read onto the + // shared inbox feed (the header bell and the `global:notifications` block + // mount it too), so the request Home sends is the request the bell sends. + const { markRead: markNotificationRead } = useInboxBell(); // The drafts row of the My-work section — `null` with nothing to publish. const draftsRow = usePendingDraftsRow(t, language); // Home renders OUTSIDE the `/apps/:appName/*` router, so there is no @@ -833,23 +839,20 @@ export function HomePage() { so it has to be hosted under an app before it is a route — navigating it verbatim is objectui#5179, where the console catch-all forwarded the unmatched path to `/` and the landing - resolver dropped the user on the default app's home with the - notification already marked read. `resolveNotificationTarget` - is shared with the top-bar bell so the two cannot answer this - differently. The fallback arm runs whenever a notification - carries no link at all, and opens the inbox. */ - onOpenNotification={(n) => { - const target = resolveNotificationTarget(n.actionUrl, hostAppSegment); - if (!target) { - navigate(`/apps/${hostAppSegment}/sys_inbox_message?view=mine`); - return; - } - if (target.kind === 'external') { - window.open(target.url, '_blank', 'noopener,noreferrer'); - return; - } - navigate(target.path); - }} + resolver dropped the user on the default app's home. + `openInboxNotification` is the top-bar bell's own open action, + so the two doors cannot answer differently: it marks the + notification read first (this row used to skip that, and a + notification opened from Home stayed unread on the server and + badged on the bell), then follows the resolved link — or, when + the notification carries no link at all, opens the inbox. */ + onOpenNotification={(n) => + openInboxNotification(n, hostAppSegment, { + markRead: markNotificationRead, + goTo: (path) => navigate(path), + openInbox: () => navigate(`/apps/${hostAppSegment}/sys_inbox_message?view=mine`), + }) + } t={t} /> diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.aiStudioDisabled.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.aiStudioDisabled.test.tsx index 421f36c108a..3046114c64d 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.aiStudioDisabled.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.aiStudioDisabled.test.tsx @@ -128,6 +128,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({ })); import { initRuntimeConfig, resetRuntimeConfigForTesting } from '../../../runtime-config'; +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; /** diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx index 103e26eb35d..ecdc4c01614 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx @@ -131,6 +131,10 @@ vi.mock('../../../runtime-config', () => ({ isAiStudioEnabled: () => true, })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; /* ── The `_drafts` double (objectui#7307) ───────────────────────────────────── diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx index 3a205f826eb..ac04b32b674 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx @@ -140,6 +140,10 @@ vi.mock('../../../runtime-config', () => ({ isAiStudioEnabled: () => true, })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; /* ── The `_drafts` double (objectui#7307) ───────────────────────────────────── diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.draftsReadGate-25894.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.draftsReadGate-25894.test.tsx index 66e07efb403..734bfcdd4d1 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.draftsReadGate-25894.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.draftsReadGate-25894.test.tsx @@ -86,6 +86,10 @@ vi.mock('../../../runtime-config', () => ({ isAiStudioEnabled: () => true, })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; const DRAFTS_ROUTE = '/api/v1/meta/_drafts'; diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingAndApps-12601.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingAndApps-12601.test.tsx index 4929f9f966f..f35199cdfa6 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingAndApps-12601.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingAndApps-12601.test.tsx @@ -147,6 +147,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({ usePublishAllDrafts: () => ({ publishAll: async () => ({ ok: true }), publishing: false }), })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; const CRM = { name: 'crm', label: 'CRM', icon: 'Building2' }; diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingLocale-11689.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingLocale-11689.test.tsx index bd5f9b2a650..3b864bd1d19 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingLocale-11689.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingLocale-11689.test.tsx @@ -100,6 +100,10 @@ vi.mock('../../../runtime-config', () => ({ isAiStudioEnabled: () => true, })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; /* ── The `_drafts` double ───────────────────────────────────────────────────── diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx index ff16e1a9e7a..014adca8a49 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx @@ -134,6 +134,10 @@ vi.mock('../../../runtime-config', () => ({ isAiStudioEnabled: () => true, })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; /* ── The `_drafts` double (objectui#7307) ───────────────────────────────────── diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.launcherHiddenApps-26838.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.launcherHiddenApps-26838.test.tsx index 3c23c9f11b8..13b9306abdd 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.launcherHiddenApps-26838.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.launcherHiddenApps-26838.test.tsx @@ -133,6 +133,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({ usePublishAllDrafts: () => ({ publishAll: async () => ({ ok: true }), publishing: false }), })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; import { LauncherAppsProvider } from '../../../hooks/useLauncherApps'; diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.marketplaceDisabled.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.marketplaceDisabled.test.tsx index 9708c6f94a5..c10b69a3fc2 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.marketplaceDisabled.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.marketplaceDisabled.test.tsx @@ -107,6 +107,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({ })); import { initRuntimeConfig, resetRuntimeConfigForTesting } from '../../../runtime-config'; +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; /** A `GET /api/v1/runtime/config` answer, as the server sends it. */ diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.myWork-12602.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.myWork-12602.test.tsx index 6d56b21c618..4122ff11775 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.myWork-12602.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.myWork-12602.test.tsx @@ -145,6 +145,10 @@ vi.mock('../../../views/studio-design/packages-io', async (importOriginal) => ({ fetchPackages: () => (state.packages ? Promise.resolve(state.packages) : new Promise(() => {})), })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; const CRM = { name: 'crm', label: 'CRM', icon: 'Building2' }; diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.noAgentsBuildChip-26614.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.noAgentsBuildChip-26614.test.tsx index 17e61371262..bad6d913caa 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.noAgentsBuildChip-26614.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.noAgentsBuildChip-26614.test.tsx @@ -134,6 +134,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({ usePublishAllDrafts: () => ({ publishAll: async () => ({ ok: true }), publishing: false }), })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; import { STUDIO_NEW_PACKAGE_PATH } from '../../../views/studio-design/studioNewPackage'; diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx index ba4e697e12f..fe26877e842 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.notificationDeepLink.test.tsx @@ -130,6 +130,10 @@ vi.mock('../../../runtime-config', () => ({ isAiStudioEnabled: () => true, })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; /* ── The `_drafts` double (objectui#7307) ───────────────────────────────────── diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.openMarksRead.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.openMarksRead.test.tsx new file mode 100644 index 00000000000..f74862b6a91 --- /dev/null +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.openMarksRead.test.tsx @@ -0,0 +1,260 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Opening a notification from Home's "My work → Notifications" card marks it + * read, exactly as opening it from the top-bar bell does. + * + * ## The defect + * + * The bell's row click marked the message read and then followed its link. + * Home's row click only followed the link. Nothing else ever marks a message + * read, so a notification opened from Home stayed unread on the server: no + * `POST /api/v1/notifications/read` went out, `unreadCount` did not move, and + * the bell kept its badge after a reload. + * + * Both doors now call `openInboxNotification`, and Home hands it the bell's own + * write (`useInboxBell().markRead`) — so the request below is the bell's + * request by construction, not a second copy of it. + * + * ## What this drives + * + * The REAL `HomePage`, the real `useHomeInbox` and `useInboxBell`, and the real + * shared inbox feed, over one fake data adapter (the inbox rows and their + * receipts) and one recording `fetch` double that plays the server: a + * `POST /api/v1/notifications/read` writes `read` receipts for the ids it + * names, so the next feed read answers with them. What is asserted is the + * request on the wire — method, path and body — its order against the + * navigation, and the card's unread count after the feed re-reads. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { act, cleanup, render, screen, waitFor, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +/** Every observable step, in the order it happened: requests and navigations. */ +let timeline: string[] = []; + +const navigateMock = vi.fn((to: string) => { + timeline.push(`navigate ${to}`); +}); + +vi.mock('react-router-dom', async () => { + const { createElement } = await import('react'); + return { + useNavigate: () => navigateMock, + // The bell wiring's arrival announcer reads the route's app segment. + useParams: () => ({}), + Link: ({ to, ...rest }: { to: string } & Record) => + createElement('a', { ...rest, href: to }), + }; +}); + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + language: 'en', + }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada', email: 'ada@example.com' } }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), +})); + +vi.mock('@object-ui/plugin-chatbot/core', async (importOriginal) => ({ + ...(await importOriginal()), + useAgents: () => ({ agents: [] }), + isAskAgent: () => false, + agentHasCapability: () => false, +})); + +vi.mock('../../../providers/MetadataProvider', () => ({ + useMetadata: () => ({ apps: [{ name: 'crm', label: 'crm' }], loading: false }), +})); + +vi.mock('../../../context/NavigationContext', () => ({ + useNavigationContext: () => ({ currentAppName: 'crm' }), +})); + +vi.mock('../../../hooks/useRecentItems', () => ({ useRecentItems: () => ({ recentItems: [] }) })); +vi.mock('../../../hooks/useFavorites', () => ({ useFavorites: () => ({ favorites: [] }) })); +vi.mock('../../../hooks/useAiSurface', () => ({ resolveAiApiBase: () => '' })); +vi.mock('../../../views/metadata-admin/useMetadata', () => ({ + useMetadataClient: () => ({ listDrafts: async () => [] }), +})); +vi.mock('../../../preview/usePublishAllDrafts', () => ({ + usePublishAllDrafts: () => ({ publishAll: async () => ({ ok: true }), publishing: false }), +})); +vi.mock('../../../runtime-config', () => ({ + getRuntimeConfig: () => ({ branding: { productName: 'ObjectStack' } }), + isMarketplaceEnabled: () => true, + isAiStudioEnabled: () => true, +})); + +// ── The server's side of the inbox: messages, and receipts keyed by event id ── + +/** Two unread messages, so the count has somewhere to drop to. */ +const INBOX = [ + { + id: 'ibx_1', + user_id: 'u1', + notification_id: 'ntf_1', + topic: 'task.assigned', + title: 'Task T-1 was assigned to you', + action_url: '/showcase_task/t_1', + created_at: '2026-10-29T09:00:00Z', + }, + { + id: 'ibx_2', + user_id: 'u1', + notification_id: 'ntf_2', + topic: 'task.mention', + title: 'You were mentioned in Task T-2', + action_url: '/showcase_task/t_2', + created_at: '2026-10-29T08:00:00Z', + }, +]; + +/** Event ids the server holds a `read` receipt for. */ +let readIds = new Set(); + +const fakeAdapter = { + find: (object: string) => { + if (object === 'sys_inbox_message') return Promise.resolve({ data: INBOX }); + if (object === 'sys_notification_receipt') { + return Promise.resolve({ + data: INBOX.map((r, i) => ({ + id: `rcp_${i + 1}`, + notification_id: r.notification_id, + user_id: 'u1', + channel: 'inbox', + state: readIds.has(r.notification_id) ? 'read' : 'delivered', + })), + }); + } + return Promise.resolve({ data: [] }); + }, + getClient: () => undefined, +}; + +vi.mock('../../../providers/AdapterProvider', () => ({ useAdapter: () => fakeAdapter })); + +/** The bodies of every mark-read request, in order. */ +let markReadBodies: unknown[] = []; +/** Every URL the global `fetch` was handed that this double does not serve. */ +let unserved: string[] = []; + +const routeOf = (url: string) => new URL(url, 'http://localhost').pathname; + +function installServerDouble() { + vi.stubGlobal( + 'fetch', + vi.fn(async (input: unknown, init?: { method?: string; body?: unknown }) => { + const url = String(input); + const method = (init?.method ?? 'GET').toUpperCase(); + const path = routeOf(url); + if (method === 'POST' && path === '/api/v1/notifications/read') { + const body = JSON.parse(String(init?.body ?? '{}')) as { ids?: string[] }; + timeline.push(`POST ${path}`); + markReadBodies.push(body); + for (const id of body.ids ?? []) readIds.add(id); + return { ok: true, status: 200, json: async () => ({ success: true }) }; + } + if (method === 'GET' && path === '/api/v1/meta/_drafts') { + return { ok: true, status: 200, json: async () => ({ drafts: [] }) }; + } + if (method === 'GET' && path === '/api/v1/approvals/requests') { + return { ok: true, status: 200, json: async () => ({ data: [], total: 0 }) }; + } + unserved.push(`${method} ${url}`); + return { ok: false, status: 404, json: async () => ({}) }; + }), + ); +} + +import { HomePage } from '../HomePage'; +import { __resetSharedUserFeeds } from '../../../hooks/sharedUserFeeds'; + +beforeEach(() => { + timeline = []; + markReadBodies = []; + unserved = []; + readIds = new Set(); + navigateMock.mockClear(); + __resetSharedUserFeeds(); + installServerDouble(); +}); + +afterEach(() => { + // A router, not a sink: a request to anything this double does not serve + // fails here instead of vanishing into a best-effort `catch`. + expect(unserved).toEqual([]); + cleanup(); + vi.unstubAllGlobals(); + __resetSharedUserFeeds(); +}); + +const notificationsCard = () => screen.getByTestId('home-my-work-notifications'); +const unreadPill = () => within(notificationsCard()).queryByTestId('home-my-work-notifications-count'); + +describe('Home "My work → Notifications": opening a notification marks it read', () => { + it('sends the bell mark-read request for the opened row, before following its link', async () => { + const user = userEvent.setup(); + render(); + await screen.findByText('Task T-1 was assigned to you'); + + await user.click(screen.getByText('Task T-1 was assigned to you')); + + // The request the bell sends: one POST, keyed by the notification EVENT + // id (`notification_id`), not the inbox row id. + expect(markReadBodies).toEqual([{ ids: ['ntf_1'] }]); + // Spent first, then followed — the bell's order. + expect(timeline).toEqual([ + 'POST /api/v1/notifications/read', + 'navigate /apps/crm/showcase_task/t_1', + ]); + }); + + it('drops the unread count once the feed re-reads the receipt the request wrote', async () => { + const user = userEvent.setup(); + render(); + await screen.findByText('Task T-1 was assigned to you'); + expect(unreadPill()).toHaveTextContent('2'); + + await user.click(screen.getByText('Task T-1 was assigned to you')); + + // Returning to the tab re-reads the shared feed at once — the same read + // the bell's badge is cut from. + await act(async () => { + document.dispatchEvent(new Event('visibilitychange')); + }); + + await waitFor(() => expect(unreadPill()).toHaveTextContent('1')); + expect(screen.queryByText('Task T-1 was assigned to you')).not.toBeInTheDocument(); + expect(screen.getByText('You were mentioned in Task T-2')).toBeInTheDocument(); + }); + + it('marks a linkless notification read too, then opens the inbox', async () => { + const original = INBOX[0].action_url; + (INBOX[0] as { action_url?: string }).action_url = undefined; + try { + const user = userEvent.setup(); + render(); + await screen.findByText('Task T-1 was assigned to you'); + + await user.click(screen.getByText('Task T-1 was assigned to you')); + + expect(markReadBodies).toEqual([{ ids: ['ntf_1'] }]); + expect(timeline).toEqual([ + 'POST /api/v1/notifications/read', + 'navigate /apps/crm/sys_inbox_message?view=mine', + ]); + } finally { + INBOX[0].action_url = original; + } + }); +}); diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.pendingDraftsPackageName-13136.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.pendingDraftsPackageName-13136.test.tsx index cf0cff76b8b..e046f03e990 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.pendingDraftsPackageName-13136.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.pendingDraftsPackageName-13136.test.tsx @@ -116,6 +116,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({ usePublishAllDrafts: () => ({ publishAll: vi.fn(async () => ({ ok: true })), publishing: false }), })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; const DRAFTS_ROUTE = '/api/v1/meta/_drafts'; diff --git a/ui/packages/app-shell/src/console/home/__tests__/HomePage.recentlyAccessed-12604.test.tsx b/ui/packages/app-shell/src/console/home/__tests__/HomePage.recentlyAccessed-12604.test.tsx index bd08513e4c9..a4acdfb8983 100644 --- a/ui/packages/app-shell/src/console/home/__tests__/HomePage.recentlyAccessed-12604.test.tsx +++ b/ui/packages/app-shell/src/console/home/__tests__/HomePage.recentlyAccessed-12604.test.tsx @@ -133,6 +133,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({ usePublishAllDrafts: () => ({ publishAll: async () => ({ ok: true }), publishing: false }), })); +// Home marks an opened notification read through the bell's own write +// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed. +vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) })); + import { HomePage } from '../HomePage'; const CRM = { name: 'crm', label: 'HotCRM', icon: 'Building2', _packageId: 'com.acme.crm' }; diff --git a/ui/packages/app-shell/src/layout/InboxPopover.tsx b/ui/packages/app-shell/src/layout/InboxPopover.tsx index 23129d25d35..3c98312cca9 100644 --- a/ui/packages/app-shell/src/layout/InboxPopover.tsx +++ b/ui/packages/app-shell/src/layout/InboxPopover.tsx @@ -38,10 +38,11 @@ import { useObjectTranslation, useDisplayLocale } from '@object-ui/i18n'; import type { ActivityFeedReading } from '../hooks/sharedUserFeeds.js'; import { useNavigationContext } from '../context/NavigationContext.js'; import { useMetadata } from '../providers/MetadataProvider.js'; -import { resolveHostAppSegment, resolveNotificationTarget } from '../utils/appRoute.js'; +import { resolveHostAppSegment } from '../utils/appRoute.js'; import { timeAgo } from '../utils/relativeTime.js'; import { groupNotifications } from './inboxGrouping.js'; import type { InboxNotification, NotificationGroup } from './inboxGrouping.js'; +import { openInboxNotification } from './openInboxNotification.js'; import { ActivityObjectLabel } from './ActivityObjectLabel.js'; import { confirmUnsavedLeave } from '../chrome/unsavedEdits.js'; @@ -279,46 +280,43 @@ export function InboxPopover({ leaveTo(`/apps/${hostAppSegment}/sys_activity`); }; - const handleNotificationClick = (n: InboxNotification) => { - onMarkRead(n.id); - // Prefer the materialization's action_url (ADR-0030). The messaging - // pipeline synthesizes an app-relative `/{object}/{id}` link from the - // event's source when a producer didn't set an explicit url — so it names - // a RECORD, and `hostAppSegment` (already resolved above for the two "see - // all" drills) is what turns it into a route. - // - // This used to host it under a raw `currentAppName ?? params.appName` and - // navigate BARE when neither named an app — which is the state on every - // surface outside `/apps/:appName/*` that mounts this bell (`/home`, - // `/organizations`, the full-page AI screen). The bare path matched no - // route, the console catch-all forwarded it to `/`, and the landing - // resolver dropped the user on the default app's home: objectui#5179. - // Reusing the drills' own resolver also stops an unchecked remembered app - // from addressing a link into an app that has since been deactivated. - const target = resolveNotificationTarget(n.action_url, hostAppSegment); - if (!target) { - // No link at all — a real state, not a defect: the producer leaves - // `action_url` undefined when an emit carries neither a `payload.url` nor - // a `source`. `onMarkRead` above has already consumed the row, so - // returning here spent the notification and left the popover sitting open - // on it, having navigated nowhere (objectui#5190). - // - // Home's action centre answers this by opening the full inbox - // (objectui#4074), and that is the ruled behaviour — so the bell reuses - // the drill it already has one screen up rather than writing a second - // answer to the same question. `goToAllNotifications` closes the popover - // and lands on the same `?view=mine` page the row came from, which is - // where a linkless notification can still be read in full. - goToAllNotifications(); - return; - } - setOpen(false); - if (target.kind === 'external') { - window.open(target.url, '_blank', 'noopener,noreferrer'); - return; - } - leaveTo(target.path); - }; + // Opening a row is `openInboxNotification`, the ONE open action — Home's + // "My work → Notifications" row is the other door onto it, so the two + // cannot disagree about what opening a notification does: mark it read + // first, then follow its link. + // + // The link is the materialization's action_url (ADR-0030). The messaging + // pipeline synthesizes an app-relative `/{object}/{id}` link from the + // event's source when a producer didn't set an explicit url — so it names + // a RECORD, and `hostAppSegment` (already resolved above for the two "see + // all" drills) is what turns it into a route. + // + // This used to host it under a raw `currentAppName ?? params.appName` and + // navigate BARE when neither named an app — which is the state on every + // surface outside `/apps/:appName/*` that mounts this bell (`/home`, + // `/organizations`, the full-page AI screen). The bare path matched no + // route, the console catch-all forwarded it to `/`, and the landing + // resolver dropped the user on the default app's home: objectui#5179. + // Reusing the drills' own resolver also stops an unchecked remembered app + // from addressing a link into an app that has since been deactivated. + // + // No link at all is a real state, not a defect: the producer leaves + // `action_url` undefined when an emit carries neither a `payload.url` nor a + // `source`. The row is already spent by then, and stopping there left the + // popover sitting open on it, having navigated nowhere (objectui#5190). + // Home's action centre answers this by opening the full inbox + // (objectui#4074), and that is the ruled behaviour — so the bell reuses the + // drill it already has one screen up rather than writing a second answer to + // the same question. `goToAllNotifications` closes the popover and lands on + // the same `?view=mine` page the row came from, which is where a linkless + // notification can still be read in full. + const handleNotificationClick = (n: InboxNotification) => + openInboxNotification({ id: n.id, actionUrl: n.action_url }, hostAppSegment, { + markRead: onMarkRead, + goTo: leaveTo, + openInbox: goToAllNotifications, + beforeFollow: () => setOpen(false), + }); const toggleGroup = (key: string) => { setExpandedGroups((prev) => { diff --git a/ui/packages/app-shell/src/layout/__tests__/openInboxNotification.test.ts b/ui/packages/app-shell/src/layout/__tests__/openInboxNotification.test.ts new file mode 100644 index 00000000000..594c8c6924a --- /dev/null +++ b/ui/packages/app-shell/src/layout/__tests__/openInboxNotification.test.ts @@ -0,0 +1,57 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `openInboxNotification` — the one open action both notification doors (the + * bell's popover row and Home's My-work row) call. Each arm is asserted as the + * ORDER of what the door was asked to do, because "marked read" and "followed + * the link" are both true of the defect it replaced, only in a different + * sequence or with the first step missing. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { openInboxNotification, type NotificationDoor } from '../openInboxNotification'; + +function recordingDoor(withBeforeFollow = true): { door: NotificationDoor; steps: string[] } { + const steps: string[] = []; + const door: NotificationDoor = { + markRead: (id) => { + steps.push(`markRead ${id}`); + }, + goTo: (path) => steps.push(`goTo ${path}`), + openInbox: () => steps.push('openInbox'), + ...(withBeforeFollow ? { beforeFollow: () => steps.push('beforeFollow') } : {}), + }; + return { door, steps }; +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('openInboxNotification', () => { + it('marks read, then follows an app-relative link hosted under the app', () => { + const { door, steps } = recordingDoor(); + openInboxNotification({ id: 'ibx_1', actionUrl: '/showcase_task/t_1' }, 'crm', door); + expect(steps).toEqual(['markRead ibx_1', 'beforeFollow', 'goTo /apps/crm/showcase_task/t_1']); + }); + + it('marks read, then opens the inbox when there is no link — without beforeFollow', () => { + const { door, steps } = recordingDoor(); + openInboxNotification({ id: 'ibx_1', actionUrl: undefined }, 'crm', door); + expect(steps).toEqual(['markRead ibx_1', 'openInbox']); + }); + + it('marks read, then opens an external link in a new tab instead of routing', () => { + const open = vi.spyOn(window, 'open').mockImplementation(() => null); + const { door, steps } = recordingDoor(); + openInboxNotification({ id: 'ibx_1', actionUrl: 'https://example.com/x' }, 'crm', door); + expect(steps).toEqual(['markRead ibx_1', 'beforeFollow']); + expect(open).toHaveBeenCalledWith('https://example.com/x', '_blank', 'noopener,noreferrer'); + }); + + it('works for a door that has nothing to do before following a link', () => { + const { door, steps } = recordingDoor(false); + openInboxNotification({ id: 'ibx_1', actionUrl: '/apps/acme/invoice/42' }, 'crm', door); + expect(steps).toEqual(['markRead ibx_1', 'goTo /apps/acme/invoice/42']); + }); +}); diff --git a/ui/packages/app-shell/src/layout/openInboxNotification.ts b/ui/packages/app-shell/src/layout/openInboxNotification.ts new file mode 100644 index 00000000000..c7cf00bfeb4 --- /dev/null +++ b/ui/packages/app-shell/src/layout/openInboxNotification.ts @@ -0,0 +1,80 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * openInboxNotification — the one "open a notification" action. + * + * A notification has more than one door: the top-bar bell's popover row and + * the Home page's "My work → Notifications" row open the same message. They + * used to carry two copies of the open logic, and the copies drifted: the bell + * marked the message read before following its link, while the Home row only + * followed the link, so a notification opened from Home stayed unread on the + * server and kept the bell badged. Both doors now call this function, so they + * cannot answer differently about what opening a notification does. + * + * Opening a notification, in order: + * + * 1. Spend it: `markRead(id)`. Always first, and whatever the link turns out + * to be — a linkless notification is still read once the user opened it. + * 2. Resolve its `action_url` with `resolveNotificationTarget` (app-relative + * links are hosted under `hostAppSegment`). + * 3. No link at all ⇒ `openInbox()` — the full inbox page, where a linkless + * notification can still be read in full. + * 4. A link ⇒ `beforeFollow()` (the bell closes its popover), then an + * external URL opens in a new tab and a console route goes to `goTo`. + * + * What differs per door is passed in, never re-decided here: how the door + * leaves the page (the bell asks first while the page holds unsaved edits), + * what it does before following a link, and where its "open the inbox" lands. + * + * @module + */ +import { resolveNotificationTarget } from '../utils/appRoute.js'; + +/** The two fields of a notification row that opening it reads. */ +export interface OpenableNotification { + /** The inbox row id — the id the door's `markRead` is keyed by. */ + id: string; + /** The materialization's `action_url` (ADR-0030 L5); absent ⇒ no link. */ + actionUrl?: string | null; +} + +/** What one door supplies to the shared action. */ +export interface NotificationDoor { + /** Mark the row read — the inbox's own write, never a second copy of it. */ + markRead: (id: string) => void | Promise; + /** Leave for a console route (already hosted under an app). */ + goTo: (path: string) => void; + /** The notification carries no link: open the full inbox instead. */ + openInbox: () => void; + /** Runs once a link is about to be followed (e.g. close a popover). */ + beforeFollow?: () => void; +} + +/** + * Open one notification through `door`: mark it read, then follow its link or, + * without one, open the inbox. See the module docblock for the order. + */ +export function openInboxNotification( + notification: OpenableNotification, + hostAppSegment: string, + door: NotificationDoor, +): void { + void door.markRead(notification.id); + const target = resolveNotificationTarget(notification.actionUrl, hostAppSegment); + if (!target) { + door.openInbox(); + return; + } + door.beforeFollow?.(); + if (target.kind === 'external') { + window.open(target.url, '_blank', 'noopener,noreferrer'); + return; + } + door.goTo(target.path); +} From 2ba5bd3fd7037e96a7e871868f7b173902f3c082 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 14:58:00 +0000 Subject: [PATCH 2/3] test(app-shell): stub window for the node-run open-action test Claude-Session: https://claude.ai/code/session_01Utp6CbDAucNyQ4UtzCJKyh Co-authored-by: Claude --- .../src/layout/__tests__/openInboxNotification.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/packages/app-shell/src/layout/__tests__/openInboxNotification.test.ts b/ui/packages/app-shell/src/layout/__tests__/openInboxNotification.test.ts index 594c8c6924a..74e328915cf 100644 --- a/ui/packages/app-shell/src/layout/__tests__/openInboxNotification.test.ts +++ b/ui/packages/app-shell/src/layout/__tests__/openInboxNotification.test.ts @@ -25,7 +25,7 @@ function recordingDoor(withBeforeFollow = true): { door: NotificationDoor; steps } afterEach(() => { - vi.restoreAllMocks(); + vi.unstubAllGlobals(); }); describe('openInboxNotification', () => { @@ -42,7 +42,9 @@ describe('openInboxNotification', () => { }); it('marks read, then opens an external link in a new tab instead of routing', () => { - const open = vi.spyOn(window, 'open').mockImplementation(() => null); + // This file runs in the node project: no DOM, so `window` is stubbed. + const open = vi.fn(() => null); + vi.stubGlobal('window', { open }); const { door, steps } = recordingDoor(); openInboxNotification({ id: 'ibx_1', actionUrl: 'https://example.com/x' }, 'crm', door); expect(steps).toEqual(['markRead ibx_1', 'beforeFollow']); From 4b00ae8fbda4684bd6b58b4d8cbcf4256205e7f2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 14:59:14 +0000 Subject: [PATCH 3/3] chore(changeset): Home notification open marks it read Claude-Session: https://claude.ai/code/session_01Utp6CbDAucNyQ4UtzCJKyh Co-authored-by: Claude --- .changeset/27447-home-notification-marks-read.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changeset/27447-home-notification-marks-read.md diff --git a/.changeset/27447-home-notification-marks-read.md b/.changeset/27447-home-notification-marks-read.md new file mode 100644 index 00000000000..27a08251d41 --- /dev/null +++ b/.changeset/27447-home-notification-marks-read.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/console': patch +--- + +fix(app-shell): opening a notification from Home's "My work → Notifications" card marks it read, as the bell does + +Clause-②: no +Metadata-protocol: none + +Clicking a notification in the top-bar bell marked it read and then opened its link. Clicking the same notification on the Home page's "My work → Notifications" card only opened the link: no `POST /api/v1/notifications/read` was sent, the server's `unreadCount` did not change, and the bell kept its badge after a reload. + +Both places now use one open action, `openInboxNotification`. It marks the notification read first, then opens its link, or opens the inbox when the notification has no link. The Home card marks read through the bell's own write (`useInboxBell().markRead`), so it sends the same request, keyed by the notification's event id. Once the inbox feed re-reads, the Home card's unread count and the bell's badge drop.