diff --git a/.changeset/12061-chart-refuse-before-fetch.md b/.changeset/12061-chart-refuse-before-fetch.md new file mode 100644 index 0000000000..0d10c6ac6a --- /dev/null +++ b/.changeset/12061-chart-refuse-before-fetch.md @@ -0,0 +1,9 @@ +--- +'@object-ui/plugin-charts': patch +--- + +`ObjectChart` decides its missing-category-axis refusal before it fetches, so a chart that refuses reads nothing (objectui#12061). + +The refusal (`chart-missing-category-axis`, objectui#8168) is drawn when an object-bound chart declares no category by any spelling (`aggregate.groupBy`, `xAxisKey`, `xAxis.field`). It was a render-time return only: the fetch ran anyway, issuing the object's `find` with no `$top` (or `aggregate` with no `groupBy`), and the rows were then discarded. Since objectui#6152 round 15, every chart list view that names no `dataset` composes that node, so each render of such a view paid for the read. + +Now the refusal and the fetch read one value. A refused chart issues no `find`, no `aggregate` and no comparison query, and a data-invalidation event for its object does not start one. The refusal's code, `role="alert"` and wording are unchanged, and it is still the first paint, with no loading skeleton before it. A chart with a category fetches exactly as before. A chart that is refused at mount and later given a category (for example, a spec `xAxis` added in a designer preview) fetches at that point. diff --git a/apps/console/src/__tests__/public-block-binding-reach.test.tsx b/apps/console/src/__tests__/public-block-binding-reach.test.tsx index f8eb8e5384..8dec81897f 100644 --- a/apps/console/src/__tests__/public-block-binding-reach.test.tsx +++ b/apps/console/src/__tests__/public-block-binding-reach.test.tsx @@ -65,7 +65,7 @@ * compensate for: a claim wider than the thing behind it. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { render, act } from '@testing-library/react'; import { ComponentRegistry } from '@object-ui/core'; import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; @@ -79,6 +79,26 @@ import '../register-plugins'; /** The object name every probed block is bound to; must appear in a data call. */ const PROBE_OBJECT = 'probe_object__c'; +/** + * objectui#12061: the `object-chart` fixture now declares a category (see + * {@link sampleFor}), and a chart with a category also reads its option + * colours through the GLOBAL `fetch`, at `/api/v1/meta/object/OBJECT`, because + * this provider passes no `apiFetch`. That read is not a data-layer call and + * not what this probe measures. So it is answered here with an empty document, + * and every other request still goes to the network-escape guard's `fetch`, so + * any other escape stays a named red. Installed once, at module scope and never + * torn down, as the guard's own `Fix:` text prescribes for a read that can land + * after a test body returns. + */ +const guardedFetch = globalThis.fetch; +vi.stubGlobal('fetch', ((input: unknown, init?: RequestInit) => { + const url = typeof input === 'string' ? input : String((input as { url?: string })?.url ?? input); + if (url.endsWith(`/api/v1/meta/object/${PROBE_OBJECT}`)) { + return Promise.resolve({ ok: true, status: 200, json: async () => ({}) }); + } + return guardedFetch(input as Parameters[0], init); +}) as typeof globalThis.fetch); + /** * `DataSource` methods the recording stub carries as real own properties, so a * block that derives a source by spreading (`{...dataSource, …}`) still gets @@ -347,8 +367,19 @@ const SUPERSEDES_BINDING = new Set(['data', 'staticData', 'customFields']); * for as long as nobody looked. Both samples below are spec-valid on their own * merit, not as crash avoidance. */ -const sampleFor = (input: any): unknown => { +const sampleFor = (input: any, blockType?: string): unknown => { if (input.name === 'objectName') return PROBE_OBJECT; + // objectui#12061: `object-chart` refuses a chart with no category axis BEFORE + // it fetches, and the generic `object` sample `{}` names none. It is also not + // an aggregate any author could publish: the spec's `ChartAggregateSchema` + // requires `function` and `groupBy`. So the chart gets the smallest aggregate + // that schema accepts, which gives it a category and keeps `objectName`'s data + // reach measured. Keyed by block TYPE as well as by name, because + // `object-metric` declares an `aggregate` too, and a metric reads it with its + // own requiredness (`groupBy` optional). + if (blockType === 'object-chart' && input.name === 'aggregate') { + return { function: 'count', groupBy: 'name' }; + } // `record:related_list.add` — the generic `object` sample below is `{}`, and // `{}` is not a valid `add`: the spec makes `picker` required. An invalid one // did not merely under-configure this block, it CRASHED it @@ -461,7 +492,7 @@ async function dataCallsFor(cfg: any): Promise { const schema: Record = { type: cfg.type }; for (const input of cfg.inputs ?? []) { if (SUPERSEDES_BINDING.has(input.name)) continue; - schema[input.name] = sampleFor(input); + schema[input.name] = sampleFor(input, cfg.type); } const view = render( diff --git a/packages/plugin-charts/src/ObjectChart.refuseBeforeFetch-12061.test.tsx b/packages/plugin-charts/src/ObjectChart.refuseBeforeFetch-12061.test.tsx new file mode 100644 index 0000000000..5b188d209e --- /dev/null +++ b/packages/plugin-charts/src/ObjectChart.refuseBeforeFetch-12061.test.tsx @@ -0,0 +1,232 @@ +/** + * 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#12061 — a chart that REFUSES for a missing category axis decides so + * BEFORE it fetches, and reads nothing. + * + * objectui#8168's refusal (`chart-missing-category-axis`) is a render-time + * return, but the object-bound fetch effect did not read it: an object-bound + * node naming no category still issued `ds.find(objectName, { $filter })` — + * with no `$top`, so bounded only by the data source's own default — and then + * threw the rows away to draw the refusal. Since objectui#6152 round 15 every + * chart list view that names no `dataset` composes exactly that node, on three + * relays (ListView's `chart` case, app-shell's `renderListView`, plugin-view's + * `generateViewSchema`), so every render of such a view paid for the read. + * + * The fix is objectui#7390's shape for `ObjectGallery`: decide the binding + * first, then fetch. The refusal's predicate is ONE value in `ObjectChart`, + * read by the render-time return and by the fetch gate alike, so the two + * cannot disagree about which chart refuses. ⛔ Not a `$top` cap: a cap would + * shrink the read, not remove it. + * + * ## What each case measures + * + * - The round-15 probe's node (`{ objectName: 'task', chartType: 'bar', + * filter: [] }`): `find` zero times, `aggregate` zero times, and the refusal + * still draws — on the FIRST paint, with no loading skeleton before it. + * - A declared measure with no `groupBy` — the arm the refusal file calls the + * card's exact bag. It reaches `ds.aggregate` rather than `ds.find`, so the + * gate has to sit before both legs, not on one of them. + * - A `compareTo` overlay: the comparison window is not queried either. + * - A write to the object does not wake a refused chart into a fetch: the + * invalidation subscription keys on the same gate. + * - Control, both legs of a bound chart: it fetches exactly as before — one + * query, on the leg its binding selects, with the same arguments. + * - A chart refused at mount that is later given a category fetches then: the + * gate is a dependency of the fetch effect, not a mount-time snapshot. + * + * `ChartRenderer` is a stand-in: the refusal is drawn by `ObjectChart` itself, + * and the control only needs to see that rows reached the renderer. + */ +import React from 'react'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, cleanup, waitFor, act } from '@testing-library/react'; +import { notifyDataChanged } from '@object-ui/react'; + +vi.mock('./ChartRenderer', () => ({ + ChartRenderer: ({ schema }: { schema?: { data?: unknown } }) => ( +
+ ), +})); + +import { ObjectChart } from './ObjectChart'; +import type { ObjectChartSchema } from '@object-ui/types'; + +const REFUSAL = 'chart-missing-category-axis'; + +/** Rows a populated data source answers with, so "no rows" never explains a zero. */ +const ROWS = [ + { stage: 'won', amount: 10 }, + { stage: 'lost', amount: 4 }, +]; + +/** Every read a data source could be asked for on the object-bound path. */ +const makeDataSource = () => ({ + find: vi.fn(async () => ROWS), + aggregate: vi.fn(async () => ROWS), + getObjectSchema: vi.fn(async () => ({ name: 'task', fields: {} })), +}); + +let metadataFetch: ReturnType; + +beforeEach(() => { + // The option-colour metadata probe reads through the global fetch when no + // host `apiFetch` is mounted. Counted, so "reads nothing" covers it too. + metadataFetch = vi.fn(async () => ({ ok: true, json: async () => ({}) })); + vi.stubGlobal('fetch', metadataFetch); +}); +afterEach(() => { + vi.unstubAllGlobals(); + cleanup(); +}); + +/** Let every effect and every resolved promise land before counting reads. */ +const settle = () => act(() => new Promise((resolve) => setTimeout(resolve, 50))); + +/** + * The round-15 probe's node, transcribed from the card: what the three relays + * compose for a chart list view that names no dataset. + */ +const PROBE_NODE: ObjectChartSchema = { + type: 'object-chart', + objectName: 'task', + chartType: 'bar', + filter: [], +}; + +/** + * A declared measure with no category. `ChartAggregate` requires `groupBy`, so + * this is a document the authoring door refuses — cast here, once, as the + * objectui#8168 file casts it (`UNAUTHORABLE`), because every live producer + * forwards `aggregate` untyped and the shape still arrives at runtime. + */ +const MEASURE_WITHOUT_CATEGORY = { + field: 'amount', + function: 'sum', +} as unknown as ObjectChartSchema['aggregate']; + +describe('objectui#12061 — a chart that refuses for a missing category axis reads nothing', () => { + it('the round-15 probe node: find zero times, aggregate zero times, and the refusal still draws', async () => { + const ds = makeDataSource(); + render(); + + // The FIRST paint is the refusal — no loading skeleton flashes before it. + expect(screen.getByTestId(REFUSAL)).toHaveAttribute('role', 'alert'); + expect(screen.queryByTestId('chart-loading')).toBeNull(); + + await settle(); + + expect(ds.find).toHaveBeenCalledTimes(0); + expect(ds.aggregate).toHaveBeenCalledTimes(0); + expect(ds.getObjectSchema).toHaveBeenCalledTimes(0); + expect(metadataFetch).toHaveBeenCalledTimes(0); + expect(screen.getByTestId(REFUSAL)).toHaveAttribute('role', 'alert'); + expect(screen.queryByTestId('chart-loading')).toBeNull(); + }); + + it('a declared measure with no groupBy: the aggregate leg is not issued either', async () => { + const ds = makeDataSource(); + render(); + + await settle(); + + expect(ds.aggregate).toHaveBeenCalledTimes(0); + expect(ds.find).toHaveBeenCalledTimes(0); + expect(screen.getByTestId(REFUSAL)).toHaveAttribute('role', 'alert'); + }); + + it('a compareTo overlay on a refused chart: neither window is queried', async () => { + // The comparison window rides the same object-bound query as the current + // one, so a refused bar chart asking for `compareTo` would read twice. + const ds = makeDataSource(); + render(); + + await settle(); + + expect(ds.find).toHaveBeenCalledTimes(0); + expect(ds.aggregate).toHaveBeenCalledTimes(0); + expect(screen.getByTestId(REFUSAL)).toHaveAttribute('role', 'alert'); + }); + + it('a write to the object does not wake a refused chart into a fetch', async () => { + const ds = makeDataSource(); + render(); + await settle(); + + act(() => notifyDataChanged({ objectName: 'task' })); + await settle(); + + expect(ds.find).toHaveBeenCalledTimes(0); + expect(ds.aggregate).toHaveBeenCalledTimes(0); + expect(screen.getByTestId(REFUSAL)).toHaveAttribute('role', 'alert'); + }); +}); + +describe('objectui#12061 control — a bound chart fetches exactly as before', () => { + it('a category on xAxisKey and no aggregate: one find over the object, no aggregate', async () => { + const ds = makeDataSource(); + render(); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + await settle(); + + expect(ds.find).toHaveBeenCalledTimes(1); + expect(ds.find).toHaveBeenCalledWith('task', { $filter: [] }); + expect(ds.aggregate).toHaveBeenCalledTimes(0); + expect(screen.queryByTestId(REFUSAL)).toBeNull(); + expect(screen.getByTestId('chart-renderer')).toHaveAttribute('data-rows', String(ROWS.length)); + }); + + it('a category on aggregate.groupBy: one aggregate over the object, no find', async () => { + const ds = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(ds.aggregate).toHaveBeenCalled()); + await settle(); + + expect(ds.aggregate).toHaveBeenCalledTimes(1); + expect(ds.aggregate).toHaveBeenCalledWith('task', { + field: 'amount', + function: 'sum', + groupBy: 'stage', + filter: [], + }); + expect(ds.find).toHaveBeenCalledTimes(0); + expect(screen.queryByTestId(REFUSAL)).toBeNull(); + expect(screen.getByTestId('chart-renderer')).toHaveAttribute('data-rows', String(ROWS.length)); + }); + + it('a chart refused at mount fetches once a spec xAxis declares its category', async () => { + // The spec's `xAxis: { field }` moves none of the fetch effect's other + // inputs (no `xAxisKey`, no `aggregate`), so only the gate itself can + // re-run the effect when the category arrives — as it does when a + // designer preview edits the node in place. + const ds = makeDataSource(); + const { rerender } = render(); + await settle(); + expect(ds.find).toHaveBeenCalledTimes(0); + + rerender(); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + await settle(); + + expect(ds.find).toHaveBeenCalledTimes(1); + expect(ds.find).toHaveBeenCalledWith('task', { $filter: [] }); + expect(screen.queryByTestId(REFUSAL)).toBeNull(); + expect(screen.getByTestId('chart-renderer')).toHaveAttribute('data-rows', String(ROWS.length)); + }); +}); diff --git a/packages/plugin-charts/src/ObjectChart.tsx b/packages/plugin-charts/src/ObjectChart.tsx index 37e93971c9..9bdf6c1ff5 100644 --- a/packages/plugin-charts/src/ObjectChart.tsx +++ b/packages/plugin-charts/src/ObjectChart.tsx @@ -988,7 +988,28 @@ export const ObjectChart = (props: ObjectChartProps) => { // The re-read keeps the chart mounted (`RefreshIndicator` over the current // rows, not the skeleton), so an open drill drawer and the chart's own state // survive a save. - const fetchesForItself = !!(schema.objectName || schema.dataset) && !boundData && !schema.data; + // + // The path the objectui#8168 screen guards, as ONE predicate that screen, + // the category forward (`groupByCategoryKey`) and the fetch gate below all + // read: an object-bound chart (`objectName`, no `dataset`) drawing rows it + // fetched itself, not rows the author handed over (`data`, or a `bind` + // scope). + const hasAuthoredRows = !!boundData || Array.isArray(schema.data); + const drawsFetchedObjectRows = !!schema.objectName && !schema.dataset && !hasAuthoredRows; + // objectui#12061 — the refusal is decided BEFORE the fetch, the shape + // objectui#7390 gave `ObjectGallery`: decide the binding first, then fetch. + // The refusal used to be a render-time return only, so the fetch effect + // still issued the object-bound query (`find` with no `$top`, or + // `aggregate` with no `groupBy`) and threw the rows away to draw the + // refusal. Every chart list view naming no `dataset` composes exactly that + // node since objectui#6152 round 15. This one value is what the refusal + // below returns on and what keeps the fetch, and the invalidation + // subscription that would re-run it, from starting — ⛔ never a second copy + // of the rule, and ⛔ never a `$top` cap, which would shrink the read + // rather than remove it. + const refusesMissingCategoryAxis = drawsFetchedObjectRows && !resolveChartCategoryField(schema); + const fetchesForItself = + !!(schema.objectName || schema.dataset) && !boundData && !schema.data && !refusesMissingCategoryAxis; const invalidationNonce = useDataInvalidation( fetchesForItself ? (schema.dataset ? datasetObject : schema.objectName) : undefined, ); @@ -999,12 +1020,17 @@ export const ObjectChart = (props: ObjectChartProps) => { if (fetchesForItself) { fetchData(dataSource, mounted); } else if (mounted.current) { - // Have inline / bound data — won't fetch; clear loading. + // Have inline / bound data, or refused for a missing category axis — + // won't fetch; clear loading. setLoading(false); } return () => { mounted.current = false; }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [schema.objectName, datasetKey, dataSource, boundData, schema.data, filterKey, aggregateKey, compareToKey, fetchData, invalidationNonce]); + // `fetchesForItself` is listed for the category half of its predicate: a + // spec `xAxis` that arrives later moves no other dependency here, and a + // chart refused at mount must fetch once a category is declared. Listing + // it is also what made this list exhaustive, so it carries no lint + // suppression. + }, [schema.objectName, datasetKey, dataSource, boundData, schema.data, filterKey, aggregateKey, compareToKey, fetchData, invalidationNonce, fetchesForItself]); const rawData = boundData || schema.data || fetchedData; const finalData = Array.isArray(rawData) ? rawData : []; @@ -1247,12 +1273,8 @@ export const ObjectChart = (props: ObjectChartProps) => { ) : null; - // The path the objectui#8168 screen further down guards, as ONE predicate - // that screen and the category forward just below both read: an object-bound - // chart (`objectName`, no `dataset`) drawing rows it fetched itself, not rows - // the author handed over (`data`, or a `bind` scope). - const hasAuthoredRows = !!boundData || Array.isArray(schema.data); - const drawsFetchedObjectRows = !!schema.objectName && !schema.dataset && !hasAuthoredRows; + // `drawsFetchedObjectRows` — the path the objectui#8168 screen further down + // guards — is declared beside the fetch gate above, which reads it too. /** * objectui#10634 — the category column an object-bound chart forwards when @@ -1387,6 +1409,9 @@ export const ObjectChart = (props: ObjectChartProps) => { * as the timeline's: a static authoring fact that no fetch outcome * changes. A skeleton that resolves into a refusal, or a network error * shown first, would both send the author to debug the wrong layer. + * And, since objectui#12061, decided before any fetch is issued: this + * return and the fetch gate read one value, `refusesMissingCategoryAxis`, + * so a chart that refuses reads nothing (no `find`, no `aggregate`). * * ## What this does NOT do * @@ -1402,7 +1427,7 @@ export const ObjectChart = (props: ObjectChartProps) => { * that it does NOT fire on the schema every producer composes for a chart * that declares a category. */ - if (drawsFetchedObjectRows && !resolveChartCategoryField(schema)) { + if (refusesMissingCategoryAxis) { return (
{tt(