diff --git a/.changeset/report-chart-axis-own-selection.md b/.changeset/report-chart-axis-own-selection.md new file mode 100644 index 0000000000..1ddd27ef6e --- /dev/null +++ b/.changeset/report-chart-axis-own-selection.md @@ -0,0 +1,27 @@ +--- +"@objectstack/lint": patch +--- + +`chart-axis-not-selected` resolves a report chart against its own `chart.yAxis`, not `report.values` (#15734) + +**Behaviour change — one false finding removed on the report surface.** A report chart whose `chart.yAxis` names a declared measure that `report.values` does not select no longer raises a `chart-axis-not-selected` warning. Nothing else about the rule moves, and no other surface moves at all. + +The warning stated a query consequence the renderer refutes. Read at the `@object-ui` revision this repo pins (`.objectui-sha`), `plugin-report/src/DatasetReportRenderer.tsx` does not query `report.values` for the chart at all — it runs the chart's own, narrower query out of the two axis strings: + +``` +const state = useDatasetRows( + dataset, + plan.kind === 'series' && xAxis ? [xAxis] : [], + wantsQuery && yAxis ? [yAxis] : [], +``` + +and says so in that file's own words at the `scopeOrder` docblock: *"the embedded chart queries only `chart.xAxis` × `chart.yAxis`"*. So the measure the warning said "the query does not return" is exactly the one the query asks for, and the chart plots it. `report.values` is the selection of the TABLE beneath the chart. + +Both limbs follow from that one measurement: + +- **No not-selected check at the report `chart.yAxis`.** That position IS the chart's query, so it cannot fail to select itself. `chart-measure-unknown` there is untouched: an UNDECLARED measure is still no column at all, and still an `error`. +- **`chart.series[].name` resolves against the singleton `{ chart.yAxis }`.** The entry is a display-name override paired with a DERIVED series, and the chart derives exactly one (`buildChartSeries(…, [xAxis], [yAxis], …)`). An entry naming `chart.yAxis` now lands however the table is selected, and one naming any other declared measure is still reported — including a measure `report.values` does select, which it could not reach before. + +The list-view and page-component surfaces are unchanged, and carry firing controls that say so: on both, `values` IS the measure set the query asks for (`ObjectView` hands it to the chart; `ObjectChart` queries `{ dimensions: schema.dimensions, measures: schema.values }`), so the existing resolution is the right one there. + +The per-position tier and consequence wording is untouched — only the SET the report surface resolves against moves. diff --git a/packages/lint/src/validate-chart-bindings.test.ts b/packages/lint/src/validate-chart-bindings.test.ts index 994aee1273..bdb8b5c16f 100644 --- a/packages/lint/src/validate-chart-bindings.test.ts +++ b/packages/lint/src/validate-chart-bindings.test.ts @@ -110,7 +110,14 @@ describe('validateChartBindings — report charts', () => { expect(findings[0].path).toBe('reports[0].chart.xAxis'); }); - it('warns when the yAxis measure is declared but not selected', () => { + // #15734 — this fixture WARNED until the set moved, and this pin held the + // warning in place. At the pinned `@object-ui` revision the embedded chart + // runs its OWN query out of the axis pair — `useDatasetRows(dataset, + // plan.kind === 'series' && xAxis ? [xAxis] : [], wantsQuery && yAxis ? + // [yAxis] : [], …)` — so `est_hours` IS asked for and IS plotted. The + // warning stated "the query does not return it", which that query refutes. + // `report.values` selects the TABLE beneath the chart, not the chart. + it('says nothing when the yAxis measure is declared but outside `report.values` — a report chart cannot fail to select what it queries', () => { const findings = validateChartBindings({ ...baseStack(), reports: [ @@ -122,9 +129,58 @@ describe('validateChartBindings — report charts', () => { }, ], }); + expect(findings).toEqual([]); + }); + + // The other direction of the same set move: `series[].name` is a display-name + // override paired with a DERIVED series, and the report chart derives exactly + // one — from its own `chart.yAxis`. So the singleton `{ chart.yAxis }` is + // what decides whether an entry lands, in BOTH directions. + it('a report series[].name that names `chart.yAxis` is silent even when `report.values` does not select it', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'r', + dataset: 'task_metrics', + values: ['task_count'], + chart: { + type: 'bar', + xAxis: 'status', + yAxis: 'est_hours', + series: [{ name: 'est_hours', color: '#f00' }], + }, + }, + ], + }); + expect(findings).toEqual([]); + }); + + it('a report series[].name that names a declared measure OTHER than `chart.yAxis` fires — even one `report.values` does select', () => { + const findings = validateChartBindings({ + ...baseStack(), + reports: [ + { + name: 'r', + dataset: 'task_metrics', + values: ['task_count', 'est_hours'], + chart: { + type: 'bar', + xAxis: 'status', + yAxis: 'est_hours', + series: [{ name: 'task_count' }], + }, + }, + ], + }); expect(findings).toHaveLength(1); - expect(findings[0].severity).toBe('warning'); expect(findings[0].rule).toBe(CHART_AXIS_NOT_SELECTED); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].path).toBe('reports[0].chart.series[0].name'); + // The SET the message names is the singleton the chart derives, not + // `report.values` — which here selects `task_count` and still cannot make + // the override land. + expect(findings[0].message).toContain("selected values (est_hours)"); }); it('errors on an unresolvable dataset', () => { @@ -273,13 +329,16 @@ describe('validateChartBindings — binding vs presentation positions (#15575)', expect(findings[0].hint).toContain('chart.yAxis'); }); - it('chart-axis-not-selected at the report yAxis position keeps its wording — that position IS the query', () => { + // #15734 replaced this case rather than re-worded it: the position keeps the + // wording it was given here, but nothing on this surface reaches it any more. + // A report `chart.yAxis` IS the query the chart issues, so it cannot name a + // measure that query does not ask for — the not-selected limb is gone at that + // position, and `chart-measure-unknown` (the case above) is untouched. + it('chart-axis-not-selected does not fire at the report yAxis position at all — that position IS the query', () => { const findings = validateChartBindings( reportWith({ type: 'bar', xAxis: 'status', yAxis: 'est_hours' }), ); - expect(findings).toHaveLength(1); - expect(findings[0].rule).toBe(CHART_AXIS_NOT_SELECTED); - expect(findings[0].message).toContain('the query does not return it'); + expect(findings).toEqual([]); }); // ── list-view charts ─────────────────────────────────────────────────── @@ -487,6 +546,36 @@ describe('validateChartBindings — list-view charts', () => { expect(findings[0].path).toBe('objects[0].listViews.by_status.chart.values[0]'); }); + // #15734 — the report surface moved to the chart's own `{ chart.yAxis }`. + // This surface did not, and this control shows it firing: `ObjectView` hands + // `values` to the chart as the dataset measures, so `values` IS the query's + // measure set here and a name outside the dataset still gates at its own + // position. The shape declares no `yAxis`/`series` limb, so no singleton can + // enter this surface in the first place. + it('list-view charts resolve against `values`, unchanged by #15734 — firing control', () => { + const findings = validateChartBindings({ + ...baseStack(), + views: [ + { + name: 'v', + list: { + chart: { + chartType: 'bar', + dataset: 'task_metrics', + dimensions: ['status'], + values: ['task_count', 'estimate_hours'], + }, + }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('views[0].list.chart.values[1]'); + expect(findings[0].message).toContain('this series comes back empty'); + }); + it('accepts a fully resolved list chart', () => { const findings = validateChartBindings({ ...baseStack(), @@ -539,6 +628,51 @@ describe('validateChartBindings — dataset-bound page chart components', () => expect(findings[0].severity).toBe('warning'); }); + // #15734 — the companion control on the page surface, where + // `chart-axis-not-selected` DOES have presentation limbs to fire at. + // `ObjectChart` queries `{ dimensions: schema.dimensions, measures: + // schema.values }`, so both limbs stay resolved against `values`: the SET + // named in each message is the page chart's own `values`, never a + // `{ yAxis[0].field }` singleton borrowed from the report surface. + it('page-component charts resolve against `values`, unchanged by #15734 — firing control on both limbs', () => { + const findings = validateChartBindings({ + ...baseStack(), + pages: [ + { + name: 'p', + regions: [ + { + name: 'main', + components: [ + { + type: 'object-chart', + properties: { + dataset: 'task_metrics', + dimensions: ['status'], + values: ['task_count'], + yAxis: [{ field: 'est_hours' }], + series: [{ name: 'est_hours' }], + }, + }, + ], + }, + ], + }, + ], + }); + expect(findings).toHaveLength(2); + expect(findings.every((f) => f.rule === CHART_AXIS_NOT_SELECTED)).toBe(true); + expect(findings.every((f) => f.severity === 'warning')).toBe(true); + expect(findings[0].path).toBe( + 'pages[0].regions[0].components[0].properties.yAxis[0].field', + ); + expect(findings[1].path).toBe( + 'pages[0].regions[0].components[0].properties.series[0].name', + ); + expect(findings[0].message).toContain('selected values (task_count)'); + expect(findings[1].message).toContain('selected values (task_count)'); + }); + it('accepts a resolved page chart', () => { const findings = validateChartBindings({ ...baseStack(), diff --git a/packages/lint/src/validate-chart-bindings.ts b/packages/lint/src/validate-chart-bindings.ts index 17f6f5bf45..718a2cd582 100644 --- a/packages/lint/src/validate-chart-bindings.ts +++ b/packages/lint/src/validate-chart-bindings.ts @@ -98,6 +98,33 @@ * it, so the series plots nothing"*. That is the truth at a query position and * not at a presentation one, where no series is derived for the name in the * first place, so its consequence is per position too. + * + * ## Which SET `chart-axis-not-selected` resolves against (#15734) + * + * The consequence is per POSITION; the selection it is measured against is per + * SURFACE — and on the report surface that selection is not `report.values`. + * At the same pinned revision `DatasetReportRenderer.tsx` runs + * `useDatasetRows(dataset, plan.kind === 'series' && xAxis ? [xAxis] : [], + * wantsQuery && yAxis ? [yAxis] : [], …)` and derives the plotted series with + * `buildChartSeries(…, [xAxis], [yAxis], …)` — *"the selection is exactly one + * dimension × one measure, so this takes the helper's single-dimension branch + * and returns ONE series"*. `report.values` is the selection of the TABLE + * beneath the chart; the chart's own is the axis pair. Two things follow: + * + * - **Nothing to report at the report `chart.yAxis`.** That position IS the + * chart's query, so it cannot fail to select itself. The warning that fired + * for a declared measure outside `report.values` stated a query consequence + * its own pin refutes: the chart asks for exactly that measure and plots + * it. `chart-measure-unknown` at that position is untouched — an UNDECLARED + * measure still returns no column, and still gates. + * - **`chart.series[].name` resolves against the singleton `{ chart.yAxis }`.** + * The override is paired with a DERIVED series and the chart derives one, + * so that singleton — not `report.values` — is the set an entry can land + * on. An entry naming `chart.yAxis` lands however the table is selected; + * one naming any other declared measure lands on nothing. + * + * The list-view and page-component surfaces keep `values`: there it IS the + * measure set the query asks for, so the existing resolution is correct. */ export const CHART_DIMENSION_UNKNOWN = 'chart-dimension-unknown'; @@ -204,6 +231,19 @@ interface ChartBinding { * different sentence from a series entry that pairs with nothing. */ axes?: Array<{ name: string; path: string }>; + /** + * The measures THIS chart's own query selects, when that is narrower than + * the `values` limb above (#15734). Set on the report surface and nowhere + * else: the embedded report chart queries `chart.xAxis` × `chart.yAxis` + * alone and derives exactly ONE series from it, while `report.values` is the + * selection of the table beneath it. `chart-axis-not-selected` resolves + * against this set where it is present — which is both why a `series[].name` + * override is measured against the singleton `{ chart.yAxis }`, and why the + * report `chart.yAxis` carries no not-selected check at all (a chart cannot + * fail to select what it queries). The other two surfaces leave it undefined + * and resolve against `values`, which on them IS the query's measure set. + */ + ownSelection?: string[]; where: string; /** Path of the chart container, for the dataset-level finding. */ path: string; @@ -372,6 +412,11 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { } const valSel = binding.values; const selected = new Set(valSel?.names ?? []); + // What this chart DERIVES A SERIES FOR — the set `chart-axis-not-selected` + // is measured against (#15734). Equal to the selection above wherever the + // chart IS the surface's selection; the narrower `{ chart.yAxis }` on the + // report surface, whose chart runs its own axis-pair query. + const derived = binding.ownSelection ? new Set(binding.ownSelection) : selected; if (valSel) { for (let i = 0; i < valSel.names.length; i++) { // The SELECTION itself — the names the dataset query asks for on every @@ -380,12 +425,16 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { } } if (binding.xAxis) dimensionRef(binding.xAxis.name, binding.xAxis.path); - if (binding.yAxis) measureRef(binding.yAxis.name, binding.yAxis.path, 'query', selected); + // No selection is passed at this position: the report `yAxis` IS the query + // the chart issues, so it cannot name a measure that query does not ask + // for (#15734). The `chart-measure-unknown` half still runs — an undeclared + // measure is no column at all. + if (binding.yAxis) measureRef(binding.yAxis.name, binding.yAxis.path, 'query'); // Axes before series, the order the page surface reported them in when the // two shared one limb — the split (#15575) changes the message, not the // walk. - for (const a of binding.axes ?? []) measureRef(a.name, a.path, 'page-axis', selected); - for (const s of binding.series ?? []) measureRef(s.name, s.path, s.kind, selected); + for (const a of binding.axes ?? []) measureRef(a.name, a.path, 'page-axis', derived); + for (const s of binding.series ?? []) measureRef(s.name, s.path, s.kind, derived); }; // ── 1. Report charts (report.chart + report.blocks[].chart) ── @@ -403,14 +452,23 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { path: string, ) => { if (!isRec(chart)) return; + const yAxisName = strName(chart.yAxis); check({ dataset, - // `values` is the report's measure SELECTION, not a chart ref; feeding - // it in lets the yAxis "declared but not selected" check work without - // reporting the selection itself twice. + // `values` is the REPORT's measure selection — what the table beneath + // the chart shows — not a chart ref. It is fed in so those names are + // resolved against the dataset (`chart-measure-unknown`) once, here, + // rather than reported twice or not at all. It is NOT what the chart + // queries, so it is no longer the set `chart-axis-not-selected` reads + // on this surface; `ownSelection` below is (#15734). values: { names: values, path: `${path}.values` }, xAxis: strName(chart.xAxis) ? { name: strName(chart.xAxis)!, path: `${path}.chart.xAxis` } : undefined, - yAxis: strName(chart.yAxis) ? { name: strName(chart.yAxis)!, path: `${path}.chart.yAxis` } : undefined, + yAxis: yAxisName ? { name: yAxisName, path: `${path}.chart.yAxis` } : undefined, + // The chart's own selection: the ONE measure it queries and derives a + // series from. Empty when no `yAxis` is authored — the chart plots + // nothing at all then, which the shape rules own, so no override can + // be reported as landing on nothing. + ownSelection: yAxisName ? [yAxisName] : [], series: recordsOf(chart.series) .map((s, si) => ({ name: strName(s.name),