Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/home-action-centre-badges-full-unread-count-4329.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@object-ui/app-shell': patch
---

Home's action centre badges everything that is waiting, not the five rows it has room for

`/home` showed two numbers for one question. The bell badges distinct unread topics plus pending approvals over the shared feed's full 20-row window; the action centre 200px below badged `pendingApprovalsCount + notifications.length` — and `notifications` is the list it renders, which `useHomeInbox` caps at 5. So nine unread messages read as **9** on the bell and **5** on the card, on one page, about one set of rows. The badge was reporting the size of a preview as if it were a total.

Before objectui#4225 the card could not have said anything else: its own read was `$top: 5`, so nine was not a number it had. Both surfaces now cut from one already-joined feed, so the true count is in hand at Home's call site and the cap is a presentation slice over data the card already holds.

`useHomeInbox` grows one additive field, `unreadTopicCount`, and `HomeActionCenter` takes it as a required prop: badge = `pendingApprovalsCount + unreadTopicCount`, list = the same unread set, newest first, still capped at 5. Badge means "how much needs you", list means "the newest few of it" — two semantics, each truthful, one number.

The count is the bell's own fold (`groupNotifications`, by `(topic, title)`) applied to the bell's own rows, deliberately, rather than the pre-slice length of Home's list. That length is title-folded and drops blank titles, so it would agree with the bell on ordinary data and disagree whenever two topics share a title — and "two derivations of one number that agree usually" is exactly the defect objectui#4316 was. One fold, applied twice, cannot drift.

