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
35 changes: 35 additions & 0 deletions .changeset/shared-inbox-feed-4225.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
'@object-ui/app-shell': patch
---

Home's action centre stops counting messages the user has already read, and the inbox is read once per page instead of twice (#4316, #4225)

`useHomeInbox` read `sys_inbox_message` and nothing else — it never joined
`sys_notification_receipt`, where ADR-0030 (resolved decision 2) puts read-state.
So Home's "Needs your attention" card could not tell a read message from an
unread one: it listed the five most recent unconditionally and badged them. A
user who opened the bell, read all nine messages and returned to Home still found
up to five of them filed as work waiting on them — while the bell two hundred
pixels above correctly showed zero, because its own poll did join the receipts.
One page load, two panels, opposite claims about the same rows (#4316).

The fix is the one #4225 sketched: `hooks/sharedUserFeeds.ts` gains an inbox feed
holding the bell's already-joined 20-row window, polled once at the bell's 10s
cadence, and BOTH consumers derive from it — the bell lists the window and badges
its unread topics, Home takes the unread ones newest-first and caps them at its
own smaller limit. Home's second query is gone (one `sys_inbox_message` read and
one `sys_notification_receipt` read per page, not two and one), and the two
surfaces can no longer disagree about a row's read-state, because there is no
second read left to drift from the first.

Two supporting changes travel with it, both visible only when something goes
wrong. The shared store now reports per-feed status in the same four words the
rest of the console uses (`idle` / `loading` / `ready` / `error`, per #4300's
one-dialect ruling): it used to swallow every failure into "keep the last value"
and say nothing, which is indistinguishable from a successful re-read, and would
have turned #4235's hard-won `error` state back into stale-but-confident data on
its way through the store. A missing object (404 / `OBJECT_NOT_FOUND`) is still
an answer — the deployment has no inbox, so nothing is waiting — and a denial
still is not. The bell's hidden-tab throttle, its return-to-tab refetch and its
failure backoff moved into the store with the poll rather than being dropped, and
now apply to every shared feed.
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,22 @@ vi.mock('../../../providers/AdapterProvider', () => ({ useAdapter: () => adapter
/**
* The other two feeds are #4197's shared store and are not this card's subject;
* stubbing them keeps the approvals addend a dial this suite can set directly.
*
* Partial, not wholesale (#4225): the inbox read now lives in this same module
* (`useSharedInboxFeed`) and it IS this suite's subject, so it has to stay
* real. Keeping the original module underneath leaves the hop under test
* exactly where it was — `dataSource.find` → rejected promise → status — while
* still holding the approvals addend as a dial.
*/
let approvalsFixture = 0;
vi.mock('../../../hooks/sharedUserFeeds', () => ({
vi.mock('../../../hooks/sharedUserFeeds', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
useSharedPendingApprovalsCount: () => approvalsFixture,
useHumanActivityFeed: () => [],
}));

import { useHomeInbox } from '../../../hooks/useHomeInbox';
import { __resetSharedUserFeeds } from '../../../hooks/sharedUserFeeds';
import { HomeActionCenter } from '../HomeRail';

/** Exactly `HomePage`'s wiring of the two — the seam the card indicts. */
Expand Down Expand Up @@ -163,6 +171,9 @@ beforeEach(() => {
approvalsFixture = 0;
inboxBehaviour = async () => ({ data: [] });
findCalls.length = 0;
// The inbox feed is a module-scoped store that deliberately outlives any one
// render tree, so cases would otherwise inherit each other's rows and status.
__resetSharedUserFeeds();
});

describe('Home action centre — the affirmative empty state needs an ANSWER (#4235)', () => {
Expand Down Expand Up @@ -254,8 +265,16 @@ describe('Home action centre — nine unread rows are listed and badged (#4235)'
await waitFor(() =>
expect(screen.getByText('Approval request 1 needs your decision')).toBeInTheDocument(),
);
expect(screen.getAllByText(/Approval request \d+ needs your decision/)).toHaveLength(9);
expect(badgeText()).toBe('9');
// 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.
expect(screen.getAllByText(/Approval request \d+ needs your decision/)).toHaveLength(5);
expect(badgeText()).toBe('5');
expect(screen.queryByText(CAUGHT_UP)).not.toBeInTheDocument();
expect(screen.queryByTestId('home-action-unanswered')).not.toBeInTheDocument();
});
Expand All @@ -275,7 +294,10 @@ describe('Home action centre — nine unread rows are listed and badged (#4235)'
expect(inboxRead?.query).toMatchObject({
$filter: { user_id: 'u1' },
$orderby: { created_at: 'desc' },
$top: 5,
// 20, not 5: this is the bell's read now, and Home cuts its five from the
// superset (#4225). The `mine` scope and the ordering — what ADR-0030
// actually names, and what this case is pinning — are untouched.
$top: 20,
});
});

Expand Down
Loading
Loading