From c71bbff9fe74553599a96ef05666057829382cae Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 17:13:39 +0000 Subject: [PATCH] fix(fields,plugin-detail,plugin-list,app-shell,console): read count/value answers as the contract declares them Six sites outside the fences of objectui#6726 / #6840 / #6839 still carried a `count` or `value` arm. Each got its OWN in-repo producer census, with the control sitting on the producer->consumer JOIN rather than on the word -- objectui#6840's zero is seam-local and is not transferable (the same sweep read 0 producers for `value` at one seam and 5 at another in one pass). ARM A -- a real precedence inversion, repaired WITHOUT deleting the arm. `packages/fields`' lookup chip read `result?.value || result?.data || []`: `value` ahead of `data`, the one rows member `QueryResult` declares. It now delegates to `@object-ui/core`'s `extractRecords` -- same accepted set (bare array, `data`, `value`), contract order. The `value` arm STAYS: its census found 8 live `find()` doubles emitting `{ value: [...] }` (3 plugin-kanban, 3 plugin-calendar, 2 plugin-grid), so deleting it -- the plausible wrong fix -- would have broken them. 0 producers emit BOTH members, which is why this was a "potentially answers wrong" and graded p2 rather than p1. FIVE DEAD ARMS deleted, each on its own measured zero: `count` at the DataSource.find() seam -- plugin-detail's reference rail and plugin-list's ListView. 0 / 592 find() producers emit `count`; controls `data` (312) and `total` (150) lit on the same pass, plus a superset sweep over all 418 producer-bearing files adjudicated to zero envelope members. `value` at the client.meta.getItems() seam -- app-shell's help menu, and the console's Public Forms and Flow Runs pages. These three do NOT sit on the find()/QueryResult seam at all, contrary to how the card described them, so they were measured on their own join: 0 / 28 meta.getItems producers emit `value`; control `items` (18) lit. The canonical readers of that envelope (`MetadataProvider.extractItems`, `MetadataService.getItems`) have never had a `value` arm either. `QueryResult` is NOT widened to bless `count` or `value` -- a published-type change and the maintainer's call, the floor #6726 / #6840 / #6839 all held. One refusal pin per module (`*.contractEnvelope-6917.*`), each keeping the live arms green beside the deleted one, and -- only where an inversion existed -- a case feeding both members with different contents, the sole input that separates the two orders. Also repaired: two plugin-grid test doubles answered `{ value: [], '@odata.count': 0 }` while ObjectGrid reads `result.data` / `result.total`. Inert only while the arrays were empty; the first row put in one would have been silently dropped. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .../6917-queryresult-value-count-arms.md | 55 ++++ .../src/pages/developer/FlowRunsPage.tsx | 7 +- .../src/pages/developer/PublicFormsPage.tsx | 7 +- ...ataEnvelope.contractEnvelope-6917.test.tsx | 165 +++++++++++ packages/app-shell/src/layout/AppHeader.tsx | 9 +- .../AppHeader.contractEnvelope-6917.test.tsx | 261 ++++++++++++++++++ .../lookupCell.contractEnvelope-6917.test.tsx | 158 +++++++++++ packages/fields/src/index.tsx | 14 +- ...erence-rail.contractEnvelope-6917.test.tsx | 151 ++++++++++ .../src/renderers/record-reference-rail.tsx | 13 +- .../src/__tests__/columnIdentity.test.tsx | 8 +- ...indEnvelope.contractEnvelope-6917.test.tsx | 105 +++++++ .../recordIdFieldProjection.test.tsx | 8 +- packages/plugin-list/src/ListView.tsx | 5 +- .../ListView.contractEnvelope-6917.test.tsx | 153 ++++++++++ 15 files changed, 1097 insertions(+), 22 deletions(-) create mode 100644 .changeset/6917-queryresult-value-count-arms.md create mode 100644 apps/console/src/pages/developer/developerMetadataEnvelope.contractEnvelope-6917.test.tsx create mode 100644 packages/app-shell/src/layout/__tests__/AppHeader.contractEnvelope-6917.test.tsx create mode 100644 packages/fields/src/__tests__/lookupCell.contractEnvelope-6917.test.tsx create mode 100644 packages/plugin-detail/src/renderers/__tests__/record-reference-rail.contractEnvelope-6917.test.tsx create mode 100644 packages/plugin-grid/src/__tests__/gridFindEnvelope.contractEnvelope-6917.test.tsx create mode 100644 packages/plugin-list/src/__tests__/ListView.contractEnvelope-6917.test.tsx diff --git a/.changeset/6917-queryresult-value-count-arms.md b/.changeset/6917-queryresult-value-count-arms.md new file mode 100644 index 0000000000..a8e0e454e0 --- /dev/null +++ b/.changeset/6917-queryresult-value-count-arms.md @@ -0,0 +1,55 @@ +--- +'@object-ui/fields': minor +'@object-ui/plugin-detail': minor +'@object-ui/plugin-list': minor +'@object-ui/app-shell': minor +'@object-ui/console': minor +--- + +Read `count` / `value` answers as the contract declares them at six more seams +(objectui#6917, following objectui#5945 / #6726 / #6840 / #6839). + +**One precedence inversion, repaired without deleting the arm.** +`@object-ui/fields`' lookup chip resolved fetch-on-demand rows with +`result?.value || result?.data || []` — `value` AHEAD of `data`, the one rows +member `QueryResult` (`@object-ui/types`) declares. A producer emitting both was +resolved to the undeclared key. It now reads through `@object-ui/core`'s +`extractRecords`, whose accepted set is identical (bare array, `data`, `value`) +and whose order is the contract's. The `value` arm is **kept**: its own producer +census measured eight live `find()` doubles emitting `{ value: [...] }` at this +seam (3 plugin-kanban, 3 plugin-calendar, 2 plugin-grid), so deleting it would +break them. Only the RANK was wrong. + +**Five dead arms deleted, each on its own measured zero.** Every module got its +own census with the control sitting on the producer→consumer join, because +objectui#6840's zero is seam-local and is not transferable — the same sweep read +0 producers for `value` at one seam and 5 at another in a single pass. + +- `count` at the `DataSource.find()` seam — `plugin-detail`'s reference rail and + `plugin-list`'s ListView. 0 of 592 `find()` producers emit `count`; controls + `data` (312) and `total` (150) lit on the same pass. Both adapters' + `normalizeQueryResult` already fold `count` into `total` below every consumer. +- `value` at the `client.meta.getItems()` seam — `app-shell`'s help menu and the + console's Public Forms and Flow Runs pages. **These three do not sit on the + `DataSource.find()` seam at all**, so they were measured on their own join: 0 + of 28 `meta.getItems` producers emit `value`; control `items` (18) lit. The + canonical readers of that envelope (`MetadataProvider.extractItems`, + `MetadataService.getItems`) have never had a `value` arm either. + +No producer changes behaviour, because at these five sites there is no producer; +what changes is that a non-conforming one is refused rather than silently +absorbed (AGENTS.md #0.1). + +`QueryResult` is **not** widened to bless `count` or `value` — a published-type +change and the maintainer's call, the floor objectui#6726, #6840 and #6839 all +held. A producer that really speaks either belongs behind an adapter that folds +it, which is what both adapters already do. + +One refusal pin per module, each keeping the live arms green beside the deleted +one, and — where an inversion actually existed — a case feeding both members with +different contents, the only input that can tell the two orders apart. + +Also repaired: two `plugin-grid` test doubles answered `{ value: [], +'@odata.count': 0 }` while `ObjectGrid` reads `result.data` / `result.total`. +Inert only while the arrays were empty; the first row put in one would have been +silently dropped. Test-only. diff --git a/apps/console/src/pages/developer/FlowRunsPage.tsx b/apps/console/src/pages/developer/FlowRunsPage.tsx index 2428a15d64..3066b6fe47 100644 --- a/apps/console/src/pages/developer/FlowRunsPage.tsx +++ b/apps/console/src/pages/developer/FlowRunsPage.tsx @@ -148,13 +148,14 @@ export function FlowRunsPage() { setFlowsError(null); try { const result: any = await client.meta.getItems('flow'); + // `meta.getItems` answers `{ type, items: [...] }`, or a bare array on + // the ADR-0037 preview path. The trailing `value` arm is gone on a + // measured zero at this seam (objectui#6917). const items: any[] = Array.isArray(result) ? result : Array.isArray(result?.items) ? result.items - : Array.isArray(result?.value) - ? result.value - : []; + : []; const normalized: FlowItem[] = items.map(it => ({ name: it?.spec?.name ?? it?.name, label: it?.spec?.label, diff --git a/apps/console/src/pages/developer/PublicFormsPage.tsx b/apps/console/src/pages/developer/PublicFormsPage.tsx index fc823e111c..2b8e834794 100644 --- a/apps/console/src/pages/developer/PublicFormsPage.tsx +++ b/apps/console/src/pages/developer/PublicFormsPage.tsx @@ -128,13 +128,14 @@ export function PublicFormsPage() { setError(null); try { const result: any = await client.meta.getItems('view'); + // `meta.getItems` answers `{ type, items: [...] }`, or a bare array on + // the ADR-0037 preview path. The trailing `value` arm is gone on a + // measured zero at this seam (objectui#6917). const items: any[] = Array.isArray(result) ? result : Array.isArray(result?.items) ? result.items - : Array.isArray(result?.value) - ? result.value - : []; + : []; const forms: PublicFormRow[] = []; const candidates: PublishableFormRow[] = []; for (const it of items) { diff --git a/apps/console/src/pages/developer/developerMetadataEnvelope.contractEnvelope-6917.test.tsx b/apps/console/src/pages/developer/developerMetadataEnvelope.contractEnvelope-6917.test.tsx new file mode 100644 index 0000000000..12c8becf69 --- /dev/null +++ b/apps/console/src/pages/developer/developerMetadataEnvelope.contractEnvelope-6917.test.tsx @@ -0,0 +1,165 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The two developer pages read a `client.meta.getItems()` answer as the + * metadata API DECLARES it — and do NOT read `value` (objectui#6917 arm B). + * + * ── The seam is NOT `DataSource.find()`, and that changed the measurement ── + * objectui#6917 filed these two sites as "the same arm objectui#6840 removed", + * i.e. a non-`QueryResult` key at the `DataSource.find()` seam. They are not. + * `QueryResult` (`@object-ui/types`) never reaches here: these pages call + * `client.meta.getItems(type)`, whose envelope is `{ type, items: [...] }`, or + * a bare array on the ADR-0037 preview path. `data` is not one of its members + * either — `MetadataProvider.extractItems` pins `{ data: [...] }` to `[]`. + * + * That is objectui#6917's own central rule turned on the card: a zero measured + * at one seam says nothing about another. So the census below was run on the + * `meta.getItems` JOIN, not on the `find()` join the card named: + * + * CELL every `meta.getItems` producer body in the repo .... 28 producers + * CONTROL `items` emitted as an envelope member ............. 18 producers + * SUBJECT `value` emitted as an envelope member ............ 0 producers + * + * Superset sweep, so a shape assembled outside a producer body still surfaces: + * of the 25 files holding a producer, 6 contain the token `value:` anywhere, + * and all 6 are filter-condition values, select options, a DOM helper + * parameter or a storage shim — none an envelope member. The control sits on + * the JOIN (same cell, same pass, same extraction), so the zero is a reading. + * + * The two canonical in-repo readers of this envelope agree and neither has ever + * had a `value` arm: `MetadataProvider.extractItems` (bare array | `items`) and + * `MetadataService.getItems` (`items` only). + * + * ⛔ The fix is the deletion, NOT widening any published type to bless `value` + * — the floor objectui#6726, #6840 and #6839 all held. + * + * The live arms are pinned beside the dead one, because live-versus-dead is the + * whole distinction. No precedence case appears here: `value` sat LAST, behind + * both live arms, so no ordering was ever observable at these two sites. (The + * one site on this card that DID invert is `packages/fields`, pinned there.) + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; + +/** + * One mutable answer, read by BOTH pages' `meta.getItems`. A stable singleton + * adapter: a fresh object per render loops the pages' load effects. + */ +const { ADAPTER, state } = vi.hoisted(() => { + const state: { answer: unknown } = { answer: [] }; + const ADAPTER = { + getClient: () => ({ + meta: { + getItems: async () => state.answer, + saveItem: vi.fn(async () => ({ ok: true })), + }, + automation: { execute: vi.fn(), listRuns: async () => ({ runs: [] }) }, + }), + }; + return { ADAPTER, state }; +}); + +vi.mock('@object-ui/app-shell', async (importOriginal) => ({ + ...(await importOriginal>()), + useAdapter: () => ADAPTER, + useMetadata: () => ({ objects: [] }), +})); +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +// Imported AFTER the mocks so the pages pick them up. +import { FlowRunsPage } from './FlowRunsPage'; +import { PublicFormsPage } from './PublicFormsPage'; + +/** A flow definition, as `meta.getItems('flow')` rows carry it. */ +const FLOW = { + spec: { + name: 'reassign_wizard', + label: 'Reassign', + variables: [{ name: 'recordId', type: 'text', isInput: true }], + }, +}; + +/** A published public form — the page lists a row only when both + * `sharing.allowAnonymous` and a parseable `publicLink` are present. */ +const FORM = { + spec: { + name: 'showcase_task.public', + label: 'Log Time', + object: 'showcase_task', + type: 'simple', + sections: [{ label: 'Task', fields: ['title'] }], + sharing: { enabled: true, allowAnonymous: true, publicLink: '/forms/log-time' }, + }, +}; + +beforeEach(() => { + state.answer = []; +}); +afterEach(cleanup); + +describe('FlowRunsPage — meta.getItems envelope (objectui#6917)', () => { + it("still reads the envelope's `items` member", async () => { + state.answer = { items: [FLOW] }; + render(); + await waitFor(() => { + expect(screen.getByText('Reassign (reassign_wizard)')).toBeInTheDocument(); + }); + }); + + it('still reads a bare array — the ADR-0037 preview path answers with one', async () => { + state.answer = [FLOW]; + render(); + await waitFor(() => { + expect(screen.getByText('Reassign (reassign_wizard)')).toBeInTheDocument(); + }); + }); + + it('does NOT read `value` — not a member of this envelope', async () => { + // Before the fix this listed the flow. The page now reports the honest + // "nothing here" rather than legitimising a second de-facto contract. + state.answer = { value: [FLOW] }; + render(); + await waitFor(() => { + expect(screen.getByText('No flow definitions found.')).toBeInTheDocument(); + }); + expect(screen.queryByText('Reassign (reassign_wizard)')).not.toBeInTheDocument(); + }); + + it('does NOT read `data` either — `extractItems` pins that to empty as well', async () => { + // The caricature guard: a reader that returned the first array it could + // find in the envelope, whatever the key, would list the flow here. + state.answer = { data: [FLOW] }; + render(); + await waitFor(() => { + expect(screen.getByText('No flow definitions found.')).toBeInTheDocument(); + }); + }); +}); + +describe('PublicFormsPage — meta.getItems envelope (objectui#6917)', () => { + it("still reads the envelope's `items` member", async () => { + state.answer = { items: [FORM] }; + render(); + await waitFor(() => { + expect(screen.getByText('Log Time')).toBeInTheDocument(); + }); + }); + + it('still reads a bare array', async () => { + state.answer = [FORM]; + render(); + await waitFor(() => { + expect(screen.getByText('Log Time')).toBeInTheDocument(); + }); + }); + + it('does NOT read `value` — not a member of this envelope', async () => { + state.answer = { value: [FORM] }; + render(); + await waitFor(() => { + expect(screen.getByText('No public forms yet')).toBeInTheDocument(); + }); + expect(screen.queryByText('Log Time')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/app-shell/src/layout/AppHeader.tsx b/packages/app-shell/src/layout/AppHeader.tsx index 1ea6f4dc86..d9c1c64bed 100644 --- a/packages/app-shell/src/layout/AppHeader.tsx +++ b/packages/app-shell/src/layout/AppHeader.tsx @@ -179,13 +179,16 @@ export function AppHeader({ return; } const result: any = await client.meta.getItems('doc'); + // `meta.getItems` answers the `{ type, items: [...] }` envelope (or a + // bare array from the ADR-0037 preview path) — the two shapes + // `MetadataProvider`'s `extractItems` accepts. A `value` arm used to + // trail them; measured zero producers emit `value` at THIS seam + // (objectui#6917), so it is gone (AGENTS.md #0.1). const items: any[] = Array.isArray(result) ? result : Array.isArray(result?.items) ? result.items - : Array.isArray(result?.value) - ? result.value - : []; + : []; setHelpDocs( items .map((it) => ({ name: it?.name, label: it?.label, _packageId: it?._packageId })) diff --git a/packages/app-shell/src/layout/__tests__/AppHeader.contractEnvelope-6917.test.tsx b/packages/app-shell/src/layout/__tests__/AppHeader.contractEnvelope-6917.test.tsx new file mode 100644 index 0000000000..43c3446a13 --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/AppHeader.contractEnvelope-6917.test.tsx @@ -0,0 +1,261 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * AppHeader's help menu reads a `meta.getItems()` answer the way the metadata + * API DECLARES it — and does NOT read `value` (objectui#6917 arm B). + * + * ── The seam is NOT `DataSource.find()`, which changed the measurement ───── + * objectui#6917 filed this site as "the same arm objectui#6840 removed", i.e. + * a non-`QueryResult` key at the `DataSource.find()` seam. It is not. + * `QueryResult` (`@object-ui/types`) never reaches here: `loadHelpDocs` calls + * `client.meta.getItems('doc')`, whose envelope is `{ type, items: [...] }`, + * or a bare array on the ADR-0037 preview path. `data` is not a member of it + * either — this package's own `MetadataProvider.extractItems` pins + * `{ data: [...] }` to `[]`, and `MetadataService.getItems` reads `items` and + * nothing else. + * + * That is objectui#6917's own central rule turned back on the card: a zero + * measured at one seam says nothing about another. So the census below sits on + * the `meta.getItems` JOIN, not on the `find()` join the card named: + * + * CELL every `meta.getItems` producer body in the repo ... 28 producers + * CONTROL `items` emitted as an envelope member ............. 18 producers + * SUBJECT `value` emitted as an envelope member ............ 0 producers + * + * Superset sweep, so a shape assembled outside a producer body still surfaces: + * of the 25 files holding a producer, 6 contain the token `value:` anywhere, + * and all 6 are filter-condition values, select options, a DOM helper parameter + * or a storage shim — none an envelope member. The control sits on the JOIN + * (same cell, same pass, same extraction), so the zero is a reading. + * + * ⛔ The fix is the deletion, NOT widening any published type to bless `value` + * — the floor objectui#6726, #6840 and #6839 all held. + * + * No precedence case appears here: `value` sat LAST, behind both live arms, so + * no ordering was ever observable at this site. (The one site on this card that + * DID invert is `packages/fields`, pinned there.) The live arms are pinned + * beside the dead one, because live-versus-dead is the whole distinction. + */ +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; + +vi.mock('react-router-dom', () => ({ + useLocation: () => ({ pathname: '/apps/crm/home', search: '', hash: '', state: null, key: 't' }), + useParams: () => ({ appName: 'crm' }), + useNavigate: () => vi.fn(), + useSearchParams: () => [new URLSearchParams(), vi.fn()] as const, + Link: ({ children, to, ...p }: any) => {children}, +})); + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + language: 'en', + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + }), + useObjectLabel: () => ({ + objectLabel: (n: string) => n, + dashboardLabel: (n: string) => n, + pageLabel: (n: string) => n, + reportLabel: (n: string) => n, + viewLabel: (n: string) => n, + appLabel: (n: string) => n, + }), +})); + +/** + * Passthrough menu primitives: the help dropdown's CONTENT is the subject, so + * `DropdownMenuContent` must render its children — Radix keeps a closed menu + * unmounted in jsdom. `DropdownMenu` must forward `onOpenChange`, because that + * is what triggers the lazy `loadHelpDocs()` fetch under measurement. + */ +vi.mock('@object-ui/components', async (importOriginal) => { + const actual = await importOriginal>(); + const stripProps = (p: any) => { + const { asChild, variant, size, align, sideOffset, onOpenChange, ...rest } = p ?? {}; + return rest; + }; + const Pass = ({ children, ...p }: any) =>
{children}
; + const OpeningMenu = ({ children, onOpenChange, ...p }: any) => { + // Fire "opened" once on mount: the header only fetches docs on first open. + const fired = (globalThis as any).__oi_fired ?? ((globalThis as any).__oi_fired = new WeakSet()); + if (onOpenChange && !fired.has(onOpenChange)) { + fired.add(onOpenChange); + queueMicrotask(() => onOpenChange(true)); + } + return
{children}
; + }; + return { + ...actual, + Button: ({ children, asChild, variant, size, ...p }: any) => ( + + ), + DropdownMenu: OpeningMenu, + DropdownMenuTrigger: Pass, + DropdownMenuContent: Pass, + DropdownMenuItem: ({ children, onClick, ...p }: any) => ( + + ), + DropdownMenuLabel: Pass, + DropdownMenuSeparator: () => null, + DropdownMenuGroup: Pass, + Avatar: Pass, + AvatarImage: () => null, + AvatarFallback: Pass, + Popover: Pass, + PopoverTrigger: Pass, + PopoverContent: () => null, + Tabs: Pass, + TabsList: Pass, + TabsTrigger: ({ children }: any) => , + TabsContent: Pass, + cn: (...c: any[]) => c.filter(Boolean).join(' '), + }; +}); + +vi.mock('lucide-react', () => { + const Icon = () => ; + return new Proxy({ __esModule: true } as Record, { + get: (target, prop) => { + if (prop === 'then' || prop === '__esModule' || typeof prop === 'symbol') return target[prop]; + return Icon; + }, + has: (_target, prop) => prop !== 'then', + }); +}); + +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useOffline: () => ({ isOnline: true }), +})); + +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + PresenceAvatars: () => null, + useTenantPresence: () => [], +})); + +vi.mock('../ModeToggle', () => ({ ModeToggle: () => null })); +vi.mock('../LocaleSwitcher', () => ({ LocaleSwitcher: () => null })); +vi.mock('../ConnectionStatus', () => ({ ConnectionStatus: () => null })); +vi.mock('../AppSwitcher', () => ({ AppSwitcher: () => null })); +vi.mock('../LocalizedSidebarTrigger', () => ({ LocalizedSidebarTrigger: () => null })); +vi.mock('../PreviewBadge', () => ({ PreviewBadge: () => null })); +vi.mock('../WorkspaceSwitcher', () => ({ WorkspaceSwitcher: () => null })); + +/** + * ONE fetch double at MODULE scope, deliberately never torn down + * (objectui#6640 / objectui#7439). The header mounts pollers — the AI agent + * probe and the pending-approvals bell — that fire and forget; happy-dom + * resolves their relative URLs against `http://localhost:3000`, so a real + * socket is attempted and the network-escape guard fails the file. Because + * those reads can happen AFTER a test body returns, restoring the real fetch + * in an afterEach would simply move the escape rather than close it. + */ +vi.stubGlobal( + 'fetch', + vi.fn(async () => + new Response(JSON.stringify({ data: [], items: [], agents: [], requests: [] }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }), + ), +); + +const PACKAGE_ID = 'pkg_crm'; + +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => ({ + apps: [{ name: 'crm', label: 'CRM', _packageId: PACKAGE_ID }], + dashboards: [], pages: [], reports: [], + }), +})); + +vi.mock('../../context/NavigationContext.js', () => ({ + useNavigationContext: () => ({ currentAppName: 'crm', recordTitle: undefined }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => { + // ONE stable identity for the whole file: AppHeader resolves this in a + // `useEffect(..., [getAuthConfig])` that ends in setState, so a fresh closure + // per render loops effect → setState → render until the heap dies. + const getAuthConfig = () => Promise.resolve({ features: { multiOrgEnabled: false } }); + return { + ...(await importOriginal>()), + useAuth: () => ({ + user: { id: 'u1', name: 'Zhang San', email: 'zs@example.com' }, + signOut: vi.fn(), + isAuthEnabled: true, + organizations: [], + activeOrganization: null, + isOrganizationsLoading: false, + switchOrganization: vi.fn(), + getAuthConfig, + }), + getUserInitials: () => 'ZS', + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), + }; +}); + +/** The answer `meta.getItems('doc')` gives for the current case. */ +const state: { answer: unknown } = { answer: [] }; + +/** Stable adapter identity — the header's pollers key effects off it, so a + * fresh object per call would re-arm them on every render. */ +const fakeAdapter = { + find: () => Promise.resolve({ data: [] }), + getClient: () => ({ meta: { getItems: async () => state.answer } }), +}; + +vi.mock('../../providers/AdapterProvider', () => ({ useAdapter: () => fakeAdapter })); + +import { AppHeader } from '../AppHeader'; + +/** One `doc` row owned by the current app — the only rows the menu surfaces. */ +const DOC = { name: 'getting-started', label: 'Getting Started', _packageId: PACKAGE_ID }; + +beforeEach(() => { state.answer = []; }); +afterEach(cleanup); + +/** True once the header has decided whether the app owns any docs. */ +async function appDocsEntryAppears(answer: unknown): Promise { + state.answer = answer; + render(); + // "All documentation" is unconditional — waiting on it proves the help menu + // rendered at all, so a false negative below cannot be "the menu never opened". + await waitFor(() => { + expect(screen.getAllByText('All documentation').length).toBeGreaterThan(0); + }); + // …then let the lazy fetch settle before reading the conditional entry. + await waitFor(() => expect(state.answer).toBe(answer)); + await new Promise((r) => setTimeout(r, 0)); + return screen.queryAllByText("This app's docs").length > 0; +} + +describe('AppHeader help docs — meta.getItems envelope (objectui#6917)', () => { + it("still reads the envelope's `items` member", async () => { + expect(await appDocsEntryAppears({ items: [DOC] })).toBe(true); + }); + + it('still reads a bare array — the ADR-0037 preview path answers with one', async () => { + expect(await appDocsEntryAppears([DOC])).toBe(true); + }); + + it('does NOT read `value` — not a member of this envelope', async () => { + // Before the fix the entry appeared off an undeclared key. + expect(await appDocsEntryAppears({ value: [DOC] })).toBe(false); + }); + + it('does NOT read `data` either — `extractItems` pins that to empty as well', async () => { + // The caricature guard: a reader returning the first array it found under + // any key would surface the entry here. + expect(await appDocsEntryAppears({ data: [DOC] })).toBe(false); + }); +}); diff --git a/packages/fields/src/__tests__/lookupCell.contractEnvelope-6917.test.tsx b/packages/fields/src/__tests__/lookupCell.contractEnvelope-6917.test.tsx new file mode 100644 index 0000000000..bcdcd242f9 --- /dev/null +++ b/packages/fields/src/__tests__/lookupCell.contractEnvelope-6917.test.tsx @@ -0,0 +1,158 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `LookupCellRenderer`'s fetch-on-demand resolver reads a `find()` answer with + * the ONE rows member `QueryResult` DECLARES first — `data` (objectui#6917 + * arm A, following objectui#6726 / #6840 / #6839). + * + * Before this pin, `useLookupName` spelled its unwrap + * + * Array.isArray(result) ? result : (result?.value || result?.data || []) + * + * — `value` AHEAD of `data`. `QueryResult` (`@object-ui/types`) declares + * exactly one rows member and it is `data`; `value` is the OData spelling. + * That is a PRECEDENCE INVERSION, not merely a dead arm: a producer emitting + * both members was resolved to the undeclared one, and the declared one was + * ignored. It is the same inversion objectui#5945 named and objectui#6726 + * repaired for `records`, standing on the key those cards did not measure. + * + * ── Why the arm is KEPT and only the ORDER changed ──────────────────────── + * MEASURED on this tree for objectui#6917, per seam rather than carried over + * from objectui#6840's zero (that zero is seam-local and says nothing here): + * + * CELL every `find()` producer body in the repo, bracket-scanned + * through chained calls ....................... 592 producers + * CONTROL `data` emitted as an envelope member ....... 312 producers + * CONTROL `total` emitted as an envelope member ....... 150 producers + * SUBJECT `value` emitted as an envelope member ....... 8 producers + * (3 plugin-kanban, 3 plugin-calendar, 2 plugin-grid) + * SUBJECT `value` AND `data` both, as envelope members 0 producers + * + * The controls sit on the JOIN — same cell, same pass, same extraction — so + * these are readings, not unmeasured cells. + * + * `value` is therefore LIVE at this seam and the arm STAYS: deleting it would + * break the eight producers above, which is the plausible wrong fix this file + * exists to refuse. What was wrong was only its RANK. Because 0 producers emit + * both members today, the inversion was a "potentially answers wrong" rather + * than an "answers wrong" — which is why objectui#6917 was graded p2, not p1. + * + * The unwrap now delegates to `@object-ui/core`'s `extractRecords`, whose + * accepted set (bare array, `data`, `value`) is identical and whose order is + * the contract's. One measured implementation instead of a fourth hand-rolled + * ladder. + * + * ⛔ The fix is the ordering, NOT widening `QueryResult` to bless `value` — + * that is a published-type change and the maintainer's call, the floor + * objectui#6726, #6840 and #6839 all held. + * + * ── What separates the two orders ───────────────────────────────────────── + * ONLY a result carrying BOTH members with DIFFERENT contents. A fixture + * carrying just `data` passes on the correct order AND on the inverted one, so + * it cannot see precedence at all. The live arms are pinned beside it, because + * live-versus-dead is the whole distinction. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { LookupCellRenderer } from '../index'; +import { SchemaRendererProvider } from '@object-ui/react'; + +/** + * A data source WITHOUT `findOne` — that is what routes the resolver down the + * `find()` branch this file measures. With `findOne` present the envelope is + * never consulted at all, and every case below would pass vacuously. + */ +function dsWithoutFindOne(objectName: string, answer: unknown) { + return { + find: vi.fn(async () => answer), + getObjectSchema: vi.fn(async (object: string) => + object === objectName ? { nameField: 'name' } : null, + ), + } as any; +} + +/** Distinct object name + id per case: the module-level name cache is keyed + * `${referenceTo}:${id}:${displayField}` and outlives a single test. */ +async function renderChip(objectName: string, id: string, answer: unknown) { + const ds = dsWithoutFindOne(objectName, answer); + render( + + + , + ); + return ds; +} + +describe('LookupCellRenderer find() envelope — objectui#6917 arm A', () => { + it('does NOT let `value` OUTRANK `data` — the precedence inversion itself', async () => { + // The sharp end, and the ONLY input that separates the two orders: both + // members present and DISAGREEING. `data` is the contract's, so + // "From data" is the only correct answer; the pre-fix order answered + // "From value". + await renderChip('env_both_6917', 'id_both', { + value: [{ id: 'id_both', name: 'From value' }], + data: [{ id: 'id_both', name: 'From data' }], + }); + await waitFor(() => { + expect(screen.getByText('From data')).toBeInTheDocument(); + }); + expect(screen.queryByText('From value')).not.toBeInTheDocument(); + }); + + it("still reads the contract's `data` member on its own", async () => { + await renderChip('env_data_6917', 'id_data', { + data: [{ id: 'id_data', name: 'Only data' }], + }); + await waitFor(() => { + expect(screen.getByText('Only data')).toBeInTheDocument(); + }); + }); + + it('still reads `value` on its own — the arm is LIVE at this seam, 8 producers', async () => { + // The non-regression axis. The plausible WRONG fix deletes this arm + // outright; three plugin-kanban, three plugin-calendar and two plugin-grid + // `find()` doubles emit `{ value: [...] }` today, so a result carrying only + // `value` must still resolve. + await renderChip('env_value_6917', 'id_value', { + value: [{ id: 'id_value', name: 'Only value' }], + }); + await waitFor(() => { + expect(screen.getByText('Only value')).toBeInTheDocument(); + }); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + await renderChip('env_bare_6917', 'id_bare', [{ id: 'id_bare', name: 'Bare array' }]); + await waitFor(() => { + expect(screen.getByText('Bare array')).toBeInTheDocument(); + }); + }); + + it('does NOT read `records` — folded into `data` below the adapter', async () => { + // `records` is the below-the-adapter spelling; both adapters' + // `normalizeQueryResult` return `data` before an answer reaches here. The + // chip falls back to the raw id rather than legitimising a second + // de-facto contract. This is also the caricature guard: an extractor that + // returned a constant, or always returned the first row of whatever it was + // handed, would answer "Should not resolve" here. + await renderChip('env_records_6917', 'id_records', { + records: [{ id: 'id_records', name: 'Should not resolve' }], + }); + await waitFor(() => { + expect(screen.getByText('id_records')).toBeInTheDocument(); + }); + expect(screen.queryByText('Should not resolve')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index 4709796be3..48b3d279e3 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -8,7 +8,7 @@ import React from 'react'; import type { DateTimeFieldMetadata, FieldMetadata, SelectOptionMetadata } from '@object-ui/types'; -import { ComponentRegistry, percentDisplayValue, getRecordDisplayName, humanizeLabel, isMissingForRequired, formatDate, formatDateTime, formatDateTimeCompactParts, formatRelativeDate, type ComponentMeta, type DateDisplayOptions } from '@object-ui/core'; +import { ComponentRegistry, percentDisplayValue, getRecordDisplayName, humanizeLabel, isMissingForRequired, formatDate, formatDateTime, formatDateTimeCompactParts, formatRelativeDate, extractRecords, type ComponentMeta, type DateDisplayOptions } from '@object-ui/core'; // The platform's own value-shape contract, asked rather than restated // (objectui#6744). See `locationStoredValueSchemaFor` below for why this is a // runtime import in the barrel and not a hand-written coordinate range. @@ -245,9 +245,15 @@ function useLookupName( $filter: { id: value }, $top: 1, }); - const records: any[] = Array.isArray(result) - ? result - : (result?.value || result?.data || []); + // Read the rows through `@object-ui/core`'s `extractRecords` rather + // than a fourth hand-rolled ladder. This site used to spell it + // `result?.value || result?.data || []` — `value` AHEAD of `data`, + // the ONE rows member `QueryResult` (`@object-ui/types`) declares. + // A producer emitting both was resolved to the undeclared key + // (objectui#6917 arm A). `extractRecords` accepts the same shapes + // (bare array, `data`, `value`) in the order the contract implies, + // and is the single measured answer for this seam (objectui#6839). + const records: any[] = extractRecords(result); record = records[0]; } // Resolve through the referenced object's schema (nameField / diff --git a/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.contractEnvelope-6917.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.contractEnvelope-6917.test.tsx new file mode 100644 index 0000000000..96f9bd60ee --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.contractEnvelope-6917.test.tsx @@ -0,0 +1,151 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The reference rail counts a `find()` answer as `QueryResult` DECLARES it — + * and does NOT read `count` (objectui#6917 arm B, following objectui#6840). + * + * `QueryResult` (`@object-ui/types`) declares exactly one count member: + * `total`. Before this pin the rail's count resolution read + * + * typeof res?.total === 'number' ? res.total + * : typeof res?.count === 'number' ? res.count + * : items.length; + * + * — the SAME ladder objectui#6840 deleted from `RelatedCountStore`, in a module + * that card was not fenced to touch. It decides a rendered number: the count + * badge on each rail card. + * + * MEASURED on this tree for objectui#6917 — its OWN producer census, NOT + * objectui#6840's numbers. That zero is seam-local and says nothing here; the + * same sweep that read 0 producers for `value` at `ObjectView`'s seam read 5 at + * `extractRecords`' seam (objectui#6839), in one pass. + * + * CELL every `find()` producer body in the repo, bracket-scanned + * through chained calls ......................... 592 producers + * CONTROL `data` emitted as an envelope member ......... 312 producers + * CONTROL `total` emitted as an envelope member ......... 150 producers + * SUBJECT `count` emitted as an envelope member ........ 0 producers + * + * The controls sit on the JOIN — same cell, same pass, same extraction — so the + * zero is a reading and not an unmeasured cell. Superset sweep as well: of the + * 418 files holding a `find()` producer, 25 contain the token `count:` + * anywhere, and every one is the DataSource's own sibling `count()` method, a + * row field, an i18n interpolation parameter, an aggregate function name, React + * state, a DOM assertion, a comment, or objectui#6840's own refusal pin — none + * an envelope member. + * + * `'@odata.count'` is a DIFFERENT key and does not answer this question: + * `res?.count` cannot read it. Two producers emit it (both plugin-grid + * fixtures, repaired on this same card). + * + * `count` IS read below the adapter, which is exactly why nothing re-emits it + * above: `ObjectStackAdapter.normalizeQueryResult` and + * `ApiDataSource.normalizeQueryResult` both fold `count` into `total` before + * returning. This rail calls `dataSource.find()` strictly ABOVE that fold, so + * the arm was unreachable — and an unreachable tolerant arm is precisely where + * a non-conforming producer keeps working unrejected (AGENTS.md #0.1). + * + * ⛔ The fix is the deletion, NOT widening `QueryResult` to bless `count` — + * a published-type change and the maintainer's call (the floor objectui#6726, + * #6840 and #6839 all held). + * + * The live arms are pinned here too, because live and dead is the whole + * distinction: `total` (what `$count: true` asks the server for), the + * `data` row count, and the bare array. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import { RecordContextProvider } from '@object-ui/react'; + +import { RecordReferenceRailRenderer } from '../record-reference-rail'; + +/** The rail gates its queries on an IntersectionObserver. Report intersecting + * immediately so the fetch effect runs deterministically under jsdom. */ +class ImmediateIO { + constructor(private cb: (records: { isIntersecting: boolean }[]) => void) {} + observe() { this.cb([{ isIntersecting: true }]); } + disconnect() {} + unobserve() {} +} + +beforeEach(() => { + vi.stubGlobal('IntersectionObserver', ImmediateIO as unknown as typeof IntersectionObserver); +}); +afterEach(() => { + vi.unstubAllGlobals(); + cleanup(); +}); + +const ROWS = [{ id: 'c1', name: 'Ada' }, { id: 'c2', name: 'Grace' }, { id: 'c3', name: 'Alan' }]; + +/** + * Render one rail entry against a `find()` answering `answer`, and return the + * badge's text. The badge is read by CLASS, not by its digits: a total that + * happens to equal a row's text would make a `getByText` query ambiguous, and + * `queryByText` throws on multiple matches just as it does on none. + */ +async function badgeFor(answer: unknown): Promise { + const dataSource = { find: vi.fn(async () => answer) }; + const { container } = render( + + + + + , + ); + await waitFor(() => { + expect(container.querySelector('.tabular-nums')).not.toBeNull(); + }); + return container.querySelector('.tabular-nums')!.textContent ?? ''; +} + +describe('reference rail find() envelope — objectui#6917', () => { + it('still prefers the server-side `total` — the reason `$count: true` is sent', async () => { + expect(await badgeFor({ total: 42, data: [{ id: 'c1' }] })).toBe('42'); + }); + + it("still counts the contract's `data` member", async () => { + expect(await badgeFor({ data: ROWS })).toBe('3'); + }); + + it('still counts a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await badgeFor(ROWS)).toBe('3'); + }); + + it('does NOT count `count` — not a QueryResult member', async () => { + // Before the fix this badge read 7. It now reports the honest "no + // countable answer" 0 rather than legitimising a second de-facto contract. + expect(await badgeFor({ count: 7 })).toBe('0'); + }); + + it('does NOT let `count` OUTRANK the `data` row count', async () => { + // Both members present and disagreeing. `data` is the contract's, so 2 is + // the only correct answer; the pre-fix ladder answered 7. + expect(await badgeFor({ count: 7, data: [{ id: 'a' }, { id: 'b' }] })).toBe('2'); + }); + + it('`total` still outranks `count` — unchanged, and the control for the two above', async () => { + // Green before AND after the fix: the arm that was always correct. Its + // presence is what makes the two refusals above a reading of THIS deletion + // rather than of a rail that stopped counting. + expect(await badgeFor({ total: 42, count: 7 })).toBe('42'); + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-reference-rail.tsx b/packages/plugin-detail/src/renderers/record-reference-rail.tsx index 0c256f10ad..a5086aa261 100644 --- a/packages/plugin-detail/src/renderers/record-reference-rail.tsx +++ b/packages/plugin-detail/src/renderers/record-reference-rail.tsx @@ -190,12 +190,13 @@ export const RecordReferenceRailRenderer: React.FC ({ ...prev, [key]: { loading: false, total, items } })); } catch (err: any) { if (!mountedRef.current) return; diff --git a/packages/plugin-grid/src/__tests__/columnIdentity.test.tsx b/packages/plugin-grid/src/__tests__/columnIdentity.test.tsx index a4abca2d56..59d1531349 100644 --- a/packages/plugin-grid/src/__tests__/columnIdentity.test.tsx +++ b/packages/plugin-grid/src/__tests__/columnIdentity.test.tsx @@ -28,7 +28,13 @@ import { ObjectGrid } from '../ObjectGrid'; import { ActionProvider } from '@object-ui/react'; const makeDataSource = () => ({ - find: vi.fn().mockResolvedValue({ value: [], '@odata.count': 0 }), + // `ObjectGrid` reads `result.data` for rows and `result.total` for the + // match count (ObjectGrid.tsx). This double used to answer + // `{ value: [], '@odata.count': 0 }` — neither key is read, so it read as + // though it supplied rows while supplying none. Inert only while the array + // was empty; the first person to put a row in it would have been handed + // zero silently (objectui#6917). + find: vi.fn().mockResolvedValue({ data: [], total: 0 }), findOne: vi.fn(), create: vi.fn(), update: vi.fn(), diff --git a/packages/plugin-grid/src/__tests__/gridFindEnvelope.contractEnvelope-6917.test.tsx b/packages/plugin-grid/src/__tests__/gridFindEnvelope.contractEnvelope-6917.test.tsx new file mode 100644 index 0000000000..007a1a5549 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/gridFindEnvelope.contractEnvelope-6917.test.tsx @@ -0,0 +1,105 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The shape `ObjectGrid` actually reads out of a `find()` answer — pinned so + * this package's test doubles cannot drift away from it again + * (objectui#6917). + * + * ## The hazard this file closes + * + * Two doubles in this package answered `{ value: [], '@odata.count': 0 }`. + * `ObjectGrid` reads `result.data` for rows and `result.total` for the match + * count, and NEITHER of those keys is one it reads. So the fixtures read as + * though they supplied rows and supplied none. + * + * That was inert only because the arrays were EMPTY — both suites assert on + * `$select` projection and on column identity, neither on rendered rows, so + * zero rows was the answer they wanted anyway. The first person to put a row + * in one would have been handed nothing, silently, with a fixture that reads + * as if it had supplied one. Repaired to `{ data: [], total: 0 }` on this card. + * + * A fixture is not covered by the suites it feeds — it is the input to them — + * so the repair needs a pin of its own, and this is it: the contract stated + * directly, against ROWS rather than against an empty array, which is the one + * condition under which the two spellings differ. + * + * ## Why `value` is refused here and KEPT in `packages/fields` + * + * Not a contradiction — the two are different consumers, and objectui#6917's + * central rule is that a measurement is seam-local. `@object-ui/core`'s + * `extractRecords` accepts `value` because eight in-repo `find()` producers + * still emit it into that helper. `ObjectGrid` does not go through + * `extractRecords`; it reads `result.data` directly and always has. Pinning + * what THIS consumer reads is what keeps its fixtures honest, and it is also + * why the repair below is to the fixtures and not to `ObjectGrid`: + * + * ⛔ do NOT "fix" this by teaching `ObjectGrid` to read `value`, and ⛔ do NOT + * widen `QueryResult` to bless `value` or `@odata.count` — a published-type + * change and the maintainer's call (the floor objectui#6726, #6840 and #6839 + * all held). `'@odata.count'` in particular is a WIRE spelling that both + * adapters' `normalizeQueryResult` already fold into `total` below the fold. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { ActionProvider } from '@object-ui/react'; + +const ROW = { id: 'c1', name: 'Ada Lovelace' }; + +const makeDataSource = (answer: unknown) => ({ + find: vi.fn().mockResolvedValue(answer), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'contacts', + fields: { name: { type: 'text' } }, + }), +}); + +async function rowRenders(answer: unknown): Promise { + const ds = makeDataSource(answer); + const schema: any = { type: 'object-grid', objectName: 'contacts', columns: [{ field: 'name' }] }; + render( + + + , + ); + // Wait on the FETCH, not on the row: waiting on the row would make every + // refusal case a timeout rather than a reading, and would take the same + // amount of time whether the grid was empty or merely slow. + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + await new Promise((r) => setTimeout(r, 0)); + return screen.queryAllByText('Ada Lovelace').length > 0; +} + +describe('ObjectGrid find() envelope — the shape its fixtures must speak (objectui#6917)', () => { + beforeEach(() => vi.clearAllMocks()); + + it("reads the contract's `data` member — the shape the repaired doubles use", async () => { + expect(await rowRenders({ data: [ROW], total: 1 })).toBe(true); + }); + + it('does NOT read `value` — the shape the doubles used to answer with', async () => { + // This is the whole defect, made visible: identical fixture, one row in it, + // and the grid is handed nothing. + expect(await rowRenders({ value: [ROW], '@odata.count': 1 })).toBe(false); + }); + + it('does NOT read `records` either — folded into `data` below the adapter', async () => { + // Caricature guard: a grid that rendered the first array it found under any + // key would show the row here, and both refusals above would be vacuous. + expect(await rowRenders({ records: [ROW] })).toBe(false); + }); +}); diff --git a/packages/plugin-grid/src/__tests__/recordIdFieldProjection.test.tsx b/packages/plugin-grid/src/__tests__/recordIdFieldProjection.test.tsx index ea07f53021..2f7020d193 100644 --- a/packages/plugin-grid/src/__tests__/recordIdFieldProjection.test.tsx +++ b/packages/plugin-grid/src/__tests__/recordIdFieldProjection.test.tsx @@ -45,7 +45,13 @@ const OBJECT_FIELDS = { }; const makeDataSource = (actions?: unknown[]) => ({ - find: vi.fn().mockResolvedValue({ value: [], '@odata.count': 0 }), + // `ObjectGrid` reads `result.data` for rows and `result.total` for the + // match count (ObjectGrid.tsx). This double used to answer + // `{ value: [], '@odata.count': 0 }` — neither key is read, so it read as + // though it supplied rows while supplying none. Inert only while the array + // was empty; the first person to put a row in it would have been handed + // zero silently (objectui#6917). + find: vi.fn().mockResolvedValue({ data: [], total: 0 }), findOne: vi.fn(), create: vi.fn(), update: vi.fn(), diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index bdd7f0c78c..ef7cf802d7 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -2073,8 +2073,11 @@ export const ListView = React.forwardRef(({ // With a known total the grid pages server-side, so the "showing first N" // cap warning no longer applies; without one we fall back to the old // single-window behaviour and keep the warning. + // `total` only — the ONE count member `QueryResult` declares. The + // `count` arm that used to sit behind it is gone on a measured zero + // (objectui#6917): no `find()` producer in the repo emits `count`. const rawTotal = (results && typeof results === 'object') - ? ((results as any).total ?? (results as any).count) + ? (results as any).total : undefined; const knownTotal = typeof rawTotal === 'number' ? rawTotal : null; setServerTotal(paginate ? knownTotal : null); diff --git a/packages/plugin-list/src/__tests__/ListView.contractEnvelope-6917.test.tsx b/packages/plugin-list/src/__tests__/ListView.contractEnvelope-6917.test.tsx new file mode 100644 index 0000000000..7b7e65703d --- /dev/null +++ b/packages/plugin-list/src/__tests__/ListView.contractEnvelope-6917.test.tsx @@ -0,0 +1,153 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * ListView reads its match total as `QueryResult` DECLARES it — and does NOT + * read `count` (objectui#6917 arm B, following objectui#6840). + * + * `QueryResult` (`@object-ui/types`) declares exactly one count member: + * `total`. Before this pin the fetch effect read + * + * (results as any).total ?? (results as any).count + * + * — the same arm objectui#6840 deleted from `RelatedCountStore`, in a module + * that card was not fenced to touch. It decides a rendered number (the record + * count bar) and, through `serverTotal`, whether the grid pages server-side at + * all. + * + * MEASURED on this tree for objectui#6917 — its OWN producer census. #6840's + * zero is seam-local and must not be carried here: the same sweep read 0 + * producers for `value` at `ObjectView`'s seam and 5 at `extractRecords`' + * seam (objectui#6839) in one and the same pass. + * + * CELL every `find()` producer body in the repo, bracket-scanned + * through chained calls ......................... 592 producers + * CONTROL `data` emitted as an envelope member ......... 312 producers + * CONTROL `total` emitted as an envelope member ......... 150 producers + * SUBJECT `count` emitted as an envelope member ........ 0 producers + * + * The controls sit on the JOIN — same cell, same pass, same extraction — so + * the zero is a reading. Superset sweep too: of the 418 files holding a + * `find()` producer, 25 contain the token `count:` anywhere and not one is an + * envelope member (the DataSource's own sibling `count()` method, row fields, + * i18n interpolation parameters, aggregate function names, React state, DOM + * assertions, comments, and objectui#6840's own refusal pin). + * + * Both adapters' `normalizeQueryResult` fold `count` into `total` below the + * fold; ListView calls `dataSource.find()` strictly above it, so the arm was + * unreachable — exactly where a non-conforming producer keeps working + * unrejected (AGENTS.md #0.1). + * + * ⛔ The fix is the deletion, NOT widening `QueryResult` to bless `count`. + * + * ⚠️ NOT fixed here, and deliberately: this same fetch effect ALSO unwraps rows + * with `data → records → value`. That ladder is the `records`/`value` tolerance + * set owned by objectui#6839's `extractRecords` family, not this card's + * `count`/`value` residue, and objectui#6917 does not enumerate it. Reported + * for #6839 rather than half-repaired here. + */ + +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { render, cleanup, waitFor, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { I18nProvider, SchemaRendererProvider } from '@object-ui/react'; +import { ListView } from '../ListView'; +import type { ListViewSchema } from '@object-ui/types'; + +const OBJECT = 'showcase_contact'; +const PAGE_SIZE = 10; +/** Three rows — fewer than the page size, so the row count and any server + * total are free to disagree and the disagreement is the whole measurement. */ +const ROWS = [ + { id: 'c-0', name: 'Ada' }, + { id: 'c-1', name: 'Grace' }, + { id: 'c-2', name: 'Alan' }, +]; + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); +afterEach(cleanup); + +function makeDataSource(answer: unknown) { + return { + find: vi.fn(async () => answer), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: async (name: string) => ({ + name, + fields: { id: { type: 'text' }, name: { type: 'text' } }, + }), + } as any; +} + +const listSchema = (): ListViewSchema => ({ + type: 'list-view', + objectName: OBJECT, + fields: ['name'], + // A page size is what turns on ListView's server-paging branch — the branch + // that reads a server total at all. + pagination: { pageSize: PAGE_SIZE }, +} as unknown as ListViewSchema); + +/** + * Render against a `find()` answering `answer` and return the record-count + * bar's text. Scoped by test id rather than queried globally: the digits of a + * total can collide with row text, and `queryByText` throws on multiple + * matches exactly as it does on none. + */ +async function recordCountText(answer: unknown): Promise { + const ds = makeDataSource(answer); + const { getByTestId } = render( + // Without an English provider the `{{count}}` interpolation never runs and + // the raw template reaches the DOM, which would make the count unassertable. + + + + + , + ); + let bar!: HTMLElement; + await waitFor(() => { + bar = getByTestId('record-count-bar'); + expect(within(bar).getByText(/record/)).toBeInTheDocument(); + }); + return within(bar).getByText(/record/).textContent ?? ''; +} + +describe('ListView find() envelope — objectui#6917', () => { + it('still reads the server-side `total` — the contract\'s count member', async () => { + expect(await recordCountText({ data: ROWS, total: 42 })).toBe('42 records'); + }); + + it('does NOT read `count` — falls back to the honest page-local row count', async () => { + // Before the fix this bar read "7 records" off an undeclared key. It now + // reports what it can actually see rather than legitimising a second + // de-facto contract. + expect(await recordCountText({ data: ROWS, count: 7 })).toBe('3 records'); + }); + + it('does NOT let `count` stand in when `total` is absent, even alongside rows', async () => { + // The caricature guard: a reader that returned any number it could find in + // the envelope would answer "7 records" here. + expect(await recordCountText({ data: ROWS, count: 99 })).toBe('3 records'); + }); + + it('`total` still outranks `count` — unchanged, and the control for the two above', async () => { + // Green before AND after the fix: the arm that was always correct. Its + // presence is what makes the refusals above a reading of THIS deletion + // rather than of a ListView that stopped counting. + expect(await recordCountText({ data: ROWS, total: 42, count: 7 })).toBe('42 records'); + }); +});