From 4fed69acc715548e0ca337c793abc93986a5476e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 01:35:52 +0000 Subject: [PATCH] fix(app-shell): forward the authored chart block WHOLE to ListView (objectui#7823) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The object-view relay handed `ListView` a hand-listed projection of exactly six keys — `chartType`, `xAxisField`, `yAxisFields`, `aggregation`, `series`, `config` — which is the pre-ADR-0021 key set, frozen. The whole ADR-0021 (objectui#1890) authoring shape (`dataset` / `dimensions` / `values`) and the legacy `categoryField` / `valueField` spelling had no rung, so a view that declared them reached `ListView` with its binding stripped and `ListView` could not tell that from a view that declared nothing. Once objectui#7544 gave `ListView.availableViews` a chart capability check, the gate was handed six `undefined` keys and correctly answered "nothing declared" about a view whose author declared everything: ADR-0047 filtered the author's own whitelist down to `['grid']`, so no Chart toggle appeared and no diagnostic was emitted. The legacy `xAxisField` spelling did survive the projection and did resolve, so the two authoring shapes behaved differently on this route for reasons that lived entirely in that one object literal. The projection is replaced by a pointer, not widened from six keys to nine: a hand-listed key set is a copy, and copies rot silently — three more keys would buy this ADR's correctness while re-arming the identical trap for the next block key, with nothing to fire, because `viewDef` is `Record` and a missing rung is invisible to tsc (objectui#7559 owns that mechanism and disclaims this census). Forwarding whole is safe because `ListView` reads the block BY NAME at both readers — `resolveListChartBinding` and `case 'chart'` — and never spreads it, so no key this relay stops dropping can collide downstream. It is the shape the sibling `gantt` / `timeline` / `tree` blocks in the same literal already use. An undeclared block now forwards `undefined` rather than the old permanently-truthy husk of six `undefined` keys, so the gate still offers no Chart toggle to a view that configured none. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3 --- ...7823-objectview-chart-relay-whole-block.md | 38 ++ .../views/ObjectView.chartRelay-7823.test.tsx | 340 ++++++++++++++++++ packages/app-shell/src/views/ObjectView.tsx | 41 ++- 3 files changed, 411 insertions(+), 8 deletions(-) create mode 100644 .changeset/7823-objectview-chart-relay-whole-block.md create mode 100644 packages/app-shell/src/views/ObjectView.chartRelay-7823.test.tsx diff --git a/.changeset/7823-objectview-chart-relay-whole-block.md b/.changeset/7823-objectview-chart-relay-whole-block.md new file mode 100644 index 0000000000..028d672f43 --- /dev/null +++ b/.changeset/7823-objectview-chart-relay-whole-block.md @@ -0,0 +1,38 @@ +--- +'@object-ui/app-shell': patch +--- + +Forward the authored `chart:` block WHOLE from the object-view relay to `ListView` +(objectui#7823). + +`ObjectView`'s `renderListView` builds the list schema by spreading the object's list +schema and then relaying the active view's visualization blocks under `options`. Every +sibling block is forwarded whole; `chart` was a hand-listed projection of exactly six +keys — `chartType`, `xAxisField`, `yAxisFields`, `aggregation`, `series`, `config` — the +pre-ADR-0021 key set, frozen. The whole ADR-0021 (objectui#1890) authoring shape +(`dataset` / `dimensions` / `values`) and the legacy `categoryField` / `valueField` +spelling had no rung, so a view that declared them reached `ListView` with its binding +stripped, and `ListView` could not tell that from a view that had declared nothing. + +**What changes for authors.** On the object-view route, a list view that declares an +ADR-0021 chart block and whitelists `chart` in `appearance.allowedVisualizations` is now +offered the Chart toggle and renders from the dataset it named. Before this fix the +capability gate added by objectui#7544 was handed six `undefined` keys, correctly +answered "nothing declared" about a view whose author had declared everything, and +ADR-0047 filtered the author's own whitelist down to `['grid']` — no toggle, no +diagnostic. The legacy `xAxisField` / `yAxisFields` spelling did survive the projection +and did resolve, so the two authoring shapes behaved differently on this route for +reasons that lived entirely in that one object literal. + +**Not a wider whitelist.** The projection is replaced by a pointer, not extended from +six keys to nine: a hand-listed key set is a copy, and copies rot silently — three more +keys would buy ADR-0021's correctness while re-arming the identical trap for the next +block key, and nothing would fire then either, because the relay's view definition is +`Record` and a missing rung is invisible to `tsc` (objectui#7559 owns that +mechanism). Forwarding whole is safe because `ListView` reads this block by name at both +of its readers — the capability gate's `resolveListChartBinding` and the `chart` render +branch — and never spreads it. + +A view that declares no chart block, an empty block, or a block with no binding at all +is still offered no Chart toggle: the relay now forwards `undefined` instead of the old +permanently-truthy husk of six `undefined` keys. diff --git a/packages/app-shell/src/views/ObjectView.chartRelay-7823.test.tsx b/packages/app-shell/src/views/ObjectView.chartRelay-7823.test.tsx new file mode 100644 index 0000000000..c06da2c77a --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.chartRelay-7823.test.tsx @@ -0,0 +1,340 @@ +/** + * 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. + */ + +/** + * objectui#7823 — the object-view relay handed `ListView` a SIX-KEY PROJECTION + * of the authored `chart:` block. + * + * ## The defect this pins + * + * `renderListView` builds `fullSchema` by spreading the OBJECT's `listSchema` + * and then relaying the active view's visualization blocks under `options`. + * Every sibling block is relayed WHOLE — `gantt: ganttViewOptions(viewDef)`, + * `timeline: timelineViewOptions(viewDef)`, `tree`/`gallery` by spread — but + * `chart` was a hand-listed copy of exactly six keys: + * + * chart: { + * chartType, xAxisField, yAxisFields, aggregation, series, config + * } + * + * That list is the PRE-ADR-0021 key set, frozen. `dataset`, `dimensions` and + * `values` — the whole ADR-0021 (#1890) authoring shape — plus the legacy + * `categoryField` / `valueField` spelling had no rung, so they were dropped + * here and could not reach the renderer at all on this route. + * + * ## Why it became load-bearing + * + * objectui#7544 gave `ListView.availableViews` a chart capability check that + * asks `resolveListChartBinding` — the render branch's own resolver — whether + * the block it was handed binds to names the author wrote. Through this + * projection an ADR-0021 block arrived as six `undefined` keys, so the gate + * correctly answered "nothing declared" about a view whose author declared + * everything: no Chart toggle, no diagnostic. The legacy + * `xAxisField` / `yAxisFields` spelling survived the projection and did + * resolve, so the two authoring shapes behaved differently on this route for + * reasons that lived entirely in this one object literal. + * + * ## The fix is a POINTER, not a wider copy + * + * Widening the list from six keys to nine would buy ADR-0021's correctness and + * re-arm the identical trap for the next block key — and nothing would fire + * then either, because `viewDef` is `Record` and a missing rung is + * invisible to `tsc` (that mechanism is objectui#7559's, and #7559 explicitly + * disclaims the census this card belongs to). A hand-listed key projection is a + * COPY of a schema's key set, and copies rot silently. So the relay forwards + * the authored block itself. + * + * The whole relay is safe to write because `ListView` never SPREADS this block: + * `resolveListChartBinding` and `case 'chart'` both read it BY NAME + * (`dataset` / `dimensions` / `values` / `chartType` / `xAxisField` / + * `yAxisFields` / `categoryField` / `valueField` / `aggregation` / `series`). + * Extra authored keys arrive and are ignored, exactly as they are for `gantt` + * and `tree`, so no key this relay stops dropping can collide downstream. + * + * ## Why the REAL `ListView` renders here + * + * The sibling relay tests in this directory stub `ListView` and inspect the + * captured schema, which answers "what does this file hand down". This card's + * claim is about the CAPABILITY GATE one seam further on, so the real component + * renders and the assertion is the Chart toggle's presence in the DOM — the + * objectui#6318 standard, that it renders differently. The captured schema is + * asserted too, as the more precise statement of the same fact. + * + * ## Direction, written before the run (reverse verification) + * + * The six-key literal was PREDICTED to turn the four `THE FIX` arms RED (no + * Chart tab, and `options.chart` carrying six `undefined` keys) while leaving + * every CONTROL's behaviour assertion GREEN — the legacy `xAxisField` arm, + * whose keys the projection already carried, and the three negative controls, + * which declare no usable binding and must be offered nothing in either world. + * + * MEASURED on the unmodified tree, before the fix was written: 5 failed, 3 + * passed. Four of the five are the `THE FIX` arms. The fifth is the SCHEMA half + * of the first negative control (`options.chart` was the six-`undefined`-key + * husk, not absent) — its DOM half, the toggle, was green there as predicted + * and stayed green after. After the fix: 8 passed. + * + * That asymmetry is the point: the old relay emitted a permanently truthy block + * for EVERY view, so a "fix" that merely added three keys to the husk would + * still hand the gate an object for a view that declared nothing. The negative + * controls are what refuse that shape. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor, screen, fireEvent } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; + +vi.mock('@object-ui/permissions', () => ({ + usePermissions: () => ({ + check: () => ({ allowed: true }), + checkField: () => true, + getFieldPermissions: () => [], + getRowFilter: () => undefined, + getObjectApiOperations: () => undefined, + roles: [], + isLoaded: false, + hasCapabilities: () => true, + can: () => true, + cannot: () => false, + }), + useFieldPermissions: () => ({ canRead: () => true, canWrite: () => true, permissions: [] }), +})); + +vi.mock('@object-ui/auth', () => ({ + useAuth: () => ({ user: { id: 'u1', name: 'Ada' }, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', () => ({ + useRealtimeSubscription: () => ({ lastMessage: null }), + useConflictResolution: () => ({ hasConflicts: false, resolveAllConflicts: () => {} }), +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +/** + * The list schema this page hands down — captured on the way INTO the real + * `ListView`, which then renders. Both halves of the evidence come from one + * mount: what the relay emitted, and what the capability gate did with it. + */ +let captured: any = null; +vi.mock('@object-ui/plugin-list', async (importOriginal) => { + const actual = await importOriginal>(); + const { createElement } = await import('react'); + return { + ...actual, + ListView: (props: any) => { + captured = props.schema; + return createElement(actual.ListView as any, props); + }, + }; +}); + +vi.mock('@object-ui/plugin-view', async (importOriginal) => ({ + ...(await importOriginal>()), + ObjectView: (props: any) => + props.renderListView?.({ + schema: { ...(props.schema ?? {}) }, + dataSource: props.dataSource, + onEdit: props.onEdit, + className: '', + refreshKey: 0, + }) ?? null, + ViewTabBar: () => null, + ManageViewsDialog: () => null, +})); + +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); +vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null })); + +import { ObjectView } from './ObjectView'; +import { ExpressionProvider } from '../providers/ExpressionProvider'; + +const OBJECT_NAME = 'duly_task'; + +/** The ADR-0021 (#1890) authoring shape — a semantic dataset selected BY NAME. */ +const ADR_0021_BLOCK = { + dataset: 'task_throughput', + dimensions: ['status'], + values: ['hours'], + chartType: 'bar', +}; + +function objectsWith(view: Record) { + return [ + { + name: OBJECT_NAME, + label: 'Task', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + status: { type: 'text', label: 'Status' }, + hours: { type: 'number', label: 'Hours' }, + }, + listViews: { + by_unit: { label: 'By business unit', type: 'grid', columns: ['name'], ...view }, + }, + }, + ]; +} + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: [], total: 0 })), + findOne: vi.fn(async () => null), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + getObjectSchema: vi.fn(async () => ({ name: OBJECT_NAME, fields: {} })), + } as any; +} + +/** Mount the object list for one authored view and wait for the relay to run. */ +async function mountObjectList(view: Record) { + captured = null; + const dataSource = makeDataSource(); + render( + + + + {}} />} + /> + + + , + ); + // `options` is built unconditionally by the same object literal as the rung + // under test, so its arrival is the signal that the relay actually ran — + // waiting on `options.chart` itself would hang rather than fail. + await waitFor(() => { + expect(captured?.options).toBeTruthy(); + }); +} + +/** + * Find a visualization option by accessible name, in either switcher form — + * two-to-four resolvable types render an inline segmented control exposing + * `role="tab"`, five or more collapse into a dropdown of plain buttons. + * Mirrors the helper in `plugin-list`'s `ListView.chart-capability-7544`. + */ +const queryViewOption = (name: string) => + screen.queryByRole('tab', { name }) ?? screen.queryByRole('button', { name }); + +/** + * Is the Chart toggle offered for a view whitelisting exactly + * `['grid', 'chart']`? The whitelist alone never suffices — ADR-0047 + * intersects it with what the capability gate finds RESOLVABLE, which is the + * question this relay's output answers. + */ +async function chartOffered(view: Record): Promise { + await mountObjectList({ appearance: { allowedVisualizations: ['grid', 'chart'] }, ...view }); + const trigger = screen.queryByTestId('view-switcher-dropdown'); + if (trigger) fireEvent.click(trigger); + return Boolean(queryViewOption('Chart')); +} + +beforeEach(() => { + cleanup(); + captured = null; + vi.stubGlobal( + 'fetch', + vi.fn(async () => + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); +}); + +describe('the object-view relay forwards the authored chart block WHOLE (objectui#7823)', () => { + it('THE FIX: an ADR-0021 view is offered the Chart toggle', async () => { + // THE DISCRIMINATING ARM, and the card's whole report in one line. Before + // the fix this read `false`: the six-key projection dropped `dataset` / + // `dimensions` / `values`, the gate found nothing declared, and ADR-0047 + // filtered the author's own whitelist down to `['grid']`. + expect(await chartOffered({ chart: ADR_0021_BLOCK })).toBe(true); + }); + + it('THE FIX: the ADR-0021 keys arrive at the renderer, not six undefined ones', async () => { + // The precise statement of the same fact. `toEqual` against the authored + // object pins BOTH directions: no declared key is dropped, and no key the + // author did not write is invented on the way through. + await mountObjectList({ chart: ADR_0021_BLOCK }); + expect(captured.options.chart).toEqual(ADR_0021_BLOCK); + }); + + it('THE FIX: the legacy `categoryField` / `valueField` spelling survives too', async () => { + // Absent from the six-key list exactly as the ADR-0021 keys were, and read + // by the same resolver. One projection dropped both; one pointer carries + // both. + expect(await chartOffered({ chart: { categoryField: 'status', valueField: 'hours' } })).toBe(true); + await mountObjectList({ chart: { categoryField: 'status', valueField: 'hours' } }); + expect(captured.options.chart).toEqual({ categoryField: 'status', valueField: 'hours' }); + }); + + it('THE FIX: keys beyond the block are relayed verbatim, not whitelisted', async () => { + // The rot this card is about is a key set frozen at a moment in time. A + // relay that carried today's nine keys would fail this the day the block + // grows a tenth, which is precisely the trap being closed. + const block = { ...ADR_0021_BLOCK, filter: [{ field: 'status', op: 'eq', value: 'open' }], config: { stacked: true } }; + await mountObjectList({ chart: block }); + expect(captured.options.chart).toEqual(block); + }); + + it('CONTROL: the legacy `xAxisField` / `yAxisFields` block still resolves', async () => { + // GREEN in either world — these keys the old projection already carried. + // It is here so a regression in the relay's shape cannot hide behind the + // arms above, and to pin that the two authoring shapes now behave the same + // on this route, which is the asymmetry the card reported. + expect(await chartOffered({ chart: { xAxisField: 'status', yAxisFields: ['hours'] } })).toBe(true); + }); + + it('NEGATIVE CONTROL: a view declaring no chart block is offered no toggle', async () => { + // GREEN in either world, and the reason the relay must forward `undefined` + // rather than a permanently truthy husk: the old projection was ALWAYS a + // truthy object, and a fix that kept that shape while adding the three keys + // would still be handing the gate an object for a view that declared + // nothing. `resolveListChartBinding` reads no binding out of either, so the + // observable answer is the same — this pins that it stays the same. + expect(await chartOffered({})).toBe(false); + await mountObjectList({}); + expect(captured.options.chart).toBeUndefined(); + }); + + it('NEGATIVE CONTROL: an EMPTY chart block is offered no toggle', async () => { + // The half-written declaration: `allowedVisualizations: ['grid','chart']` + // with nothing under `chart:`. It must stay half-written all the way down — + // the gate offers only blocks that render from names the author wrote. + expect(await chartOffered({ chart: {} })).toBe(false); + }); + + it('NEGATIVE CONTROL: a block with no binding at all is offered no toggle', async () => { + // `chartType` alone chooses a shape, not a binding. Offering this would + // route into the legacy branch's invented `'name'` / `'value'` floors — + // the objectui#7547 / #7029 / #7070 family, deliberately untouched here. + expect(await chartOffered({ chart: { chartType: 'pie' } })).toBe(false); + }); +}); diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 12af054ae3..6070f34293 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -2432,14 +2432,39 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an ...((viewDef as any).tree || {}), labelField: (viewDef as any).tree?.labelField || (viewDef as any).tree?.titleField || 'name', }, - chart: { - chartType: viewDef.chart?.chartType, - xAxisField: viewDef.chart?.xAxisField, - yAxisFields: viewDef.chart?.yAxisFields, - aggregation: viewDef.chart?.aggregation, - series: viewDef.chart?.series, - config: viewDef.chart?.config, - }, + // The chart block the view DECLARED, forwarded WHOLE — a + // pointer, not a copy of its key set (objectui#7823). + // + // This was a hand-listed projection of six keys (`chartType` / + // `xAxisField` / `yAxisFields` / `aggregation` / `series` / + // `config`): the PRE-ADR-0021 set, frozen. The whole ADR-0021 + // (#1890) authoring shape (`dataset` / `dimensions` / `values`) + // and the legacy `categoryField` / `valueField` spelling had no + // rung here, so an author who declared them reached `ListView` + // with the binding stripped. Once objectui#7544 gave + // `ListView.availableViews` a chart capability check, that gate + // was handed six `undefined` keys and correctly answered + // "nothing declared" about a view whose author declared + // everything — the Chart toggle stayed hidden, with no + // diagnostic, while the legacy `xAxisField` spelling (which the + // projection did carry) went on resolving. + // + // ⛔ NOT widened to nine keys. A hand-listed projection is a + // COPY of a schema's key set and copies rot silently: three more + // keys would buy this ADR's correctness and re-arm the identical + // trap for the next block key, with nothing to fire — `viewDef` + // is `Record`, so a missing rung is invisible to + // tsc (objectui#7559 owns that mechanism and disclaims the + // census this belongs to). + // + // Forwarding whole is safe because `ListView` never SPREADS this + // block — `resolveListChartBinding` and `case 'chart'` both read + // it BY NAME — so keys it does not consult are ignored exactly + // as they are for the `gantt` / `timeline` / `tree` blocks above, + // which are already relayed whole. Undeclared stays `undefined` + // rather than the old permanently-truthy husk, which is what + // keeps the gate from offering a chart nobody configured. + chart: viewDef.chart, }, };