fix(console): System Hub 计数失败回落 null 而非冒充 0,单卡隔离 (#3679)

.changeset/system-hub-count-error-state-3679.md

@@ -0,0 +1,23 @@
+---
+'@object-ui/console': patch
+---
+
+System Hub: a card count that failed to load no longer renders as `0`
+
+Each count on the System Hub fetched one object and caught its own failure with
+an empty page, so a 500, a 401, a 403 or a dropped connection all produced the
+same confident `0` as a table that really is empty — no error, no retry, and no
+way to tell the two apart. The most reachable case was a permission denial on a
+single object: an administrator who may open the hub but cannot read
+`sys_audit_log` was shown "0 entries" rather than being told anything at all.
+
+A failed lookup now leaves that card's count unknown, and the badge — which
+already renders only for a known count — is omitted, so the card shows no
+number instead of a wrong one. The catch stays on each call rather than around
+the batch, so one object's failure blanks only its own card and the cards beside
+it keep the real numbers they received.
+
+Unchanged: an object the backend does not have still counts `0`. The adapter
+resolves an unregistered object as an empty page by design (callers read empty
+as "feature unavailable"), so that never was an error and is not treated as one
+here.

apps/console/src/pages/system/SystemHubPage.tsx

@@ -55,6 +55,21 @@ interface HubCard {
   adminOnly?: boolean;
 }
 
+/**
+ * One card's count from one `find` result: the row count when the lookup
+ * succeeded, `null` when it did not.
+ *
+ * `null` is not a formatting preference — it is the only shape this page has
+ * for "we do not know". The badge renders on `count !== null`, so an unknown
+ * count shows no badge at all, while `0` is a claim: the backend answered, and
+ * the answer was none. Collapsing a failed lookup into `0` prints a number
+ * nothing ever confirmed, and a 500 / 401 / 403 / offline then looks exactly
+ * like an empty table (objectui#3679).
+ */
+function countOrUnknown(result: { data?: unknown[] } | null): number | null {
+  return result === null ? null : (result.data?.length ?? 0);
+}
+
 export function SystemHubPage() {
   const navigate = useNavigate();
   const { appName } = useParams();
@@ -104,22 +119,46 @@ export function SystemHubPage() {
       // MEASUREMENT case in this page's test rather than quietly re-aimed.
       //
       // TODO: Replace with count-specific API endpoint when available
+      //
+      // Each `.catch` resolves to `null`, NOT to an empty page. What these
+      // catches actually cover is the class the adapter does NOT absorb: it
+      // rethrows everything that is not a 404, so a 500 / 401 / 403 / offline
+      // / timeout lands here. Answering that with `{ data: [] }` used to print
+      // a confident `0` — the same pixel as "there really are none", with no
+      // error, no retry and no way for an administrator to tell the two apart
+      // (objectui#3679). `null` flows into `counts` and the badge's existing
+      // `count !== null` branch simply omits the badge.
+      //
+      // Per call rather than once around the `Promise.all`, because the most
+      // reachable failure is a permission denial on ONE object — an admin who
+      // may open this hub but cannot read `sys_audit_log` should lose that
+      // card's number only, not the four beside it that answered fine.
+      //
+      // A 404 still does not reach here and still renders `0`: the adapter
+      // resolves unregistered objects as an empty page on purpose (see above).
+      // That is its contract, not a failure — the one card still riding on it
+      // is Permissions, which is objectui#3655's decision to close.
       const [usersRes, orgsRes, positionsRes, permsRes, logsRes] = await Promise.all([
-        dataSource.find('sys_user').catch(() => ({ data: [] })),
-        dataSource.find('sys_organization').catch(() => ({ data: [] })),
-        dataSource.find('sys_position').catch(() => ({ data: [] })),
-        dataSource.find('sys_permission').catch(() => ({ data: [] })),
-        dataSource.find('sys_audit_log').catch(() => ({ data: [] })),
+        dataSource.find('sys_user').catch(() => null),
+        dataSource.find('sys_organization').catch(() => null),
+        dataSource.find('sys_position').catch(() => null),
+        dataSource.find('sys_permission').catch(() => null),
+        dataSource.find('sys_audit_log').catch(() => null),
       ]);
       setCounts({
-        users: usersRes.data?.length ?? 0,
-        orgs: orgsRes.data?.length ?? 0,
-        positions: positionsRes.data?.length ?? 0,
-        permissions: permsRes.data?.length ?? 0,
-        auditLogs: logsRes.data?.length ?? 0,
+        users: countOrUnknown(usersRes),
+        orgs: countOrUnknown(orgsRes),
+        positions: countOrUnknown(positionsRes),
+        permissions: countOrUnknown(permsRes),
+        auditLogs: countOrUnknown(logsRes),
       });
     } catch {
-      // Keep nulls on failure
+      // Keep nulls on failure. Only a SYNCHRONOUS throw from `dataSource.find`
+      // can arrive here: the `.catch`es above are attached to the returned
+      // promises, so nothing makes `Promise.all` reject. That is why this
+      // branch is not where the fix went — leaving the counts untouched only
+      // says "unknown" because they are still `null`, and `setCounts` above is
+      // the only place a lookup's outcome is ever written.
     } finally {
       setLoading(false);
     }

apps/console/src/pages/system/__tests__/SystemHubPage.counts.test.tsx

@@ -31,11 +31,22 @@
  * `SystemHubPage` itself is the real component, as in the sibling
  * `SystemHubPage.metadataCards.test.tsx` — a transcribed copy of the card list
  * is precisely how a wrong name survives.
+ *
+ * ── objectui#3679 ──────────────────────────────────────────────────────────
+ * A second describe block was added below for the other half of the same
+ * pixel. The names above decide WHICH object is counted; the error handling
+ * decides whether a count that never arrived is allowed to be spelled `0`. It
+ * no longer is: a non-404 rejection (500 / 401 / 403 / offline — the only
+ * class the adapter does not absorb) leaves that card's count `null`, and the
+ * badge's existing `count !== null` branch drops the badge. The MEASUREMENT
+ * case that pinned the old collapse-into-0 was rewritten there, as its own
+ * comment asked for; the other two MEASUREMENTs still pin objectui#3655's gap
+ * and are untouched.
  */
 
 import '@testing-library/jest-dom/vitest';
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { render, screen, within, cleanup } from '@testing-library/react';
+import { render, screen, within, cleanup, waitFor } from '@testing-library/react';
 import { MemoryRouter, Routes, Route } from 'react-router-dom';
 
 // Hoisted so the vi.mock factories below can close over them, and so the
@@ -143,6 +154,54 @@ async function badge(cardTestId: string, text: string) {
   return within(card).findByText(text);
 }
 
+/**
+ * A card's count badge, or `null` when the card shows none — which is how this
+ * page says "unknown" (the badge renders only on `count !== null`).
+ *
+ * Anchored `^…$` on purpose: the card's title and description carry the same
+ * label word ("Manage system users and accounts"), so an unanchored match
+ * would find text on a card that has no badge at all.
+ */
+function countBadge(cardTestId: string, label: string) {
+  return within(screen.getByTestId(cardTestId)).queryByText(
+    new RegExp(`^\\d+\\s+${label}$`),
+  );
+}
+
+/**
+ * Settles the fetch when NO badge is expected to appear, so awaiting one is not
+ * an option. `fetchCounts` clears `loading` in its `finally`, so the spinner
+ * going away is the single anchor that holds whether the calls resolved,
+ * rejected, or threw synchronously.
+ */
+async function settleWithoutBadges() {
+  await waitFor(() =>
+    expect(screen.queryByText('Loading statistics...')).not.toBeInTheDocument(),
+  );
+}
+
+/** Every card that carries a count, as [testid, countLabel]. */
+const COUNTED_CARDS: ReadonlyArray<readonly [string, string]> = [
+  ['hub-card-users', 'users'],
+  ['hub-card-organizations', 'organizations'],
+  ['hub-card-positions', 'positions'],
+  ['hub-card-permissions', 'permissions'],
+  ['hub-card-audit-log', 'entries'],
+];
+
+/**
+ * The object names the page asks for, in call order — including
+ * `sys_permission`, which the framework does not register (objectui#3655) and
+ * so is absent from the fixture registry above.
+ */
+const QUERIED_OBJECT_NAMES = [
+  'sys_user',
+  'sys_organization',
+  'sys_position',
+  'sys_permission',
+  'sys_audit_log',
+];
+
 describe('System Hub card counts — object names (objectui#3670)', () => {
   it('counts Organizations through sys_organization, the name the framework registers', async () => {
     renderHub();
@@ -183,10 +242,16 @@ describe('System Hub card counts — object names (objectui#3670)', () => {
   });
 
   // ── MEASUREMENT ────────────────────────────────────────────────────────────
-  // The three cases below pin the CURRENT behaviour, not the desired one. They
+  // The two cases below pin the CURRENT behaviour, not the desired one. They
   // exist so the remaining gap is visible in the suite instead of being read as
   // a missed line, and so whoever resolves objectui#3655 has a failing anchor
   // to rewrite rather than a silent pass.
+  //
+  // There were three. The third — "a non-404 failure is collapsed into 0 as
+  // well, with no error affordance" — named objectui#3679 as the work that
+  // would rewrite it, and that work is done: it now lives in the next describe
+  // block with its expectation inverted. These two stay measurements because
+  // objectui#3655 is still open.
 
   it('MEASUREMENT: Permissions still reads 0 while both candidate objects hold rows', async () => {
     renderHub();
@@ -216,20 +281,96 @@ describe('System Hub card counts — object names (objectui#3670)', () => {
     expect(within(screen.getByTestId('hub-card-permissions')).getByText('0 permissions')).toBeInTheDocument();
   });
 
-  it('MEASUREMENT: a non-404 failure is collapsed into 0 as well, with no error affordance', async () => {
-    // The 404 never reaches the page's `.catch` — the adapter ate it upstream.
-    // What that `.catch` really covers is this: a 500 (or 401 / 403 / offline)
-    // on ONE object, rendered as a confident `0` on that card while its
-    // neighbours show real numbers. Recorded here only; changing the error
-    // handling is a separate class of work, filed as objectui#3679.
+});
+
+describe('System Hub card counts — a lookup that failed is not a `0` (objectui#3679)', () => {
+  it('a 500 on one object blanks that card instead of collapsing it into 0', async () => {
+    // This is the rewrite of PR #3680's third MEASUREMENT ("a non-404 failure
+    // is collapsed into 0 as well, with no error affordance") — same fixture,
+    // inverted expectation. The 404 never reaches the page's `.catch`; the
+    // adapter ate it upstream. What that `.catch` really covers is this.
     state.failures.sys_user = Object.assign(new Error('Internal Server Error'), {
       status: 500,
     });
     renderHub();
 
-    expect(await badge('hub-card-users', '0 users')).toBeInTheDocument();
-    // The per-call `.catch` also keeps `Promise.all` from rejecting, so the
-    // other four cards still resolve — including the one this PR fixed.
-    expect(within(screen.getByTestId('hub-card-organizations')).getByText('2 organizations')).toBeInTheDocument();
+    // Organizations answered, so the whole wall has settled once its badge is
+    // up — all five counts land in one `setCounts`.
+    expect(await badge('hub-card-organizations', '2 organizations')).toBeInTheDocument();
+    expect(countBadge('hub-card-users', 'users')).toBeNull();
+    // Spelled out, because `0 users` is the exact string this issue is about.
+    expect(screen.queryByText('0 users')).not.toBeInTheDocument();
+  });
+
+  it('blanks only the card that failed and leaves its neighbours their real counts', async () => {
+    // The issue's most reachable scenario, and the one an administrator is
+    // most likely to act on: someone who may open the hub but has no read
+    // permission on `sys_audit_log`. The backend answers 403, the adapter
+    // rethrows (it absorbs 404s only), and this card used to read "0 entries"
+    // — an audit log that looks empty to the person auditing it.
+    state.failures.sys_audit_log = Object.assign(new Error('Forbidden'), {
+      status: 403,
+    });
+    renderHub();
+
+    await badge('hub-card-users', '3 users');
+    expect(countBadge('hub-card-audit-log', 'entries')).toBeNull();
+    // Single-card isolation — the reason the `.catch` stayed on each call
+    // instead of moving out around the `Promise.all`. The other four answered,
+    // so they still show their numbers.
+    expect(countBadge('hub-card-users', 'users')).toHaveTextContent('3 users');
+    expect(countBadge('hub-card-organizations', 'organizations')).toHaveTextContent(
+      '2 organizations',
+    );
+    expect(countBadge('hub-card-positions', 'positions')).toHaveTextContent('4 positions');
+    expect(countBadge('hub-card-permissions', 'permissions')).toHaveTextContent(
+      '0 permissions',
+    );
+  });
+
+  it('keeps `0` for the two things that really are zero, and blanks only the failure', async () => {
+    // All three rows of the issue's behaviour matrix in one render. Only the
+    // third moves; the first two are the adapter's contract and stay as they
+    // are (whether Permissions should be riding on row two at all is
+    // objectui#3655, not this change):
+    //   sys_audit_log   registered, genuinely empty  -> `0 entries`
+    //   sys_permission  unregistered, adapter resolves empty -> `0 permissions`
+    //   sys_position    403, adapter rethrows        -> no badge
+    state.registry.sys_audit_log = [];
+    state.failures.sys_position = Object.assign(new Error('Forbidden'), { status: 403 });
+    renderHub();
+
+    await badge('hub-card-users', '3 users');
+    expect(countBadge('hub-card-audit-log', 'entries')).toHaveTextContent('0 entries');
+    expect(countBadge('hub-card-permissions', 'permissions')).toHaveTextContent(
+      '0 permissions',
+    );
+    expect(countBadge('hub-card-positions', 'positions')).toBeNull();
   });
+
+  it('shows no counts at all when every lookup fails, and still renders the hub', async () => {
+    // Offline, or the backend down. Every counted card loses its badge rather
+    // than reporting a system with nothing in it; the cards that never carried
+    // a count are unaffected and every link still works.
+    for (const objectName of QUERIED_OBJECT_NAMES) {
+      state.failures[objectName] = Object.assign(new Error('Failed to fetch'), {
+        status: 0,
+      });
+    }
+    renderHub();
+
+    await settleWithoutBadges();
+    for (const [testId, label] of COUNTED_CARDS) {
+      expect(countBadge(testId, label)).toBeNull();
+    }
+    expect(screen.getByTestId('hub-card-settings')).toBeInTheDocument();
+  });
+
+  // No case is written for `fetchCounts`'s OUTER `catch`. It is reachable only
+  // by a synchronous throw from `dataSource.find` (the per-call `.catch`es keep
+  // `Promise.all` from ever rejecting), and on that path the counts are still
+  // at their initial `null` — so an assertion that "no card shows a number"
+  // would pass whatever the outer catch did, including nothing. It would be
+  // green for an empty reason rather than because the page is right, so the
+  // fix stayed where the outcome is actually written, and so did the tests.
 });