diff --git a/.changeset/gridlayout-legacy-retired-fallback-4612.md b/.changeset/gridlayout-legacy-retired-fallback-4612.md new file mode 100644 index 000000000..40384a78b --- /dev/null +++ b/.changeset/gridlayout-legacy-retired-fallback-4612.md @@ -0,0 +1,11 @@ +--- +'@object-ui/plugin-dashboard': minor +--- + +One legacy detector, two dashboard surfaces — the editable grid stops rendering a silent blank chart + +framework#3320 retired the pre-ADR-0021 inline-analytics widget shape (top-level `object` + `categoryField` / `valueField` / `aggregate`, pivot `rowField` / `columnField`) and shipped a graceful fallback for the stored metadata that still carries it: a visible tile reading "This widget uses a retired data format. Edit it to bind a dataset." The fallback was applied to `DashboardRenderer` and to nothing else. `DashboardGridLayout` — separately exported, and registered as the `dashboard-grid` SDUI component — had no sentinel at all, so the identical stored widget fell through to its static-data branch with `data: []`. Same metadata, same product, two different outcomes: a rebind prompt on one surface, a silent blank chart on the other. The blank is worse for the author than the pre-retirement state, because it carries no chart, no diagnostic and no path to fix — the exact outcome the retirement's own test header says must not happen. + +The fix is not a second copy of the condition, because one copy is why the defect existed. The detector and the placeholder now live in a single module (`legacyRetiredWidget.ts`) that both surfaces import; `DashboardRenderer`'s observable behaviour is unchanged, pinned by its existing suite, and the new grid suite mirrors that suite's structure on the surface nobody had pinned. Four positive cases go from blank to placeholder, two of them the widget shapes stored byte-for-byte in the schema catalog's `filtered-dashboard` entry. + +The negative controls are the load-bearing half, because the retired shape is one character away from a live one. `options.data = { provider: 'object', … }` carries its OWN nested `object` and `aggregate`: it is a different, still-live authoring surface, read off the widget's data rather than off the widget top level, and it keeps rendering untouched — as do dataset-bound widgets and static-data widgets, on both surfaces. `DashboardRenderer`'s pivot arm stays deliberately surface-local rather than shared: it returns the placeholder for the entire pivot family because that surface emits no pivot block at all, which is a fact about what it can draw, not about the widget being legacy. The grid does draw pivots, from static data and from the provider config, so exporting that arm would have retired two working branches. A legacy pivot is caught on both surfaces by the shared sentinel instead, via the top-level `object` it carries. diff --git a/packages/plugin-dashboard/src/DashboardGridLayout.tsx b/packages/plugin-dashboard/src/DashboardGridLayout.tsx index c57a86ed0..34944a22b 100644 --- a/packages/plugin-dashboard/src/DashboardGridLayout.tsx +++ b/packages/plugin-dashboard/src/DashboardGridLayout.tsx @@ -8,6 +8,7 @@ import { useObjectTranslation, pickLocalized } from '@object-ui/i18n'; import type { BaseSchema, DashboardComponentSchema, DashboardWidgetSchema } from '@object-ui/types'; import { isObjectProvider } from './utils'; import { classifyWidgetType } from './widgetDispatch'; +import { LEGACY_RETIRED_WIDGET_SCHEMA, isLegacyRetiredWidget } from './legacyRetiredWidget'; /** Bridges editMode transitions to the ObjectUI DnD system when a DndProvider is present. */ function DndEditModeBridge({ editMode }: { editMode: boolean }) { @@ -174,6 +175,16 @@ export const DashboardGridLayout: React.FC = ({ const getComponentSchema = React.useCallback((widget: DashboardWidgetSchema) => { if (widget.component) return widget.component; + // Retired legacy inline-analytics widget (framework#3320) — the SAME + // detector `DashboardRenderer` uses, imported rather than restated + // (objectui#4612). Without it this stored shape reached the branches below + // with no data at all and rendered a silent blank chart / empty table / + // em-dash metric: no diagnostic and no path to fix, which is worse for the + // author than the pre-retirement state. It must be tested BEFORE the + // dispatch branches, exactly as on the sibling surface, because those + // branches are what swallow it. + if (isLegacyRetiredWidget(widget)) return LEGACY_RETIRED_WIDGET_SCHEMA; + const widgetType = widget.type; const options = (widget.options || {}) as Record; // One shared classification (./widgetDispatch) — this surface used to name diff --git a/packages/plugin-dashboard/src/DashboardRenderer.tsx b/packages/plugin-dashboard/src/DashboardRenderer.tsx index 2901951ee..757a29ae6 100644 --- a/packages/plugin-dashboard/src/DashboardRenderer.tsx +++ b/packages/plugin-dashboard/src/DashboardRenderer.tsx @@ -39,6 +39,7 @@ import { import { CSS } from '@dnd-kit/utilities'; import { isObjectProvider } from './utils'; import { classifyWidgetType, METRIC_LIKE_TYPES } from './widgetDispatch'; +import { LEGACY_RETIRED_WIDGET_SCHEMA, isLegacyRetiredWidget } from './legacyRetiredWidget'; import { DatasetWidget } from './DatasetWidget'; import { DashboardFilterBar } from './DashboardFilterBar'; @@ -138,21 +139,16 @@ const FILTERABLE_COMPONENT_TYPES = new Set([ 'object-data-table', ]); -/** - * Placeholder schema for a widget that still carries the retired pre-ADR-0021 - * inline-analytics binding — top-level `object` + `categoryField`/`valueField`/ - * `aggregate` — with no `dataset` and no inline `options.data`. No authoring - * surface emits this shape anymore (framework#3320); any surviving stored - * metadata renders this visible error placeholder (not a blank) so an author can - * rebind the widget to a dataset. +/* + * The retired-widget placeholder and its detector used to be declared right + * here, private to this file — and that is the whole reason objectui#4612 + * existed: the sibling surface `DashboardGridLayout` could not consume what it + * could not see, so the graceful fallback covered one of the two surfaces and + * the other rendered a silent blank chart for the identical stored metadata. + * Both now import the single declaration from `./legacyRetiredWidget`; the + * behaviour on this surface is unchanged (pinned by + * `__tests__/DashboardRenderer.legacyRetired.test.tsx`). */ -const LEGACY_RETIRED_WIDGET_SCHEMA = { - type: 'text', - value: 'This widget uses a retired data format. Edit it to bind a dataset.', - variant: 'caption', - align: 'center', - className: 'flex h-full w-full items-center justify-center rounded border border-dashed border-destructive/40 bg-destructive/5 p-4 text-center text-destructive', -} as const; /** * The dashboard renderer's props. @@ -545,8 +541,10 @@ const DashboardRendererInner = forwardRef): DashboardComponentSchema => + ({ type: 'dashboard', widgets: [widget] }) as unknown as DashboardComponentSchema; + +describe('DashboardGridLayout retired legacy widgets (#4612)', () => { + it.each([ + ['chart', { id: 'w1', type: 'bar', object: 'invoices', categoryField: 'month', valueField: 'amount', aggregate: 'sum' }], + // Byte-for-byte `examples/schema-catalog/src/schemas/plugin-dashboard/ + // filtered-dashboard.json` → `widgets[0]`: the real stored shape, not a + // fixture invented to match the detector. + ['catalog bar', { id: 'invoices_by_status', title: 'Invoices by Status', type: 'bar', object: 'invoices', categoryField: 'status', aggregate: 'count' }], + // The pivot family, whose legacy spelling names `rowField`/`columnField`. + ['pivot', { id: 'w1', type: 'pivot', object: 'invoices', rowField: 'region', valueField: 'amount' }], + // `widgets[2]` of the same catalog entry. + ['metric', { id: 'w1', type: 'metric', object: 'invoices', aggregate: 'count' }], + ])('renders the visible placeholder for a legacy %s widget', (_kind, widget) => { + render(); + expect(screen.getByText(/retired data format/i)).toBeInTheDocument(); + }); + + it('states the rebind affordance verbatim, not merely "something is wrong"', () => { + // The message IS the fix path — a placeholder that does not say what to do + // is only a prettier blank. Pinned verbatim, and identically to the + // DashboardRenderer surface (one shared constant, one wording). + render(); + expect( + screen.getByText('This widget uses a retired data format. Edit it to bind a dataset.'), + ).toBeInTheDocument(); + }); + + /** + * The must-not-change half. Each control asserts BOTH that the placeholder is + * absent AND that the widget was really rendered — otherwise "no placeholder" + * would also be satisfied by a grid that threw or drew nothing, and the + * control would pass for the wrong reason. + */ + const renderOne = (widget: Record) => { + const { container } = render(); + expect(container.querySelector('[data-testid="grid-layout"]')).toBeInTheDocument(); + return container; + }; + + it('does NOT show the placeholder for a dataset-bound widget', () => { + renderOne({ id: 'w1', type: 'bar', dataset: 'invoices', values: ['count'] }); + expect(screen.queryByText(/retired data format/i)).not.toBeInTheDocument(); + }); + + it('does NOT show the placeholder for an options.data provider widget', () => { + // The nested `{ provider: 'object', object, aggregate }` config is a + // separate, LIVE surface. Its `object` is read off `widgetData`, never off + // the widget top level — conflating the two would retire a working feature. + renderOne({ + id: 'w1', + type: 'bar', + options: { data: { provider: 'object', object: 'invoices', aggregate: { field: 'amount', function: 'sum' } } }, + }); + expect(screen.queryByText(/retired data format/i)).not.toBeInTheDocument(); + }); + + it('does NOT show the placeholder for a static-data widget', () => { + renderOne({ id: 'w1', type: 'bar', options: { data: [{ name: 'A', value: 1 }] } }); + expect(screen.queryByText(/retired data format/i)).not.toBeInTheDocument(); + }); + + it('does NOT show the placeholder for a static-data pivot widget', () => { + // DashboardRenderer's pivot arm returns the placeholder for the WHOLE family + // because that surface has no pivot renderer at all. This surface does, and + // a static-data pivot is a live static-data widget — so the shared detector + // is the inline-analytics sentinel (which the legacy pivot above matches via + // its top-level `object`), NOT the family arm. Mirroring the family arm here + // would have retired a working branch. + renderOne({ id: 'w1', type: 'pivot', options: { data: [{ region: 'EMEA', amount: 1 }] } }); + expect(screen.queryByText(/retired data format/i)).not.toBeInTheDocument(); + }); +}); diff --git a/packages/plugin-dashboard/src/legacyRetiredWidget.ts b/packages/plugin-dashboard/src/legacyRetiredWidget.ts new file mode 100644 index 000000000..22f03f051 --- /dev/null +++ b/packages/plugin-dashboard/src/legacyRetiredWidget.ts @@ -0,0 +1,110 @@ +/** + * 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. + */ + +/** + * ONE legacy detector and ONE placeholder, shared by every dashboard surface + * (objectui#4612). + * + * framework#3320 retired the pre-ADR-0021 inline-analytics binding — a widget + * carrying top-level `object` + `categoryField` / `valueField` / `aggregate` + * (pivot: `rowField` / `columnField`) instead of a semantic-layer `dataset`. No + * authoring surface emits that shape anymore (`WidgetConfigPanel` actively + * scrubs it on save via `LEGACY_ANALYTICS_KEYS`), so any widget still carrying + * it is stale STORED metadata — and stored metadata is exactly what a renderer + * cannot refuse to receive. + * + * The retirement therefore came with a graceful fallback: render a VISIBLE error + * placeholder naming the fix, never a blank widget. That fallback was applied to + * `DashboardRenderer` and to nothing else, while `DashboardGridLayout` — a + * separately exported surface, registered as the `dashboard-grid` SDUI component + * — kept falling through to its static-data branch with `data: []`. Same stored + * metadata, same product: a rebind prompt on one surface and a silent blank + * chart on the other, which is worse for the user than the pre-retirement state + * (no chart, no diagnostic, no path to fix). + * + * The defect existed *because* there were two surfaces and one fix, so the cure + * is not a second copy of the condition: both surfaces consume this module. + * + * ## What this deliberately does NOT match + * + * `options.data = { provider: 'object', object, aggregate }` is a DIFFERENT and + * still-LIVE authoring surface — the async data-provider config, which carries + * its OWN nested `object`/`aggregate`. Every read of it is on `widgetData`, not + * on the widget top level (`DashboardRenderer.tsx` / `DashboardGridLayout.tsx`, + * the `isObjectProvider` branches). Conflating the two would retire a working + * feature, so the detector requires the absence of any widget-level data before + * it looks at `object` at all — the same order `DashboardRenderer` has always + * used. + */ + +import type { DashboardWidgetSchema } from '@object-ui/types'; + +/** + * The placeholder schema rendered in place of a retired inline-analytics widget. + * + * A `text` node rather than a thrown error or an empty box: it has to reach the + * author *inside the dashboard*, in the tile where the chart used to be, and it + * has to say what to do next — a placeholder that only signals "something is + * wrong" is a prettier blank. The wording is contract (one condition, one + * wording) and is pinned verbatim by both surfaces' `legacyRetired` suites. + */ +export const LEGACY_RETIRED_WIDGET_SCHEMA = { + type: 'text', + value: 'This widget uses a retired data format. Edit it to bind a dataset.', + variant: 'caption', + align: 'center', + className: 'flex h-full w-full items-center justify-center rounded border border-dashed border-destructive/40 bg-destructive/5 p-4 text-center text-destructive', +} as const; + +/** + * The four keys this detector reads, and the reason it needs a shape of its own: + * two of them are not on `DashboardWidgetSchema` at all. `object` was removed + * from the widget vocabulary by the retirement — reading it is the whole point + * here — and `data` is a renderer-internal spelling that never had a declaration. + * Naming them once, here, is what keeps `as any` out of both call sites. + */ +type LegacyRetiredReadKeys = { + /** Semantic-layer binding (ADR-0021). Its presence means the widget is current. */ + dataset?: unknown; + /** Renderer-internal inline data array (the widget-level spelling). */ + data?: unknown; + /** Renderer-internal inline data / nested `provider: 'object'` config. */ + options?: { data?: unknown } | null; + /** The retired pre-ADR-0021 top-level object binding. */ + object?: unknown; +}; + +/** + * Does this widget still carry the retired pre-ADR-0021 inline-analytics shape? + * + * True when all three hold, in this order: + * + * 1. no `dataset` — a dataset-bound widget is the CURRENT shape and renders + * through the governed `queryDataset` path; + * 2. no renderer-internal data — neither `widget.data` nor `options.data`, which + * covers both the static inline array and the live `provider: 'object'` + * nested config; + * 3. a top-level `object` — the retired binding itself. + * + * `||` (not `??`) between the two data spellings, preserving `DashboardRenderer` + * byte for byte. + * + * Step 1 is stated here rather than inherited from a caller's render fork: + * `DashboardRenderer` never reached the placeholder for a dataset-bound widget + * because `datasetBound` picks `DatasetWidget` at the render site regardless of + * what `getComponentSchema` returned — so naming the condition costs that + * surface no observable behavior, and it keeps the predicate true on its own + * terms for any surface that has no such fork. + */ +export function isLegacyRetiredWidget(widget: DashboardWidgetSchema | null | undefined): boolean { + if (!widget) return false; + const w = widget as LegacyRetiredReadKeys; + if (w.dataset) return false; + const widgetData = w.data || w.options?.data; + return !widgetData && !!w.object; +}