fix(plugin-charts): ObjectChart decides its missing-category-axis refusal before it fetches, so a refused chart reads nothing (objectui#12061)

.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.

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<typeof fetch>[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<Mount> {
   const schema: Record<string, unknown> = { 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(

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 } }) => (
+    <div
+      data-testid="chart-renderer"
+      data-rows={Array.isArray(schema?.data) ? schema.data.length : -1}
+    />
+  ),
+}));
+
+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<typeof vi.fn>;
+
+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<void>((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(<ObjectChart schema={PROBE_NODE} dataSource={ds} />);
+
+    // 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(<ObjectChart schema={{ ...PROBE_NODE, aggregate: MEASURE_WITHOUT_CATEGORY }} dataSource={ds} />);
+
+    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(<ObjectChart schema={{ ...PROBE_NODE, compareTo: { kind: 'previousYear' } }} dataSource={ds} />);
+
+    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(<ObjectChart schema={PROBE_NODE} dataSource={ds} />);
+    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(<ObjectChart schema={{ ...PROBE_NODE, xAxisKey: 'stage' }} dataSource={ds} />);
+
+    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(
+      <ObjectChart
+        schema={{ ...PROBE_NODE, aggregate: { field: 'amount', function: 'sum', groupBy: 'stage' } }}
+        dataSource={ds}
+      />,
+    );
+
+    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(<ObjectChart schema={PROBE_NODE} dataSource={ds} />);
+    await settle();
+    expect(ds.find).toHaveBeenCalledTimes(0);
+
+    rerender(<ObjectChart schema={{ ...PROBE_NODE, xAxis: { field: 'stage' } }} dataSource={ds} />);
+    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));
+  });
+});

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 (
         <div className={"p-4 text-destructive " + (schema.className || '')} data-testid="chart-missing-category-axis" role="alert">
             {tt(