diff --git a/.changeset/8269-dashboard-category-axis-groupby.md b/.changeset/8269-dashboard-category-axis-groupby.md new file mode 100644 index 0000000000..d6e554dd06 --- /dev/null +++ b/.changeset/8269-dashboard-category-axis-groupby.md @@ -0,0 +1,45 @@ +--- +'@object-ui/core': patch +'@object-ui/plugin-dashboard': patch +'@object-ui/plugin-charts': patch +--- + +Fix a dashboard chart widget that declares its category as `aggregate.groupBy` being +refused for lacking a `name` column (objectui#8269). + +A widget bound to an object with `aggregate: { function: 'count', groupBy: 'status' }` +and no `options.xField` rendered a refusal instead of a chart: + +> This chart cannot plot its category axis: no row has a `name` field. + +The author wrote `groupBy: 'status'`. Nothing on screen said `groupBy` was the key that +had been ignored, and `name` appeared nowhere in their metadata — so the diagnostic sent +them to debug the wrong layer. + +**Cause.** The two dashboard relays (`DashboardGridLayout`, `DashboardRenderer`) each +floored the category binding on a literal — `options.xField || 'name'` — and handed it to +the `object-chart` node without ever consulting the aggregate that decides it. An +object-bound aggregate returns one row per group keyed by the raw `groupBy` field, so no +row carried `name` and the category-axis guard (framework#4033) fired correctly on a +binding that was already wrong when it arrived. + +**Fix.** `chartCategoryKey` is a new `@object-ui/core` export delegating to +`chartAggregateCategoryKey` in `@objectstack/spec/ui` — the contract's own derivation of +"the category column an object-bound aggregate produces", and the published sibling of the +`chartAggregateValueKey` that objectui#8266 adopted for the measure axis. Both relays now +consult it for the object-provider branch. + +**What moves on screen.** A widget that rendered a refusal now draws. Measured through +`ChartRenderer` at 480x320 over the rows a fieldless count returns +(`[{status:'open',count:2},{status:'paid',count:5}]`): the composed binding went from +`xAxisKey: 'name'` — a `missing-category-key` refusal, 0 marks — to `xAxisKey: 'status'`, +1 series and 2 marks with the category ticks drawn. + +**Unaffected.** A chart with no `aggregate` at all keeps the author's `xField` (its rows +are raw records, so that key is the right one), an UNGROUPED aggregate keeps it too (it +returns a single row with no category column), and the authored-literal-rows branch — the +`chart` node composed after the object-provider check fails — keeps its floor unchanged. +One authored key changes meaning, exactly as objectui#8266's `yField` did: an `xField` +written on an object-bound chart that ALSO declares a `groupBy` no longer wins over the +aggregate's own column — it named a record column a grouped aggregate never returns, so it +produced the same refusal before. diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index dc1dcfaabf..68c48e5abe 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -76,6 +76,11 @@ export * from './utils/chart-series.js'; // projection and the series binding alike, delegated to the spec's own // derivation so the two cannot drift (objectui#8266). export * from './utils/chart-measure-key.js'; +// "Which result column carries the CATEGORY?" — the same move on the other +// axis, delegated to the contract's own published derivation so a relay cannot +// floor the x-axis binding on a literal the aggregate contradicts +// (objectui#8269). +export * from './utils/chart-category-key.js'; // The AUTHORED half of a dataset-bound chart (objectui#4229's data/presentation // split), shared by the dashboard widget and the report's embedded chart so the // same spec keys are lowered identically on both (objectui#4877). diff --git a/packages/core/src/utils/chart-category-key.test.ts b/packages/core/src/utils/chart-category-key.test.ts new file mode 100644 index 0000000000..adff90fed3 --- /dev/null +++ b/packages/core/src/utils/chart-category-key.test.ts @@ -0,0 +1,82 @@ +/** + * 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#8269 — the ONE answer to "which result column carries the category?". + * + * The regression arm is the FIRST one: a `groupBy` with no `options.xField` + * beside it, which is the whole authoring shape the card is about. The relays + * answered `'name'` there and the rows carry the raw groupBy field, so + * `hasNoCategoryKey` refused the widget by the name of a key nobody wrote. + * + * The fallback arms are the other half: a floor that fired too eagerly would + * take the caller's `xField` back over a `groupBy` the contract CAN answer for, + * or invent a category column for an UNGROUPED aggregate that returns exactly + * one row and has none. + */ +import { describe, it, expect } from 'vitest'; +import { chartCategoryKey } from './chart-category-key'; + +describe('chartCategoryKey — the contract answers', () => { + it('is the raw groupBy field, never the caller floor', () => { + // THE REGRESSION. `options.xField || 'name'` answered 'name' here, and the + // rows carry 'status', so the chart refused, naming 'name'. + expect(chartCategoryKey({ function: 'count', groupBy: 'status' }, 'name')).toBe('status'); + }); + + it('ignores a caller floor the author chose, when the groupBy answers', () => { + // An authored `xField` names a column of the RECORDS, and a grouped + // aggregate does not return records. Honouring it refuses the chart for + // exactly the same reason 'name' did. + expect(chartCategoryKey({ function: 'count', groupBy: 'status' }, 'stage')).toBe('status'); + }); + + it('answers for a field-bearing aggregate the same way', () => { + expect(chartCategoryKey({ field: 'amount', function: 'sum', groupBy: 'stage' }, 'name')).toBe('stage'); + }); + + it('reads a structured groupBy node by its FIELD', () => { + expect( + chartCategoryKey({ function: 'count', groupBy: { field: 'closed_at', dateGranularity: 'month' } }, 'name'), + ).toBe('closed_at'); + }); + + it('prefers a structured groupBy ALIAS, because the alias renames the projected column', () => { + // The one place this seam and `plugin-charts`' `resolveChartCategoryField` + // deliberately disagree: that resolver answers "which FIELD?" (and returns + // `closed_at`, which is what a field-metadata probe needs), this one + // answers "which COLUMN do the rows carry?" — and `ObjectChart`'s own fetch + // path keys those rows `alias || field`. + expect( + chartCategoryKey({ function: 'count', groupBy: { field: 'closed_at', alias: 'month' } }, 'name'), + ).toBe('month'); + }); +}); + +describe('chartCategoryKey — the caller floor, only where the contract is silent', () => { + it('falls back when there is no aggregate at all', () => { + // A provider whose rows are raw records: the author's xField IS the key. + expect(chartCategoryKey(undefined, 'name')).toBe('name'); + expect(chartCategoryKey(undefined, 'stage')).toBe('stage'); + }); + + it('falls back for an UNGROUPED aggregate, which returns no category column', () => { + // One row, one number. There is no category to name, so the caller's floor + // is the only answer available — and refusing to invent one here is what + // keeps a single-value aggregate from being bound to a column that would + // never exist. + expect(chartCategoryKey({ field: 'amount', function: 'sum' }, 'name')).toBe('name'); + }); + + it('falls back for a groupBy shape ChartGroupBySchema rejects', () => { + expect(chartCategoryKey({ function: 'count', groupBy: '' }, 'name')).toBe('name'); + expect(chartCategoryKey({ function: 'count', groupBy: {} }, 'name')).toBe('name'); + expect(chartCategoryKey({ function: 'count', groupBy: ['status'] }, 'name')).toBe('name'); + expect(chartCategoryKey({}, 'name')).toBe('name'); + }); +}); diff --git a/packages/core/src/utils/chart-category-key.ts b/packages/core/src/utils/chart-category-key.ts new file mode 100644 index 0000000000..1f6946c4ce --- /dev/null +++ b/packages/core/src/utils/chart-category-key.ts @@ -0,0 +1,90 @@ +/** + * 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. + */ + +/** + * chart-category-key — the ONE answer to "which result column carries the + * CATEGORY?" for an object-bound chart (objectui#8269). + * + * The mirror of `chart-measure-key` (objectui#8266) on the other axis. That + * card landed `chartMeasureKey` over the contract's `chartAggregateValueKey`; + * the contract publishes a CATEGORY sibling in the same module — + * `chartAggregateCategoryKey` — and nothing in this repository read it. + * + * ## The disagreement this replaces + * + * Both dashboard relays composed the category binding as a LITERAL FLOOR: + * + * const xAxisKey = options.xField || 'name'; + * + * and handed it to the `object-chart` node without ever consulting the + * `aggregate` that decides it. An object-bound aggregate returns one row per + * group, keyed by the raw `groupBy` field, so a widget declaring + * `aggregate: { function: 'count', groupBy: 'status' }` and no `options.xField` + * bound `'name'` against rows keyed `'status'`. + * + * Unlike the measure half, this one is LOUD: `hasNoCategoryKey` + * (`plugin-charts/src/AdvancedChartImpl.tsx`, framework#4033) fires and the + * author reads "This chart cannot plot its category axis: no row has a `name` + * field." The diagnostic is wrong-CAUSE — it names a binding the author never + * wrote and never mentions the `groupBy` they did — so it points at the wrong + * layer. Being loud makes it a lesser harm than objectui#8266, not a + * non-defect: measured over the rows a fieldless count returns, + * `[{status:'open',count:2},{status:'paid',count:5}]`, an `xAxisKey` of + * `'name'` refuses under BOTH series bindings, i.e. objectui#8266's fix does + * not reach this authoring shape at all. + * + * ## Why the answer is delegated rather than restated + * + * Restating the rule here would make this file a second opinion of a question + * the contract already answers — the objectui#5042 / #7544 / #8193 / #8168 + * drift shape that `chart-measure-key` exists to end. So the rule stays + * upstream in `@objectstack/spec/ui` and this is the seam objectui-side callers + * share. + * + * ## ⚠️ NOT the same function as `resolveChartCategoryField` + * + * `plugin-charts`' own `resolveChartCategoryField` (objectui#8168) reads + * `aggregate.groupBy` first too — which is exactly why `ObjectChart` does NOT + * refuse this shape at its own level, and then forwards `schema.xAxisKey` + * verbatim as the render binding anyway. But the two answer DIFFERENT + * questions and must not be collapsed: + * + * - `resolveChartCategoryField` answers "which FIELD is the category?" — its + * structured-`groupBy` leg returns `node.field`, because its two readers + * are the refusal (does the author name a category at all?) and the + * field-metadata probe that loads that field's option labels and colours. + * - this function answers "which COLUMN do the returned rows carry it + * under?" — `groupBy.alias ?? groupBy.field` per the contract, because an + * `alias` (admitted by `ChartGroupBySchema`) renames the projected column. + * + * They coincide whenever no alias is written, which is why the distinction is + * easy to miss; conflating them would either bind an axis to a column the rows + * do not carry, or probe field metadata for a field that does not exist. + */ + +import { chartAggregateCategoryKey, type ChartAggregateLike } from '@objectstack/spec/ui'; + +/** + * The result column a chart's category axis / x-axis binding must name. + * + * `fallback` is the caller's own floor, used ONLY when the contract has no + * answer — a chart that declares no `aggregate` at all (its rows are raw + * records or authored literals, where the author's `xField` is the right key), + * an UNGROUPED aggregate (one row, no category column), or a `groupBy` shape + * `ChartGroupBySchema` already rejects. It is never a second opinion about an + * aggregate the contract CAN answer for. + * + * @param aggregate the chart's inline aggregate, or `undefined` + * @param fallback the key to bind when the contract has no answer + */ +export function chartCategoryKey( + aggregate: ChartAggregateLike | undefined, + fallback: string, +): string { + return chartAggregateCategoryKey(aggregate) ?? fallback; +} diff --git a/packages/plugin-charts/src/ObjectChart.categoryAxisKeyRender-8269.test.tsx b/packages/plugin-charts/src/ObjectChart.categoryAxisKeyRender-8269.test.tsx new file mode 100644 index 0000000000..102f33c427 --- /dev/null +++ b/packages/plugin-charts/src/ObjectChart.categoryAxisKeyRender-8269.test.tsx @@ -0,0 +1,270 @@ +/** + * 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#8269, the RENDER half — what a category binding naming a column the + * rows do not carry actually draws, and what the fixed binding draws instead. + * + * `plugin-dashboard/src/__tests__/DashboardChart.categoryAxisKey-8269.test.tsx` + * pins which column the two dashboard relays name. That is necessary and not + * sufficient: the card's own bar is that "a widget that renders a refusal today + * would start drawing, so it needs a render measurement, not only a seam + * assertion". So this file renders the real chain — `ChartRenderer` → + * `normalizeChartSchema` → `AdvancedChartImpl` — over the rows a fieldless + * count actually returns, and reads the DOM. + * + * It lives here for the reason the objectui#8266 twin states: `recharts` + * resolves inside `plugin-charts` alone, so this is the only package that can + * mock `ResponsiveContainer` to a measured box and count marks at all. + * (Re-verified rather than inherited: `require.resolve('recharts')` from + * `packages/plugin-dashboard` is MODULE_NOT_FOUND.) + * + * ## The baseline this reproduces, measured on `origin/main` `3c6394cb2` + * + * Rows `[{status:'open',count:2},{status:'paid',count:5}]`, `ChartRenderer` at + * 480x320: + * + * xAxisKey 'status' + dataKey 'value' -> surface, ticks open/paid, 0 marks, + * no refusal (that is objectui#8266) + * xAxisKey 'status' + dataKey 'count' -> 2 marks, y ticks 0..8 + * (objectui#8266, after PR 8272) + * xAxisKey 'name' + dataKey 'value' -> refusal `missing-category-key` + * xAxisKey 'name' + dataKey 'count' -> refusal `missing-category-key` + * + * The LAST row is this card: the binding both relays composed for a + * `groupBy`-only widget, AFTER objectui#8266's fix had already corrected the + * measure. It is the one the second block below shows changing. + * + * ⚠️ Mark counts are harness-bound (`ResponsiveContainer` is fixed at 480x320 + * here); they are re-derived in this file and never carried in from another. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; + +// Recharts measures via ResizeObserver, which reports 0x0 under the headless +// DOM, so nothing paints. Fix its size — the shim every render test in this +// package uses. +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +// `ChartRenderer` reaches the component under test through +// `React.lazy(() => import('./AdvancedChartImpl'))`. Importing the SAME +// specifier at module scope puts that load in the import phase, where no test +// or hook timeout applies (AGENTS.md, the flaky-test rule): under a saturated +// transform pipeline a first dynamic import can spend most of a `waitFor` +// budget, and this file's assertions would then be racing the module loader. +import './AdvancedChartImpl'; +import { ChartRenderer } from './ChartRenderer'; +import { ObjectChart, aggregateRecords } from './ObjectChart'; +import { chartCategoryKey, chartMeasureKey } from '@object-ui/core'; + +afterEach(cleanup); + +/** + * Not transcribed — produced by the very builder the row projection uses, so a + * change to the projected column names lands in this fixture instead of leaving + * the pin asserting against a shape the product stopped emitting. + */ +const COUNT_ROWS = aggregateRecords( + [ + { status: 'open' }, { status: 'open' }, + { status: 'paid' }, { status: 'paid' }, { status: 'paid' }, { status: 'paid' }, { status: 'paid' }, + ], + { function: 'count', groupBy: 'status' }, +); + +/** The aggregate the card's widget declares — its category named ONCE, as groupBy. */ +const AGGREGATE = { function: 'count', groupBy: 'status' } as const; + +/** The two literal floors the relays used to bind unconditionally. */ +const CATEGORY_FLOOR = 'name'; +const MEASURE_FLOOR = 'value'; + +const readChart = (container: HTMLElement) => ({ + marks: container.querySelectorAll('.recharts-rectangle').length, + series: container.querySelectorAll('.recharts-bar').length, + refusal: container.querySelector('[data-chart-error]')?.getAttribute('data-chart-error') ?? null, + refusalText: container.querySelector('[data-chart-error]')?.textContent ?? null, + emptyState: !!screen.queryByTestId('chart-empty-state'), + ticks: Array.from(container.querySelectorAll('.recharts-cartesian-axis-tick-value')).map((n) => n.textContent), +}); + +/** + * Wait for a TERMINAL state — a plot or a refusal. Either satisfies it, so a + * refusal is never mistaken for a timeout and a timeout is never read as "it + * drew nothing". + */ +const settleChart = async (container: HTMLElement) => { + await waitFor(() => { + if (!container.querySelector('.recharts-surface') && !container.querySelector('[data-chart-error]')) { + throw new Error('neither a plot nor a refusal'); + } + }, { timeout: 5000 }); + return readChart(container); +}; + +const drawWith = async (xAxisKey: string, dataKey: string) => { + const { container } = render( + , + ); + return settleChart(container); +}; + +describe('the cardized baseline: which (xAxisKey, dataKey) pairs draw (objectui#8269)', () => { + it('is the shape the row builder emits — the premise the rest of the file rests on', () => { + expect(COUNT_ROWS).toEqual([ + { status: 'open', count: 2 }, + { status: 'paid', count: 5 }, + ]); + }); + + it('status/count — the only pair that draws', async () => { + const drawn = await drawWith('status', 'count'); + expect(drawn.refusal).toBeNull(); + expect(drawn.series).toBe(1); + expect(drawn.marks).toBe(2); + expect(drawn.ticks).toEqual(expect.arrayContaining(['open', 'paid', '0', '2', '4'])); + }); + + it('status/value — objectui#8266: a frame with the categories on it and nothing in it', async () => { + const drawn = await drawWith('status', MEASURE_FLOOR); + expect(drawn.ticks).toEqual(['open', 'paid']); + expect(drawn.marks).toBe(0); + expect(drawn.refusal).toBeNull(); + expect(drawn.emptyState).toBe(false); + }); + + it.each([MEASURE_FLOOR, 'count'])( + 'name/%s — refused, and the message names the key the author never wrote', + async (dataKey) => { + const drawn = await drawWith(CATEGORY_FLOOR, dataKey); + expect(drawn.refusal).toBe('missing-category-key'); + // The wrong-CAUSE half of the defect: `name` is named, `status` is not. + expect(drawn.refusalText).toContain(CATEGORY_FLOOR); + expect(drawn.refusalText).not.toContain('status'); + expect(drawn.marks).toBe(0); + }, + ); +}); + +describe('the binding the relays compose now draws (objectui#8269)', () => { + // Computed by the very seam the relays call, not transcribed: if + // `chartCategoryKey` stops answering for this aggregate, this file measures + // the binding that actually ships rather than a stale copy of it. + const composedCategory = chartCategoryKey(AGGREGATE, CATEGORY_FLOOR); + const composedMeasure = chartMeasureKey(AGGREGATE, MEASURE_FLOOR); + + it('resolves the pair away from BOTH literal floors', () => { + expect(composedCategory).toBe('status'); + expect(composedMeasure).toBe('count'); + }); + + it('draws marks where the pre-fix binding rendered a refusal', async () => { + // Before: (name, count) — the last row of the baseline table, a refusal. + const before = await drawWith(CATEGORY_FLOOR, composedMeasure); + expect(before.refusal).toBe('missing-category-key'); + expect(before.marks).toBe(0); + + cleanup(); + + // After: the pair the relays compose today. + const after = await drawWith(composedCategory, composedMeasure); + expect(after.refusal).toBeNull(); + expect(after.series).toBe(1); + expect(after.marks).toBe(2); + expect(after.ticks).toEqual(expect.arrayContaining(['open', 'paid'])); + }); +}); + +/** + * The ordering question the card raises, MEASURED rather than reasoned about: + * + * > `resolveGroupByLabels` rewrites the groupBy column in place — check the + * > resolved key is still valid at the point the axis reads it. + * + * A resolver returning the right key is worthless if a later pass renames the + * column under it. This block runs the WHOLE `ObjectChart` fetch pipeline — + * `runAggregate` → comparison merge → `resolveGroupByLabels` → `ChartRenderer` + * → `AdvancedChartImpl` — against a data source whose groupBy field carries + * picklist options, so the label pass really fires. Marks drawn UNDER + * HUMANIZED TICKS is the two-in-one reading: the rewrite happened (the ticks + * are the labels, not the raw enum values) AND the resolved key survived it + * (had the column been renamed, `hasNoCategoryKey` would refuse instead). + */ +describe('the resolved key survives the groupBy label rewrite (objectui#8269)', () => { + const OBJECT_SCHEMA = { + fields: { + status: { + type: 'select', + options: [ + { value: 'open', label: 'Open cases' }, + { value: 'paid', label: 'Paid cases' }, + ], + }, + }, + }; + + beforeEach(() => { + // `ObjectChart` probes object metadata for option colours on the global + // fetch. Answered from a double so the render is offline and deterministic. + vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, json: async () => ({}) }))); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('draws the marks with the LABELS on the axis, not a refusal', async () => { + const dataSource = { + aggregate: vi.fn(async () => COUNT_ROWS.map((row) => ({ ...row }))), + getObjectSchema: vi.fn(async () => OBJECT_SCHEMA), + }; + + const { container } = render( + , + ); + + const drawn = await settleChart(container); + expect(dataSource.getObjectSchema).toHaveBeenCalled(); + expect(drawn.refusal).toBeNull(); + expect(drawn.marks).toBe(2); + // The label pass ran… + expect(drawn.ticks).toEqual(expect.arrayContaining(['Open cases', 'Paid cases'])); + // …and it replaced the VALUES, never the column name — the raw enums are + // gone from the axis, and the axis still found its key. + expect(drawn.ticks).not.toContain('open'); + expect(drawn.ticks).not.toContain('paid'); + }); +}); diff --git a/packages/plugin-dashboard/src/DashboardGridLayout.tsx b/packages/plugin-dashboard/src/DashboardGridLayout.tsx index 70a5a2cc69..c69fd67dcc 100644 --- a/packages/plugin-dashboard/src/DashboardGridLayout.tsx +++ b/packages/plugin-dashboard/src/DashboardGridLayout.tsx @@ -6,7 +6,7 @@ import { Edit, GripVertical, Save, X, RefreshCw } from 'lucide-react'; import { SchemaRenderer, useHasDndProvider, useDnd } from '@object-ui/react'; import { useObjectTranslation, pickLocalized } from '@object-ui/i18n'; import type { BaseSchema, DashboardComponentSchema, DashboardWidgetSchema } from '@object-ui/types'; -import { chartMeasureKey } from '@object-ui/core'; +import { chartCategoryKey, chartMeasureKey } from '@object-ui/core'; import { isObjectProvider, deriveStaticTableColumns } from './utils'; import { classifyWidgetType } from './widgetDispatch'; import { LEGACY_RETIRED_WIDGET_SCHEMA, isLegacyRetiredWidget } from './legacyRetiredWidget'; @@ -245,12 +245,22 @@ export const DashboardGridLayout: React.FC = ({ // the floor for a provider with NO aggregate, whose rows are raw // records — there the author's `yField` really is the key. const effectiveYField = chartMeasureKey(effectiveAggregate, yField); + // Which column carries the CATEGORY is the same question on the other + // axis, and this relay was answering it with a literal it never checked + // against the aggregate that decides it: `xField || 'name'` bound + // `'name'` against the rows a `groupBy: 'status'` aggregate returns, so + // `hasNoCategoryKey` refused the widget by the name of a key its author + // never wrote (objectui#8269). `xAxisKey` survives as the floor for the + // shapes the contract has no answer for — a provider with NO aggregate + // (rows are raw records) and an UNGROUPED one (a single row with no + // category column at all). + const effectiveXAxisKey = chartCategoryKey(effectiveAggregate, xAxisKey); return { type: 'object-chart', chartType: dispatch.chartType, objectName: widgetData.object, aggregate: effectiveAggregate, - xAxisKey: xAxisKey, + xAxisKey: effectiveXAxisKey, series: [{ dataKey: effectiveYField }], colors: CHART_COLORS, // Deterministic first paint inside the grid (#2756). diff --git a/packages/plugin-dashboard/src/DashboardRenderer.tsx b/packages/plugin-dashboard/src/DashboardRenderer.tsx index f955fcd598..d42a37a445 100644 --- a/packages/plugin-dashboard/src/DashboardRenderer.tsx +++ b/packages/plugin-dashboard/src/DashboardRenderer.tsx @@ -16,6 +16,7 @@ import { buildWidgetScopedFilter, mergeFilters, toDomProps, + chartCategoryKey, chartMeasureKey, } from '@object-ui/core'; import { cn, Card, CardHeader, CardTitle, CardContent, Button, getLazyIcon } from '@object-ui/components'; @@ -623,13 +624,20 @@ const DashboardRendererInner = forwardRef { + composed.push(props.schema ?? props); + return null; +}; +for (const type of ['object-chart', 'chart'] as const) { + ComponentRegistry.register(type, recorder as any, { + namespace: 'test', + label: 'recorder', + category: 'plugin', + } as any); +} + +afterEach(cleanup); + +const dataSource = { aggregate: async () => [], find: async () => [] }; + +/** Render one widget through a relay and return the node it composed. */ +const composeVia = async (surface: 'grid' | 'renderer', widget: Record) => { + composed.length = 0; + render( + + {surface === 'grid' ? ( + + ) : ( + + )} + , + ); + await waitFor(() => expect(composed.length).toBeGreaterThan(0)); + const node = composed[composed.length - 1]; + cleanup(); + return node; +}; + +/** + * The card's authoring shape: `options` carries NO `xField`, which is the whole + * point — the category is declared once, as `aggregate.groupBy`. + */ +const objectWidget = (aggregate: unknown, options: Record = {}) => ({ + id: 'w1', + type: 'bar', + title: 'By status', + options, + data: { provider: 'object', object: 'crm_case', ...(aggregate ? { aggregate } : {}) }, +}); + +const SURFACES = ['grid', 'renderer'] as const; + +describe.each(SURFACES)('%s relay — object-bound category axis key (objectui#8269)', (surface) => { + it('binds a groupBy-only widget to the column the rows carry, not to the name floor', async () => { + const node = await composeVia(surface, objectWidget({ function: 'count', groupBy: 'status' })); + // The regression: 'name' here, against rows keyed 'status' — and unlike + // objectui#8266 this one was LOUD, refusing the whole widget. + expect(node.xAxisKey).toBe('status'); + }); + + it('ignores an authored xField that an object-bound aggregate cannot project', async () => { + // `xField` names a column of the RECORDS, and a grouped aggregate does not + // return records. Honouring it refuses the chart for exactly the same + // reason 'name' did, so the aggregate's own column wins — the same verdict + // objectui#8266 reached for `yField`. + const node = await composeVia( + surface, + objectWidget({ function: 'count', groupBy: 'status' }, { xField: 'title' }), + ); + expect(node.xAxisKey).toBe('status'); + }); + + it('binds a field-bearing aggregate to its groupBy just the same', async () => { + const node = await composeVia(surface, objectWidget({ function: 'sum', field: 'amount', groupBy: 'stage' })); + expect(node.xAxisKey).toBe('stage'); + }); + + it('reads a structured groupBy node, and prefers its alias', async () => { + // `ChartGroupBySchema` admits both spellings; `ObjectChart`'s own fetch path + // keys the returned rows `alias || field`, so the binding must agree. + expect( + (await composeVia(surface, objectWidget({ function: 'count', groupBy: { field: 'closed_at', dateGranularity: 'month' } }))) + .xAxisKey, + ).toBe('closed_at'); + expect( + (await composeVia(surface, objectWidget({ function: 'count', groupBy: { field: 'closed_at', alias: 'month' } }))) + .xAxisKey, + ).toBe('month'); + }); + + it('keeps the xField floor for an object provider with NO aggregate', async () => { + // Rows are raw records here, so the author's xField really is the key — + // this is the arm a floor that fired too eagerly would have broken. + expect((await composeVia(surface, objectWidget(null))).xAxisKey).toBe('name'); + expect((await composeVia(surface, objectWidget(null, { xField: 'title' }))).xAxisKey).toBe('title'); + }); + + it('keeps the xField floor for an UNGROUPED aggregate, which has no category column', async () => { + // One row, one number: there is no category for the contract to name, so + // inventing one would bind an axis to a column that never exists. + const agg = { function: 'sum', field: 'amount' }; + expect((await composeVia(surface, objectWidget(agg))).xAxisKey).toBe('name'); + expect((await composeVia(surface, objectWidget(agg, { xField: 'title' }))).xAxisKey).toBe('title'); + }); + + it('still composes the object node, and still names the measure column', async () => { + // The category fix rides on the same site objectui#8266 fixed; asserting + // both halves here means a regression in either is attributed correctly + // rather than showing up as an unexplained shape change. + const node = await composeVia(surface, objectWidget({ function: 'count', groupBy: 'status' })); + expect(node.type).toBe('object-chart'); + expect(node.series[0].dataKey).toBe('count'); + }); +}); + +describe.each(SURFACES)('%s relay — the authored-rows branch is NOT the same decision', (surface) => { + const literalWidget = (options: Record) => ({ + id: 'w2', + type: 'bar', + title: 'Literal', + options, + data: [ + { name: 'open', value: 2, bucket: 'a' }, + { name: 'paid', value: 5, bucket: 'b' }, + ], + }); + + it('binds the authored xField, because there is no aggregate to consult', async () => { + expect((await composeVia(surface, literalWidget({}))).xAxisKey).toBe('name'); + expect((await composeVia(surface, literalWidget({ xField: 'bucket' }))).xAxisKey).toBe('bucket'); + }); + + it('composes the literal `chart` node, not `object-chart`', async () => { + // The branch discriminator itself: these rows never reach the object path, + // which is why consulting an aggregate there would be meaningless. + const node = await composeVia(surface, literalWidget({ xField: 'bucket' })); + expect(node.type).toBe('chart'); + expect(node.aggregate).toBeUndefined(); + expect(Array.isArray(node.data)).toBe(true); + }); +});