From 5dc25e3f5aab482f1d066afdfd6ec824a952ff13 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 22:48:13 +0000 Subject: [PATCH] fix(core): read find() answers as QueryResult declares them in extractRecords MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the `records` arm from the shared `extractRecords` normaliser and pin the contract read per module. `QueryResult` (`@object-ui/types`) declares exactly one rows member, `data`. The ladder was `array -> records -> data -> value`, i.e. the undeclared spelling AHEAD of the contract's own member — the precedence inversion objectui#5945 was filed about and objectui#6726 repaired by hand at seven other seams. `records` is the below-the-adapter spelling that `ObjectStackAdapter.normalizeQueryResult` and `ApiDataSource .normalizeQueryResult` already fold into `data` below every consumer. Reach re-derived on this tree: ten call sites in nine packages. Seven direct (ObjectChart x2, ObjectDataTable, ObjectPivotTable, ObjectGantt, ObjectKanban, ObjectTimeline) and four through `applyNonGridRowCeiling` (`@object-ui/react`), itself a published export and a sink of its own (ObjectCalendar, ObjectGantt's rows, ObjectMap, ObjectTree). Producer measurement per consumer: no `find()` in those nine packages, nor in the apps and examples mounting them, emits a `records` envelope. Control — the same sweep finds `records` envelopes on other seams, including one live `find()` double at `plugin-list`'s ObjectGallery, a consumer with its own unwrap ladder, untouched here. The `value` arm stays: five `find()` doubles feed it today (plugin-kanban x3, plugin-calendar x2), and objectui#6840 explicitly refused to transfer its own zero to this seam. `QueryResult` is NOT widened. One refusal pin per module (`*.contractEnvelope-6839.*`), each keeping the live arms green alongside the deleted one, plus a direct pin on the helper for the precedence question a per-module render pin cannot ask. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .../6839-extract-records-records-arm.md | 56 +++++ ...ract-records.contractEnvelope-6839.test.ts | 112 ++++++++++ packages/core/src/utils/extract-records.ts | 71 ++++++- ...ectCalendar.contractEnvelope-6839.test.tsx | 157 ++++++++++++++ ...ObjectChart.contractEnvelope-6839.test.tsx | 194 +++++++++++++++++ ...ctDataTable.contractEnvelope-6839.test.tsx | 130 ++++++++++++ ...tPivotTable.contractEnvelope-6839.test.tsx | 125 +++++++++++ ...ObjectGantt.contractEnvelope-6839.test.tsx | 197 ++++++++++++++++++ ...bjectKanban.contractEnvelope-6839.test.tsx | 160 ++++++++++++++ .../ObjectMap.contractEnvelope-6839.test.tsx | 142 +++++++++++++ ...ectTimeline.contractEnvelope-6839.test.tsx | 131 ++++++++++++ .../ObjectTree.contractEnvelope-6839.test.tsx | 141 +++++++++++++ ...idRowCeiling.contractEnvelope-6839.test.ts | 104 +++++++++ 13 files changed, 1712 insertions(+), 8 deletions(-) create mode 100644 .changeset/6839-extract-records-records-arm.md create mode 100644 packages/core/src/utils/__tests__/extract-records.contractEnvelope-6839.test.ts create mode 100644 packages/plugin-calendar/src/ObjectCalendar.contractEnvelope-6839.test.tsx create mode 100644 packages/plugin-charts/src/ObjectChart.contractEnvelope-6839.test.tsx create mode 100644 packages/plugin-dashboard/src/__tests__/ObjectDataTable.contractEnvelope-6839.test.tsx create mode 100644 packages/plugin-dashboard/src/__tests__/ObjectPivotTable.contractEnvelope-6839.test.tsx create mode 100644 packages/plugin-gantt/src/ObjectGantt.contractEnvelope-6839.test.tsx create mode 100644 packages/plugin-kanban/src/ObjectKanban.contractEnvelope-6839.test.tsx create mode 100644 packages/plugin-map/src/ObjectMap.contractEnvelope-6839.test.tsx create mode 100644 packages/plugin-timeline/src/ObjectTimeline.contractEnvelope-6839.test.tsx create mode 100644 packages/plugin-tree/src/ObjectTree.contractEnvelope-6839.test.tsx create mode 100644 packages/react/src/utils/nonGridRowCeiling.contractEnvelope-6839.test.ts diff --git a/.changeset/6839-extract-records-records-arm.md b/.changeset/6839-extract-records-records-arm.md new file mode 100644 index 0000000000..f90ea5b9bc --- /dev/null +++ b/.changeset/6839-extract-records-records-arm.md @@ -0,0 +1,56 @@ +--- +'@object-ui/core': minor +--- + +`extractRecords` reads a `find()` answer as `QueryResult` declares it: the +`records` arm is gone (objectui#6839, following objectui#5945 / #6726 / #6840). + +`QueryResult` (`@object-ui/types`) declares exactly one rows member, `data`. +This shared normaliser's ladder was `array -> records -> data -> value`, i.e. +the undeclared spelling AHEAD of the contract's own member — the same +precedence inversion objectui#5945 was filed about and objectui#6726 repaired by +hand at seven other seams. The ladder is now `array -> data -> value`. + +`records` is the below-the-adapter spelling: `ObjectStackAdapter +.normalizeQueryResult` and `ApiDataSource.normalizeQueryResult` (its +`['data','items','results','records','value']` envelope loop) both CONSUME the +server/SDK `records` envelope and return `data` before an answer reaches this +helper, and every consumer calls it strictly above that fold. So no producer +changes behaviour, because there is no producer; what changes is that a +non-conforming one is refused instead of silently absorbed — and, being first, +the arm used to outrank `data` when a producer emitted both. + +Reach, re-derived on this tree rather than taken from the card (which was +measured six days before filing and is stale in three places): ten call sites in +nine packages. Seven call `extractRecords` directly — `ObjectChart` (x2: the +chart rows, and the group-by lookup label domain inside the exported +`resolveGroupByLabels`), `ObjectDataTable`, `ObjectPivotTable`, `ObjectGantt` +(the quick-filter option domain), `ObjectKanban`, `ObjectTimeline`. Four more +reach it through `applyNonGridRowCeiling` (`@object-ui/react`), which is itself +a published export and a sink in its own right: `ObjectCalendar`, `ObjectGantt` +(its rows), `ObjectMap`, `ObjectTree`. + +Producer measurement, per consumer rather than once for all of them: no `find()` +in any of those nine packages, nor in the apps and examples that mount them, +emits a `records` envelope. CONTROL, so the zero is a reading rather than a +miss — the same sweep finds `records` envelopes elsewhere: `ViewDataProvider`'s +own `ResolvedData` (served by that module's own private reader of the same +name), the raw Cloud HTTP payloads, the client-SDK doubles below +`normalizeQueryResult`, the record-visibility batch route stubs, and one live +`find()` double at `plugin-list`'s ObjectGallery — a consumer with its OWN +unwrap ladder, which does not come through here and is untouched. + +The `value` arm STAYS. objectui#6840 removed `value` from `ObjectView`'s ladder +on a measured zero at that seam and stated that its zero must not transfer here; +it does not. Five `find()` doubles emit `{ value: [...] }` into this helper +today (three in `plugin-kanban`, two in `plugin-calendar`), so the arm is live +and its removal is a separate card with its own measurement. + +`QueryResult` is NOT widened to bless `records` — that is a published-type +change and the maintainer's call, the same floor objectui#6726 and #6840 +respected. + +One refusal pin per module (`*.contractEnvelope-6839.*`), each keeping the live +arms green alongside the deleted one, because live and dead is the whole +distinction — plus a direct pin on the helper for the precedence question a +per-module render pin cannot ask ("when BOTH keys are present, which wins"). diff --git a/packages/core/src/utils/__tests__/extract-records.contractEnvelope-6839.test.ts b/packages/core/src/utils/__tests__/extract-records.contractEnvelope-6839.test.ts new file mode 100644 index 0000000000..ab298fdfbe --- /dev/null +++ b/packages/core/src/utils/__tests__/extract-records.contractEnvelope-6839.test.ts @@ -0,0 +1,112 @@ +/** + * 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. + */ + +/** + * `extractRecords` reads a `find()` answer as `QueryResult` DECLARES it — and + * does NOT read `records` (objectui#6839, following #5945 / #6726 / #6840). + * + * `QueryResult` (`@object-ui/types`) declares exactly one rows member: `data`. + * This helper's ladder was `array -> records -> data -> value`, i.e. `records` + * AHEAD of the contract's own member — the same precedence inversion #5945 was + * filed about and #6726 repaired by hand at seven other seams. `records` is + * the below-the-adapter spelling that `ObjectStackAdapter.normalizeQueryResult` + * and `ApiDataSource.normalizeQueryResult` already fold into `data` BELOW every + * consumer of this helper. + * + * ## What this file measures that the per-module pins cannot + * + * The nine renderers get their own `*.contractEnvelope-6839.*` pins, because a + * single "nothing reads `records`" assertion would pass even if one module + * routed around the helper. This file is the other half: the ARM ORDER itself. + * A per-module render pin answers "did a `records` envelope paint rows"; only a + * direct call can answer "when BOTH keys are present, which one wins" — and + * that question is the defect this card names in its title. + * + * ⚠️ Read the refusal cases together with the live ones. Every "returns []" + * assertion below is also satisfied by a helper that returns `[]` for + * EVERYTHING — an implementation strictly worse than the bug. The `data`, bare + * array and `value` cases are what refuse that implementation, and the two + * precedence cases refuse a helper that merely REORDERED the arms instead of + * deleting `records` (a reorder leaves `records` reachable whenever `data` is + * absent, which is precisely the tolerance AGENTS.md #0.1 is about). + * + * ⛔ `value` is NOT under test for removal here. It is LIVE at this seam — five + * `find()` doubles in `plugin-kanban` (3) and `plugin-calendar` (2) answer with + * `{ value: [...] }` today — and objectui#6840 explicitly refused to let its own + * zero at `ObjectView`'s seam transfer here. Its case below is a NON-REGRESSION + * case, not an endorsement of the key. + */ + +import { describe, it, expect } from 'vitest'; +import { extractRecords } from '../extract-records'; + +const ROWS = [{ id: 'r1', name: 'Ada' }, { id: 'r2', name: 'Grace' }]; +/** Distinct rows, so "which arm answered" is readable off the RESULT. */ +const OTHER = [{ id: 'x9', name: 'Nobody' }]; + +describe('extractRecords — the find() envelope it reads (objectui#6839)', () => { + describe('the shapes it still reads — the live arms', () => { + it("reads the contract's `data` member", () => { + expect(extractRecords({ data: ROWS, total: 2 })).toEqual(ROWS); + }); + + it('reads a bare array — the live non-envelope shape fakes answer with', () => { + expect(extractRecords(ROWS)).toEqual(ROWS); + }); + + it('still reads `value` — LIVE at this seam (objectui#6840 refused to transfer its zero)', () => { + expect(extractRecords({ value: ROWS, total: 2 })).toEqual(ROWS); + }); + }); + + describe('the shape it refuses', () => { + it('does NOT read `records` — not a QueryResult member', () => { + // Before the fix this returned the two rows, off a key the adapters + // below this seam have already folded into `data`. + expect(extractRecords({ records: ROWS, total: 2 })).toEqual([]); + }); + }); + + describe('the arm ORDER — the defect this card is named for', () => { + it('`data` OUTRANKS `records`: a producer emitting both is read as QueryResult', () => { + // Before the fix `records` was tried FIRST, so this returned OTHER — the + // contract's own member was ignored in favour of a key it does not + // declare. This is the case a mere reorder would also pass, which is why + // the `records`-only case above is asserted alongside it. + expect(extractRecords({ data: ROWS, records: OTHER })).toEqual(ROWS); + }); + + it('`value` outranks `records` too — the deleted arm outranks nothing', () => { + expect(extractRecords({ value: ROWS, records: OTHER })).toEqual(ROWS); + }); + + it('a bare array outranks every envelope key', () => { + // The array leg sits above the object branch, so an array that also + // carries envelope-shaped own properties is read as the array it is. + // `Array.from` strips those own properties before the compare — + // `toEqual` weighs them, and would fail on the decoration rather than + // on the ELEMENTS, which is the only thing this case is about. + const withProps = Object.assign([...ROWS], { records: OTHER, data: OTHER }); + expect(Array.from(extractRecords(withProps))).toEqual(ROWS); + }); + }); + + describe('the shapes that were never rows', () => { + it.each([ + ['null', null], + ['undefined', undefined], + ['a number', 42], + ['a string', 'rows'], + ['an empty object', {}], + ['a non-array `data`', { data: { id: 'r1' } }], + ['a non-array `value`', { value: 'rows' }], + ])('answers [] for %s', (_label, input) => { + expect(extractRecords(input)).toEqual([]); + }); + }); +}); diff --git a/packages/core/src/utils/extract-records.ts b/packages/core/src/utils/extract-records.ts index be5f09df86..d36ba54fe0 100644 --- a/packages/core/src/utils/extract-records.ts +++ b/packages/core/src/utils/extract-records.ts @@ -7,21 +7,76 @@ */ /** - * Extract an array of records from various API response formats. - * Supports: raw array, { records: [] }, { data: [] }, { value: [] }. + * Extract the rows array out of a `DataSource.find()` answer. * - * This utility normalises the different shapes returned by ObjectStack, - * OData, and MSW mock endpoints so that every data-fetching component - * can rely on a single extraction path. + * Accepted shapes, in the order they are tried: + * + * 1. a bare array — the live non-envelope shape fakes and `ValueDataSource` + * answer with; + * 2. `{ data: [] }` — the ONE rows member `QueryResult` (`@object-ui/types`) + * declares; + * 3. `{ value: [] }` — the OData spelling, still LIVE at this seam (see + * below). + * + * ## `records` is NOT read here (objectui#6839, following #5945 / #6726) + * + * A `records` arm used to sit AHEAD of `data` — the precedence inversion + * objectui#5945 was filed about and objectui#6726 repaired by hand in seven + * modules. `records` is the below-the-adapter spelling: both + * `ObjectStackAdapter.normalizeQueryResult` (`@object-ui/data-objectstack`) + * and `ApiDataSource.normalizeQueryResult` (its `['data','items','results', + * 'records','value']` envelope loop) CONSUME the server/SDK `records` envelope + * and return `data` before the answer ever reaches this helper. Every consumer + * below calls it strictly ABOVE that fold, so the arm was unreachable — and an + * unreachable tolerant arm is exactly where a non-conforming producer keeps + * working unrejected (AGENTS.md #0.1). Worse, being FIRST, it outranked the + * contract's own member: a producer emitting both would have had `data` + * ignored. + * + * Measured on this tree, per consumer rather than once for all of them, since + * this helper is reached from ten call sites in nine packages: + * + * packages/plugin-charts/src/ObjectChart.tsx (x2: rows, ref labels) + * packages/plugin-dashboard/src/ObjectDataTable.tsx + * packages/plugin-dashboard/src/ObjectPivotTable.tsx + * packages/plugin-gantt/src/ObjectGantt.tsx (ref option labels) + * packages/plugin-kanban/src/ObjectKanban.tsx + * packages/plugin-timeline/src/ObjectTimeline.tsx + * packages/react/src/utils/nonGridRowCeiling.tsx (applyNonGridRowCeiling, + * itself the seam for ObjectCalendar, ObjectGantt, ObjectMap, ObjectTree) + * + * ZERO of them can be reached by a `records` envelope: no `find()` in any of + * those packages, nor in the apps and examples that mount them, emits one. + * CONTROL, so the zero is a reading rather than a miss — the same sweep DOES + * find `records` envelopes elsewhere: `ViewDataProvider`'s own `ResolvedData` + * (a different contract, served by that module's own private reader), the raw + * Cloud HTTP payloads, the client-SDK doubles below `normalizeQueryResult`, + * the record-visibility batch route stubs, and one live `find()` double at + * `plugin-list`'s ObjectGallery — a consumer with its OWN unwrap ladder, which + * does not come through here. + * + * ⛔ Do not restore the arm, and ⛔ do not widen `QueryResult` to bless + * `records` instead — that is a published-type change and the maintainer's + * call, the same floor objectui#6726 and #6840 respected. A producer that + * really does speak `records` belongs behind an adapter that folds it into + * `data`, which is what both adapters above already do. + * + * ## `value` STAYS — it is live here, and that is seam-local + * + * objectui#6840 / PR #6916 deleted the `value` arm from `ObjectView`'s ladder + * on a measured zero at THAT seam, and said in as many words that its zero + * must not be carried here. It does not: five `find()` doubles emit + * `{ value: [...] }` into this helper today (three in `plugin-kanban`, two in + * `plugin-calendar`). Deleting it here would break them. Whether `value` + * should survive at this seam is its own card with its own measurement. + * + * Pinned per module by the `*.contractEnvelope-6839.*` suites. */ export function extractRecords(results: unknown): any[] { if (Array.isArray(results)) { return results; } if (results && typeof results === 'object') { - if (Array.isArray((results as any).records)) { - return (results as any).records; - } if (Array.isArray((results as any).data)) { return (results as any).data; } diff --git a/packages/plugin-calendar/src/ObjectCalendar.contractEnvelope-6839.test.tsx b/packages/plugin-calendar/src/ObjectCalendar.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..1a3956733e --- /dev/null +++ b/packages/plugin-calendar/src/ObjectCalendar.contractEnvelope-6839.test.tsx @@ -0,0 +1,157 @@ +/** + * 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. + */ + +/** + * `ObjectCalendar` reads its `find()` answer as `QueryResult` DECLARES it — and + * does NOT read `records` (objectui#6839). + * + * ⭐ This module reaches the shared reader INDIRECTLY: it does not call + * `extractRecords` at all, it hands its `find()` answer to + * `applyNonGridRowCeiling` (`@object-ui/react`), which unwraps it. A card + * enumerating the helper's direct callers would not list this file, and a + * repo-wide "nothing reads `records`" assertion would pass over it in silence. + * The route is what is measured here, at the events. + * + * ## This module is one of the two where the `value` arm is LIVE + * + * TWO `find()` doubles in this package answer `{ value: [...] }` today, and + * objectui#6840 — which deleted the `value` arm from `ObjectView`'s own ladder + * on a measured zero at THAT seam — said in as many words that its zero must + * not be carried to this one. So the `value` case below is a NON-REGRESSION + * case: it is what refuses an `extractRecords` that answers `[]` for + * everything, an implementation strictly worse than the bug. + * + * MEASURED for this module: no `find()` in `plugin-calendar`, nor in any app or + * example mounting a calendar, emits a `records` envelope — the package's two + * `records:` occurrences are a doc comment and a record-VISIBILITY batch route + * stub (`fetch`, not `find`). CONTROL, so the zero is a reading: the same sweep + * finds a live `find()` double emitting `{ records: [...] }` at `plugin-list`'s + * ObjectGallery, a consumer with its own unwrap ladder. + */ + +import React from 'react'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; + +vi.mock('@object-ui/plugin-detail', async (importOriginal) => ({ + ...(await importOriginal()), + RecordDetailDrawer: () => null, + deriveRecordPageHref: () => null, +})); + +// The month grid draws at most four events per day cell, so it cannot report a +// count. Stubbing the child puts the number on an attribute an assertion can +// reach — the same idiom `ObjectCalendar.rowCeiling-7210.test.tsx` uses, and +// `importOriginal` keeps the module's other exports live. +vi.mock('./CalendarView', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + CalendarView: ({ events }: any) => ( +
+ ), + }; +}); + +import { ObjectCalendar } from './ObjectCalendar'; + +const NOW = new Date(); + +/** Events inside the month the calendar opens on, so they are drawable at all. */ +const ROWS = [1, 2].map((n) => { + const d = new Date(NOW.getFullYear(), NOW.getMonth(), n); + const iso = `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, '0')}-${String( + d.getDate(), + ).padStart(2, '0')}`; + return { id: String(n), subject: `Event ${n}`, start_at: iso, end_at: iso }; +}); + +const schema: any = { + type: 'calendar', + objectName: 'event', + calendar: { titleField: 'subject', startDateField: 'start_at', endDateField: 'end_at' }, + data: { provider: 'object', object: 'event' }, +}; + +/** 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 }); +const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); + +/** + * Mount the calendar over a `find()` answering `envelope`, return events drawn. + * + * ⛔ Call ONCE per case, never inside a `waitFor` predicate (objectui#7802): + * it renders, and `waitFor` re-runs its callback on DOM mutations, so a + * predicate that renders feeds itself and leaks a container div per run. + */ +async function eventsThrough(envelope: Envelope): Promise { + const find = vi.fn(async () => envelope(ROWS)); + const ds: any = { + find, + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async () => ({ + name: 'event', + fields: { + id: { name: 'id', type: 'text' }, + subject: { name: 'subject', type: 'text' }, + start_at: { name: 'start_at', type: 'date' }, + end_at: { name: 'end_at', type: 'date' }, + }, + })), + }; + render(); + await waitFor(() => expect(find).toHaveBeenCalled()); + // `find`'s OWN answer, settled — a pure read of the mock's call record that + // touches no DOM. Without it "no events" is satisfied by the mount's initial + // empty state, which every arm renders identically. + await find.mock.results[0].value; + // The stubbed grid mounts on every arm, refused or not — a mount signal + // rather than a rows signal, which is what makes it the one wait shared by + // the live cases and the refusal case. + await waitFor(() => expect(screen.queryByTestId('calendar-view')).toBeTruthy()); + return Number(screen.getByTestId('calendar-view').getAttribute('data-event-count')); +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('ObjectCalendar — the find() envelope it reads (objectui#6839)', () => { + it("still reads the contract's `data` member", async () => { + expect(await eventsThrough(asData), 'the declared rows member must still draw').toBe(2); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await eventsThrough(asBareArray), 'the bare-array arm must still draw').toBe(2); + }); + + it('still reads `value` — LIVE at this seam, two doubles in this package emit it', async () => { + expect( + await eventsThrough(asValue), + 'objectui#6840 refused to transfer its `ObjectView` zero here; deleting this arm would ' + + 'break two doubles in this package', + ).toBe(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two events drew off a key `QueryResult` does not + // declare, and did so AHEAD of `data`. + expect( + await eventsThrough(asRecords), + 'a `records` envelope must reach the grid as zero events, not as the rows it names', + ).toBe(0); + }); +}); diff --git a/packages/plugin-charts/src/ObjectChart.contractEnvelope-6839.test.tsx b/packages/plugin-charts/src/ObjectChart.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..c3d2540f8d --- /dev/null +++ b/packages/plugin-charts/src/ObjectChart.contractEnvelope-6839.test.tsx @@ -0,0 +1,194 @@ +/** + * 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. + */ + +/** + * `ObjectChart` reads its `find()` answers as `QueryResult` DECLARES them — and + * does NOT read `records` (objectui#6839). + * + * ⭐ This module has TWO call sites into the shared reader, and they behave + * differently — objectui#6839's own table names only one of them, so the second + * is re-derived here rather than inherited: + * + * 1. THE ROWS — `ds.find(objectName)` -> `extractRecords`. A refused envelope + * paints the empty state. + * 2. THE GROUP-BY LABEL DOMAIN — `dataSource.find(referenceTo)` -> + * `extractRecords`, inside the exported `resolveGroupByLabels`. A refused + * envelope here does NOT empty the chart: every bar still draws, and the + * only casualty is that a lookup dimension keeps its RAW FOREIGN KEY as + * its axis label. A chart of `a1f3c…` instead of `Apollo` is a chart that + * looks like it worked, which is exactly the failure a rows-only pin + * cannot see. Site 2 is asserted through the exported function directly — + * it is public API of this module, and calling it is a sharper instrument + * than inferring the axis text through a full chart mount. + * + * MEASURED for this module: no `find()` in `plugin-charts`, nor in any app or + * example mounting a chart, emits a `records` envelope — the package's single + * `records:` occurrence is an `aggregateRecords(records: any[], …)` parameter + * name. CONTROL, so the zero is a reading: the same sweep finds a live `find()` + * double emitting `{ records: [...] }` at `plugin-list`'s ObjectGallery, a + * consumer with its own unwrap ladder. + * + * ⚠️ Every refusal case is ALSO satisfied by an `extractRecords` that returns + * `[]` for everything — an implementation strictly worse than the bug. The + * `data` and bare-array cases refuse it, on BOTH sites. + * + * NOTE: `plugin-charts` also RE-EXPORTS `extractRecords` + * (`export { extractRecords } from '@object-ui/core'`), so this package's + * published surface carries the behaviour change too. That re-export is + * identity-pinned by `ObjectChart.humanizeLabelReexport.test.ts`'s family and + * is not re-asserted here. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; + +let lastSchema: any = null; + +vi.mock('./ChartRenderer', () => ({ + ChartRenderer: (props: any) => { + lastSchema = props.schema; + return null; + }, +})); + +import { ObjectChart, resolveGroupByLabels } from './ObjectChart'; + +const ROWS = [ + { stage: 'p1', amount: 10 }, + { stage: 'p1', amount: 20 }, +]; + +/** The referenced object's rows — the id→label domain site 2 resolves. */ +const PROJECTS = [{ id: 'p1', name: 'Apollo' }]; + +/** 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 }); + +const schema = { + type: 'object-chart', + chartType: 'bar' as const, + objectName: 'crm_opportunity', + xAxisKey: 'stage', + series: [{ dataKey: 'amount', label: 'Amount' }], + isAnimationActive: false, +}; + +/** A plain text dimension — site 2 stays out of the way of site 1's cases. */ +const TEXT_SCHEMA = { + name: 'crm_opportunity', + fields: { stage: { type: 'text' }, amount: { type: 'number' } }, +}; + +/** A lookup dimension — this is what puts site 2 on the path. */ +const LOOKUP_FIELD = { type: 'lookup', reference: 'projects' }; + +beforeEach(() => { + lastSchema = null; + vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, json: async () => ({}) }))); +}); +afterEach(() => { + vi.unstubAllGlobals(); + cleanup(); + lastSchema = null; +}); + +/** + * Mount the chart over a `find()` answering `envelope`, and hand back what it + * settled on: the number of rows that reached `ChartRenderer`, or + * `'empty-state'` when it drew objectui#7130's "No data yet" tile instead. + * + * The two outcomes are DIFFERENT DOM, not a count of 0 — `ObjectChart` returns + * the empty tile on an empty result set and never mounts the renderer at all. + * Reporting which one happened keeps a refused envelope distinguishable from a + * mount that never rendered. + * + * ⛔ Call ONCE per case, never inside a `waitFor` predicate (objectui#7802): + * it renders, and `waitFor` re-runs its callback on DOM mutations, so a + * predicate that renders feeds itself and leaks a container div per run. + */ +async function settledOn(envelope: Envelope): Promise { + const find = vi.fn(async () => envelope(ROWS)); + const ds: any = { find, getObjectSchema: vi.fn(async () => TEXT_SCHEMA) }; + render(); + await waitFor(() => expect(find).toHaveBeenCalled()); + // `find`'s OWN answer, settled — a pure read of the mock's call record that + // touches no DOM. Without it "no rows" is satisfied by the loading state, + // which every arm passes through identically. + await find.mock.results[0].value; + await waitFor(() => + expect(lastSchema ?? screen.queryByTestId('chart-empty-state')).not.toBeNull(), + ); + return lastSchema ? (lastSchema.data ?? []).length : 'empty-state'; +} + +/** + * Run site 2 directly: resolve one `lookup` dimension's labels over a `find()` + * answering `envelope`, and hand back the axis label the row ended up with. + * + * `'Apollo'` means the domain was read; `'P1'` is what `humanizeLabel` makes of + * the unresolved foreign key, i.e. the silent-looking failure. + */ +async function labelThrough(envelope: Envelope): Promise { + const ds: any = { find: vi.fn(async () => envelope(PROJECTS)) }; + const out = await resolveGroupByLabels( + [{ stage: 'p1', amount: 10 }], + 'stage', + { name: 'crm_opportunity', fields: { stage: LOOKUP_FIELD } }, + ds, + ); + return String(out[0].stage); +} + +describe('ObjectChart — the find() envelopes it reads (objectui#6839)', () => { + describe('site 1 — the chart rows', () => { + it("still reads the contract's `data` member", async () => { + expect(await settledOn(asData), 'the declared rows member must still draw').toBe(2); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await settledOn(asBareArray), 'the bare-array arm must still draw').toBe(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two rows drew off a key `QueryResult` does not + // declare, and did so AHEAD of `data`. The chart now settles on + // objectui#7130's empty tile — a DIFFERENT node from the renderer, which + // is what makes this a reading of the refusal rather than of a mount + // that never happened. + expect( + await settledOn(asRecords), + 'a `records` envelope must reach the chart as no rows at all, not as the rows it names', + ).toBe('empty-state'); + }); + }); + + describe('site 2 — the group-by label domain (resolveGroupByLabels)', () => { + it("still reads the contract's `data` member", async () => { + expect(await labelThrough(asData), 'the lookup label must still resolve').toBe('Apollo'); + }); + + it('still reads a bare array', async () => { + expect(await labelThrough(asBareArray)).toBe('Apollo'); + }); + + it('does NOT read `records` — and the chart is NOT how you would notice', async () => { + // The sharp half. The bar count is identical either way; what changes is + // that the axis now carries the raw foreign key. A rows-only pin would + // have called this module green. + expect( + await labelThrough(asRecords), + 'a `records` envelope must not resolve the referenced domain', + ).not.toBe('Apollo'); + }); + }); +}); diff --git a/packages/plugin-dashboard/src/__tests__/ObjectDataTable.contractEnvelope-6839.test.tsx b/packages/plugin-dashboard/src/__tests__/ObjectDataTable.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..b98e1804b6 --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/ObjectDataTable.contractEnvelope-6839.test.tsx @@ -0,0 +1,130 @@ +/** + * 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. + */ + +/** + * `ObjectDataTable` reads its `find()` answer as `QueryResult` DECLARES it — + * and does NOT read `records` (objectui#6839). + * + * The widget unwraps through `extractRecords` (`@object-ui/core`). The read + * lives in ONE shared helper, so this file is not asking "was this copy + * repaired" (objectui#6726's question, where each of seven consumers carried + * its own copy) — it asks whether THIS module actually routes its answer + * through the helper. A module that unwrapped the envelope itself, as + * `plugin-list`'s `ListView` still does, would inherit nothing from the + * helper's own pin while a repo-wide "nothing reads `records`" assertion passed + * over it in silence. + * + * MEASURED for this module: no `find()` in `plugin-dashboard`, nor in any app + * or example mounting this widget, emits a `records` envelope — the package's + * three `records:` occurrences are a `.then((records: any) =>` parameter name + * and two doc comments. CONTROL, so the zero is a reading: the same sweep + * finds a live `find()` double emitting `{ records: [...] }` at `plugin-list`'s + * ObjectGallery, and `records`-keyed payloads on the raw-HTTP and client-SDK + * seams below the adapters. + * + * ⚠️ The refusal case is ALSO satisfied by an `extractRecords` that returns + * `[]` for everything — an implementation strictly worse than the bug. The + * `data` and bare-array cases refuse it: same rows, same mount. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import React from 'react'; + +/** + * The underlying `data-table` renderer, replaced by a reader of the ROWS the + * widget handed it. The real one pulls the whole component registry, and what + * this file observes is the row count, not the table's chrome. + */ +vi.mock('@object-ui/react', async () => { + const actual: any = await vi.importActual('@object-ui/react'); + return { + ...actual, + SchemaRenderer: ({ schema }: any) => ( +
+ ), + useDataScope: () => undefined, + }; +}); + +import { ObjectDataTable } from '../ObjectDataTable'; + +const ROWS = [{ name: 'Acme' }, { name: 'Initech' }]; + +const FIELDS = { fields: { name: { type: 'text', label: 'Name' } } }; + +const schema: any = { + type: 'object-data-table', + objectName: 'account', + columns: [{ header: 'Name', accessorKey: 'name' }], +}; + +/** 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 }); + +/** + * Mount the widget over a `find()` answering `envelope`, and hand back what it + * settled on: the number of rows it delivered, or `'empty-state'` when it drew + * its own "no records" panel instead. + * + * The two outcomes are DIFFERENT DOM, not a count of 0 — this widget returns + * early on an empty result set and never mounts the table at all. Reporting + * which one happened is what keeps a refused envelope distinguishable from a + * mount that simply never rendered. + * + * ⛔ Call ONCE per case, never inside a `waitFor` predicate (objectui#7802): + * it renders, and `waitFor` re-runs its callback on DOM mutations, so a + * predicate that renders feeds itself and leaks a container div per run. + */ +async function settledOn(envelope: Envelope): Promise { + const find = vi.fn(async () => envelope(ROWS)); + const ds: any = { find, getObjectSchema: vi.fn(async () => FIELDS) }; + render(); + await waitFor(() => expect(find).toHaveBeenCalled()); + // `find`'s OWN answer, settled — a pure read of the mock's call record that + // touches no DOM. Without it the read below is satisfied by the loading + // skeleton every arm passes through identically. + await find.mock.results[0].value; + await waitFor(() => + expect( + screen.queryByTestId('rows') ?? screen.queryByTestId('table-empty-state'), + ).toBeTruthy(), + ); + const rows = screen.queryByTestId('rows'); + return rows ? Number(rows.getAttribute('data-row-count')) : 'empty-state'; +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('ObjectDataTable — the find() envelope it reads (objectui#6839)', () => { + it("still reads the contract's `data` member", async () => { + expect(await settledOn(asData), 'the declared rows member must still draw').toBe(2); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await settledOn(asBareArray), 'the bare-array arm must still draw').toBe(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two rows drew off a key `QueryResult` does not + // declare, and did so AHEAD of `data`. The widget now settles on its own + // "no records" panel — a DIFFERENT node from the table, which is what makes + // this a reading of the refusal rather than of a mount that never happened. + expect( + await settledOn(asRecords), + 'a `records` envelope must reach the widget as no rows at all, not as the rows it names', + ).toBe('empty-state'); + }); +}); diff --git a/packages/plugin-dashboard/src/__tests__/ObjectPivotTable.contractEnvelope-6839.test.tsx b/packages/plugin-dashboard/src/__tests__/ObjectPivotTable.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..cba76a7d17 --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/ObjectPivotTable.contractEnvelope-6839.test.tsx @@ -0,0 +1,125 @@ +/** + * 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. + */ + +/** + * `ObjectPivotTable` reads its `find()` answer as `QueryResult` DECLARES it — + * and does NOT read `records` (objectui#6839). + * + * The widget unwraps through `extractRecords` (`@object-ui/core`). The read + * lives in ONE shared helper, so this file is not asking "was this copy + * repaired" — it asks whether THIS module actually routes its answer through + * the helper, which a repo-wide "nothing reads `records`" assertion cannot say. + * + * ⭐ This sink behaves DIFFERENTLY from its sibling `ObjectDataTable`, which is + * why it gets a pin rather than sharing one. `ObjectDataTable` returns early on + * an empty result set and never mounts its table; this one hands `finalData` + * straight through to `PivotTable` as the identity key of the cross-tabulation + * memo, so a refused envelope arrives as a real, empty `schema.data` — the same + * rows slot the live arms fill. Reading it there is what makes the refusal + * legible as a refusal. + * + * MEASURED for this module: no `find()` in `plugin-dashboard`, nor in any app + * or example mounting this widget, emits a `records` envelope. CONTROL, so the + * zero is a reading: the same sweep finds a live `find()` double emitting + * `{ records: [...] }` at `plugin-list`'s ObjectGallery. + * + * ⚠️ The refusal case is ALSO satisfied by an `extractRecords` that returns + * `[]` for everything — an implementation strictly worse than the bug. The + * `data` and bare-array cases refuse it: same rows, same mount, same slot. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import React from 'react'; + +/** + * `PivotTable`, replaced by a reader of the rows slot. The real one + * cross-tabulates, which is orthogonal to how many rows reached it. + */ +vi.mock('../PivotTable', () => ({ + PivotTable: ({ schema }: any) => ( +
+ ), +})); + +import { ObjectPivotTable } from '../ObjectPivotTable'; + +const ROWS = [ + { stage: 'won', owner: 'ada', amount: 10 }, + { stage: 'lost', owner: 'grace', amount: 20 }, +]; + +const schema: any = { + type: 'pivot-table', + objectName: 'opportunity', + rowField: 'stage', + columnField: 'owner', + valueField: 'amount', +}; + +/** 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 }); + +/** + * Mount the widget over a `find()` answering `envelope`, return the row count + * that reached `PivotTable`. + * + * ⛔ Call ONCE per case, never inside a `waitFor` predicate (objectui#7802): + * it renders, and `waitFor` re-runs its callback on DOM mutations, so a + * predicate that renders feeds itself and leaks a container div per run. + */ +async function rowsThrough(envelope: Envelope): Promise { + const find = vi.fn(async () => envelope(ROWS)); + const ds: any = { + find, + getObjectSchema: vi.fn(async () => ({ + name: 'opportunity', + fields: { + stage: { type: 'text', label: 'Stage' }, + owner: { type: 'text', label: 'Owner' }, + amount: { type: 'number', label: 'Amount' }, + }, + })), + }; + render(); + await waitFor(() => expect(find).toHaveBeenCalled()); + // `find`'s OWN answer, settled — a pure read of the mock's call record that + // touches no DOM. Without it the read below is satisfied by the mount's + // initial empty `data`, which every arm passes through identically. + await find.mock.results[0].value; + await waitFor(() => expect(screen.queryByTestId('pivot')).toBeTruthy()); + return Number(screen.getByTestId('pivot').getAttribute('data-row-count')); +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('ObjectPivotTable — the find() envelope it reads (objectui#6839)', () => { + it("still reads the contract's `data` member", async () => { + expect(await rowsThrough(asData), 'the declared rows member must still reach the pivot').toBe(2); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await rowsThrough(asBareArray), 'the bare-array arm must still reach the pivot').toBe(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two rows cross-tabulated off a key `QueryResult` + // does not declare, and did so AHEAD of `data`. + expect( + await rowsThrough(asRecords), + 'a `records` envelope must reach the pivot as zero rows, not as the rows it names', + ).toBe(0); + }); +}); diff --git a/packages/plugin-gantt/src/ObjectGantt.contractEnvelope-6839.test.tsx b/packages/plugin-gantt/src/ObjectGantt.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..44c7ae49bd --- /dev/null +++ b/packages/plugin-gantt/src/ObjectGantt.contractEnvelope-6839.test.tsx @@ -0,0 +1,197 @@ +/** + * 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. + */ + +/** + * `ObjectGantt` reads its `find()` answers as `QueryResult` DECLARES them — and + * does NOT read `records` (objectui#6839). + * + * ⭐ This module is the one with TWO sinks, reached by two different routes, + * and they behave differently — which is why they are pinned separately here + * rather than folded into one "the gantt rendered" case: + * + * 1. THE ROWS — `find(task)` -> `applyNonGridRowCeiling` (`@object-ui/react`) + * -> `extractRecords`. Indirect: this component never names the helper on + * this path, so a card enumerating the helper's direct callers would not + * list it. A refused envelope draws an empty chart. + * 2. THE QUICK-FILTER DOMAIN — `find(projects)` -> `extractRecords`, called + * directly. A refused envelope here does NOT empty the chart: the bars + * still draw, and the only casualty is the dropdown's option list, which + * silently narrows to the values present in the loaded rows. That failure + * is invisible to any rows-only assertion, and it is the reason this file + * asserts the domain separately. + * + * MEASURED for this module: no `find()` in `plugin-gantt`, nor in any app or + * example mounting a gantt, emits a `records` envelope — the package's four + * `records:` occurrences are three doc comments and a record-VISIBILITY batch + * route stub (`fetch`, not `find`). CONTROL, so the zero is a reading: the same + * sweep finds a live `find()` double emitting `{ records: [...] }` at + * `plugin-list`'s ObjectGallery, a consumer with its own unwrap ladder. + * + * ⚠️ Every refusal case is ALSO satisfied by an `extractRecords` that returns + * `[]` for everything — an implementation strictly worse than the bug. The + * `data` and bare-array cases refuse it: they push the SAME rows through the + * SAME mounts, on BOTH sinks. + * + * MODULE RESOLUTION: this file imports the component by relative source path + * and `@object-ui/core` is aliased by the root `vitest.config.mts` to + * `packages/core/src`, so both legs resolve to SOURCE — no package `exports` + * hop, no `dist`, and therefore no rebuild leg to get wrong. + */ + +import React from 'react'; +import { render, fireEvent, waitFor, within, cleanup } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; + +/** + * `GanttView` is mocked to a thin shell surfacing the task count, exactly as + * `ObjectGantt.quickfilter.test.tsx` and `ObjectGantt.referenceArms-6837 + * .test.tsx` do — how the chart paints bars is not what this file observes. + */ +vi.mock('./GanttView', () => ({ + GanttView: ({ tasks }: any) =>
, +})); + +import { ObjectGantt } from './ObjectGantt'; + +afterEach(cleanup); + +/** Both loaded rows point at `p1`, so `p2`/`p3` can only come from the domain. */ +const TASKS = [ + { id: '1', name: 'Alpha', start: '2024-01-01', end: '2024-01-05', project: 'p1' }, + { id: '2', name: 'Beta', start: '2024-02-01', end: '2024-02-10', project: 'p1' }, +]; + +/** The referenced object's full domain — reachable ONLY through sink 2. */ +const PROJECTS = [ + { id: 'p1', name: 'Apollo' }, + { id: 'p2', name: 'Borealis' }, + { id: 'p3', name: 'Cygnus' }, +]; + +/** 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 }); + +const GANTT_SCHEMA = { + type: 'gantt', + objectName: 'task', + startDateField: 'start', + endDateField: 'end', + titleField: 'name', + quickFilters: [{ field: 'project', label: 'Project' }], +} as any; + +/** + * Mount over a `find()` that wraps BOTH answers in `envelope`, and wait for the + * schema-dependent commit. + * + * The settle signal is envelope-INDEPENDENT: once `objectSchema` lands, the + * record query is re-issued carrying `$expand`, and `buildExpandFields` decides + * that from the field's `type` alone. So the recorded `find('task', {$expand})` + * proves the component consumed the schema for a REFUSED envelope exactly as it + * does for a live one — and the option-fetch effect shares that commit, so a + * resolving arm has already recorded `find('projects', …)` by then. + * + * ⛔ Call ONCE per case, never inside a `waitFor` predicate (objectui#7802). + */ +async function mount(envelope: Envelope) { + const ds: any = { + find: vi.fn(async (object: string) => + object === 'projects' ? envelope(PROJECTS) : envelope(TASKS), + ), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'task', + fields: { + name: { type: 'text' }, + start: { type: 'date' }, + end: { type: 'date' }, + project: { type: 'lookup', reference: 'projects' }, + }, + }), + }; + const view = render(); + await waitFor(() => + expect( + ds.find.mock.calls.some((c: any[]) => c[0] === 'task' && c[1]?.$expand?.includes('project')), + ).toBe(true), + ); + return { ds, view }; +} + +/** Bars the chart drew — sink 1. */ +const bars = (view: any) => Number(view.getByTestId('gantt-view').getAttribute('data-count')); + +/** + * Is `p3` — a project NO loaded row points at — offered in the dropdown? + * + * This is the only observation that separates sink 2 from sink 1: the fallback + * arm ("distinct values present in the loaded data") would still offer `p1` + * whether or not the domain fetch was read. + */ +async function offersUnloadedProject(view: any): Promise { + fireEvent.click(view.getByTestId('quick-filter-trigger-project')); + const panel = await view.findByTestId('quick-filter-panel-project'); + return within(panel).queryByTestId('quick-filter-option-project-p3') !== null; +} + +describe('ObjectGantt — the find() envelopes it reads (objectui#6839)', () => { + describe('sink 1 — the rows, via applyNonGridRowCeiling', () => { + it("still reads the contract's `data` member", async () => { + const { view } = await mount(asData); + await waitFor(() => expect(bars(view), 'the declared rows member must still draw').toBe(2)); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + const { view } = await mount(asBareArray); + await waitFor(() => expect(bars(view), 'the bare-array arm must still draw').toBe(2)); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two tasks drew off a key `QueryResult` does not + // declare, and did so AHEAD of `data`. + const { view } = await mount(asRecords); + expect( + bars(view), + 'a `records` envelope must reach the chart as zero bars, not as the tasks it names', + ).toBe(0); + }); + }); + + describe('sink 2 — the quick-filter option domain, read directly', () => { + it("still reads the contract's `data` member", async () => { + const { view } = await mount(asData); + expect( + await offersUnloadedProject(view), + 'the full referenced domain must still widen the dropdown past the loaded rows', + ).toBe(true); + }); + + it('still reads a bare array', async () => { + const { view } = await mount(asBareArray); + expect(await offersUnloadedProject(view)).toBe(true); + }); + + it('does NOT read `records` — and the chart is NOT how you would notice', async () => { + // The sharp half of this file. The bars are unaffected by sink 2, so a + // rows-only pin would have called this module green while the dropdown + // silently narrowed to the values already on screen. + const { view } = await mount(asRecords); + expect( + await offersUnloadedProject(view), + 'a `records` envelope must not resolve the referenced domain', + ).toBe(false); + }); + }); +}); diff --git a/packages/plugin-kanban/src/ObjectKanban.contractEnvelope-6839.test.tsx b/packages/plugin-kanban/src/ObjectKanban.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..9dcaf19459 --- /dev/null +++ b/packages/plugin-kanban/src/ObjectKanban.contractEnvelope-6839.test.tsx @@ -0,0 +1,160 @@ +/** + * 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. + */ + +/** + * `ObjectKanban` reads its `find()` answer as `QueryResult` DECLARES it — and + * does NOT read `records` (objectui#6839). + * + * ## Why per module, when the read lives in ONE shared helper + * + * The board unwraps through `extractRecords` (`@object-ui/core`). A repo-wide + * "nothing reads `records`" assertion would pass over a module that unwrapped + * the envelope itself instead — as `plugin-list`'s `ListView` still does — so + * what this file measures is the ROUTE, at the cards. + * + * ## This module is where the `value` arm is LIVE, which is why it is asserted + * + * `value` is not a leftover here: THREE `find()` doubles in this package answer + * `{ value: [...] }` today, and objectui#6840 (which deleted `value` from + * `ObjectView`'s own ladder on a zero at THAT seam) said in as many words that + * its zero must not be carried to this one. So the `value` case below is a + * NON-REGRESSION case — it is what refuses an `extractRecords` that answers + * `[]` for everything, an implementation strictly worse than the bug. + * + * MEASURED for this module: no `find()` in `plugin-kanban`, nor in any app or + * example mounting a board, emits a `records` envelope. The package's four + * `records:` object literals are a doc comment, an authoring-metadata + * description, and two record-VISIBILITY batch route stubs (`fetch`, not + * `find`). CONTROL, so the zero is a reading: the same sweep finds a live + * `find()` double emitting `{ records: [...] }` at `plugin-list`'s + * ObjectGallery, a consumer with its own unwrap ladder. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup, within } from '@testing-library/react'; +import React from 'react'; +import type { DataSource, ObjectKanbanSchema } from '@object-ui/types'; +import { ObjectKanban } from './ObjectKanban'; + +const schema = { + type: 'object-kanban', + objectName: 'opportunity', + groupBy: 'stage', + cardTitle: 'name', + columns: [{ id: 'negotiation', title: 'Negotiation' }], +} satisfies ObjectKanbanSchema; + +const ROWS = [ + { id: 'o1', name: 'Northwind renewal', stage: 'negotiation' }, + { id: 'o2', name: 'Contoso expansion', stage: 'negotiation' }, +]; + +const DEF = { + name: 'opportunity', + fields: { + name: { type: 'text', label: 'Name' }, + stage: { + type: 'picklist', + label: 'Stage', + options: [{ value: 'negotiation', label: 'Negotiation' }], + }, + }, +}; + +/** 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 }); +const asRecords: Envelope = (rows) => ({ records: rows, total: rows.length }); + +/** Cards the board actually painted, by their `aria-label`. */ +function cards(): string[] { + const list = screen.queryByRole('list', { name: 'Negotiation cards' }); + if (!list) return []; + return within(list) + .queryAllByRole('listitem') + .map((el) => el.getAttribute('aria-label') ?? ''); +} + +/** + * Mount the board over a `find()` answering `envelope`, and hand back the cards + * it drew. + * + * ⛔ Call this ONCE per case and NEVER from inside a `waitFor` predicate + * (objectui#7802) — it renders, and `waitFor` re-runs its callback on DOM + * mutations, so a predicate that renders feeds itself and leaks a container + * div per run. + * + * ⚠️ Mounted with NO `data` prop: `ObjectKanban` skips its own fetch when + * external data is supplied, and a board handed its rows directly would answer + * every case identically — measuring nothing. + */ +async function cardsThrough(envelope: Envelope): Promise { + const find = vi.fn(async () => envelope(ROWS)); + const ds = { + getObjectSchema: vi.fn(async () => DEF), + find, + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + } as unknown as DataSource; + render(); + await waitFor(() => expect(find).toHaveBeenCalled()); + // `find`'s OWN answer, settled — a pure read of the mock's call record that + // touches no DOM. Without it, "no cards" is satisfied by the mount's initial + // empty state, which every arm renders identically. + await find.mock.results[0].value; + // The column header lands on every arm, refused or not, so it is a mount + // signal rather than a rows signal — which is exactly what makes it the + // right thing to wait on before reading the cards. + await waitFor(() => expect(screen.queryByText('Negotiation')).toBeTruthy()); + return cards(); +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe('ObjectKanban — the find() envelope it reads (objectui#6839)', () => { + it("still reads the contract's `data` member", async () => { + const drawn = await cardsThrough(asData); + expect(drawn.length, 'the declared rows member must still draw both cards').toBe(2); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + const drawn = await cardsThrough(asBareArray); + expect(drawn.length, 'the bare-array arm must still draw both cards').toBe(2); + }); + + it('still reads `value` — LIVE at this seam, three doubles in this package emit it', async () => { + const drawn = await cardsThrough(asValue); + expect( + drawn.length, + 'objectui#6840 refused to transfer its `ObjectView` zero here; deleting this arm would ' + + 'break three doubles in this package', + ).toBe(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two cards drew off a key `QueryResult` does not + // declare, and did so AHEAD of `data`. + const drawn = await cardsThrough(asRecords); + expect( + drawn, + 'a `records` envelope must reach the board as zero cards, not as the rows it names', + ).toEqual([]); + }); +}); diff --git a/packages/plugin-map/src/ObjectMap.contractEnvelope-6839.test.tsx b/packages/plugin-map/src/ObjectMap.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..829e417b5f --- /dev/null +++ b/packages/plugin-map/src/ObjectMap.contractEnvelope-6839.test.tsx @@ -0,0 +1,142 @@ +/** + * 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. + */ + +/** + * `ObjectMap` reads its `find()` answer as `QueryResult` DECLARES it — and does + * NOT read `records` (objectui#6839). + * + * ⭐ This module reaches the shared reader INDIRECTLY: it does not call + * `extractRecords` at all, it hands its `find()` answer to + * `applyNonGridRowCeiling` (`@object-ui/react`), which unwraps it. A card + * enumerating the helper's direct callers would not list this file, and a + * repo-wide "nothing reads `records`" assertion would pass over it in silence. + * The route is what is measured here, at the markers. + * + * ⚠️ `enableClustering={false}` is load-bearing, for the same reason + * `ObjectMap.rowCeiling-7210.test.tsx` gives: the map clusters above 100 + * markers, and a cluster bubble is a marker count folded into one DOM node. + * With clustering on, "two markers" and "no markers" are not reliably + * different pictures. Clustering is a pure function of the marker array, so + * turning it off changes what is on screen, never what reached the view. + * + * MEASURED for this module: no `find()` in `plugin-map`, nor in any app or + * example mounting a map, emits a `records` envelope — the package's four + * `records:` occurrences are two doc comments and two `mockResolvedValue(records)` + * calls whose `records` is a local BARE ARRAY, not an envelope key. CONTROL, so + * the zero is a reading: the same sweep finds a live `find()` double emitting + * `{ records: [...] }` at `plugin-list`'s ObjectGallery. + * + * ⚠️ The refusal case is ALSO satisfied by an `extractRecords` that returns + * `[]` for everything — an implementation strictly worse than the bug. The + * `data` and bare-array cases refuse it: same rows, same mount. + */ + +import React from 'react'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; + +// The MapLibre canvas, replaced by DOM the test can count. Copied from +// `ObjectMap.configMemo.test.tsx`, which is where this package's map double +// lives; `Marker` is the node the marker array actually produces. +vi.mock('react-map-gl/maplibre', () => { + const MapImpl = ({ children }: any) =>
{children}
; + return { + default: MapImpl, + Map: MapImpl, + NavigationControl: () =>
, + Marker: ({ children, longitude, latitude }: any) => ( +
+ {children} +
+ ), + Popup: ({ children }: any) =>
{children}
, + }; +}); + +import { ObjectMap } from './ObjectMap'; + +const ROWS = [ + { id: '1', name: 'Harbour Depot', latitude: 47.6062, longitude: -122.3321 }, + { id: '2', name: 'Ridge Yard', latitude: 37.7749, longitude: -122.4194 }, +]; + +const schema: any = { + type: 'map', + objectName: 'store', + map: { latitudeField: 'latitude', longitudeField: 'longitude', titleField: 'name' }, + data: { provider: 'object', object: 'store' }, +}; + +/** 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 }); + +/** + * Mount the map over a `find()` answering `envelope`, return markers plotted. + * + * ⛔ Call ONCE per case, never inside a `waitFor` predicate (objectui#7802): + * it renders, and `waitFor` re-runs its callback on DOM mutations, so a + * predicate that renders feeds itself and leaks a container div per run. + */ +async function markersThrough(envelope: Envelope): Promise { + const find = vi.fn(async () => envelope(ROWS)); + const ds: any = { + find, + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async () => ({ + name: 'store', + fields: { + id: { name: 'id', type: 'text' }, + name: { name: 'name', type: 'text' }, + latitude: { name: 'latitude', type: 'number' }, + longitude: { name: 'longitude', type: 'number' }, + }, + })), + }; + render(); + await waitFor(() => expect(find).toHaveBeenCalled()); + // `find`'s OWN answer, settled — a pure read of the mock's call record that + // touches no DOM. Without it "no markers" is satisfied by the mount's + // initial empty state, which every arm renders identically. + await find.mock.results[0].value; + // The loading panel clears on EVERY arm, refused or not — a settle signal + // rather than a rows signal, which is exactly what makes it usable as the + // one wait shared by the live cases and the refusal case. + await waitFor(() => expect(screen.queryByText('Loading map...')).toBeNull()); + return screen.queryAllByTestId('map-marker').length; +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('ObjectMap — the find() envelope it reads (objectui#6839)', () => { + it("still reads the contract's `data` member", async () => { + expect(await markersThrough(asData), 'the declared rows member must still plot').toBe(2); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await markersThrough(asBareArray), 'the bare-array arm must still plot').toBe(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two sites plotted off a key `QueryResult` does not + // declare, and did so AHEAD of `data`. + expect( + await markersThrough(asRecords), + 'a `records` envelope must reach the map as zero markers, not as the sites it names', + ).toBe(0); + }); +}); diff --git a/packages/plugin-timeline/src/ObjectTimeline.contractEnvelope-6839.test.tsx b/packages/plugin-timeline/src/ObjectTimeline.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..e5e244554e --- /dev/null +++ b/packages/plugin-timeline/src/ObjectTimeline.contractEnvelope-6839.test.tsx @@ -0,0 +1,131 @@ +/** + * 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. + */ + +/** + * `ObjectTimeline` reads its `find()` answer as `QueryResult` DECLARES it — and + * does NOT read `records` (objectui#6839). + * + * ## Why per module, when the read lives in ONE shared helper + * + * objectui#6726 repaired seven consumers that each carried their OWN copy of + * the read, so its per-module pins answered "were all seven repaired". Here + * there is one read site (`extractRecords`, `@object-ui/core`), and the + * per-module question is a different one: does THIS module actually route its + * answer through the helper? A module that unwrapped the envelope itself — as + * `plugin-list`'s `ListView` still does — would inherit nothing from the + * helper's own pin, and a repo-wide "nothing reads `records`" assertion would + * pass over it in silence. This file measures the route, at the rows. + * + * MEASURED for this module: no `find()` in `plugin-timeline`, nor in any app or + * example that mounts a timeline, emits a `records` envelope. CONTROL, so the + * zero is a reading — the same sweep finds `records` envelopes at other seams, + * including one live `find()` double (`plugin-list`'s ObjectGallery) and the + * record-visibility batch route stubs in this repo's own sibling packages. + * + * ⚠️ The refusal case below is ALSO satisfied by an `extractRecords` that + * returns `[]` for everything — an implementation strictly worse than the bug. + * The `data` and bare-array cases are the ones that refuse it: they push the + * SAME rows through the SAME mount, so an arm that delivers nothing delivered + * nothing because the envelope was refused. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, waitFor, cleanup, screen } from '@testing-library/react'; +import React from 'react'; + +// The timeline's own visualisation is orthogonal to what this file observes +// (how many rows reached it), and rendering it pulls a chart's worth of DOM +// per case. The rows arrive as `schema.items`. +vi.mock('./renderer', () => ({ + TimelineRenderer: ({ schema }: any) => ( +
+ ), +})); + +import { ObjectTimeline } from './ObjectTimeline'; + +const OBJECT = 'duty_task'; +const ROWS = [ + { id: 't1', subject: 'Ship it', starts_at: '2026-01-01T09:00:00Z' }, + { id: 't2', subject: 'Ship it again', starts_at: '2026-01-02T09:00:00Z' }, +]; + +/** 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 }); + +const schema: any = { + type: 'timeline', + objectName: OBJECT, + titleField: 'subject', + startDateField: 'starts_at', +}; + +/** Mount the timeline over a `find()` answering `envelope`, return rows drawn. */ +async function itemsThrough(envelope: Envelope): Promise { + const ds: Record = { + find: vi.fn(async () => envelope(ROWS)), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async (name: string) => ({ + name, + fields: { + id: { type: 'text', label: 'Id' }, + subject: { type: 'text', label: 'Subject' }, + starts_at: { type: 'datetime', label: 'Start' }, + }, + })), + }; + render(); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + // `find`'s OWN answer, settled — a pure read of the mock's call record that + // touches no DOM. Without it the assertion can be satisfied by the mount's + // initial empty state, which every arm renders identically. + await ds.find.mock.results[0].value; + await waitFor(() => + expect(screen.getByTestId('timeline-renderer').getAttribute('data-item-count')).not.toBeNull(), + ); + return Number(screen.getByTestId('timeline-renderer').getAttribute('data-item-count')); +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe('ObjectTimeline — the find() envelope it reads (objectui#6839)', () => { + it("still reads the contract's `data` member", async () => { + // ⛔ Never call `itemsThrough` from inside a `waitFor` predicate + // (objectui#7802): it renders, and `waitFor` re-runs its callback on DOM + // mutations, so the predicate feeds itself and leaks a container div per + // run. The render happens once, out here, and the case reads its answer. + expect(await itemsThrough(asData), 'the declared rows member must still draw').toBe(2); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await itemsThrough(asBareArray), 'the bare-array arm must still draw').toBe(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two rows drew off a key `QueryResult` does not + // declare, and did so AHEAD of `data`. + expect( + await itemsThrough(asRecords), + 'a `records` envelope must reach the rail as zero rows, not as the rows it names', + ).toBe(0); + }); +}); diff --git a/packages/plugin-tree/src/ObjectTree.contractEnvelope-6839.test.tsx b/packages/plugin-tree/src/ObjectTree.contractEnvelope-6839.test.tsx new file mode 100644 index 0000000000..03a74cec1f --- /dev/null +++ b/packages/plugin-tree/src/ObjectTree.contractEnvelope-6839.test.tsx @@ -0,0 +1,141 @@ +/** + * 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. + */ + +/** + * `ObjectTree` reads its `find()` answer as `QueryResult` DECLARES it — and + * does NOT read `records` (objectui#6839). + * + * ⭐ This module reaches the shared reader INDIRECTLY, and that is the whole + * reason it needs its own pin. It does not call `extractRecords` at all: it + * hands its `find()` answer to `applyNonGridRowCeiling` (`@object-ui/react`), + * which unwraps it. A card that enumerated the helper's direct callers would + * not list this file, and a repo-wide "nothing reads `records`" assertion would + * pass over it in silence — so the route itself is what is measured here, at + * the rows the forest draws. + * + * MEASURED for this module: no `find()` in `plugin-tree`, nor in any app or + * example mounting a tree, emits a `records` envelope — the package's single + * `records:` occurrence is a `buildForest(records: any[], …)` parameter name. + * CONTROL, so the zero is a reading: the same sweep finds a live `find()` + * double emitting `{ records: [...] }` at `plugin-list`'s ObjectGallery, a + * consumer with its own unwrap ladder. + * + * ⚠️ The refusal case is ALSO satisfied by an `extractRecords` that returns + * `[]` for everything — an implementation strictly worse than the bug. The + * `data` and bare-array cases refuse it: same rows, same mount. + */ + +import React from 'react'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { ObjectTree } from './ObjectTree'; + +// objectui#6892 slice 9 — inherit the real surface through `` rather than +// `typeof import('@object-ui/plugin-detail')`. `plugin-tree` does NOT declare +// that package and `ObjectTree`'s module graph reaches zero of its modules, so +// a type-position `import()` of it would be a real specifier to +// `check-phantom-dependencies`, which would then correctly demand a dependency +// the runtime does not have. +vi.mock('@object-ui/plugin-detail', async (importOriginal) => ({ + ...((await importOriginal()) as Record), + RecordDetailDrawer: () => null, + deriveRecordPageHref: () => null, +})); + +const ROWS = [ + { id: '1', name: 'Root', parent_id: null }, + { id: '2', name: 'Child', parent_id: '1' }, +]; + +const schema: any = { + type: 'object-tree', + objectName: 'node', + tree: { parentField: 'parent_id', labelField: 'name' }, + data: { provider: 'object', object: 'node' }, +}; + +/** 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 }); + +/** + * Mount the tree over a `find()` answering `envelope`, and hand back what it + * settled on: the number of rows it drew, or `'empty-state'` when it drew its + * own "No records" panel instead. + * + * The two outcomes are DIFFERENT DOM, not a count of 0 — `ObjectTree` returns + * early on an empty forest and never mounts the table. Reporting which one + * happened is what keeps a refused envelope distinguishable from a mount that + * never rendered. + * + * ⛔ Call ONCE per case, never inside a `waitFor` predicate (objectui#7802): + * it renders, and `waitFor` re-runs its callback on DOM mutations, so a + * predicate that renders feeds itself and leaks a container div per run. + */ +async function settledOn(envelope: Envelope): Promise { + const find = vi.fn(async () => envelope(ROWS)); + const ds: any = { + find, + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn(async () => ({ + name: 'node', + fields: { + id: { name: 'id', type: 'text' }, + name: { name: 'name', type: 'text' }, + parent_id: { name: 'parent_id', type: 'text' }, + }, + })), + }; + const { container } = render(); + await waitFor(() => expect(find).toHaveBeenCalled()); + // `find`'s OWN answer, settled — a pure read of the mock's call record that + // touches no DOM. Without it "no rows" is satisfied by the mount's initial + // empty state, which every arm renders identically. + await find.mock.results[0].value; + await waitFor(() => + expect( + container.querySelector('[data-testid="object-tree"]') ?? + screen.queryByText('No records'), + ).not.toBeNull(), + ); + return container.querySelector('[data-testid="object-tree"]') + ? container.querySelectorAll('tbody tr').length + : 'empty-state'; +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('ObjectTree — the find() envelope it reads (objectui#6839)', () => { + it("still reads the contract's `data` member", async () => { + expect(await settledOn(asData), 'the declared rows member must still draw').toBe(2); + }); + + it('still reads a bare array — the live non-envelope shape fakes answer with', async () => { + expect(await settledOn(asBareArray), 'the bare-array arm must still draw').toBe(2); + }); + + it('does NOT read `records` — not a QueryResult member', async () => { + // Before the fix these two nodes drew off a key `QueryResult` does not + // declare, and did so AHEAD of `data`. The tree now settles on its own + // "No records" panel — a DIFFERENT node from the table, which is what makes + // this a reading of the refusal rather than of a mount that never happened. + expect( + await settledOn(asRecords), + 'a `records` envelope must reach the forest as no rows at all, not as the rows it names', + ).toBe('empty-state'); + }); +}); diff --git a/packages/react/src/utils/nonGridRowCeiling.contractEnvelope-6839.test.ts b/packages/react/src/utils/nonGridRowCeiling.contractEnvelope-6839.test.ts new file mode 100644 index 0000000000..57d324cf00 --- /dev/null +++ b/packages/react/src/utils/nonGridRowCeiling.contractEnvelope-6839.test.ts @@ -0,0 +1,104 @@ +/** + * 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. + */ + +/** + * `applyNonGridRowCeiling` reads a `find()` answer as `QueryResult` DECLARES it + * — and does NOT read `records` (objectui#6839). + * + * ## Why this module gets its own pin + * + * It is a PUBLISHED `@object-ui/react` export in its own right (README + * "NON_GRID_ROW_CEILING"), so an app calls it directly, without any of the nine + * renderers in the picture. It is also the seam through which FOUR of them + * reach `extractRecords` — `ObjectCalendar`, `ObjectGantt`, `ObjectMap` and + * `ObjectTree` all hand it their `find()` answer rather than unwrapping it + * themselves. So it is one module by the pin's definition and four by the + * card's, and it behaves DIFFERENTLY from the other sinks: the rows it + * extracts also decide `truncated`, the probe-row verdict the loud footnote is + * drawn from. + * + * That second reading is the one a rows-only pin would miss. A refused envelope + * does not merely paint zero rows here — it also reports `truncated: false` + * over a result set that really was cut, i.e. it would have SILENCED the + * footnote. Both halves are asserted below. + * + * ⚠️ Every "0 rows" assertion here is also satisfied by an `extractRecords` + * that returns `[]` for everything — an implementation strictly worse than the + * bug. The `data` / bare-array / `value` cases are what refuse that, and they + * push the SAME rows through the SAME call. + */ + +import { describe, it, expect } from 'vitest'; +import { + NON_GRID_ROW_CEILING, + NON_GRID_ROW_CEILING_TOP, + applyNonGridRowCeiling, +} from './nonGridRowCeiling'; + +const rows = (n: number) => Array.from({ length: n }, (_, i) => ({ id: String(i + 1) })); + +describe('applyNonGridRowCeiling — the find() envelope it reads (objectui#6839)', () => { + describe('the shapes it still reads — the live arms', () => { + it("reads the contract's `data` member", () => { + const capped = applyNonGridRowCeiling({ data: rows(3), total: 3 }); + expect(capped.rows).toHaveLength(3); + expect(capped.total).toBe(3); + }); + + it('reads a bare array', () => { + expect(applyNonGridRowCeiling(rows(3)).rows).toHaveLength(3); + }); + + it('still reads `value` — LIVE at this seam', () => { + expect(applyNonGridRowCeiling({ value: rows(3) }).rows).toHaveLength(3); + }); + }); + + describe('the shape it refuses', () => { + it('does NOT read `records` — not a QueryResult member', () => { + const capped = applyNonGridRowCeiling({ records: rows(3), total: 3 }); + expect(capped.rows).toHaveLength(0); + }); + + it('`data` OUTRANKS `records` when a producer emits both', () => { + // Before the fix `records` was tried FIRST: this answered the one row + // under the undeclared key and dropped the three the contract declared. + const capped = applyNonGridRowCeiling({ data: rows(3), records: rows(1) }); + expect(capped.rows).toHaveLength(3); + }); + }); + + describe('the truncation verdict, which is this sink’s own behaviour', () => { + it('a `data` envelope over the ceiling still reports truncated, with the ceiling drawn', () => { + const capped = applyNonGridRowCeiling({ + data: rows(NON_GRID_ROW_CEILING_TOP), + total: 41234, + }); + expect(capped.truncated).toBe(true); + expect(capped.rows).toHaveLength(NON_GRID_ROW_CEILING); + expect(capped.total).toBe(41234); + }); + + it('a refused `records` envelope reports NOT truncated — the footnote it would have silenced', () => { + // The sharp half. `truncated` is decided from the extracted rows, so a + // refused envelope does not just draw nothing: it also states that + // nothing was cut, over a result set of 2001 rows. That is why this sink + // needs its own pin rather than inheriting the helper's. + const capped = applyNonGridRowCeiling({ + records: rows(NON_GRID_ROW_CEILING_TOP), + total: 41234, + }); + expect(capped.rows).toHaveLength(0); + expect(capped.truncated).toBe(false); + // `total` is read off the envelope independently of the rows, so it + // survives the refusal — stated so the zero above is not misread as + // "the whole object was rejected". + expect(capped.total).toBe(41234); + }); + }); +});