Skip to content

Commit 2a5f04a

Browse files
authored
feat(spec,lint): pin <ObjectChart> aggregate result-column naming and validate its axes (#3701) (#3725)
#3684 extended ADR-0021 axis checking to report charts, list-view charts and dataset-bound page chart components, but had to leave the react <ObjectChart> block out: it is OBJECT-bound (objectName + an inline aggregate), and nothing in the repo said what the aggregated result columns were called. Record the convention rather than invent one: an object-bound aggregate returns rows keyed by the RAW FIELD NAMES — `groupBy` for the category column, `field` for the value column, the literal `count` for a fieldless count, plus `<field>__comparison` under a comparison overlay. The deliberate opposite of the dataset path, whose rows are keyed by the declared measure name. spec: ChartAggregateSchema/ChartGroupBySchema/ChartAggregateFunctionSchema replace the '{ field, function, groupBy }' description string and reject a non-count function with no field; chart-aggregate.ts records the convention and exports chartAggregateCategoryKey/chartAggregateValueKey/ chartAggregateResultKeys. The <ObjectChart> contract now names the props the block actually reads (chartType/xAxisKey/series join the overlay; ChartConfig's inert xAxis/yAxis/series leave dataProps per ADR-0078). lint: validate-react-page-props now reads attribute VALUES for <ObjectChart> — react-chart-field-unknown, react-chart-aggregate-invalid, react-chart-axis-unknown, react-chart-axis-inert. Static literals only; state- driven props, spreads, inline data, and cross-package objects are skipped. The end state remains runtime-honors-ChartConfig: tracked in #3729 (framework contract/lint flip) + objectui#2880 (renderer implementation), after which the overlay dialect retires.
1 parent 1bd5652 commit 2a5f04a

19 files changed

Lines changed: 1023 additions & 52 deletions
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/lint': minor
4+
---
5+
6+
`<ObjectChart>` aggregate result-column naming is now a contract, and its axis bindings are validated (issue #3701)
7+
8+
Split out of #3583 Phase 2 (#3684), which extended ADR-0021 axis checking to
9+
report charts, list-view charts, and dataset-bound page chart components but had
10+
to leave the react `<ObjectChart>` block out: it is OBJECT-bound (`objectName` +
11+
an inline `aggregate`), `aggregate` existed in the contract only as the
12+
description string `'{ field, function, groupBy }'`, and nothing in the repo said
13+
what the aggregated result columns were called. Without that, `xAxis`/`yAxis` had
14+
nothing to resolve against, and guessing a convention would have manufactured
15+
false positives (ADR-0072 D1).
16+
17+
**The convention, recorded rather than invented.** Every path that can serve an
18+
object-bound chart already agreed — the engine's structured-`groupBy` aggregate
19+
(whose alias objectui sets to `field || function`), the legacy analytics query
20+
(which remaps its measure key back to `field`), the client-side fallback, and the
21+
console's own chart-view wiring (`xAxisKey: groupBy`, `series[].dataKey: field`).
22+
`packages/spec/src/ui/chart-aggregate.ts` writes it down and exports it:
23+
24+
* an object-bound aggregate returns rows keyed by the **raw field names**
25+
`groupBy` for the category column, `field` for the value column, the literal
26+
`count` for a fieldless count, plus `<field>__comparison` under a comparison
27+
overlay;
28+
* `chartAggregateCategoryKey` / `chartAggregateValueKey` / `chartAggregateResultKeys`
29+
derive those columns so producers and checkers cannot re-derive them apart;
30+
* `ChartAggregateSchema` replaces the description string with a real Zod schema
31+
and rejects a non-`count` function with no `field` (which used to reach the
32+
renderer as `sum(undefined)` and render blank).
33+
34+
This is the deliberate opposite of the dataset path, whose rows are keyed by the
35+
declared measure `name` (`sum_amount`) — the trap `chart-measure-unknown` catches.
36+
Only the dataset path has an author-chosen name to key by.
37+
38+
**`<ObjectChart>`'s contract now names the props it actually reads.** The block
39+
consumes `xAxisKey` and `series[].dataKey`; `ChartConfig`'s `xAxis`/`yAxis`/`series`
40+
shapes reached it and were silently dropped, which ADR-0078 forbids. They are
41+
removed from the block's `dataProps`; `chartType`, `xAxisKey`, and `series` are
42+
declared in the React overlay where the other bindings live.
43+
44+
**`validate-react-page-props` now reads attribute VALUES**, not just names, for
45+
`<ObjectChart>`:
46+
47+
* `react-chart-field-unknown` (error) — `aggregate.field` / `aggregate.groupBy`
48+
naming a field the bound object does not declare;
49+
* `react-chart-aggregate-invalid` (error) — an unimplemented aggregation
50+
function, or a non-`count` function with nothing to aggregate;
51+
* `react-chart-axis-unknown` (error) — `xAxisKey` / `series[].dataKey` naming a
52+
column the aggregate does not return (including a dataset-style `sum_total`),
53+
or a category axis bound to the value column;
54+
* `react-chart-axis-inert` (warning) — the `xAxis` / `yAxis` shapes this block
55+
never reads.
56+
57+
Value reading is opt-in per block and evaluates only static literals: a prop
58+
driven by React state or a variable, a usage carrying a `{...spread}`, a chart
59+
given inline `data`, and objects another package defines are all skipped
60+
silently — an unresolvable binding is not a wrong one.

content/docs/deployment/validating-metadata.mdx

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,12 @@ chart: { type: 'bar', xAxis: 'status', yAxis: 'estimate_hours' }
156156

157157
An axis naming a measure the dataset declares but this chart does not *select*
158158
(not in `values`) is a **warning**: the query never returns it, so it plots
159-
nothing. The react `<ObjectChart>` block is object-bound rather than
160-
dataset-bound and is not checked here.
159+
nothing.
160+
161+
The react `<ObjectChart>` block is **object-bound** (`objectName` + an inline
162+
`aggregate`) rather than dataset-bound, so it is checked by the react-page prop
163+
gate below against a different rule — its result rows are keyed by the raw field
164+
names, exactly the opposite of the dataset case.
161165

162166
### 7. Navigation exposing objects nobody can read
163167

@@ -177,6 +181,34 @@ ships. Skipped for platform-provided objects (whose own packages grant them),
177181
for stacks that declare no permission sets at all, and when any set carries a
178182
wildcard (`objects: { '*': … }`) grant.
179183

184+
### 8. `<ObjectChart>` axes bound to the wrong result column
185+
186+
A `kind:'react'` page's `<ObjectChart>` is **object-bound**: `objectName` plus an
187+
inline `aggregate`, run as one ad-hoc query. Its rows come back keyed by the
188+
**raw field names** the aggregate was given — `groupBy` names the category
189+
column, `field` names the value column (the literal `count` for a fieldless
190+
`count`). That is the exact opposite of the dataset case in §6, and mixing the
191+
two conventions up is the usual cause of a chart that renders axes and no bars.
192+
193+
```jsx
194+
<ObjectChart objectName="invoice" chartType="bar"
195+
aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }}
196+
xAxisKey="status" series={[{ dataKey: 'sum_total' }]} />
197+
// ↑ a dataset-style measure name; the rows
198+
// are keyed `total` → error
199+
```
200+
201+
Both halves are checked: `aggregate.field` / `aggregate.groupBy` must be fields
202+
the object declares, and `xAxisKey` / `series[].dataKey` must name a column the
203+
aggregate actually returns (plus `<field>__comparison` when a comparison overlay
204+
is on). `ChartConfig`'s `xAxis` / `yAxis` shapes are **not** read by this block
205+
and are reported as inert (warning).
206+
207+
Skipped, to keep false positives at zero: any prop whose value is not a static
208+
literal (it comes from React state or a variable), a usage carrying a `{...spread}`,
209+
a chart given static `data` (its columns are the author's own), and objects
210+
another package defines.
211+
180212
## The one gate, two entry points
181213

182214
`os validate` and `os build` (alias of `os compile`) run the **same** validator:

content/docs/references/ui/chart.mdx

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,41 @@ Provides a comprehensive set of chart types for data visualization.
1818
## TypeScript Usage
1919

2020
```typescript
21-
import { ChartAnnotation, ChartAxis, ChartConfig, ChartInteraction, ChartSeries, ChartType } from '@objectstack/spec/ui';
22-
import type { ChartAnnotation, ChartAxis, ChartConfig, ChartInteraction, ChartSeries, ChartType } from '@objectstack/spec/ui';
21+
import { ChartAggregate, ChartAggregateFunction, ChartAnnotation, ChartAxis, ChartConfig, ChartGroupBy, ChartInteraction, ChartSeries, ChartType } from '@objectstack/spec/ui';
22+
import type { ChartAggregate, ChartAggregateFunction, ChartAnnotation, ChartAxis, ChartConfig, ChartGroupBy, ChartInteraction, ChartSeries, ChartType } from '@objectstack/spec/ui';
2323

2424
// Validate data
25-
const result = ChartAnnotation.parse(data);
25+
const result = ChartAggregate.parse(data);
2626
```
2727

28+
---
29+
30+
## ChartAggregate
31+
32+
Inline aggregation for an object-bound chart
33+
34+
### Properties
35+
36+
| Property | Type | Required | Description |
37+
| :--- | :--- | :--- | :--- |
38+
| **field** | `string` | optional | Field to aggregate — required for sum/avg/min/max, optional for count |
39+
| **function** | `Enum<'count' \| 'sum' \| 'avg' \| 'min' \| 'max'>` || Aggregation function |
40+
| **groupBy** | `string \| { field: string; dateGranularity?: Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; alias?: string }` || Field the rows are grouped by — the chart category axis |
41+
42+
43+
---
44+
45+
## ChartAggregateFunction
46+
47+
### Allowed Values
48+
49+
* `count`
50+
* `sum`
51+
* `avg`
52+
* `min`
53+
* `max`
54+
55+
2856
---
2957

3058
## ChartAnnotation
@@ -85,6 +113,35 @@ const result = ChartAnnotation.parse(data);
85113
| **aria** | `{ ariaLabel?: string; ariaDescribedBy?: string; role?: string }` | optional | ARIA accessibility attributes |
86114

87115

116+
---
117+
118+
## ChartGroupBy
119+
120+
### Union Options
121+
122+
This schema accepts one of the following structures:
123+
124+
#### Option 1
125+
126+
Field to group by
127+
128+
Type: `string`
129+
130+
---
131+
132+
#### Option 2
133+
134+
### Properties
135+
136+
| Property | Type | Required | Description |
137+
| :--- | :--- | :--- | :--- |
138+
| **field** | `string` || Field to group by |
139+
| **dateGranularity** | `Enum<'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>` | optional | Bucket date values into uniform periods |
140+
| **alias** | `string` | optional | Alias for the projected group value (defaults to `field`) — this becomes the category column |
141+
142+
---
143+
144+
88145
---
89146

90147
## ChartInteraction

examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ import { definePage } from '@objectstack/spec/ui';
1818
* cross-object reads declaratively (zero data code).
1919
* (This comparison absorbed the former Account Cockpit page.)
2020
*
21+
* The chart's axes are bound explicitly (#3701) to show the object-bound
22+
* result-column rule: an inline `aggregate` returns rows keyed by the RAW
23+
* FIELD NAMES — `status` (its `groupBy`) and `total` (its `field`) — not by a
24+
* dataset-style measure name. `os validate` now checks both halves.
25+
*
2126
* Styling (ADR-0065): no Tailwind — inline `style={{}}` with `hsl(var(--token))`;
2227
* data blocks and the drawer bring their own compiled styling. The drawer sets
2328
* NO pixel width: per #2578 pixel widths are deprecated (the author can't know
@@ -114,7 +119,7 @@ function Page() {
114119
<Stat label="Open AR" value={related.openInvoices} accent="hsl(38 92% 50%)" />
115120
</div>
116121
117-
<ObjectChart objectName="showcase_invoice" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} title="Invoice value by status" showLegend={true} />
122+
<ObjectChart objectName="showcase_invoice" chartType="bar" aggregate={{ field: 'total', function: 'sum', groupBy: 'status' }} xAxisKey="status" series={[{ dataKey: 'total', label: 'Invoice value' }]} title="Invoice value by status" showLegend={true} />
118123
119124
<RecordRelatedList objectName="showcase_account" recordId={sel} relationshipField="account" columns={['name', 'status', 'total']} limit={5} showViewAll={true} title="Invoices" />
120125

packages/lint/src/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,13 @@ export { validateJsxPages } from './validate-jsx-pages.js';
7575
export type { JsxPageFinding, JsxPageSeverity } from './validate-jsx-pages.js';
7676
export { validateReactPages } from './validate-react-pages.js';
7777
export type { ReactPageFinding, ReactPageSeverity } from './validate-react-pages.js';
78-
export { validateReactPageProps } from './validate-react-page-props.js';
78+
export {
79+
validateReactPageProps,
80+
REACT_CHART_FIELD_UNKNOWN,
81+
REACT_CHART_AGGREGATE_INVALID,
82+
REACT_CHART_AXIS_UNKNOWN,
83+
REACT_CHART_AXIS_INERT,
84+
} from './validate-react-page-props.js';
7985
export type { ReactPropFinding, ReactPropSeverity } from './validate-react-page-props.js';
8086
export { validatePageSourceStyling, PAGE_SOURCE_CLASSNAME } from './validate-page-source-styling.js';
8187
export type { SourceStyleFinding, SourceStyleSeverity } from './validate-page-source-styling.js';

packages/lint/src/validate-chart-bindings.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,14 @@
2727
* binding shape as a list chart, but it arrives through the untyped
2828
* `properties` bag.
2929
*
30-
* Deliberately NOT covered: the react `<ObjectChart>` block. It is
31-
* OBJECT-bound (`objectName` + an inline `aggregate`), not dataset-bound, and
32-
* nothing in the repo pins down what the runtime names the aggregated result
33-
* column — so there is no defensible answer for what its `yAxis[].field`
34-
* should resolve against. Guessing one would manufacture false positives on
35-
* the single authored usage. Its `aggregate.field` / `groupBy` ARE checkable
36-
* against the object's fields, but only by reading JSX attribute values, which
37-
* `validate-react-page-props` does not do today. Left for a follow-up rather
38-
* than half-built (ADR-0078 §5: verify, then enforce).
30+
* Not covered HERE, and deliberately so: the react `<ObjectChart>` block. It is
31+
* OBJECT-bound (`objectName` + an inline `aggregate`), so its result rows are
32+
* keyed by the RAW FIELD NAMES rather than by a measure name — the opposite
33+
* convention, which would make `chart-measure-unknown`'s message a lie. It also
34+
* arrives as JSX rather than config, so it needs the TypeScript compiler this
35+
* rule has no business loading. It is checked by `validate-react-page-props`
36+
* instead, against the naming convention `chartAggregateResultKeys`
37+
* (`@objectstack/spec/ui`) now pins down (#3701).
3938
*/
4039

4140
export const CHART_DIMENSION_UNKNOWN = 'chart-dimension-unknown';

0 commit comments

Comments
 (0)