Skip to content

Commit e16a7af

Browse files
fix(app-shell): opening a notification from Home's My-work card marks it read, as the bell does (#27653)
domain:ui#4 · os-bill · session_01Utp6CbDAucNyQ4UtzCJKyh <!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgDFH4S5pR1C8YvYCWWsZfXx)_ Fixes #27447 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:首页「我的工作 → 通知」卡片点开一条通知时,会先把它标为已读,再跳转;与顶栏铃铛的行为一致。 - **为什么改**:之前首页卡片只跳转、不标已读,服务端未读数不变,刷新后铃铛角标仍在。 - **风险与回滚**:风险低,只动控制台前端的点击处理,铃铛行为不变;回滚即 revert 本 PR。 - **先看**:`ui/packages/app-shell/src/layout/openInboxNotification.ts`(共享的打开动作);`HomePage.tsx` 的 `onOpenNotification`;`InboxPopover.tsx` 的 `handleNotificationClick`。 - **两分钟验证**:以有一条未读通知的成员登录首页,点「通知」卡片里的那条;预期发出 `POST /api/v1/notifications/read`,回到首页后未读数减一、铃铛角标消失。 - **需要你做的**:无,席位落地。 ## What changes - **One open action for both doors.** New `openInboxNotification` (`ui/packages/app-shell/src/layout/openInboxNotification.ts`): mark the row read, resolve its `action_url` with `resolveNotificationTarget`, then open the inbox (no link), open a new tab (external link) or go to the console route. What differs per door is passed in: how it leaves the page, what it does before following a link, where "open the inbox" lands. - **The bell** (`InboxPopover.handleNotificationClick`) now calls it with `onMarkRead`, `leaveTo` (the unsaved-edits guard), `goToAllNotifications` and "close the popover". Behaviour unchanged. - **Home** (`HomePage` `onOpenNotification`) now calls it with `useInboxBell().markRead`, the bell's own write. Before, it only navigated. The request Home sends is the bell's `POST /api/v1/notifications/read` `{ "ids": [notification_id] }`, keyed by the notification event id. Measured mechanism (premise holds): the bell's mark-read is `useInboxBell().markRead` (AppHeader, `global:notifications`). Home's card rows come from `useHomeInbox`, which cuts the same `useSharedInboxFeed` but had no write. Same feed, different hook. Only the bell's hook had the write. ## Tests - `console/home/__tests__/HomePage.openMarksRead.test.tsx` (new). It renders the real `HomePage` with the real `useHomeInbox`, `useInboxBell` and shared inbox feed, over a fake data adapter and a recording `fetch` double that writes `read` receipts when the mark-read POST arrives. It clicks the Home card row and asserts: - the exact request (`POST /api/v1/notifications/read`, body `{"ids":["ntf_1"]}`), - that the POST comes before the navigation, - the card's unread pill going 2 → 1 once the feed re-reads, - the linkless arm (mark read, then the inbox). - `layout/__tests__/openInboxNotification.test.ts` (new): the four arms of the action, as an ordered step log. - The 14 existing `HomePage.*` harnesses stub `useInboxBell`, because they serve no inbox feed. The bell's own pins (`InboxPopover.notificationDeepLink`, `InboxPopover.linklessFallback`, `InboxPopover.unsavedEditsLeave-13253`) pass unchanged. - Ablation: replacing Home's `markRead: markNotificationRead,` with a no-op turns all 3 cases of the new Home test red. The request list is `[]` and the pill stays `2`. Restored from `HEAD` and verified by blob equality. ## Acceptance notes - **Optimistic flip is per surface, by design.** After a Home click, the header bell's badge drops on the shared feed's next read: the 10 s poll, or at once when the tab becomes visible again. The bell's own clicks work the same way toward Home. `useInboxBell` keeps its optimistic overlay per surface on purpose ("one surface's optimistic flip must not become another's fact before the server agrees"). - **A second bell wiring is mounted on `/home`.** `HomePage` now mounts `useInboxBell` next to the header's. Its own docblock says two mounts are safe: the shared feed is read once, and the arrival announcer's seen set is module-scoped, so nothing is announced twice. - **A third door exists outside this card's file surface.** The arrival toast and system notification (`useInboxArrivalNotifier`'s `openMessage`) carry their own copy of the same open logic. That copy already marks read first, so it is not a defect. Moving it onto `openInboxNotification` would be a one-line follow-up. Noted, not changed. ## Verification All readings below are on head `86c3d08d`, after `origin/main` was merged in. <details><summary>Targeted tests and typecheck</summary> - `pnpm --dir ui exec vitest run --maxWorkers=2 packages/app-shell/src/console/home/ packages/app-shell/src/layout/ packages/app-shell/src/hooks/__tests__/sharedInboxFeed packages/app-shell/src/hooks/__tests__/useHomeInbox packages/app-shell/src/views/__tests__/global-notifications`: Test Files 158 passed (158), Tests 1018 passed (1018). - `pnpm --filter @object-ui/app-shell type-check` (src and `tsconfig.test.json`): exit 0. - Ablation of Home's `markRead` (via `scripts/ablation-replace.mjs`): `HomePage.openMarksRead.test.tsx` failed 3 of 3, then was restored to `HEAD` (blob equal, `git diff HEAD` empty). - The whole `@object-ui/app-shell` suite (1883 files) was started locally but stopped after 61 min. It was holding the shared verification lock. CI's sharded `Test UI` runs it in full. </details> <details><summary>Derived gates (dispatch-gates --commands, reconciled with --ran: 34 derived, 34 run, 0 not measured)</summary> Every derived family exited 0. That includes `node scripts/check-sdui-manifest.mjs` and its `--self-test`, the changeset gates, `pnpm check:type-check-coverage`, `pnpm check:type-check-debt` (re-measured under the lock) and `pnpm --dir ui exec vitest run scripts/` (169 files passed, 2 skipped). `pnpm --dir ui check:new-line-citations`: 0 new citations. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CbDAucNyQ4UtzCJKyh)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e5c5035 commit e16a7af

