diff --git a/.changeset/6840-queryresult-count-value-arms.md b/.changeset/6840-queryresult-count-value-arms.md new file mode 100644 index 000000000..82b439e3e --- /dev/null +++ b/.changeset/6840-queryresult-count-value-arms.md @@ -0,0 +1,48 @@ +--- +'@object-ui/components': minor +'@object-ui/plugin-view': minor +--- + +Read `find()` answers as `QueryResult` declares them on two more seams: the +related-count badge store no longer reads `count`, and `ObjectView`'s non-grid +unwrap no longer reads `value` (objectui#6840, following objectui#6726). + +`QueryResult` (`@object-ui/types`) declares exactly one rows member, `data`, and +exactly one count member, `total`. objectui#6726 removed the `records` arm from +seven consumers after measuring that nothing produces it at the +`DataSource.find()` seam, and deliberately left two arms reading *other* +undeclared keys standing in the same expressions — because it had measured +`records` and not them. Its own pin says so in as many words. This is the +measurement it deferred. + +- `related-count-store.ts` dropped `typeof res?.count === 'number' ? res.count`, + which was tried second and *ahead of the contract's `data`* — the same + precedence inversion objectui#5945/#6726 were filed about, on the key those + cards did not measure. The store already asks the server for the count with + `$count: true` and reads it back as `total`, which is a declared member. +- `ObjectView.tsx` dropped the ladder's last branch, + `Array.isArray((results as any).value)`. Unlike the store's arm this was a + pure fallback, not an inversion — `data` was already read first. + +Both keys are the raw-payload spellings that `ObjectStackAdapter.normalizeQueryResult` +and `ApiDataSource.normalizeQueryResult` already fold into `total` / `data` +*below* this seam, so nothing above it emits them. A producer sweep over every +`find()` definition body in the repo (452 bodies / 331 files, bracket-scanned so +a body cannot leak into sibling properties) found `count` emitted **0** times, +against controls `total` (85 hits / 75 files) and `data` (135 hits / 103 files) +drawn from the same cells. Narrowed to the 25 bodies reachable by `ObjectView`, +`value` is emitted **0** times against the same controls (6 and 6). + +No producer changes behaviour, because there is no producer; what changes is +that a non-conforming one is now refused instead of silently absorbed — which +is the point (AGENTS.md #0.1). Each module gets its own refusal pin +(`*.contractEnvelope-6840.*`), and the pins keep the live arms green alongside +the deleted ones, because live and dead is the whole distinction. + +Deliberately not done: `QueryResult` is **not** widened to bless `count` or +`value`. That is a published-type change and the maintainer's call, the same +floor objectui#6726 respected. + +The `value` reading here is **seam-local** and does not transfer: at +`extractRecords` (`@object-ui/core`, objectui#6839) the same key is still LIVE — +five test doubles in plugin-calendar / plugin-kanban emit it today. diff --git a/packages/components/src/__tests__/related-count-store.contractEnvelope-6840.test.ts b/packages/components/src/__tests__/related-count-store.contractEnvelope-6840.test.ts new file mode 100644 index 000000000..06bd1eb73 --- /dev/null +++ b/packages/components/src/__tests__/related-count-store.contractEnvelope-6840.test.ts @@ -0,0 +1,108 @@ +/** + * 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 `count` (objectui#6840, following objectui#6726). + * + * `QueryResult` (`@object-ui/types`) declares exactly one count member: + * `total`. Before this pin the store's count resolution read + * + * typeof res?.total === 'number' ? res.total + * : typeof res?.count === 'number' ? res.count : ... + * + * — `count` SECOND, but still ahead of the contract's rows member `data`. That + * is the same precedence inversion objectui#6726 removed for `records`, on the + * key objectui#6726 did not measure, and this module is where it decides a + * rendered number: the tab-strip badge on a record detail ("Contacts (12)"). + * + * MEASURED on this tree (objectui#6840) — a producer sweep of its own, NOT + * objectui#6726's `records` numbers, which say nothing about this key: + * + * cell every `find()` DEFINITION body in the repo (452 bodies / 331 files), + * bracket-scanned so a body cannot leak into sibling properties + * SUBJECT `count` emitted as an envelope key .... 0 hits / 0 files + * CONTROL `total` emitted as an envelope key .... 85 hits / 75 files + * CONTROL `data` emitted as an envelope key .... 135 hits / 103 files + * + * The controls sit on the JOIN — the same cell the zero lives in, extracted by + * the same pass — so the zero is a reading and not an unmeasured cell. + * + * `count` IS read below the adapter, on the raw payload, which is exactly why + * nothing re-emits it above: `ObjectStackAdapter.normalizeQueryResult` + * (`data-objectstack/src/index.ts:3382`) and `ApiDataSource.normalizeQueryResult` + * (`core/src/adapters/ApiDataSource.ts:402`) both fold `count` into `total` + * before returning. The store's `probe` is bound strictly ABOVE that fold — + * `containers.tsx` hands it `(object, query) => ds.find(object, query)` — so the + * arm was unreachable, and an unreachable tolerant arm is precisely where a + * non-conforming producer would keep working unrejected (AGENTS.md #0.1). + * + * ⛔ The fix is the deletion, NOT widening `QueryResult` to bless `count` — + * that is a published-type change and the maintainer's call (same floor as + * objectui#6726). + * + * The live arms are pinned here as well, because live and dead is the whole + * distinction: `total` (what `$count: true` asks the server for), `data` + * (the contract's rows member), and the bare array. + */ + +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#6840)', () => { + 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 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 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 `count` — not a QueryResult member', async () => { + const probe = vi.fn(async () => ({ count: 7 }) as never); + // Before the fix this returned 7. 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 `count` OUTRANK `data` — the precedence inversion itself', async () => { + // The sharp end: both members present and disagreeing. `data` is the + // contract's, so 2 is the only correct answer; the pre-fix order answered 7. + const probe = vi.fn(async () => ({ count: 7, data: [{ id: 'a' }, { id: 'b' }] }) as never); + expect(await RelatedCountStore.fetch(probe, 'contact', 'account_id', 'A1')).toBe(2); + }); + + it('does NOT let `count` OUTRANK a bare array either', async () => { + // A bare array carries no `count`; this case exists so the deletion is + // pinned on BOTH live rows shapes, not only the envelope one. + const probe = vi.fn(async () => ROWS as never); + expect(await RelatedCountStore.fetch(probe, 'contact', 'account_id', 'A1')).toBe(3); + }); + + it('`total` still outranks `count` — unchanged, and the control for the two above', async () => { + // Green before AND after the fix: it is the arm that was always correct. + // Its presence is what makes the four refusals above a reading of THIS + // deletion rather than of a store that stopped counting. + const probe = vi.fn(async () => ({ total: 42, count: 7 }) as never); + expect(await RelatedCountStore.fetch(probe, 'contact', 'account_id', 'A1')).toBe(42); + }); +}); diff --git a/packages/components/src/hooks/related-count-store.ts b/packages/components/src/hooks/related-count-store.ts index 837aaf2ce..b114e864e 100644 --- a/packages/components/src/hooks/related-count-store.ts +++ b/packages/components/src/hooks/related-count-store.ts @@ -102,27 +102,34 @@ async function fetchCount( // 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). + // The count reads `total` — the ONE count member `QueryResult` + // (`@object-ui/types`) declares — then falls back to the ONE rows member + // it declares, `data`. Nothing else. + // + // Two tolerant arms were removed from this expression, each on its own + // measurement. `records` FIRST, ahead of `data` (objectui#5945, + // objectui#6726). Then `count` SECOND, still ahead of `data` + // (objectui#6840): `count` is the RAW-payload spelling that + // `ObjectStackAdapter.normalizeQueryResult` and + // `ApiDataSource.normalizeQueryResult` both fold into `total` BELOW this + // seam, so no producer emits it here — a sweep of all 452 `find()` + // definition bodies in the repo found `count` emitted 0 times against + // controls `total` (85) and `data` (135) drawn from the same cells. + // `ProbeFn` above never declared either spelling — only the `any` here + // let them through. Pinned by + // `related-count-store.contractEnvelope-6726.test.ts` and + // `related-count-store.contractEnvelope-6840.test.ts`; ⛔ do not re-add a + // tolerant arm, and ⛔ do not widen `QueryResult` to bless `records` or + // `count` (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?.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/plugin-view/src/ObjectView.tsx b/packages/plugin-view/src/ObjectView.tsx index 56c822823..ae4d44cdb 100644 --- a/packages/plugin-view/src/ObjectView.tsx +++ b/packages/plugin-view/src/ObjectView.tsx @@ -926,16 +926,26 @@ export const ObjectView: React.FC = ({ 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`. + // declares, and now the only one this ladder reads. Two + // below-the-adapter spellings were removed from it, each on its own + // measurement: `records` (objectui#6726) and then `value` + // (objectui#6840) — the OData spelling that + // `ObjectStackAdapter.normalizeQueryResult` and + // `ApiDataSource.normalizeQueryResult` both fold into `data` BELOW + // this seam, so no producer emits it here. A sweep of the 25 `find()` + // definition bodies reachable by this component found `value` + // emitted 0 times against controls `data` (6) and `total` (6) drawn + // from the same cells. + // + // ⚠️ That zero is SEAM-LOCAL and must not be carried elsewhere: the + // same key is LIVE at `extractRecords` + // (`core/src/utils/extract-records.ts`, objectui#6839), where five + // test doubles in plugin-calendar / plugin-kanban still emit it. + // + // Pinned by `ObjectView.contractEnvelope-6726.test.tsx` and + // `ObjectView.contractEnvelope-6840.test.tsx`. if (Array.isArray((results as any).data)) { items = (results as any).data; - } else if (Array.isArray((results as any).value)) { - items = (results as any).value; } } diff --git a/packages/plugin-view/src/__tests__/ObjectView.contractEnvelope-6840.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.contractEnvelope-6840.test.tsx new file mode 100644 index 000000000..515099017 --- /dev/null +++ b/packages/plugin-view/src/__tests__/ObjectView.contractEnvelope-6840.test.tsx @@ -0,0 +1,145 @@ +/** + * 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 `value` (objectui#6840, following + * objectui#6726). + * + * `QueryResult` (`@object-ui/types`) declares exactly one rows member: `data`. + * objectui#6726 removed the `records` branch from this unwrap ladder and said + * so explicitly in its own pin — "the `value` branch is a below-the-adapter + * spelling by the same argument, and it is left standing here ... filed + * separately rather than fixed on a card that did not measure it." This is that + * card, and this is that measurement. + * + * MEASURED on this tree (objectui#6840) — a producer sweep of its own, NOT + * objectui#6726's `records` numbers, which say nothing about this key: + * + * cell every `find()` DEFINITION body reachable by this seam — the 25 + * bodies in the 24 files that mount plugin-view's `ObjectView` + * SUBJECT `value` emitted as an envelope key .... 0 hits / 0 files + * CONTROL `data` emitted as an envelope key .... 6 hits / 6 files + * CONTROL `total` emitted as an envelope key .... 6 hits / 6 files + * + * Repo-wide the same pass over all 452 `find()` bodies finds `value` emitted 5 + * times, and every one of them is a TEST DOUBLE in `plugin-calendar` (2) or + * `plugin-kanban` (3) — components that unwrap through `extractRecords` + * (`core/src/utils/extract-records.ts`), a DIFFERENT seam, which is + * objectui#6839's subject. None of the five reaches this component. See the PR + * description for the cross-card reading; ⛔ this card's zero must NOT be + * carried over to objectui#6839's seam, where the same key is LIVE. + * + * `value` IS read below the adapter, on the raw payload — the OData spelling — + * which is exactly why nothing re-emits it above: + * `ObjectStackAdapter.normalizeQueryResult` + * (`data-objectstack/src/index.ts:3381`, `resultObj.records || resultObj.value`) + * and `ApiDataSource.normalizeQueryResult` + * (`core/src/adapters/ApiDataSource.ts:398`, the `['data','items','results', + * 'records','value']` envelope loop) both fold it into `data` before returning. + * This block calls `dataSource.find()` strictly ABOVE that fold, so the branch + * was unreachable — and an unreachable tolerant branch is precisely where a + * non-conforming producer would keep working unrejected (AGENTS.md #0.1). + * + * ⛔ The fix is the deletion, NOT widening `QueryResult` to bless `value` — + * that is a published-type change and the maintainer's call (same floor as + * objectui#6726). + * + * NOTE on shape, so the silence is not read as a verdict: unlike + * `related-count-store`'s `count` arm, `value` here was NOT a precedence + * inversion — the ladder already read `data` first, and `value` was its last + * branch. So there is no "does not outrank `data`" case to pin; the reading is + * simply that the fallback is gone. Stating that is the point — a fabricated + * inversion case would have passed both before and after and measured nothing. + * + * 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 `value` 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 (importOriginal) => { + const React = await import('react'); + return { + // Inherit the real export surface, then override only what this pin reads. + // A hand-listed factory freezes the mock at whatever was typed that day, and + // the next export any module in this file's import graph reads at module + // scope kills the file during COLLECTION -- zero failed assertions, tests + // that never ran (objectui#6768 / #6849). + ...(await importOriginal>()), + 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 asValue: Envelope = (rows) => ({ value: 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#6840)', () => { + it("still 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 `value` — not a QueryResult member, it is the OData spelling', async () => { + // Nothing delivered: the envelope was refused. Before the fix the two rows + // above reached the board off a key `QueryResult` does not declare — one + // the adapters below this seam have already folded into `data`. + expect(await deliveredThrough(asValue)).toHaveLength(0); + }); +});