diff --git a/.changeset/6726-find-envelope-records-arms.md b/.changeset/6726-find-envelope-records-arms.md new file mode 100644 index 000000000..d50092f39 --- /dev/null +++ b/.changeset/6726-find-envelope-records-arms.md @@ -0,0 +1,63 @@ +--- +'@object-ui/components': minor +'@object-ui/plugin-detail': minor +'@object-ui/plugin-view': minor +--- + +Seven more `find()` readers now read exactly what `QueryResult` declares — the +`records` arm is removed from each (objectui#6726, following objectui#5945). + +`QueryResult` (`@object-ui/types`) declares exactly one rows member — `data` — +alongside `total`, `page`, `pageSize`, `hasMore`, `cursor` and `metadata`. +`records` is not a member of it. It is the spelling the server envelope and the +client SDK use, which `ObjectStackAdapter.normalizeQueryResult` maps to `data` +before returning — a *below*-the-adapter spelling that had leaked into +above-the-adapter consumers. objectui#5945 removed it from two app-shell +readers; these are the seven the same producer sweep turned up and that card did +not name: + +| module | what it does | +| --- | --- | +| `components/src/hooks/related-count-store.ts` | related-list tab badge count | +| `components/src/renderers/basic/data-list.tsx` | `element:repeater` rows | +| `components/src/renderers/basic/elements.tsx` | `element:number` client-side aggregate | +| `components/src/renderers/basic/record-picker.tsx` | `element:record_picker` options | +| `plugin-detail/src/renderers/record-activity.tsx` | `record:activity` self-fetch | +| `plugin-detail/src/renderers/record-history.tsx` | `record:history` self-fetch | +| `plugin-view/src/ObjectView.tsx` | non-grid (kanban / calendar / gallery / timeline) fetch | + +**One of them was actively wrong, six were dead.** `related-count-store.ts` +read `records` *ahead of* `data` — the precedence inversion objectui#5945 was +filed about — so a `find()` answer carrying both would have been counted from +the key the contract does not declare. The other six read `data` first, so their +`records` arm could never be reached by a conforming producer. A dead tolerant +arm is not harmless: it is where a non-conforming producer keeps working +unrejected, and hardens into a second de-facto contract nobody is checking +(AGENTS.md #0.1). + +**What stops being accepted.** A `find()` answer shaped `{ records: [...] }` +now reads as **no rows** at these seams instead of silently resolving. Every +call site degrades rather than throws: the tab badge counts 0, the repeater and +the picker render their empty state, `element:number` reports 0, the activity +and history feeds render empty, and the non-grid views paint no rows. + +**Nothing produces that shape at this seam today**, which is why this is a +removal rather than a migration. Measured repo-wide over every tracked file: +`ObjectStackAdapter.normalizeQueryResult` CONSUMES the server/SDK `records` +envelope and returns `{ data, total, page, pageSize, hasMore }`; every other +`find()` implementation in the repo (`ApiDataSource`, `ValueDataSource`, the +runner and example mocks, the `@object-ui/types` REST example) returns `data` +or a bare array. The `records` producers that DO exist are on other seams and +are untouched: `ViewDataProvider`'s own `ResolvedData` interface, which declares +`records` legitimately; the raw Cloud HTTP payloads `marketplaceApi.ts` and +`packagedActions.ts` read; and the client-SDK doubles that sit *below* +`normalizeQueryResult`. + +**The bare-array arm is kept** wherever it existed, because it is live: fakes at +these seams answer with a plain array. Each module carries its own pin — +`*.contractEnvelope-6726.*` — asserting the contract read, the live arms, and +the refusal of `records`, so the live and the dead shapes cannot drift into each +other. + +`QueryResult` is **not** widened to bless `records`; that would be a +published-type change and a maintainer decision. diff --git a/packages/components/src/__tests__/related-count-store.contractEnvelope-6726.test.ts b/packages/components/src/__tests__/related-count-store.contractEnvelope-6726.test.ts new file mode 100644 index 000000000..7182d3aaa --- /dev/null +++ b/packages/components/src/__tests__/related-count-store.contractEnvelope-6726.test.ts @@ -0,0 +1,77 @@ +/** + * 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. + */ + +/** + * `RelatedCountStore` reads a `find()` answer as `QueryResult` DECLARES it — + * and does NOT read `records` (objectui#6726, following objectui#5945). + * + * `QueryResult` (`@object-ui/types`) declares exactly one rows member: `data`. + * Before this pin the store's row-count fallback read + * + * Array.isArray(res?.records) ? res.records.length + * : Array.isArray(res?.data) ? res.data.length : ... + * + * — `records` FIRST, ahead of the contract's `data`. That is the same + * precedence inversion objectui#5945 was filed about, and this module is where + * it actually decides a rendered number: the tab-strip badge on a record detail + * ("Contacts (12)"). + * + * MEASURED on this tree, no producer emits `records` at the `DataSource.find()` + * seam this store's `probe` is bound to (`containers.tsx` hands it + * `(object, query) => ds.find(object, query)`): + * `ObjectStackAdapter.normalizeQueryResult` CONSUMES the server/SDK `records` + * envelope and returns `{ data, total, page, pageSize, hasMore }`; every other + * `find()` implementation in the repo returns `data` too. `ProbeFn` itself + * never declared `records` either — only the `res: any` cast let it through. + * + * The two live arms are pinned here as well, because live and dead is the whole + * distinction: `total` (what `$count: true` asks the server for) and the bare + * array (what fakes at this seam really answer with). + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { RelatedCountStore } from '../hooks/related-count-store'; + +/** Three rows, wrapped by whichever envelope the case is measuring. */ +const ROWS = [{ id: 'c1' }, { id: 'c2' }, { id: 'c3' }]; + +beforeEach(() => { + RelatedCountStore._reset(); +}); + +describe('RelatedCountStore — the find() envelope it counts from (objectui#6726)', () => { + it("counts the contract's `data` member", async () => { + const probe = vi.fn(async () => ({ data: ROWS })); + expect(await RelatedCountStore.fetch(probe, 'contact', 'account_id', 'A1')).toBe(3); + }); + + it('still prefers the server-side `total` — the reason `$count: true` is sent', async () => { + const probe = vi.fn(async () => ({ total: 42, data: [{ id: 'c1' }] })); + expect(await RelatedCountStore.fetch(probe, 'contact', 'account_id', 'A1')).toBe(42); + }); + + it('still counts a bare array — the live non-envelope shape fakes answer with', async () => { + const probe = vi.fn(async () => ROWS); + expect(await RelatedCountStore.fetch(probe, 'contact', 'account_id', 'A1')).toBe(3); + }); + + it('does NOT count `records` — not a QueryResult member', async () => { + const probe = vi.fn(async () => ({ records: ROWS }) as never); + // Before the fix this returned 3. The badge now reports the honest "no + // countable answer" 0 rather than legitimising a second de-facto contract. + expect(await RelatedCountStore.fetch(probe, 'contact', 'account_id', 'A1')).toBe(0); + }); + + it('does NOT let `records` OUTRANK `data` — the precedence inversion itself', async () => { + // The sharp end of objectui#5945: both members present and disagreeing. + // `data` is the contract's, so 1 is the only correct answer; the pre-fix + // order answered 3. + const probe = vi.fn(async () => ({ records: ROWS, data: [{ id: 'only' }] }) as never); + expect(await RelatedCountStore.fetch(probe, 'contact', 'account_id', 'A1')).toBe(1); + }); +}); diff --git a/packages/components/src/hooks/related-count-store.ts b/packages/components/src/hooks/related-count-store.ts index 52e2d0a14..837aaf2ce 100644 --- a/packages/components/src/hooks/related-count-store.ts +++ b/packages/components/src/hooks/related-count-store.ts @@ -99,21 +99,30 @@ async function fetchCount( try { // Request the server-side count instead of relying on the page length. // Without `$count: true` most adapters omit `total`, and we'd fall - // back to `records.length` which is capped to `$top: 1` → badge + // back to `data.length` which is capped to `$top: 1` → badge // shows "1" no matter how many rows exist. + // + // The rows fallback reads `data` — the ONE rows member `QueryResult` + // (`@object-ui/types`) declares — and nothing else. It used to try + // `records` FIRST, ahead of `data`: a spelling no producer emits at the + // `DataSource.find()` seam, because `ObjectStackAdapter.normalizeQueryResult` + // maps the server/SDK `records` envelope to `data` before returning + // (objectui#5945, objectui#6726). `ProbeFn` above never declared it + // either — only the `any` here let it through. Pinned by + // `related-count-store.contractEnvelope-6726.test.ts`; ⛔ do not + // re-add a tolerant arm, and ⛔ do not widen `QueryResult` to bless + // `records` (a published-type change, maintainer's call). const res: any = await probe(objectName, { $filter, $top: 1, $count: true }); const total = typeof res?.total === 'number' ? res.total : typeof res?.count === 'number' ? res.count - : Array.isArray(res?.records) - ? res.records.length - : Array.isArray(res?.data) - ? res.data.length - : Array.isArray(res) - ? res.length - : 0; + : Array.isArray(res?.data) + ? res.data.length + : Array.isArray(res) + ? res.length + : 0; const n = typeof total === 'number' ? total : 0; setCount(objectName, relField, parentId, n); return n; diff --git a/packages/components/src/renderers/basic/__tests__/data-list.contractEnvelope-6726.test.tsx b/packages/components/src/renderers/basic/__tests__/data-list.contractEnvelope-6726.test.tsx new file mode 100644 index 000000000..32ffa61f4 --- /dev/null +++ b/packages/components/src/renderers/basic/__tests__/data-list.contractEnvelope-6726.test.tsx @@ -0,0 +1,100 @@ +/** + * 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. + */ + +/** + * `element:repeater` (`data-list.tsx`) reads a `find()` answer as `QueryResult` + * DECLARES it — and does NOT read `records` (objectui#6726). + * + * `QueryResult` (`@object-ui/types`) declares exactly one rows member: `data`. + * The renderer's read was `res?.data ?? res?.records ?? (Array.isArray(res) ? res : [])` + * — `records` sat between the contract's member and the bare-array arm. + * + * MEASURED on this tree, no producer emits `records` at this `DataSource.find()` + * seam: `ObjectStackAdapter.normalizeQueryResult` CONSUMES the server/SDK + * `records` envelope and returns `data`, and every other `find()` in the repo + * returns `data` (or a bare array). So the arm was dead — but a dead arm still + * costs the contract its authority: a raw SDK client handed in where a + * `DataSource` belongs would have kept working, unnoticed and unrejected + * (AGENTS.md #0.1). + * + * Both live arms are pinned alongside it. That is what makes the `records` zero + * a reading rather than a broken harness: the `data` leg and the bare-array leg + * answer with the SAME rows through the SAME mount, so a leg that renders + * nothing rendered nothing because the envelope was refused. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import * as React from 'react'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { AdapterCtx } from '@object-ui/react'; +import { SchemaRenderer } from '@object-ui/react'; +// Registers every `element:*` renderer at module scope, not in a hook +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../../../renderers'; + +afterEach(cleanup); + +const ROWS = [{ id: 'r1', name: 'Ada' }, { id: 'r2', name: 'Grace' }]; + +/** How one case wraps its rows on the way back out of `find()`. */ +type Envelope = (rows: unknown[]) => unknown; + +const asData: Envelope = (rows) => ({ data: rows, total: rows.length }); +const asBareArray: Envelope = (rows) => rows; +const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); + +function mount(envelope: Envelope) { + const adapter = { find: vi.fn(async () => envelope(ROWS)) }; + render( + + + , + ); + return adapter; +} + +/** The rows the block actually painted. */ +const painted = () => + Array.from(screen.queryByTestId('repeater')?.querySelectorAll('li') ?? []).map( + (li) => li.textContent ?? '', + ); + +describe('element:repeater — the find() envelope it reads (objectui#6726)', () => { + it("reads the contract's `data` member", async () => { + const adapter = mount(asData); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + await waitFor(() => expect(screen.queryByTestId('repeater')).toBeTruthy()); + expect(painted().join('|')).toContain('Ada'); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + const adapter = mount(asBareArray); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + await waitFor(() => expect(screen.queryByTestId('repeater')).toBeTruthy()); + expect(painted().join('|')).toContain('Ada'); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + const adapter = mount(asRecords); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + // The empty-state paragraph, not the list: before the fix this painted the + // two rows above. + await waitFor(() => expect(screen.getByText('No records')).toBeTruthy()); + expect(screen.queryByTestId('repeater')).toBeNull(); + }); +}); diff --git a/packages/components/src/renderers/basic/__tests__/element-number.contractEnvelope-6726.test.tsx b/packages/components/src/renderers/basic/__tests__/element-number.contractEnvelope-6726.test.tsx new file mode 100644 index 000000000..4b64c3b57 --- /dev/null +++ b/packages/components/src/renderers/basic/__tests__/element-number.contractEnvelope-6726.test.tsx @@ -0,0 +1,96 @@ +/** + * 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. + */ + +/** + * `element:number` (`elements.tsx`) reads a `find()` answer as `QueryResult` + * DECLARES it — and does NOT read `records` (objectui#6726). + * + * This is the block's client-side aggregate fallback: reached when the adapter + * has no `aggregate()`, it pulls rows through `find()` and counts/sums them + * locally. Its read was + * `res?.data ?? res?.records ?? (Array.isArray(res) ? res : [])` — `records` + * between the contract's one rows member and the bare-array arm. + * + * MEASURED on this tree, no producer emits `records` at this `DataSource.find()` + * seam: `ObjectStackAdapter.normalizeQueryResult` CONSUMES the server/SDK + * `records` envelope and returns `data`; every other `find()` in the repo + * returns `data` or a bare array. + * + * What makes the `records` zero a reading and not a broken harness: the `data` + * leg and the bare-array leg push the SAME three rows through the SAME mount + * and both paint `3`. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import * as React from 'react'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { AdapterCtx, SchemaRenderer } from '@object-ui/react'; +// Registers every `element:*` renderer at module scope, not in a hook +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../../../renderers'; + +afterEach(cleanup); + +const ROWS = [{ id: 'r1' }, { id: 'r2' }, { id: 'r3' }]; + +type Envelope = (rows: unknown[]) => unknown; + +const asData: Envelope = (rows) => ({ data: rows, total: rows.length }); +const asBareArray: Envelope = (rows) => rows; +const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); + +/** + * No `aggregate()` on this adapter ON PURPOSE — that is the branch under test. + * An adapter carrying one would answer from the server and never reach the + * envelope read at all. + */ +function mount(envelope: Envelope) { + const adapter = { find: vi.fn(async () => envelope(ROWS)) }; + const view = render( + + + , + ); + return { adapter, view }; +} + +/** The number the block painted (the '…' placeholder while loading). */ +const painted = (view: ReturnType) => + view.container.querySelector('.tabular-nums')?.textContent ?? ''; + +describe('element:number — the find() envelope its client-side count reads (objectui#6726)', () => { + it("counts the contract's `data` member", async () => { + const { adapter, view } = mount(asData); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + await waitFor(() => expect(painted(view)).toBe('3')); + }); + + it('still counts a bare array — the live non-envelope shape fakes answer with', async () => { + const { adapter, view } = mount(asBareArray); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + await waitFor(() => expect(painted(view)).toBe('3')); + }); + + it('does NOT count `records` — not a QueryResult member', async () => { + const { adapter, view } = mount(asRecords); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + // Zero countable rows, not three: before the fix this metric painted `3` + // off a key the contract does not declare. + await waitFor(() => expect(painted(view)).toBe('0')); + }); +}); diff --git a/packages/components/src/renderers/basic/__tests__/record-picker.contractEnvelope-6726.test.tsx b/packages/components/src/renderers/basic/__tests__/record-picker.contractEnvelope-6726.test.tsx new file mode 100644 index 000000000..6c079a3e1 --- /dev/null +++ b/packages/components/src/renderers/basic/__tests__/record-picker.contractEnvelope-6726.test.tsx @@ -0,0 +1,95 @@ +/** + * 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. + */ + +/** + * `element:record_picker` reads a `find()` answer as `QueryResult` DECLARES it + * — and does NOT read `records` (objectui#6726). + * + * The picker's read was + * `res?.data ?? res?.records ?? (Array.isArray(res) ? res : [])` — `records` + * between the contract's one rows member (`data`) and the bare-array arm. + * + * MEASURED on this tree, no producer emits `records` at this `DataSource.find()` + * seam: `ObjectStackAdapter.normalizeQueryResult` CONSUMES the server/SDK + * `records` envelope and returns `data`; every other `find()` in the repo + * returns `data` or a bare array. + * + * The observable is the picker's own empty-state line, which it renders exactly + * when the query came back with no rows — so the `records` leg is measured by + * what the user would see, not by an internal. The two live legs push the SAME + * rows through the SAME mount and both suppress that line, which is what makes + * the `records` reading a reading. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import * as React from 'react'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { AdapterCtx, SchemaRenderer } from '@object-ui/react'; +// Registers every `element:*` renderer at module scope, not in a hook +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../../../renderers'; + +afterEach(cleanup); + +const ROWS = [ + { id: 'a1', name: 'Acme' }, + { id: 'a2', name: 'Zephyr' }, +]; + +type Envelope = (rows: unknown[]) => unknown; + +const asData: Envelope = (rows) => ({ data: rows, total: rows.length }); +const asBareArray: Envelope = (rows) => rows; +const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); + +function mount(envelope: Envelope) { + const adapter = { + find: vi.fn(async () => envelope(ROWS)), + getObjectSchema: vi.fn(async () => ({ name: 'account', fields: {} })), + }; + render( + + + , + ); + return adapter; +} + +describe('element:record_picker — the find() envelope it reads (objectui#6726)', () => { + it("reads the contract's `data` member — rows offered, no empty-state line", async () => { + const adapter = mount(asData); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + await waitFor(() => expect(screen.getByTestId('record-picker-trigger')).toBeTruthy()); + await waitFor(() => expect(screen.queryByText('No records')).toBeNull()); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + const adapter = mount(asBareArray); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + await waitFor(() => expect(screen.getByTestId('record-picker-trigger')).toBeTruthy()); + await waitFor(() => expect(screen.queryByText('No records')).toBeNull()); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + const adapter = mount(asRecords); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + // The empty-state line, offered to the user: before the fix the picker + // offered the two rows above off a key the contract does not declare. + await waitFor(() => expect(screen.getByText('No records')).toBeTruthy()); + }); +}); diff --git a/packages/components/src/renderers/basic/data-list.tsx b/packages/components/src/renderers/basic/data-list.tsx index d4e29868a..08b8a66f6 100644 --- a/packages/components/src/renderers/basic/data-list.tsx +++ b/packages/components/src/renderers/basic/data-list.tsx @@ -132,7 +132,15 @@ function RepeaterRenderer({ schema }: { schema: any }) { if (props.sort) query.$orderby = props.sort; if (props.limit) query.$top = props.limit; const res = await adapter.find(props.object, query); - const data: any[] = res?.data ?? res?.records ?? (Array.isArray(res) ? res : []); + // `data` is the ONE rows member `QueryResult` (`@object-ui/types`) + // declares; the bare-array arm stays because fakes at this seam really + // do answer with a plain array. A `res?.records` arm sat between them + // until objectui#6726 — a below-the-adapter spelling + // (`ObjectStackAdapter.normalizeQueryResult` maps the server/SDK + // `records` envelope to `data` before returning), so no producer emits + // it here and the arm bought nothing. Pinned by + // `data-list.contractEnvelope-6726.test.tsx`. + const data: any[] = res?.data ?? (Array.isArray(res) ? res : []); if (!cancelled) setRows(data); } catch (e: any) { if (!cancelled) setError(e?.message ?? 'Failed to load'); diff --git a/packages/components/src/renderers/basic/elements.tsx b/packages/components/src/renderers/basic/elements.tsx index b2e2cf7d8..1921b6957 100644 --- a/packages/components/src/renderers/basic/elements.tsx +++ b/packages/components/src/renderers/basic/elements.tsx @@ -417,7 +417,15 @@ function ElementNumberRenderer({ schema }: { schema: any }) { // Last-resort: pull all rows and aggregate client-side. Costly // but matches the chart renderer fallback path. const res = await adapter.find(props.object, props.filter ? { $filter: props.filter } : undefined); - const records: any[] = res?.data ?? res?.records ?? (Array.isArray(res) ? res : []); + // `data` is the ONE rows member `QueryResult` (`@object-ui/types`) + // declares; the bare-array arm stays because fakes at this seam + // really do answer with a plain array. A `res?.records` arm sat + // between them until objectui#6726 — a below-the-adapter spelling + // (`ObjectStackAdapter.normalizeQueryResult` maps the server/SDK + // `records` envelope to `data` before returning), so no producer + // emits it here. Pinned by + // `element-number.contractEnvelope-6726.test.tsx`. + const records: any[] = res?.data ?? (Array.isArray(res) ? res : []); let v: number | null = null; if (props.aggregate === 'count') v = records.length; else if (props.field) { diff --git a/packages/components/src/renderers/basic/record-picker.tsx b/packages/components/src/renderers/basic/record-picker.tsx index 7782fd6da..4b7f69118 100644 --- a/packages/components/src/renderers/basic/record-picker.tsx +++ b/packages/components/src/renderers/basic/record-picker.tsx @@ -136,7 +136,15 @@ function ElementRecordPickerRenderer({ schema }: { schema: any }) { if (sort) query.$orderby = sort; if (limit) query.$top = limit; const res = await adapter.find(object, query); - const data: any[] = res?.data ?? res?.records ?? (Array.isArray(res) ? res : []); + // `data` is the ONE rows member `QueryResult` (`@object-ui/types`) + // declares; the bare-array arm stays because fakes at this seam really + // do answer with a plain array. A `res?.records` arm sat between them + // until objectui#6726 — a below-the-adapter spelling + // (`ObjectStackAdapter.normalizeQueryResult` maps the server/SDK + // `records` envelope to `data` before returning), so no producer emits + // it here. Pinned by + // `record-picker.contractEnvelope-6726.test.tsx`. + const data: any[] = res?.data ?? (Array.isArray(res) ? res : []); if (!cancelled) setRows(data); } catch (e: any) { if (!cancelled) setError(e?.message ?? 'Failed to load'); diff --git a/packages/plugin-detail/src/renderers/__tests__/record-activity.contractEnvelope-6726.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-activity.contractEnvelope-6726.test.tsx new file mode 100644 index 000000000..8727cb9ce --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/record-activity.contractEnvelope-6726.test.tsx @@ -0,0 +1,96 @@ +/** + * 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. + */ + +/** + * `record:activity` reads its `sys_activity` self-fetch as `QueryResult` + * DECLARES it — and does NOT read `records` (objectui#6726). + * + * `QueryResult` (`@object-ui/types`) declares exactly one rows member: `data`. + * The renderer's read was `res?.data ?? res?.records ?? []`. + * + * MEASURED on this tree, no producer emits `records` at this `DataSource.find()` + * seam: `ObjectStackAdapter.normalizeQueryResult` CONSUMES the server/SDK + * `records` envelope and returns `data` before returning, and every other + * `find()` implementation in the repo returns `data` too. So the arm was dead + * rather than actively wrong — but a dead arm is still a second de-facto + * contract nobody is checking (AGENTS.md #0.1). + * + * Note the shape here has no bare-array arm and never did: this seam is fed by + * `RecordContextValue.dataSource`, and both plugin-detail self-fetchers read + * only the envelope. The `data` leg below is what makes the `records` leg a + * reading — same rows, same mount, one paints the feed and one does not. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { RecordContextProvider } from '@object-ui/react'; +import { RecordActivityRenderer } from '../record-activity'; + +const EMPTY_FEED = 'No activity recorded'; + +/** Rows as `sys_activity` returns them for one record. */ +const ROWS = [ + { + id: 'act-1', + type: 'updated', + summary: 'Stage: draft to qualified', + timestamp: '2026-01-02T00:00:00.000Z', + actor_name: 'Grace', + }, + { + id: 'act-2', + type: 'system', + summary: 'Assignment rule ran', + timestamp: '2026-01-05T00:00:00.000Z', + }, +]; + +/** How one case wraps its rows on the way back out of `find()`. */ +type Envelope = (rows: unknown[]) => unknown; + +const asData: Envelope = (rows) => ({ data: rows, total: rows.length }); +const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); + +function mount(envelope: Envelope) { + const dataSource = { find: vi.fn(async () => envelope(ROWS)) } as any; + render( + + + , + ); + return dataSource; +} + +beforeEach(() => { + cleanup(); +}); + +describe('record:activity — the find() envelope it reads (objectui#6726)', () => { + it("reads the contract's `data` member", async () => { + const dataSource = mount(asData); + await waitFor(() => expect(dataSource.find).toHaveBeenCalled()); + expect(await screen.findByText('Stage: draft to qualified')).toBeTruthy(); + expect(screen.getByText('Assignment rule ran')).toBeTruthy(); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + const dataSource = mount(asRecords); + await waitFor(() => expect(dataSource.find).toHaveBeenCalled()); + // An empty feed, honestly: the block probed and the envelope was refused. + // Before the fix both rows above rendered off a key `QueryResult` does not + // declare. + expect(await screen.findByText(EMPTY_FEED)).toBeTruthy(); + expect(screen.queryByText('Stage: draft to qualified')).toBeNull(); + }); +}); diff --git a/packages/plugin-detail/src/renderers/__tests__/record-history.contractEnvelope-6726.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-history.contractEnvelope-6726.test.tsx new file mode 100644 index 000000000..270fbfb83 --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/record-history.contractEnvelope-6726.test.tsx @@ -0,0 +1,93 @@ +/** + * 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. + */ + +/** + * `record:history` reads its `sys_activity` self-fetch as `QueryResult` + * DECLARES it — and does NOT read `records` (objectui#6726). + * + * `QueryResult` (`@object-ui/types`) declares exactly one rows member: `data`. + * The renderer's read was `res?.data ?? res?.records ?? []`. + * + * MEASURED on this tree, no producer emits `records` at this `DataSource.find()` + * seam: `ObjectStackAdapter.normalizeQueryResult` CONSUMES the server/SDK + * `records` envelope and returns `data`; every other `find()` in the repo + * returns `data` too. The arm was dead — and a dead tolerant arm is exactly + * where a non-conforming producer would keep working unrejected. + * + * This is a SEPARATE pin from its sibling `record:activity`, deliberately: one + * blanket "nothing reads `records`" assertion would stay green with either + * module repaired wrongly. Each module states its own reading. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { RecordContextProvider } from '@object-ui/react'; +import { RecordHistoryRenderer } from '../record-history'; + +const EMPTY_HISTORY = 'No history yet'; + +/** `sys_activity` rows whose `type` is in the history set. */ +const ROWS = [ + { + id: 'h-1', + type: 'updated', + summary: 'Owner changed to Ada', + timestamp: '2026-01-02T00:00:00.000Z', + actor_name: 'Grace', + }, + { + id: 'h-2', + type: 'created', + summary: 'Record created', + timestamp: '2026-01-01T00:00:00.000Z', + actor_name: 'Ada', + }, +]; + +type Envelope = (rows: unknown[]) => unknown; + +const asData: Envelope = (rows) => ({ data: rows, total: rows.length }); +const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); + +function mount(envelope: Envelope) { + const dataSource = { find: vi.fn(async () => envelope(ROWS)) } as any; + render( + + + , + ); + return dataSource; +} + +beforeEach(() => { + cleanup(); +}); + +describe('record:history — the find() envelope it reads (objectui#6726)', () => { + it("reads the contract's `data` member", async () => { + const dataSource = mount(asData); + await waitFor(() => expect(dataSource.find).toHaveBeenCalled()); + expect(await screen.findByText('Owner changed to Ada')).toBeTruthy(); + expect(screen.getByText('Record created')).toBeTruthy(); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + const dataSource = mount(asRecords); + await waitFor(() => expect(dataSource.find).toHaveBeenCalled()); + // The empty timeline, honestly: before the fix both rows above rendered off + // a key `QueryResult` does not declare. + expect(await screen.findByText(EMPTY_HISTORY)).toBeTruthy(); + expect(screen.queryByText('Owner changed to Ada')).toBeNull(); + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-activity.tsx b/packages/plugin-detail/src/renderers/record-activity.tsx index 5089d90e7..acd360df6 100644 --- a/packages/plugin-detail/src/renderers/record-activity.tsx +++ b/packages/plugin-detail/src/renderers/record-activity.tsx @@ -183,7 +183,13 @@ export const RecordActivityRenderer: React.FC = ({ ) .then((res: any) => { if (cancelled) return; - const raw: unknown = res?.data ?? res?.records ?? []; + // `data` is the ONE rows member `QueryResult` (`@object-ui/types`) + // declares. A `res?.records` arm sat behind it until objectui#6726 — a + // below-the-adapter spelling (`ObjectStackAdapter.normalizeQueryResult` + // maps the server/SDK `records` envelope to `data` before returning), + // so no producer emits it at this `DataSource.find()` seam and the arm + // was dead. Pinned by `record-activity.contractEnvelope-6726.test.tsx`. + const raw: unknown = res?.data ?? []; const rows: SysActivityRow[] = Array.isArray(raw) ? (raw as SysActivityRow[]) : []; const mapped: FeedItem[] = []; for (const row of rows) { diff --git a/packages/plugin-detail/src/renderers/record-history.tsx b/packages/plugin-detail/src/renderers/record-history.tsx index b9ef6adf8..05d501040 100644 --- a/packages/plugin-detail/src/renderers/record-history.tsx +++ b/packages/plugin-detail/src/renderers/record-history.tsx @@ -87,7 +87,13 @@ export const RecordHistoryRenderer: React.FC = ({ ) .then((res: any) => { if (cancelled) return; - const rows: any[] = res?.data ?? res?.records ?? []; + // `data` is the ONE rows member `QueryResult` (`@object-ui/types`) + // declares. A `res?.records` arm sat behind it until objectui#6726 — a + // below-the-adapter spelling (`ObjectStackAdapter.normalizeQueryResult` + // maps the server/SDK `records` envelope to `data` before returning), + // so no producer emits it at this `DataSource.find()` seam and the arm + // was dead. Pinned by `record-history.contractEnvelope-6726.test.tsx`. + const rows: any[] = res?.data ?? []; const mapped: HistoryEntry[] = rows .filter((r) => HISTORY_TYPES.has(r?.type)) .map((r) => { diff --git a/packages/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx index 9a27f5d17..56c822823 100644 --- a/packages/plugin-view/src/ObjectView.tsx +++ b/packages/plugin-view/src/ObjectView.tsx @@ -925,10 +925,15 @@ export const ObjectView: React.FC = ({ if (Array.isArray(results)) { items = results; } else if (results && typeof results === 'object') { + // `data` is the ONE rows member `QueryResult` (`@object-ui/types`) + // declares. A `records` branch sat between `data` and `value` until + // objectui#6726 — a below-the-adapter spelling + // (`ObjectStackAdapter.normalizeQueryResult` maps the server/SDK + // `records` envelope to `data` before returning), so no producer + // emits it at this `DataSource.find()` seam and the branch was dead. + // Pinned by `ObjectView.contractEnvelope-6726.test.tsx`. if (Array.isArray((results as any).data)) { items = (results as any).data; - } else if (Array.isArray((results as any).records)) { - items = (results as any).records; } else if (Array.isArray((results as any).value)) { items = (results as any).value; } diff --git a/packages/plugin-view/src/__tests__/ObjectView.contractEnvelope-6726.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.contractEnvelope-6726.test.tsx new file mode 100644 index 000000000..810d95ce6 --- /dev/null +++ b/packages/plugin-view/src/__tests__/ObjectView.contractEnvelope-6726.test.tsx @@ -0,0 +1,109 @@ +/** + * 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. + */ + +/** + * `ObjectView`'s non-grid fetch reads a `find()` answer as `QueryResult` + * DECLARES it — and does NOT read `records` (objectui#6726). + * + * `QueryResult` (`@object-ui/types`) declares exactly one rows member: `data`. + * This block's unwrap was a four-branch ladder — bare array, `data`, `records`, + * `value` — and the `records` branch sat between the contract's member and the + * OData one. + * + * MEASURED on this tree, no producer emits `records` at this `DataSource.find()` + * seam: `ObjectStackAdapter.normalizeQueryResult` CONSUMES the server/SDK + * `records` envelope and returns `{ data, total, page, pageSize, hasMore }` + * before returning, and every other `find()` implementation in the repo returns + * `data` or a bare array. The branch was unreachable in practice — and an + * unreachable tolerant branch is precisely where a non-conforming producer + * would keep working unrejected (AGENTS.md #0.1). + * + * OUT OF THIS CARD'S FENCE, recorded so the silence is not read as a verdict: + * the `value` branch is a below-the-adapter spelling by the same argument, and + * it is left standing here. objectui#6726 names `records`; `value` is filed + * separately rather than fixed on a card that did not measure it. + * + * The rows reach the child as `data={data}`, so that prop is what this pin + * reads. The `data` and bare-array legs push the SAME rows through the SAME + * mount, which is what makes the `records` leg a reading rather than a mount + * that never rendered. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, waitFor, cleanup } from '@testing-library/react'; +import { ObjectView } from '../ObjectView'; +import type { ObjectViewSchema } from '@object-ui/types'; + +/** Every `data` prop the view handed to SchemaRenderer, in order. */ +const delivered: unknown[][] = []; + +vi.mock('@object-ui/react', async () => { + const React = await import('react'); + return { + SchemaRenderer: ({ data }: any) => { + if (Array.isArray(data)) delivered.push(data); + return
; + }, + SchemaRendererContext: React.createContext(null), + subscribeDataChanges: () => () => {}, + notifyDataChanged: () => {}, + }; +}); +vi.mock('@object-ui/plugin-grid', () => ({ ObjectGrid: () =>
})); +vi.mock('@object-ui/plugin-form', () => ({ ObjectForm: () =>
})); + +const ROWS = [{ id: 'r1', name: 'Ada' }, { id: 'r2', name: 'Grace' }]; + +/** How one case wraps its rows on the way back out of `find()`. */ +type Envelope = (rows: unknown[]) => unknown; + +const asData: Envelope = (rows) => ({ data: rows, total: rows.length }); +const asBareArray: Envelope = (rows) => rows; +const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); + +async function deliveredThrough(envelope: Envelope): Promise { + delivered.length = 0; + const ds: any = { + find: vi.fn().mockResolvedValue(envelope(ROWS)), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ name: 'store', fields: {} }), + }; + render( + , + ); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + await waitFor(() => expect(delivered.length).toBeGreaterThan(0)); + return delivered[delivered.length - 1]; +} + +beforeEach(() => { + cleanup(); +}); + +describe('ObjectView — the find() envelope its non-grid fetch reads (objectui#6726)', () => { + it("reads the contract's `data` member", async () => { + await waitFor(async () => expect(await deliveredThrough(asData)).toHaveLength(2)); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await deliveredThrough(asBareArray)).toHaveLength(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Nothing delivered: the envelope was refused. Before the fix the two rows + // above reached the board off a key `QueryResult` does not declare. + expect(await deliveredThrough(asRecords)).toHaveLength(0); + }); +});