Two adjacent behaviours are unchanged and now pinned as such: the approvals addend (distinct pending request ids from the shared REST feed, degrading to 0 on 404) and the list's own cap of five. "You're all caught up" is now gated on the total rather than on the rows on show, so it can no longer contradict the badge above it.
6 changes: 5 additions & 1 deletion packages/app-shell/src/console/home/HomePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ export function HomePage() {
const { favorites } = useFavorites();
const { user } = useAuth();
const isAdmin = useIsWorkspaceAdmin();
const { pendingApprovalsCount, notifications, notificationsStatus, activities } = useHomeInbox();
const { pendingApprovalsCount, notifications, unreadTopicCount, notificationsStatus, activities } =
useHomeInbox();
// Home renders OUTSIDE the `/apps/:appName/*` router, so there is no
// `params.appName` to read — `currentAppName` (published by ConsoleLayout on
// every app mount) is the only "which app is the user in" signal available
Expand Down Expand Up @@ -462,6 +463,9 @@ export function HomePage() {
<HomeActionCenter
pendingApprovalsCount={pendingApprovalsCount}
notifications={notifications}
/* The badge's total — every unread topic, not the capped list
(#4329); the same number the bell above shows. */
unreadTopicCount={unreadTopicCount}
notificationsStatus={notificationsStatus}
onOpenApprovals={() => navigate(`/apps/${hostAppSegment}/system/approvals`)}
/* The fallback arm runs whenever a notification carries no
Expand Down
29 changes: 28 additions & 1 deletion packages/app-shell/src/console/home/HomeRail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,27 @@ function Row({
export function HomeActionCenter({
pendingApprovalsCount,
notifications,
unreadTopicCount,
notificationsStatus,
onOpenApprovals,
onOpenNotification,
t,
}: {
pendingApprovalsCount: number;
/** The PREVIEW: newest-first, one row per title, capped by `useHomeInbox`. */
notifications: HomeNotification[];
/**
* The TOTAL waiting in the inbox — every unread topic, not just the ones this
* card has room for (#4329).
*
* Required, and separate from `notifications` for the same reason
* `notificationsStatus` is: `notifications.length` was the badge until this
* card learned the difference, which made the badge report the size of a
* capped list. Nine unread showed "9" on the bell and "5" here, on one page,
* about one set of rows. A call site that cannot say how much is waiting must
* not be able to badge its own preview length by saying nothing.
*/
unreadTopicCount: number;
/**
* Required, not optional-with-a-default: a call site that cannot say whether
* its rows are an answer must not be able to reach the affirmative copy by
Expand All @@ -131,7 +145,11 @@ export function HomeActionCenter({
t: TFn;
}) {
const { language } = useObjectTranslation();
const total = pendingApprovalsCount + notifications.length;
// "How much needs you", which is the question the badge asks and the question
// the bell answers with the same number. The list below is a preview of it —
// fewer rows than this whenever the cap or the title fold bites, and that
// gap is the point rather than a defect: badge = total, list = preview.
const total = pendingApprovalsCount + unreadTopicCount;
const answered = notificationsStatus === 'ready';
return (
<Card icon={CheckSquare} accent count={total} title={t('home.actionCenter.title', { defaultValue: 'Needs your attention' })}>
Expand All @@ -155,6 +173,15 @@ export function HomeActionCenter({
)}
</div>
)}
{/*
Gated on the TOTAL, not on the rows on show: "You're all caught up" is a
claim about the inbox, so it may only be made when the inbox is empty —
never merely because this card had nothing renderable to list. (The one
state where the two differ is an unread message with no title at all,
which the list cannot render: the card then shows its badge and no row,
rather than telling the user they are caught up while the bell above
badges the same message.)
*/}
{total === 0 ? (
answered && (
<div className="flex items-center gap-2 py-2 text-sm text-muted-foreground">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,13 @@ import { HomeActionCenter } from '../HomeRail';

/** Exactly `HomePage`'s wiring of the two — the seam the card indicts. */
function HomeActionCenterHost() {
const { pendingApprovalsCount, notifications, notificationsStatus } = useHomeInbox();
const { pendingApprovalsCount, notifications, notificationsStatus, unreadTopicCount } =
useHomeInbox();
return (
<HomeActionCenter
pendingApprovalsCount={pendingApprovalsCount}
notifications={notifications}
unreadTopicCount={unreadTopicCount}
notificationsStatus={notificationsStatus}
onOpenApprovals={() => {}}
onOpenNotification={() => {}}
Expand Down Expand Up @@ -265,16 +267,15 @@ describe('Home action centre — nine unread rows are listed and badged (#4235)'
await waitFor(() =>
expect(screen.getByText('Approval request 1 needs your decision')).toBeInTheDocument(),
);
// Five, not nine — and five is what a real deployment always showed. The
// card's cap used to travel as the read's own `$top: 5`, which THIS fake
// adapter ignores (it answers every query with the full fixture), so the
// case measured nine only because nothing here enforced the server's cut.
// #4225 moved the read to the shared feed, whose `$top` is the bell's 20,
// and the cap became a client-side slice — enforced in the test exactly as
// the server enforced it in production. The rows-are-listed-and-badged
// claim this case exists to make is unchanged.
// Five ROWS and a badge of nine (#4329). The card's cap used to travel as
// the read's own `$top: 5`; #4225 moved the read to the shared feed (whose
// `$top` is the bell's 20) and the cap became a client-side slice, so the
// list is still five. The badge counted that slice until #4329, which is
// how one page came to show 9 on the bell and 5 here for one question —
// the badge is the total waiting now, the list its newest few. The
// rows-are-listed-and-badged claim this case exists to make is unchanged.
expect(screen.getAllByText(/Approval request \d+ needs your decision/)).toHaveLength(5);
expect(badgeText()).toBe('5');
expect(badgeText()).toBe('9');
expect(screen.queryByText(CAUGHT_UP)).not.toBeInTheDocument();
expect(screen.queryByTestId('home-action-unanswered')).not.toBeInTheDocument();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,12 @@ vi.mock('../../../context/NavigationContext', () => ({
vi.mock('../../../hooks/useRecentItems', () => ({ useRecentItems: () => ({ recentItems: [] }) }));
vi.mock('../../../hooks/useFavorites', () => ({ useFavorites: () => ({ favorites: [] }) }));
vi.mock('../../../hooks/useHomeInbox', () => ({
useHomeInbox: () => ({ pendingApprovalsCount: 3, notifications: [], activities: [] }),
useHomeInbox: () => ({
pendingApprovalsCount: 3,
notifications: [],
unreadTopicCount: 0,
activities: [],
}),
}));
vi.mock('../../../hooks/useAiSurface', () => ({ resolveAiApiBase: () => '' }));
vi.mock('../../../views/metadata-admin/useMetadata', () => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ vi.mock('../../../hooks/useHomeInbox', () => ({
useHomeInbox: () => ({
pendingApprovalsCount: 0,
notifications: notificationsFixture,
unreadTopicCount: notificationsFixture.length,
activities: activitiesFixture,
}),
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function renderRail() {
notifications={[
{ id: 'n1', title: '系统文件已分配给你', createdAt: threeDaysAgo() } as any,
]}
unreadTopicCount={1}
notificationsStatus="ready"
onOpenApprovals={() => {}}
onOpenNotification={() => {}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,31 @@
* - drop only the `!m.is_read` filter ⇒ the #4316 block goes red, the
* one-read block stays GREEN — which is why the read-count pin cannot stand
* in for the read-state pin, and both are here.
*
* ## #4329 — one question, one number
*
* Read-state was only half of "the two surfaces agree". The other half is HOW
* MANY: Home badged `pendingApprovalsCount + notifications.length`, and
* `notifications` is the list it renders — which `useHomeInbox` caps at 5. So
* with nine unread the bell said 9 and the card two hundred pixels below said
* 5, for the same question about the same rows. The badge was reporting the
* size of a preview as if it were a total.
*
* The badge now counts unread TOPICS through `groupNotifications` — the bell's
* own fold, over the bell's own rows — while the list stays capped. Badge =
* "how much is waiting", list = "the newest few of it": two semantics, each
* truthful, one number.
*
* Reverse verification (predictions first, measured in PR #4344):
* - restore `total = pendingApprovalsCount + notifications.length` ⇒ the
* #4329 block goes red (Home badges its capped 5 against the bell's 9) and
* the whole #4316 read-state block stays GREEN — the cap and the join are
* different defects and neither pin stands in for the other;
* - count the pre-slice list length instead of the topic fold (title-folded,
* blank titles dropped) ⇒ every case here stays green EXCEPT
* "counts unread TOPICS, not the titles the list happens to show", which is
* the only fixture where the two folds disagree — that case is why the
* count is taken from `groupNotifications` rather than re-derived.
*/
import '@testing-library/jest-dom/vitest';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
Expand Down Expand Up @@ -198,6 +223,29 @@ const READ_TITLES = [
'Scheduled maintenance Sunday 02:00',
];

/**
* Two unread messages sharing a TITLE across two different topics — the one
* fixture where the surfaces' two folds disagree. The bell folds by
* `(topic, title)` and sees two topics; Home's LIST folds by title alone (its
* digest-collapsing rule, deliberately unchanged here) and renders one row.
* The badge is the bell's question, so it answers the bell's fold: 2.
*/
const TWIN_TITLES = [
{ id: 'ibx_t1', notification_id: 'ntf_t1', topic: 'crm.lead.assigned', title: 'Acme Corp needs you', created_at: '2026-08-11T05:00:00Z' },
{ id: 'ibx_t2', notification_id: 'ntf_t2', topic: 'approval.reminder', title: 'Acme Corp needs you', created_at: '2026-08-11T04:00:00Z' },
].map((r) => ({ ...r, user_id: 'u1', action_url: `/apps/showcase/x/record/${r.id}` }));

/** Twelve unread topics — past the bell's "9+" display clamp. */
const TWELVE = Array.from({ length: 12 }, (_, i) => ({
id: `ibx_d${i + 1}`,
user_id: 'u1',
notification_id: `ntf_d${i + 1}`,
topic: `topic.${i + 1}`,
title: `Waiting item ${i + 1}`,
action_url: `/apps/crm/x/record/d_${i + 1}`,
created_at: `2026-08-${String(i + 1).padStart(2, '0')}T09:00:00Z`,
}));

/** `read` for the listed notification ids, `delivered` (= NOT read) for the rest. */
const receiptsFor = (rows: Array<{ notification_id: string }>, readIds: string[]) =>
rows.map((r, i) => ({
Expand Down Expand Up @@ -239,12 +287,14 @@ import { __resetSharedUserFeeds } from '../sharedUserFeeds';
* works, so every assertion has to say WHICH panel it is talking about.
*/
function HomeProbe() {
const { pendingApprovalsCount, notifications, notificationsStatus } = useHomeInbox();
const { pendingApprovalsCount, notifications, notificationsStatus, unreadTopicCount } =
useHomeInbox();
return (
<div data-testid="home-cards">
<HomeActionCenter
pendingApprovalsCount={pendingApprovalsCount}
notifications={notifications}
unreadTopicCount={unreadTopicCount}
notificationsStatus={notificationsStatus}
onOpenApprovals={() => {}}
onOpenNotification={() => {}}
Expand Down Expand Up @@ -300,6 +350,25 @@ afterEach(() => {
vi.unstubAllGlobals();
});

/**
* Approvals answer with `ids` pending requests; every other fetch still 404s.
*
* The count is the number of DISTINCT request ids (`sharedUserFeeds` dedupes by
* `id` before counting), degrading to 0 on 404 — the second addend BOTH badges
* have always had, and the one rule #4329 deliberately leaves alone.
*/
const approvalsPending = (ids: string[]) =>
vi.stubGlobal(
'fetch',
vi.fn((url: unknown) =>
Promise.resolve(
String(url).includes('/api/v1/approvals/requests')
? new Response(JSON.stringify({ data: ids.map((id) => ({ id })) }), { status: 200 })
: new Response('{}', { status: 404 }),
),
),
);

describe('#4316 — an already-read message is not "needs your attention"', () => {
it('nine messages, all read: the bell badges nothing AND Home lists nothing', async () => {
// #4316's headline scenario, verbatim: the user opened the bell and read
Expand Down Expand Up @@ -386,14 +455,75 @@ describe('#4316 — an already-read message is not "needs your attention"', () =

render(<HomeSurfaces />);

await waitFor(() => expect(homeBadge()).toBe('5'));
// Home caps its list at `limit` (5) …
// Nine unread, and both surfaces say nine. Home's LIST is still capped at
// `limit` (5) — the cap is a presentation slice, and #4329 is the pin that
// it may not leak into the badge.
await waitFor(() => expect(homeBadge()).toBe('9'));
expect(within(home()).queryAllByText(/Approval request \d+ needs your decision/)).toHaveLength(5);
// … while the bell, which lists the full window, badges all nine topics.
expect(screen.getByTestId('inbox-bell-badge')).toHaveTextContent('9');
});
});

describe('#4329 — the badge is the total, the list is the preview', () => {
it('counts unread TOPICS, not the titles the list happens to show', async () => {
// The discriminating fixture. Two unread topics that share a title: the
// bell's fold sees two, a title fold sees one. Deriving Home's badge from
// its own (pre-slice) list length would agree with the bell on every other
// case in this file and disagree here — "agrees usually" is precisely the
// shape of the defect #4316 was, so the badge takes the bell's own fold of
// the bell's own rows instead of re-deriving a second one.
inboxRows = TWIN_TITLES;
receiptRows = [];

render(<HomeSurfaces />);

await waitFor(() => expect(screen.getByTestId('inbox-bell-badge')).toHaveTextContent('2'));
expect(homeBadge()).toBe('2');
// The LIST still collapses repeats by title — that is its digest-collapsing
// rule and #4329 does not touch it. One row under a badge of two is the
// badge/preview split doing its job, exactly as five rows under nine is.
expect(inHome('Acme Corp needs you')).toHaveLength(1);
expect(inBell('Acme Corp needs you')).toHaveLength(2);
});

it('adds the same pending-approvals count on both surfaces', async () => {
// Approvals are the badge's other addend and their rule is unchanged: the
// distinct pending request ids from the shared REST feed, added once on
// each surface. Four unread topics + two approvals = six, twice.
inboxRows = MIXED;
receiptRows = receiptsFor(MIXED, MIXED_READ_IDS);
approvalsPending(['ar_1', 'ar_2']);

render(<HomeSurfaces />);

await waitFor(() => expect(screen.getByTestId('inbox-bell-badge')).toHaveTextContent('6'));
expect(homeBadge()).toBe('6');
// …and the approvals row is still listed with its own count, unchanged.
expect(within(home()).getByTestId('home-action-approvals')).toHaveTextContent(
'2 pending approvals',
);
});

it('reports the same count past the bell’s "9+" display clamp', async () => {
// Measured, and pinned as measured: twelve unread topics reach both
// surfaces as twelve. The bell CLAMPS its rendering at "9+" because its
// badge is a 20px circle in the top bar (#2765); Home's card badge has the
// room and prints the number. Same count, two renderings of it — not two
// counts, which is what #4329 was filed about. Left as-is deliberately:
// "9+" and "12" do not contradict each other, and clamping Home would mean
// teaching the shared card badge a display rule it has no other use for.
inboxRows = TWELVE;
receiptRows = [];

render(<HomeSurfaces />);

await waitFor(() => expect(homeBadge()).toBe('12'));
expect(screen.getByTestId('inbox-bell-badge')).toHaveTextContent('9+');
// The list is unmoved by any of it — still five rows.
expect(within(home()).queryAllByText(/Waiting item \d+/)).toHaveLength(5);
});
});

describe('#4225 — one feed, one read, however many consumers mount', () => {
beforeEach(() => {
inboxRows = MIXED;
Expand Down
Loading
Loading