Skip to content

fix(app-shell): opening a notification from Home's My-work card marks it read, as the bell does - #27653

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-j9w213
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-j9w213

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

domain:ui#4 · os-bill · session_01Utp6CbDAucNyQ4UtzCJKyh

Requested by Bill · project thread

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.

Targeted tests and typecheck
  • 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.
Derived gates (dispatch-gates --commands, reconciled with --ran: 34 derived, 34 run, 0 not measured)

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.


Generated by Claude Code

… 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 <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:ui#4 · os-bill · session_01Utp6CbDAucNyQ4UtzCJKyh

席位核对

ACCEPT · head 86c3d08d94aff6ebbaa24fd2124d21481af8b1c9 · 2026-10-29T16:39Z

  • Shape: draft → main, Fixes #27447 on its own line, no other card number beside a closing keyword. Clause-②: no and Metadata-protocol: none hold: the new openInboxNotification lives in @object-ui/app-shell (private: true) and nothing published changes. The changeset agrees with the body.
  • Change, read:
    • openInboxNotification (layout/openInboxNotification.ts) is now the one open action. It marks the row read, then resolves action_url through resolveNotificationTarget. With no link it opens the inbox. With a link it runs beforeFollow (if set), then opens an external URL in a new tab or goes to the console route.
    • The bell's handleNotificationClick passes onMarkRead, leaveTo (unsaved-edits guard), goToAllNotifications and popover close. Its behaviour is unchanged.
    • Home's onOpenNotification passes useInboxBell().markRead, so Home sends the bell's own POST /api/v1/notifications/read.
  • Mounting useInboxBell on Home, checked: the hook fetches nothing itself, because the shared feeds own polling. Its arrival notifier keeps a module-scoped seen set, so a second mount announces nothing twice, as its docblock states. Home's own rows still come from useHomeInbox, and the unread pill drops when the feed re-reads (pinned by the new test).
  • Surface: within claim 6499718053. The 14 existing HomePage.* harnesses gain a useInboxBell stub (+4 lines each), which counts as tests beside.
  • Accepted deviations (report 6502306473):
    • The local test run was the 158-file affected subset, not the whole app-shell suite. CI's sharded Test UI ran the whole suite green on this head.
    • The signature and os-dev-report lines are in that order.
    • Trailers are model-free.
  • Noted, not filed: useInboxArrivalNotifier's openMessage is a third copy of the open logic. It already marks the row read first, so it is not a defect. Moving it onto the shared action needs hooks/ in a claim.
  • Evidence:
    • The new HomePage.openMarksRead.test.tsx mounts the real HomePage, useHomeInbox, useInboxBell and shared feed. It checks that the POST body is {ids:[ntf_1]}, that the POST comes before navigate, that the pill goes 2 → 1, and the linkless arm.
    • openInboxNotification.test.ts covers 4 arms.
    • Ablation of the Home markRead wiring turns 3/3 red, restored to HEAD.
    • Gates: 34/34 derived, 0 NOT-MEASURED.
  • Gates (check-runs on the head): 44 success, 11 skipped, 0 failure. Lint & Repo Gates, TypeScript Type Check and Check Changeset each success. check-governed-merges --pr 27653: NOT governed, 587 changed lines.
  • Landing: non-governed, all checks green and mergeable_state clean, so the seat lands it with pr_ready + automerge_enable.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

console: opening a notification from the Home "Notifications" card does not mark it read (the bell popover does)

1 participant