diff --git a/.changeset/home-action-centre-needs-an-answer-4235.md b/.changeset/home-action-centre-needs-an-answer-4235.md new file mode 100644 index 000000000..350c860f1 --- /dev/null +++ b/.changeset/home-action-centre-needs-an-answer-4235.md @@ -0,0 +1,26 @@ +--- +'@object-ui/app-shell': patch +--- + +Home's action centre no longer says "You're all caught up" to a user whose inbox it failed to read (#4235) + +`useHomeInbox` caught every failed `sys_inbox_message` read to `[]`, so a denial +arrived at `HomeActionCenter` wearing the exact shape of an empty inbox and the +panel reported a quiet day, with no badge, to a user with nine unread messages. +That is the reported symptom, and objectstack#7344 measured its mechanism in a +browser: `403 PERMISSION_DENIED` on that object for every non-admin session, +while `/api/v1/notifications` — a projection of the very same rows — answered +with their messages. It also resolves the cross-run contradiction the card +carried: two QA runs on one console pin disagreed because one was an admin and +one was not. + +The hook now reports `notificationsStatus` (`idle` / `loading` / `ready` / +`error` — `MetadataProvider`'s vocabulary, per #4300's one-dialect ruling), and +the affirmative copy renders only on `ready`. An unanswered read gets a quiet, +non-affirmative notice instead, rendered alongside the approvals row when only +that half answered. A deployment with no inbox object at all is still an answer +and still reads as caught up, unchanged. + +The source is unchanged and deliberately so: ADR-0030 names `sys_inbox_message` +as the console's consumer channel, and `/api/v1/notifications` projects the same +query one hop later. diff --git a/packages/app-shell/src/console/home/HomePage.tsx b/packages/app-shell/src/console/home/HomePage.tsx index 6cd76beb3..49257f869 100644 --- a/packages/app-shell/src/console/home/HomePage.tsx +++ b/packages/app-shell/src/console/home/HomePage.tsx @@ -253,7 +253,7 @@ export function HomePage() { const { favorites } = useFavorites(); const { user } = useAuth(); const isAdmin = useIsWorkspaceAdmin(); - const { pendingApprovalsCount, notifications, activities } = useHomeInbox(); + const { pendingApprovalsCount, notifications, 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 @@ -462,6 +462,7 @@ export function HomePage() { navigate(`/apps/${hostAppSegment}/system/approvals`)} /* The fallback arm runs whenever a notification carries no `action_url`; `?view=mine` selects the user-scoped view, so the diff --git a/packages/app-shell/src/console/home/HomeRail.tsx b/packages/app-shell/src/console/home/HomeRail.tsx index 79d93eea2..5b4c37967 100644 --- a/packages/app-shell/src/console/home/HomeRail.tsx +++ b/packages/app-shell/src/console/home/HomeRail.tsx @@ -14,11 +14,11 @@ */ import { CheckSquare, Activity, ArrowRight, CheckCheck, Bell, Clock, - FileText, Database, LayoutDashboard, File, + FileText, Database, LayoutDashboard, File, CircleAlert, } from 'lucide-react'; import { useObjectTranslation } from '@object-ui/i18n'; import type { ActivityItem } from '../../layout/ActivityFeed'; -import type { HomeNotification } from '../../hooks/useHomeInbox'; +import type { HomeInboxStatus, HomeNotification } from '../../hooks/useHomeInbox'; import type { RecentItem } from '../../hooks/useRecentItems'; import { timeAgo } from '../../utils/relativeTime'; @@ -94,28 +94,74 @@ function Row({ ); } +/** + * "You're all caught up" is an ASSERTION about the user's inbox, so it may only + * be made once the inbox has answered (#4235). + * + * `notifications` arriving empty says nothing on its own: until #4235 the hook + * behind it swallowed every failed read to `[]`, so a `403 PERMISSION_DENIED` + * on `sys_inbox_message` (objectstack#7344, browser-measured for every non-admin + * persona) reached this component wearing the exact shape of an empty inbox — + * and the panel cheerfully told a user with nine unread messages that there was + * nothing to do, with no badge. `notificationsStatus` is the missing bit, and + * gating on it is why that pair is now unreachable: an unanswered read renders + * the quiet notice below, never the affirmative copy. + * + * Same rule #4300 landed for the app list ("an unloadable app list is UNKNOWN, + * not 'no default app'"), and the same status vocabulary. + */ export function HomeActionCenter({ pendingApprovalsCount, notifications, + notificationsStatus, onOpenApprovals, onOpenNotification, t, }: { pendingApprovalsCount: number; notifications: HomeNotification[]; + /** + * 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 + * saying nothing. + */ + notificationsStatus: HomeInboxStatus; onOpenApprovals: () => void; onOpenNotification: (n: HomeNotification) => void; t: TFn; }) { const { language } = useObjectTranslation(); const total = pendingApprovalsCount + notifications.length; + const answered = notificationsStatus === 'ready'; return ( - {total === 0 ? ( -
- - {t('home.actionCenter.empty', { defaultValue: "You're all caught up" })} + {/* + Rendered ALONGSIDE the list, not only instead of it: when approvals are + known and the inbox read failed, the panel is showing half an answer, + and saying so is the same honesty the empty case owes. + */} + {!answered && ( +
+ {notificationsStatus === 'error' ? ( + <> + + {t('errors.unknown', { defaultValue: 'An unexpected error occurred.' })} + + ) : ( + t('common.loading', { defaultValue: 'Loading...' }) + )}
+ )} + {total === 0 ? ( + answered && ( +
+ + {t('home.actionCenter.empty', { defaultValue: "You're all caught up" })} +
+ ) ) : (
    {pendingApprovalsCount > 0 && ( diff --git a/packages/app-shell/src/console/home/__tests__/HomeActionCenter.unansweredInbox.test.tsx b/packages/app-shell/src/console/home/__tests__/HomeActionCenter.unansweredInbox.test.tsx new file mode 100644 index 000000000..ef5f8abae --- /dev/null +++ b/packages/app-shell/src/console/home/__tests__/HomeActionCenter.unansweredInbox.test.tsx @@ -0,0 +1,293 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #4235 — Home's action centre said "You're all caught up" to a user with nine + * unread messages, and showed no badge. + * + * ## What the card reported, and what it actually was + * + * The card read the symptom as a WRONG SOURCE: `useHomeInbox` polls + * `sys_inbox_message` rather than `/api/v1/notifications`. Measured, the source + * is right and sanctioned. ADR-0030 names the table read as the consumer + * channel in three places — the L5 row of its object model ("**the bell reads + * `sys_inbox_message`**"), the P0 phase ("UI bell reads `sys_inbox_message`"), + * and the cross-repo cut-over ("Repoint the Console bell … to + * **`sys_inbox_message`** (the `mine` view), joining `sys_notification_receipt` + * for read-state") — and `/api/v1/notifications` is a PROJECTION of those very + * rows: `MessagingService.listInbox` reads `sys_inbox_message` where + * `user_id`, ordered `created_at desc`, joined with the same receipts. Reading + * the table is reading the API's own source, one hop earlier. + * + * ## The condition that decided it — and it is not staleness + * + * Two QA runs on the SAME console pin `09987b68` recorded opposite things: + * objectstack#7514 saw this panel empty with 9 unread, objectstack#7517 used it + * as the WORKING control against the dead bell. Both are true, of different + * users. objectstack#7344 measured the mechanism in a browser on 2026-08-10: + * + * [Security] Access denied: operation 'find' on object 'sys_inbox_message' + * is not permitted for positions [org_member, contributor, finance, everyone] + * + * — every inbox read `403 PERMISSION_DENIED` for a plain member, because no + * shipped permission set granted the object, while `/api/v1/notifications` + * (a dedicated authenticated route, not the generic data API) answered with + * their unread rows. An admin session read the table and the card worked. So + * the flip is the SIGNED-IN USER'S PERMISSION SET, not the console build. + * + * The 403 itself is closed server-side by objectstack#7586 (owner-scoped member + * grants, merged 2026-08-11). What is NOT closed, and is what these cases pin, + * is that the console turned that denial into good news: `useHomeInbox` caught + * every failure to `[]`, and `HomeActionCenter` renders the affirmative empty + * copy on an empty array. Any future denial, outage or malformed answer + * reproduces the identical silent lie — the grant fixed one cause of a symptom + * that had no cause-independent guard. + * + * ## The oracle + * + * Real `useHomeInbox` + real `HomeActionCenter`, wired as `HomePage` wires them, + * against a fake adapter. Nothing about the REST layer is pinned here — the + * adapter boundary is where a denial becomes a rejected promise, and that is the + * hop under test. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + language: 'en', + t: (key: string, options?: Record) => + String(options?.defaultValue ?? key), + }), +})); + +/** `null` is the pre-auth / still-settling session, not a user with no mail. */ +let userFixture: { id: string } | null = { id: 'u1' }; +vi.mock('@object-ui/auth', () => ({ useAuth: () => ({ user: userFixture }) })); + +/** What `find('sys_inbox_message')` does for the current case. */ +let inboxBehaviour: () => Promise = async () => ({ data: [] }); +const findCalls: Array<{ object: string; query: unknown }> = []; +const fakeAdapter = { + find: (object: string, query: unknown) => { + findCalls.push({ object, query }); + if (object === 'sys_inbox_message') return inboxBehaviour(); + return Promise.resolve({ data: [] }); + }, +}; +/** `null` is "no adapter yet" — the other half of the not-asked-yet state. */ +let adapterFixture: unknown = fakeAdapter; +vi.mock('../../../providers/AdapterProvider', () => ({ useAdapter: () => adapterFixture })); + +/** + * 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. + */ +let approvalsFixture = 0; +vi.mock('../../../hooks/sharedUserFeeds', () => ({ + useSharedPendingApprovalsCount: () => approvalsFixture, + useHumanActivityFeed: () => [], +})); + +import { useHomeInbox } from '../../../hooks/useHomeInbox'; +import { HomeActionCenter } from '../HomeRail'; + +/** Exactly `HomePage`'s wiring of the two — the seam the card indicts. */ +function HomeActionCenterHost() { + const { pendingApprovalsCount, notifications, notificationsStatus } = useHomeInbox(); + return ( + {}} + onOpenNotification={() => {}} + t={(key: string, options?: any) => String(options?.defaultValue ?? key)} + /> + ); +} + +/** The QA shape: nine unread rows for this user, as the L5 channel writes them. */ +const NINE_UNREAD = Array.from({ length: 9 }, (_, i) => ({ + id: `ibx_${i + 1}`, + user_id: 'u1', + notification_id: `ntf_${i + 1}`, + topic: 'approval.reminder', + title: `Approval request ${i + 1} needs your decision`, + action_url: `/apps/crm/sys_approval_request/record/a_${i + 1}`, + created_at: `2026-08-0${i + 1}T09:00:00Z`, +})); + +const CAUGHT_UP = "You're all caught up"; +const ERROR_COPY = 'An unexpected error occurred.'; +const LOADING_COPY = 'Loading...'; + +/** + * The objectstack#7344 rejection, verbatim in shape: the generic data API + * refusing `find` on the object. NOT a 404 — the object exists, this caller may + * not read it, and that difference is the whole point of the pair below. + */ +function permissionDenied(): Promise { + const err = new Error( + "Access denied: operation 'find' on object 'sys_inbox_message' is not permitted", + ) as Error & { httpStatus?: number; code?: string }; + err.httpStatus = 403; + err.code = 'PERMISSION_DENIED'; + return Promise.reject(err); +} + +/** The deployment that never installed the messaging pipeline. */ +function objectMissing(): Promise { + const err = new Error('Object not found: sys_inbox_message') as Error & { + httpStatus?: number; + code?: string; + }; + err.httpStatus = 404; + err.code = 'OBJECT_NOT_FOUND'; + return Promise.reject(err); +} + +/** The card's badge: rendered by `Card` only when the count is > 0. */ +function badgeText(): string | null { + const heading = screen.getByText('Needs your attention'); + const badge = heading.parentElement?.querySelector('span.tabular-nums'); + return badge ? badge.textContent : null; +} + +beforeEach(() => { + userFixture = { id: 'u1' }; + adapterFixture = fakeAdapter; + approvalsFixture = 0; + inboxBehaviour = async () => ({ data: [] }); + findCalls.length = 0; +}); + +describe('Home action centre — the affirmative empty state needs an ANSWER (#4235)', () => { + it('does not claim "all caught up" when the inbox read was DENIED', async () => { + // The headline case. Nine unread rows exist; this session may not read + // them. Before #4235 the denial landed as `[]` and the panel said the user + // was done for the day — the exact pair objectstack#7514 screenshotted. + inboxBehaviour = permissionDenied; + + render(); + + await waitFor(() => expect(screen.getByTestId('home-action-unanswered')).toBeInTheDocument()); + expect(screen.queryByText(CAUGHT_UP)).not.toBeInTheDocument(); + expect(screen.getByText(ERROR_COPY)).toBeInTheDocument(); + }); + + it('does not claim it while the session is still settling (no user yet)', async () => { + // The unauthenticated / pre-auth arm of the same rule: the effect returns + // before asking anything, so `[]` here has never been an inbox answer. + userFixture = null; + inboxBehaviour = async () => ({ data: NINE_UNREAD }); + + render(); + + expect(screen.queryByText(CAUGHT_UP)).not.toBeInTheDocument(); + expect(screen.getByText(LOADING_COPY)).toBeInTheDocument(); + expect(findCalls.filter((c) => c.object === 'sys_inbox_message')).toHaveLength(0); + }); + + it('does not claim it while no adapter has been provided yet', async () => { + adapterFixture = null; + + render(); + + expect(screen.queryByText(CAUGHT_UP)).not.toBeInTheDocument(); + expect(screen.getByTestId('home-action-unanswered')).toBeInTheDocument(); + }); + + it('says so even when the approvals half DID answer', async () => { + // Half an answer is still not an empty inbox. The list renders the known + // approvals row AND the notice — the panel never silently drops the half it + // failed to read. + approvalsFixture = 2; + inboxBehaviour = permissionDenied; + + render(); + + await waitFor(() => expect(screen.getByText(ERROR_COPY)).toBeInTheDocument()); + expect(screen.getByTestId('home-action-approvals')).toBeInTheDocument(); + expect(screen.queryByText(CAUGHT_UP)).not.toBeInTheDocument(); + }); + + it('CONTROL: a successful, genuinely empty inbox still says "all caught up"', async () => { + // The copy is not being retired — it is being earned. A quiet workspace + // must still look intentional rather than broken. + inboxBehaviour = async () => ({ data: [] }); + + render(); + + await waitFor(() => expect(screen.getByText(CAUGHT_UP)).toBeInTheDocument()); + expect(screen.queryByTestId('home-action-unanswered')).not.toBeInTheDocument(); + expect(badgeText()).toBeNull(); + }); + + it('CONTROL: a deployment with no inbox object at all is an ANSWER, not an error', async () => { + // The other polarity of the same predicate, and the reason it is not "any + // rejection is an error": a community build without service-messaging has + // no inbox, so nothing is waiting — the hook's documented degradation, and + // the same `isMissingResource` split `sharedUserFeeds` applies to its feeds. + // Get this wrong and every such deployment reads an error on Home forever. + inboxBehaviour = objectMissing; + + render(); + + await waitFor(() => expect(screen.getByText(CAUGHT_UP)).toBeInTheDocument()); + expect(screen.queryByText(ERROR_COPY)).not.toBeInTheDocument(); + }); +}); + +describe('Home action centre — nine unread rows are listed and badged (#4235)', () => { + it('lists the QA payload and badges it, instead of an empty panel', async () => { + // The other half of the reported pair: no badge. With the read answering — + // which is what objectstack#7586 restored for members — the rows are on + // show and the badge carries their count. + inboxBehaviour = async () => ({ data: NINE_UNREAD }); + + render(); + + 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'); + expect(screen.queryByText(CAUGHT_UP)).not.toBeInTheDocument(); + expect(screen.queryByTestId('home-action-unanswered')).not.toBeInTheDocument(); + }); + + it('reads the ADR-0030 `mine` window — user-scoped, newest first', async () => { + // Pins the source the card questioned, so a later "route it through the + // notifications API" change has to argue with ADR-0030 rather than slip past + // it. `/api/v1/notifications` projects exactly this query one hop later. + inboxBehaviour = async () => ({ data: NINE_UNREAD }); + + render(); + + await waitFor(() => + expect(screen.getByText('Approval request 1 needs your decision')).toBeInTheDocument(), + ); + const inboxRead = findCalls.find((c) => c.object === 'sys_inbox_message'); + expect(inboxRead?.query).toMatchObject({ + $filter: { user_id: 'u1' }, + $orderby: { created_at: 'desc' }, + $top: 5, + }); + }); + + it('badges the approvals-only case without the inbox contributing (#4197 control)', async () => { + approvalsFixture = 3; + inboxBehaviour = async () => ({ data: [] }); + + render(); + + await waitFor(() => expect(screen.getByTestId('home-action-approvals')).toBeInTheDocument()); + expect(badgeText()).toBe('3'); + expect(screen.queryByText(CAUGHT_UP)).not.toBeInTheDocument(); + expect(screen.queryByTestId('home-action-unanswered')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/app-shell/src/console/home/__tests__/HomeRail.i18n.test.tsx b/packages/app-shell/src/console/home/__tests__/HomeRail.i18n.test.tsx index 8350d19a4..c8ef06e01 100644 --- a/packages/app-shell/src/console/home/__tests__/HomeRail.i18n.test.tsx +++ b/packages/app-shell/src/console/home/__tests__/HomeRail.i18n.test.tsx @@ -33,6 +33,7 @@ function renderRail() { notifications={[ { id: 'n1', title: '系统文件已分配给你', createdAt: threeDaysAgo() } as any, ]} + notificationsStatus="ready" onOpenApprovals={() => {}} onOpenNotification={() => {}} t={zh} diff --git a/packages/app-shell/src/hooks/useHomeInbox.ts b/packages/app-shell/src/hooks/useHomeInbox.ts index f96b8046f..f493c7863 100644 --- a/packages/app-shell/src/hooks/useHomeInbox.ts +++ b/packages/app-shell/src/hooks/useHomeInbox.ts @@ -6,9 +6,27 @@ * - notifications — latest in-app inbox messages (assignments/@mentions) * - activities — recent human activity feed (sys_activity) * - * Everything degrades silently to empty on 404 / error so deployments without - * the approvals plugin, the inbox pipeline, or a `sys_activity` object still - * render Home. + * A deployment without the approvals plugin, the inbox pipeline or a + * `sys_activity` object still renders Home: a MISSING object (404 / + * `OBJECT_NOT_FOUND`) is an answer — this deployment has no inbox, so nothing is + * waiting — and degrades to empty, exactly as `sharedUserFeeds.markUnavailable` + * treats its own feeds. + * + * Every OTHER failure is NOT an answer, and `notificationsStatus` is what says + * so (#4235). This hook used to swallow all of them to `[]`, which handed the + * Home action centre the empty array and nothing else — so a denial arrived + * wearing the shape of good news and the panel said "You're all caught up". + * Measured, that is not hypothetical: objectstack#7344 recorded every non-admin + * session taking `403 PERMISSION_DENIED` on this exact read + * (`operation 'find' on object 'sys_inbox_message' is not permitted for + * positions [org_member, contributor, finance, everyone]`) while + * `/api/v1/notifications` — a projection of the very same rows — answered with + * their unread messages. Admin sessions read the rows and saw the card work. + * One build, one console pin, opposite screenshots, decided by who was signed in. + * + * The vocabulary is `MetadataProvider`'s per-type status, deliberately — see + * #4300, which fixed the same class ("an unloadable app list is UNKNOWN, not + * 'no default app'") and ruled one source of truth, no second dialect. * * Approvals and activity are NOT fetched here (#4197). Both come from * `sharedUserFeeds`, which the top-bar bell reads too — on `/home` the bell and @@ -23,6 +41,7 @@ import { useEffect, useRef, useState } from 'react'; import { useAdapter } from '../providers/AdapterProvider'; import { useAuth } from '@object-ui/auth'; +import { errorCodeIs } from '@object-ui/types'; import { useHumanActivityFeed, useSharedPendingApprovalsCount } from './sharedUserFeeds'; import type { ActivityItem } from '../layout/ActivityFeed'; @@ -33,16 +52,44 @@ export interface HomeNotification { createdAt?: string; } +/** + * Whether `notifications` is an ANSWER about the user's inbox. + * + * - `idle` — not asked yet (no adapter, or no signed-in user). + * - `loading` — asked, still in flight. + * - `ready` — the read answered. `notifications` is that answer, and only + * here does an empty array mean the inbox is genuinely empty. + * - `error` — the read failed (denied, unreachable, malformed). The empty + * array is the absence of an answer, not an empty inbox. + * + * Same four words as `MetadataTypeStatus` (`providers/MetadataProvider`), on + * purpose: #4300 ruled one status dialect for this exact question. + */ +export type HomeInboxStatus = 'idle' | 'loading' | 'ready' | 'error'; + export interface HomeInboxData { pendingApprovalsCount: number; notifications: HomeNotification[]; + /** Whether `notifications` is an answer — see {@link HomeInboxStatus}. */ + notificationsStatus: HomeInboxStatus; activities: ActivityItem[]; } +/** + * A missing OBJECT, as opposed to a failed read of a present one. The + * ObjectStack client throws `httpStatus` (not `status`) with an error code — + * same predicate `sharedUserFeeds` and `AppHeader` apply to their own reads. + */ +function isMissingResource(err: unknown): boolean { + const e = err as { httpStatus?: number; status?: number } | null; + return e?.httpStatus === 404 || e?.status === 404 || errorCodeIs(err, 'OBJECT_NOT_FOUND'); +} + export function useHomeInbox(limit = 5): HomeInboxData { const dataSource = useAdapter(); const { user } = useAuth(); const [notifications, setNotifications] = useState([]); + const [notificationsStatus, setNotificationsStatus] = useState('idle'); const mountedRef = useRef(true); // Shared with the top-bar bell — one read each, not one per consumer (#4197). @@ -58,8 +105,14 @@ export function useHomeInbox(limit = 5): HomeInboxData { // Latest in-app inbox messages (assignments / @mentions / alerts). useEffect(() => { - if (!dataSource || !user?.id) return; + // Nothing asked yet — NOT an empty inbox. A console still settling its + // adapter or its session must not be reported as "all caught up". + if (!dataSource || !user?.id) { + setNotificationsStatus('idle'); + return; + } let cancelled = false; + setNotificationsStatus('loading'); Promise.resolve( dataSource.find('sys_inbox_message', { $filter: { user_id: user.id }, @@ -83,10 +136,17 @@ export function useHomeInbox(limit = 5): HomeInboxData { // — keep the most recent of each title (rows are newest-first). .filter((n) => (seenTitles.has(n.title) ? false : (seenTitles.add(n.title), true))); setNotifications(deduped); + setNotificationsStatus('ready'); }) - .catch(() => { /* inbox pipeline absent → empty */ }); + .catch((err: unknown) => { + if (cancelled || !mountedRef.current) return; + // A missing object is an answer: this deployment has no inbox pipeline, + // so nothing is waiting on the user and the empty state is honest. + // Every other failure — the objectstack#7344 denial included — is not. + setNotificationsStatus(isMissingResource(err) ? 'ready' : 'error'); + }); return () => { cancelled = true; }; }, [dataSource, user?.id, limit]); - return { pendingApprovalsCount, notifications, activities }; + return { pendingApprovalsCount, notifications, notificationsStatus, activities }; }