20 files changed

Lines changed: 528 additions & 59 deletions
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/console': patch
4+
---
5+
6+
fix(app-shell): opening a notification from Home's "My work → Notifications" card marks it read, as the bell does
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
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.
12+
13+
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.

‎ui/packages/app-shell/src/console/home/HomePage.tsx‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ import {
4646
matchAppBySegment,
4747
resolveHostAppSegment,
4848
resolveKeyedI18nLabel,
49-
resolveNotificationTarget,
5049
} from '../../utils/index.js';
50+
import { useInboxBell } from '../../hooks/useInboxBell.js';
51+
import { openInboxNotification } from '../../layout/openInboxNotification.js';
5152
import { resolveSetupAppPath } from '../ConsoleShell.js';
5253
import {
5354
Empty,
@@ -369,6 +370,11 @@ export function HomePage() {
369370
notificationsReadable,
370371
activity,
371372
} = useHomeInbox();
373+
// Opening a notification from the My-work card marks it read through the
374+
// bell's own write — `useInboxBell` is the one wiring of mark-read onto the
375+
// shared inbox feed (the header bell and the `global:notifications` block
376+
// mount it too), so the request Home sends is the request the bell sends.
377+
const { markRead: markNotificationRead } = useInboxBell();
372378
// The drafts row of the My-work section — `null` with nothing to publish.
373379
const draftsRow = usePendingDraftsRow(t, language);
374380
// Home renders OUTSIDE the `/apps/:appName/*` router, so there is no
@@ -833,23 +839,20 @@ export function HomePage() {
833839
so it has to be hosted under an app before it is a route —
834840
navigating it verbatim is objectui#5179, where the console
835841
catch-all forwarded the unmatched path to `/` and the landing
836-
resolver dropped the user on the default app's home with the
837-
notification already marked read. `resolveNotificationTarget`
838-
is shared with the top-bar bell so the two cannot answer this
839-
differently. The fallback arm runs whenever a notification
840-
carries no link at all, and opens the inbox. */
841-
onOpenNotification={(n) => {
842-
const target = resolveNotificationTarget(n.actionUrl, hostAppSegment);
843-
if (!target) {
844-
navigate(`/apps/${hostAppSegment}/sys_inbox_message?view=mine`);
845-
return;
846-
}
847-
if (target.kind === 'external') {
848-
window.open(target.url, '_blank', 'noopener,noreferrer');
849-
return;
850-
}
851-
navigate(target.path);
852-
}}
842+
resolver dropped the user on the default app's home.
843+
`openInboxNotification` is the top-bar bell's own open action,
844+
so the two doors cannot answer differently: it marks the
845+
notification read first (this row used to skip that, and a
846+
notification opened from Home stayed unread on the server and
847+
badged on the bell), then follows the resolved link — or, when
848+
the notification carries no link at all, opens the inbox. */
849+
onOpenNotification={(n) =>
850+
openInboxNotification(n, hostAppSegment, {
851+
markRead: markNotificationRead,
852+
goTo: (path) => navigate(path),
853+
openInbox: () => navigate(`/apps/${hostAppSegment}/sys_inbox_message?view=mine`),
854+
})
855+
}
853856
t={t}
854857
/>
855858

‎ui/packages/app-shell/src/console/home/__tests__/HomePage.aiStudioDisabled.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({
128128
}));
129129

130130
import { initRuntimeConfig, resetRuntimeConfigForTesting } from '../../../runtime-config';
131+
// Home marks an opened notification read through the bell's own write
132+
// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed.
133+
vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) }));
134+
131135
import { HomePage } from '../HomePage';
132136

