From 511d688f8e0ba1ead529ed08c2f8498dd903fb9a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 02:51:24 +0000 Subject: [PATCH 1/3] fix(app-shell): ReportView reads only the declared `dataSource.object` key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ReportView` accepted `resource` as a second spelling of `object` on a report's `dataSource` binding, in two places, and NAMED that spelling in a user-facing warning: :171 liveReport?.objectName || liveReport?.dataSource?.object || liveReport?.dataSource?.resource :273 dataFetchSource.dataSource.object || dataFetchSource.dataSource.resource :275 console.warn('ReportView: dataSource missing object/resource property') `resource` is not on this contract. `ElementDataSourceConfig` declares `object` / `view?` / `filter?` / `sort?` / `limit?`, its spec twin `ElementDataSourceSchema` is a strict object (an extra `resource` key is rejected, not ignored), and `isElementDataSourceConfig` decides on `object` — so a `resource`-only binding was never a binding anywhere else. `resource` is a real key on OTHER surfaces (`CRUDSchema.resource`, the `DataSource` adapter's first parameter, `LiveExportOptions.resource`); none of them is this one. Per AGENTS.md #0.1 an off-spec spelling is fixed at the producer, never as a renderer-side alias — and a producer census found nothing to fix: no site in this repo or in the `objectstack` framework repo writes `resource` onto a report `dataSource`. The limb was speculative in the commit that added it. Measured by rendering, per input shape (before -> after): object only queries `acct` -> queries `acct` (unchanged) resource only queries `acct` -> not queried, warns (REMOVED) object+resource queries `acct` -> queries `acct` (unchanged) neither not queried -> not queried, warns (unchanged) A `resource`-only binding now fails loudly (named warning, empty rows, fallback field list) instead of rendering off-spec metadata as if it were valid. The `:275` wording names only `object`, so the diagnostic no longer promises a key the contract does not declare. The local `resource` variable at the fetch site is renamed `objectName` — it was the last in-code echo of the non-contract spelling at the very line being fixed, and an open invitation to re-add the alias. The adapter's own parameter is still called `resource`; a comment records where the two vocabularies meet. Fixes #5116 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE --- .../ReportView.dataSourceObjectKey.test.tsx | 209 ++++++++++++++++++ packages/app-shell/src/views/ReportView.tsx | 15 +- 2 files changed, 218 insertions(+), 6 deletions(-) create mode 100644 packages/app-shell/src/views/ReportView.dataSourceObjectKey.test.tsx diff --git a/packages/app-shell/src/views/ReportView.dataSourceObjectKey.test.tsx b/packages/app-shell/src/views/ReportView.dataSourceObjectKey.test.tsx new file mode 100644 index 000000000..4a8e5453a --- /dev/null +++ b/packages/app-shell/src/views/ReportView.dataSourceObjectKey.test.tsx @@ -0,0 +1,209 @@ +/** + * 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. + */ + +/** + * ReportView reads `dataSource.object` — and ONLY `object` (objectui#5116). + * + * The view used to accept `resource` as a second spelling of `object` in two + * places, and to NAME that spelling in a user-facing warning: + * + * :171 liveReport?.objectName || liveReport?.dataSource?.object + * || liveReport?.dataSource?.resource + * :273 dataFetchSource.dataSource.object || dataFetchSource.dataSource.resource + * :275 console.warn('ReportView: dataSource missing object/resource property') + * + * `resource` is not on this contract. `ElementDataSourceConfig` + * (`packages/core/src/data-scope/element-data-source.ts`) declares + * `object` / `view?` / `filter?` / `sort?` / `limit?`, its spec twin + * `ElementDataSourceSchema` is `z.strictObject` (an extra `resource` key is + * REJECTED, not ignored), and the binding's own predicate + * `isElementDataSourceConfig` decides on `object` — so a `resource`-only + * binding was never a binding on any other renderer. `resource` IS a real key + * elsewhere (`CRUDSchema.resource`, the `DataSource` adapter's first + * parameter, `LiveExportOptions.resource`); none of those is this surface. + * + * Per AGENTS.md #0.1 the fix belongs at the producer, never as a renderer-side + * alias — and the producer census found no producer to fix: nothing in this + * repo or in the `objectstack` framework repo writes `resource` onto a report + * `dataSource`. The limb was speculative from the commit that introduced it. + * + * What these pin, per input shape, BY RENDERING (not by reading the source): + * + * object only -> queries that object (unchanged) + * resource only -> NOT queried; the view reports it (behaviour REMOVED) + * object+resource -> queries `object`; `resource` inert (unchanged) + * neither -> NOT queried; the view reports it (unchanged) + * + * and, on the second limb, which object the config panel's field list is + * derived from. Both limbs are measured in every shape, so a fix applied to + * only one of them fails here. + * + * Non-vacuity: the `resource`-only cases put a REAL object (`acct`, with real + * fields) behind the non-contract spelling, so "no query" and "fallback + * fields" prove the spelling was not read — not that there was nothing to + * read. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; + +/** Props the (stubbed) report renderer and config panel were handed. */ +const cap = vi.hoisted(() => ({ renderer: null as any, panel: null as any })); + +vi.mock('@object-ui/plugin-report', () => ({ + ReportRenderer: (props: any) => { + cap.renderer = props; + return null; + }, +})); +vi.mock('@object-ui/plugin-dashboard', () => ({ DrillDownDrawer: () => null })); +vi.mock('./ReportConfigPanel', () => ({ + ReportConfigPanel: (props: any) => { + cap.panel = props; + return null; + }, +})); + +const meta = vi.hoisted(() => ({ value: null as any })); +vi.mock('../providers/MetadataProvider', () => ({ useMetadata: () => meta.value })); + +vi.mock('react-router-dom', () => ({ + useParams: () => ({ reportName: 'revenue_by_month' }), + useNavigate: () => vi.fn(), + useLocation: () => ({ pathname: '/reports/revenue_by_month', search: '' }), +})); + +vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() })); +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false }), +})); +vi.mock('./metadata-admin/useMetadata', () => ({ useMetadataClient: () => ({ get: vi.fn() }) })); +vi.mock('./runtime-metadata-persistence', () => ({ persistRuntimeMetadata: vi.fn() })); +vi.mock('../providers/AdapterProvider', () => ({ useAdapter: () => ({}) })); +vi.mock('../providers/ExpressionProvider', () => ({ useExpressionContext: () => ({ app: undefined }) })); +vi.mock('@object-ui/auth', () => ({ useIsWorkspaceAdmin: () => true })); +vi.mock('@object-ui/i18n', () => ({ + useObjectTranslation: () => ({ t: (k: string) => k }), + createSafeTranslation: (defaults: Record) => () => ({ + t: (k: string) => defaults?.[k] ?? k, + }), +})); + +import { ReportView } from './ReportView'; + +/** A REAL object with REAL fields — what a read of `resource` would find. */ +const ACCT_OBJECT = { + name: 'acct', + label: 'Account', + fields: { industry: { label: 'Industry', type: 'text' } }, +}; +/** A second real object, so "both keys" can show WHICH one was read. */ +const OTHER_OBJECT = { + name: 'other', + label: 'Other', + fields: { misc: { label: 'Misc', type: 'text' } }, +}; + +/** + * Mount the view over a report whose `dataSource` is exactly `ds`, and hand + * back the two live measurements: what the adapter was asked to query, and + * which field list the config panel was handed. + */ +async function mountReport(ds: Record) { + const find = vi.fn(async () => ({ data: [{ id: '1' }] })); + meta.value = { + apps: [], + objects: [ACCT_OBJECT, OTHER_OBJECT], + dashboards: [], + reports: [{ name: 'revenue_by_month', label: 'Revenue by Month', dataSource: ds }], + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: vi.fn(async () => null), + getItemsByType: () => [], + getTypeStatus: () => 'ready', + }; + + render(); + await waitFor(() => expect(cap.panel).not.toBeNull()); + // The fetch effect is async; let it settle before reading the spy. + await waitFor(() => expect(find.mock.calls.length >= 0).toBe(true)); + await new Promise((r) => setTimeout(r, 0)); + + return { + /** First argument of the adapter query, or null when never queried. */ + queried: find.mock.calls.length ? (find.mock.calls[0][0] as string) : null, + /** Field values the config panel got, e.g. ['industry'] or the fallbacks. */ + fieldValues: (cap.panel.availableFields as any[]).map((f) => f.value), + }; +} + +let warn: ReturnType; + +beforeEach(() => { + cap.renderer = null; + cap.panel = null; + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => { + warn.mockRestore(); + vi.clearAllMocks(); +}); + +/** Every `console.warn` argument the view emitted, flattened to one string. */ +const warnText = () => warn.mock.calls.map((c) => c.join(' ')).join('\n'); + +describe('ReportView — `dataSource.object` is the only spelling (objectui#5116)', () => { + it('object only: queries that object, and derives the field list from it', async () => { + const { queried, fieldValues } = await mountReport({ object: 'acct' }); + + expect(queried).toBe('acct'); + expect(fieldValues).toEqual(['industry']); + expect(warnText()).not.toContain('dataSource missing'); + }); + + it('resource only: NOT queried — the undeclared spelling is not a binding', async () => { + const { queried, fieldValues } = await mountReport({ resource: 'acct' }); + + // `acct` is a real object with real fields; it stays unread all the same. + expect(queried).toBeNull(); + expect(fieldValues).not.toContain('industry'); + expect(warnText()).toContain('ReportView: dataSource missing object property'); + }); + + it('object + resource: `object` wins and `resource` is inert', async () => { + const { queried, fieldValues } = await mountReport({ object: 'acct', resource: 'other' }); + + expect(queried).toBe('acct'); + expect(fieldValues).toEqual(['industry']); + expect(warnText()).not.toContain('dataSource missing'); + }); + + it('neither: NOT queried, and the view reports the missing key', async () => { + const { queried } = await mountReport({}); + + expect(queried).toBeNull(); + expect(warnText()).toContain('ReportView: dataSource missing object property'); + }); + + it('the warning names `object` and does NOT teach the non-contract `resource`', async () => { + await mountReport({ resource: 'acct' }); + + const text = warnText(); + expect(text).toContain('object'); + // The old wording was 'missing object/resource property' — a user-facing + // diagnostic that promised a key the contract never declared. + expect(text).not.toContain('resource'); + expect(text).not.toContain('object/resource'); + }); +}); diff --git a/packages/app-shell/src/views/ReportView.tsx b/packages/app-shell/src/views/ReportView.tsx index c0a9e0187..13466eff4 100644 --- a/packages/app-shell/src/views/ReportView.tsx +++ b/packages/app-shell/src/views/ReportView.tsx @@ -168,7 +168,7 @@ export function ReportView({ dataSource }: { dataSource?: DataSource }) { // Uses live editSchema when available to respond to objectName changes const availableFields = useMemo(() => { const liveReport = editSchema || reportData; - const objName = liveReport?.objectName || liveReport?.dataSource?.object || liveReport?.dataSource?.resource; + const objName = liveReport?.objectName || liveReport?.dataSource?.object; return getFieldsForObject(objName) ?? FALLBACK_FIELDS; }, [editSchema, reportData, getFieldsForObject]); @@ -269,15 +269,18 @@ export function ReportView({ dataSource }: { dataSource?: DataSource }) { if (dataFetchSource.dataSource) { const fetchDataFromSource = async () => { try { - // Use the dataSource configuration to fetch data - const resource = dataFetchSource.dataSource.object || dataFetchSource.dataSource.resource; - if (!resource) { - console.warn('ReportView: dataSource missing object/resource property'); + // Use the dataSource configuration to fetch data. `object` is the + // ONLY spelling this binding declares (ElementDataSourceConfig / + // the spec's strict ElementDataSourceSchema); the adapter just + // happens to call its first parameter `resource` (objectui#5116). + const objectName = dataFetchSource.dataSource.object; + if (!objectName) { + console.warn('ReportView: dataSource missing object property'); setReportRuntimeData([]); return; } - const result = await dataSource.find(resource, { + const result = await dataSource.find(objectName, { $filter: dataFetchSource.dataSource.filter, $orderby: dataFetchSource.dataSource.sort, $top: dataFetchSource.dataSource.limit, From c550ed1ab10b72afa0ad4a6879bbabbfefc75769 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 02:54:41 +0000 Subject: [PATCH 2/3] chore(changeset): declare the ReportView `dataSource.object` narrowing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behaviour removal, declared honestly: a `resource`-only report binding used to render and now does not. Marked `minor` per AGENTS.md (objectui's own breaking semantics never take `major` — that would drag the whole fixed group off the `@objectstack` major it tracks); the breaking semantics are spelled out in the changeset body along with the per-shape before/after table. Part of #5116 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE --- .../report-view-datasource-object-key-5116.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 .changeset/report-view-datasource-object-key-5116.md diff --git a/.changeset/report-view-datasource-object-key-5116.md b/.changeset/report-view-datasource-object-key-5116.md new file mode 100644 index 000000000..2d3435113 --- /dev/null +++ b/.changeset/report-view-datasource-object-key-5116.md @@ -0,0 +1,51 @@ +--- +'@object-ui/app-shell': minor +--- + +`ReportView` reads a report's data binding through the one key the contract declares — `dataSource.object`. + +The view accepted `resource` as a second spelling of `object`, in two places, and +named that spelling in a warning the user could read: + +``` +:171 liveReport?.objectName || liveReport?.dataSource?.object + || liveReport?.dataSource?.resource +:273 dataFetchSource.dataSource.object || dataFetchSource.dataSource.resource +:275 console.warn('ReportView: dataSource missing object/resource property') +``` + +`resource` is not on this binding. `ElementDataSourceConfig` declares `object`, +`view?`, `filter?`, `sort?` and `limit?`; its `@objectstack/spec` twin +`ElementDataSourceSchema` is a strict object, so an extra `resource` key is +*rejected* there rather than ignored; and the binding's own predicate +`isElementDataSourceConfig` decides on `object`. A `resource`-only binding +therefore was never a binding on any other renderer in the system — it rendered +here and silently produced nothing anywhere else, with neither end reporting a +problem. That divergence is what a consumer-side alias buys: one renderer +answering a question the contract says has no answer. + +`resource` is a real key on other surfaces — `CRUDSchema.resource`, the +`DataSource` adapter's first parameter, `LiveExportOptions.resource` — and all +three are untouched. None of them is this one. + +Behaviour, measured by rendering each input shape before and after. Only the +`resource`-only shape moves: + +| binding | before | after | +| --- | --- | --- | +| `object` only | queries that object | unchanged | +| `resource` only | queries it as if declared | not queried; named warning, no rows, fallback field list | +| both | queries `object` | unchanged | +| neither | not queried; warning | unchanged | + +So off-spec report metadata that used to render now fails loudly instead of +appearing to work. A producer census found nothing that would notice: no site in +this repository, and none in the `objectstack` framework repository, writes +`resource` onto a report `dataSource`. The limb was speculative in the commit +that introduced it, and per AGENTS.md #0.1 an off-spec spelling is corrected at +the producer, never taught a second dialect by the renderer. + +The `:275` wording now names only `object`. A diagnostic that lists a key the +contract does not declare is not a small thing: it is the system telling an +author — increasingly, an author's code generator — that the wrong spelling is +supported. From d8a61a5ceba40237c6dc368c38f8075c88c7ef14 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 03:10:24 +0000 Subject: [PATCH 3/3] test(app-shell): type the ReportView adapter spy against DataSource.find MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tsc -p tsconfig.test.json` rejected three spots in the new pin: an untyped `vi.fn()` gave `mock.calls` an empty tuple type, so reading the captured first argument was both an impossible cast and an out-of-range index, and the warn spy's callback parameter was implicitly `any`. The spy now declares the real `DataSource.find(objectName, params)` signature, which makes the captured argument a `string` with no cast at all — the assertion reads the query the way the adapter would receive it. Part of #5116 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE --- .../src/views/ReportView.dataSourceObjectKey.test.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/app-shell/src/views/ReportView.dataSourceObjectKey.test.tsx b/packages/app-shell/src/views/ReportView.dataSourceObjectKey.test.tsx index 4a8e5453a..c295e2902 100644 --- a/packages/app-shell/src/views/ReportView.dataSourceObjectKey.test.tsx +++ b/packages/app-shell/src/views/ReportView.dataSourceObjectKey.test.tsx @@ -117,7 +117,9 @@ const OTHER_OBJECT = { * which field list the config panel was handed. */ async function mountReport(ds: Record) { - const find = vi.fn(async () => ({ data: [{ id: '1' }] })); + const find = vi.fn( + async (_objectName: string, _params?: Record) => ({ data: [{ id: '1' }] }), + ); meta.value = { apps: [], objects: [ACCT_OBJECT, OTHER_OBJECT], @@ -142,7 +144,7 @@ async function mountReport(ds: Record) { return { /** First argument of the adapter query, or null when never queried. */ - queried: find.mock.calls.length ? (find.mock.calls[0][0] as string) : null, + queried: find.mock.calls.length ? find.mock.calls[0][0] : null, /** Field values the config panel got, e.g. ['industry'] or the fallbacks. */ fieldValues: (cap.panel.availableFields as any[]).map((f) => f.value), }; @@ -161,7 +163,8 @@ afterEach(() => { }); /** Every `console.warn` argument the view emitted, flattened to one string. */ -const warnText = () => warn.mock.calls.map((c) => c.join(' ')).join('\n'); +const warnText = () => + warn.mock.calls.map((c: unknown[]) => c.join(' ')).join('\n'); describe('ReportView — `dataSource.object` is the only spelling (objectui#5116)', () => { it('object only: queries that object, and derives the field list from it', async () => {