133137
/**

‎ui/packages/app-shell/src/console/home/__tests__/HomePage.approvalsTarget.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ vi.mock('../../../runtime-config', () => ({
131131
isAiStudioEnabled: () => true,
132132
}));
133133

134+
// Home marks an opened notification read through the bell's own write
135+
// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed.
136+
vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) }));
137+
134138
import { HomePage } from '../HomePage';
135139

136140
/* ── The `_drafts` double (objectui#7307) ─────────────────────────────────────

‎ui/packages/app-shell/src/console/home/__tests__/HomePage.authoringCapabilityGate.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ vi.mock('../../../runtime-config', () => ({
140140
isAiStudioEnabled: () => true,
141141
}));
142142

143+
// Home marks an opened notification read through the bell's own write
144+
// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed.
145+
vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) }));
146+
143147
import { HomePage } from '../HomePage';
144148

145149
/* ── The `_drafts` double (objectui#7307) ─────────────────────────────────────

‎ui/packages/app-shell/src/console/home/__tests__/HomePage.draftsReadGate-25894.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,10 @@ vi.mock('../../../runtime-config', () => ({
8686
isAiStudioEnabled: () => true,
8787
}));
8888

89+
// Home marks an opened notification read through the bell's own write
90+
// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed.
91+
vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) }));
92+
8993
import { HomePage } from '../HomePage';
9094

9195
const DRAFTS_ROUTE = '/api/v1/meta/_drafts';

‎ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingAndApps-12601.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({
147147
usePublishAllDrafts: () => ({ publishAll: async () => ({ ok: true }), publishing: false }),
148148
}));
149149

150+
// Home marks an opened notification read through the bell's own write
151+
// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed.
152+
vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) }));
153+
150154
import { HomePage } from '../HomePage';
151155

152156
const CRM = { name: 'crm', label: 'CRM', icon: 'Building2' };

‎ui/packages/app-shell/src/console/home/__tests__/HomePage.greetingLocale-11689.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ vi.mock('../../../runtime-config', () => ({
100100
isAiStudioEnabled: () => true,
101101
}));
102102

103+
// Home marks an opened notification read through the bell's own write
104+
// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed.
105+
vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) }));
106+
103107
import { HomePage } from '../HomePage';
104108

105109
/* ── The `_drafts` double ─────────────────────────────────────────────────────

‎ui/packages/app-shell/src/console/home/__tests__/HomePage.inboxLinksTarget.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ vi.mock('../../../runtime-config', () => ({
134134
isAiStudioEnabled: () => true,
135135
}));
136136

137+
// Home marks an opened notification read through the bell's own write
138+
// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed.
139+
vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) }));
140+
137141
import { HomePage } from '../HomePage';
138142

139143
/* ── The `_drafts` double (objectui#7307) ─────────────────────────────────────

‎ui/packages/app-shell/src/console/home/__tests__/HomePage.launcherHiddenApps-26838.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ vi.mock('../../../preview/usePublishAllDrafts', () => ({
133133
usePublishAllDrafts: () => ({ publishAll: async () => ({ ok: true }), publishing: false }),
134134
}));
135135

136+
// Home marks an opened notification read through the bell's own write
137+
// (`useInboxBell`); this harness serves no inbox feed, so the write is stubbed.
138+
vi.mock('../../../hooks/useInboxBell', () => ({ useInboxBell: () => ({ markRead: vi.fn() }) }));
139+
136140
import { HomePage } from '../HomePage';
137141
import { LauncherAppsProvider } from '../../../hooks/useLauncherApps';
138142

0 commit comments

Comments
 (0)