Skip to content

Commit fa006fb

Browse files
os-zhuangclaude
andauthored
feat(lint): validate dashboard filter field-existence at build time (#3365) (#3382)
Since #2501 wires dashboard-level filters (`dateRange` + every `globalFilters[]`) into every widget's analytics query, a filter field absent on a widget's dataset object emits invalid SQL (`no such column: …`) and crashes the widget at render time — a build-decidable invariant that escaped `os validate` / `os build` and failed only when a user opened the dashboard. Extend `validateWidgetBindings` (ADR-0021) with a new `dashboard-filter-field-unknown` error: for each dashboard filter × widget, fail unless the filter's effective field (after any `filterBindings` re-target) exists on the widget's dataset object, or the widget opts out via `filterBindings: { <name>: false }`. The message names the dashboard, widget, filter, field, and object, mirroring the field-existence invariant ADR-0032 enforces for CEL references. Effective-field resolution mirrors the objectui runtime (opt-out / string re-target / legacy `targetWidgets` allow-list / filter default). Registry-injected system fields (e.g. `created_at`, the `dateRange` default) and objects outside the validated stack never false-positive. Adds fixture coverage for the failing and opted-out shapes; verified 0 findings against the real showcase dashboards. Claude-Session: https://claude.ai/code/session_01BDsV6nCuuRPWbJhqkNBQzF Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9a43e04 commit fa006fb

6 files changed

Lines changed: 320 additions & 2 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
Validate dashboard filter field-existence at build time (extend ADR-0021, #3365).
6+
7+
`validateWidgetBindings` now checks that every dashboard-level filter (`dateRange`
8+
+ each `globalFilters[]`) resolves to a real field on each bound widget's dataset
9+
object. Since #2501 wired these filters into every widget's analytics query, a
10+
filter field absent on a widget's object — e.g. a `dateRange` bound to
11+
`close_date` inherited by an account/contact widget over a different object —
12+
emitted invalid SQL (`no such column: close_date`) and crashed the widget at
13+
render time. That build-decidable invariant previously escaped `os validate` /
14+
`os build` and failed only when a user opened the dashboard.
15+
16+
It now fails the build (new rule `dashboard-filter-field-unknown`) with a message
17+
naming the dashboard, widget, filter, field, and object, unless the widget opts
18+
out via `filterBindings: { <name>: false }` or re-targets to an existing field —
19+
mirroring the field-existence invariant ADR-0032 enforces for CEL references.
20+
Effective-field resolution matches the runtime (`filterBindings` re-target /
21+
opt-out, legacy `targetWidgets` allow-list, filter default). Registry-injected
22+
system fields (e.g. `created_at`, the `dateRange` default) and objects outside
23+
the validated stack never false-positive.

content/docs/getting-started/validating-metadata.mdx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,16 @@ If a name doesn't resolve, the chart renders **empty** — no error (ADR-0021).
4848
`os validate` resolves every binding against the declared datasets and fails on a
4949
dangling one.
5050

51+
The same gate also checks **dashboard-level filter fields**. A dashboard's
52+
`dateRange` and each `globalFilters[]` entry are broadcast into every widget's
53+
analytics query, so a filter field that doesn't exist on a bound widget's object
54+
emits invalid SQL (`no such column: …`) and crashes that widget at render time.
55+
`os validate` fails when a filter's effective field — after any per-widget
56+
`filterBindings` re-target — is absent on the widget's dataset object, naming the
57+
dashboard, widget, filter, field, and object. Opt a widget out with
58+
`filterBindings: { <name>: false }`, or re-target the filter to one of that
59+
object's own fields.
60+
5161
## The one gate, two entry points
5262

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

packages/lint/README.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@ It never depends on a runtime and is never bundled into a frontend.
1414
## API
1515

1616
- `validateWidgetBindings(stack)` — dashboard widget → dataset → measure/dimension
17-
reference integrity, chart-config bindings, and measure-aggregation coherence
18-
(e.g. SUM of a `percent` field is meaningless).
17+
reference integrity, chart-config bindings, measure-aggregation coherence
18+
(e.g. SUM of a `percent` field is meaningless), and dashboard-level filter
19+
field existence — every `dateRange` / `globalFilters[]` field (after any
20+
`filterBindings` re-target) must exist on each bound widget's object, else the
21+
broadcast filter emits invalid SQL and the widget crashes at render (#3365).
1922
- `validateStackExpressions(stack)` — CEL/predicate validity for field
2023
conditionals, sharing rules, action `visible`/`disabled`, lifecycle hooks
2124
(ADR-0032).

packages/lint/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export {
2020
CHART_CONFIG_MISSING,
2121
TABLE_COUNT_ONLY,
2222
MEASURE_AGGREGATE_INCOHERENT,
23+
DASHBOARD_FILTER_FIELD_UNKNOWN,
2324
} from './validate-widget-bindings.js';
2425
export type { WidgetBindingFinding, WidgetBindingSeverity } from './validate-widget-bindings.js';
2526

packages/lint/src/validate-widget-bindings.test.ts

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
MEASURE_AGGREGATE_INCOHERENT,
1111
WIDGET_LEGACY_ANALYTICS_SHAPE,
1212
WIDGET_LEGACY_ANALYTICS_UNRENDERABLE,
13+
DASHBOARD_FILTER_FIELD_UNKNOWN,
1314
} from './validate-widget-bindings.js';
1415

1516
/** The downstream repro from issue #1719 — dataset with a count AND a sum
@@ -483,3 +484,135 @@ describe('validateWidgetBindings — legacy analytics shape (#1878/#1894)', () =
483484
expect(legacyOnly(validateWidgetBindings(stack))).toHaveLength(1);
484485
});
485486
});
487+
488+
describe('validateWidgetBindings (dashboard-filter-field-unknown, issue #3365)', () => {
489+
const only = (findings: ReturnType<typeof validateWidgetBindings>) =>
490+
findings.filter((f) => f.rule === DASHBOARD_FILTER_FIELD_UNKNOWN);
491+
492+
/**
493+
* The #3365 repro: a dashboard `dateRange` bound to `close_date` (which lives
494+
* only on the opportunity object) inherited by a widget over `crm_account`.
495+
* `dash` overrides the dashboard tail (dateRange/globalFilters); `widget`
496+
* overrides the single account widget.
497+
*/
498+
function stack(dash: Record<string, unknown> = {}, widget: Record<string, unknown> = {}) {
499+
return {
500+
objects: [
501+
{ name: 'crm_account', fields: [
502+
{ name: 'name', type: 'text' },
503+
{ name: 'industry', type: 'select' },
504+
{ name: 'renewal_date', type: 'date' },
505+
] },
506+
{ name: 'crm_opportunity', fields: [
507+
{ name: 'name', type: 'text' },
508+
{ name: 'close_date', type: 'date' },
509+
] },
510+
],
511+
datasets: [
512+
{ name: 'account_metrics', object: 'crm_account',
513+
dimensions: [{ name: 'industry', field: 'industry' }],
514+
measures: [{ name: 'account_count', aggregate: 'count' }] },
515+
],
516+
dashboards: [{
517+
name: 'executive_dashboard',
518+
label: 'Executive',
519+
dateRange: { field: 'close_date', defaultRange: 'this_quarter' },
520+
widgets: [{
521+
id: 'total_accounts', type: 'metric',
522+
dataset: 'account_metrics', values: ['account_count'],
523+
...widget,
524+
}],
525+
...dash,
526+
}],
527+
};
528+
}
529+
530+
it('errors on the repro: an inherited dateRange field absent on the widget object', () => {
531+
const findings = only(validateWidgetBindings(stack()));
532+
expect(findings).toHaveLength(1);
533+
const f = findings[0];
534+
expect(f.severity).toBe('error');
535+
// names the dashboard, widget, filter, field, and object (acceptance criteria)
536+
expect(f.where).toContain('executive_dashboard');
537+
expect(f.where).toContain('total_accounts');
538+
expect(f.message).toContain('dateRange');
539+
expect(f.message).toContain('close_date');
540+
expect(f.message).toContain('crm_account');
541+
expect(f.hint).toContain('filterBindings: { dateRange: false }');
542+
expect(f.path).toBe('dashboards[0].widgets[0]');
543+
});
544+
545+
it('passes when the widget opts out via filterBindings: { dateRange: false }', () => {
546+
expect(only(validateWidgetBindings(stack({}, { filterBindings: { dateRange: false } })))).toHaveLength(0);
547+
});
548+
549+
it('passes when the widget re-targets to an existing field', () => {
550+
expect(only(validateWidgetBindings(stack({}, { filterBindings: { dateRange: 'renewal_date' } })))).toHaveLength(0);
551+
});
552+
553+
it('errors (explicit wording) when a re-target names a non-existent field', () => {
554+
const findings = only(validateWidgetBindings(stack({}, { filterBindings: { dateRange: 'closed_date' } })));
555+
expect(findings).toHaveLength(1);
556+
expect(findings[0].message).toContain('via filterBindings');
557+
expect(findings[0].message).toContain('closed_date');
558+
});
559+
560+
it('passes when the inherited field exists on the object', () => {
561+
expect(only(validateWidgetBindings(stack({ dateRange: { field: 'renewal_date' } })))).toHaveLength(0);
562+
});
563+
564+
it('does not false-positive on the created_at system field (bare dateRange default)', () => {
565+
// dateRange with no `field` defaults to `created_at`, a registry-injected
566+
// system field never present in `object.fields`.
567+
expect(only(validateWidgetBindings(stack({ dateRange: { defaultRange: 'this_month' } })))).toHaveLength(0);
568+
});
569+
570+
it('checks globalFilters[] fields too (name defaults to field)', () => {
571+
const findings = only(validateWidgetBindings(stack({
572+
dateRange: undefined,
573+
globalFilters: [{ field: 'region', type: 'select' }],
574+
})));
575+
expect(findings).toHaveLength(1);
576+
expect(findings[0].message).toContain('region');
577+
expect(findings[0].message).toContain('crm_account');
578+
});
579+
580+
it('a globalFilter opt-out uses the filter name (custom name honoured)', () => {
581+
// custom `name` becomes the filterBindings key, not the raw field.
582+
expect(only(validateWidgetBindings(stack(
583+
{ dateRange: undefined, globalFilters: [{ name: 'sales_region', field: 'region', type: 'select' }] },
584+
{ filterBindings: { sales_region: false } },
585+
)))).toHaveLength(0);
586+
});
587+
588+
it('a targetWidgets allow-list gates the default binding (unlisted widget is unbound)', () => {
589+
// `region` targets only some_other_widget, so total_accounts never inherits
590+
// it — even though crm_account has no `region`.
591+
expect(only(validateWidgetBindings(stack({
592+
dateRange: undefined,
593+
globalFilters: [{ field: 'region', type: 'select', targetWidgets: ['some_other_widget'] }],
594+
})))).toHaveLength(0);
595+
});
596+
597+
it('skips a relationship-path filter field (dotted paths are engine-resolved)', () => {
598+
expect(only(validateWidgetBindings(stack({ dateRange: { field: 'account.region' } })))).toHaveLength(0);
599+
});
600+
601+
it('cannot judge — and never false-positives — when the object is not in the stack', () => {
602+
const s = stack();
603+
delete (s as { objects?: unknown }).objects;
604+
expect(only(validateWidgetBindings(s))).toHaveLength(0);
605+
});
606+
607+
it('the error is NOT suppressible via suppressWarnings', () => {
608+
const findings = only(validateWidgetBindings(stack({}, {
609+
suppressWarnings: [DASHBOARD_FILTER_FIELD_UNKNOWN],
610+
})));
611+
expect(findings).toHaveLength(1);
612+
expect(findings[0].severity).toBe('error');
613+
});
614+
615+
it('is silent on dashboards with no dashboard-level filters', () => {
616+
expect(only(validateWidgetBindings(stack({ dateRange: undefined })))).toHaveLength(0);
617+
});
618+
});

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@ import { isIncoherentAggregate } from '@objectstack/spec/data';
3434
* renders nothing. Errored (not warned) so this class of authoring mistake —
3535
* very often an AI emitting a removed shape — fails the build instead of
3636
* shipping a blank widget past human review.
37+
* - `dashboard-filter-field-unknown` (#3365) — a dashboard-level filter
38+
* (`dateRange` or a `globalFilters[]` entry) is wired into EVERY widget's
39+
* analytics query (#2501), but its EFFECTIVE field (after any `filterBindings`
40+
* re-target) does not exist on a bound widget's dataset object. The widget's
41+
* query then references a non-existent column and crashes at render time
42+
* (`no such column …`) — a build-decidable invariant that previously escaped
43+
* the static gate and failed only when a user opened the dashboard. A widget
44+
* opts out with `filterBindings: { <name>: false }` or re-targets to a real
45+
* field. This is the same field-existence invariant ADR-0032 enforces for
46+
* CEL formula / sharing-rule references, applied to dashboard filter fields.
3747
*
3848
* Advisory rules — severity `warning`, build stays green:
3949
*
@@ -72,6 +82,7 @@ export const TABLE_COUNT_ONLY = 'table-count-only';
7282
export const MEASURE_AGGREGATE_INCOHERENT = 'measure-aggregate-incoherent';
7383
export const WIDGET_LEGACY_ANALYTICS_SHAPE = 'widget-legacy-analytics-shape';
7484
export const WIDGET_LEGACY_ANALYTICS_UNRENDERABLE = 'widget-legacy-analytics-unrenderable';
85+
export const DASHBOARD_FILTER_FIELD_UNKNOWN = 'dashboard-filter-field-unknown';
7586

7687
/**
7788
* Pre-ADR-0021 inline-analytics keys. The single-form cutover replaced them
@@ -189,6 +200,97 @@ function list(names: Iterable<string>): string {
189200
return arr.length > 0 ? arr.join(', ') : '(none)';
190201
}
191202

203+
// ── dashboard-filter field-existence (#3365) ─────────────────────────────────
204+
205+
/** Reserved filter name for the dashboard's built-in date range (#2501). */
206+
const DATE_RANGE_FILTER_NAME = 'dateRange';
207+
/**
208+
* Default field of the built-in date range when `dateRange.field` is omitted.
209+
* MUST track objectui `dashboard-filters.ts` `DATE_RANGE_DEFAULT_FIELD` — the
210+
* runtime this check shadows. `created_at` is a registry-injected system field
211+
* (below), so a bare `dateRange` never false-positives.
212+
*/
213+
const DATE_RANGE_DEFAULT_FIELD = 'created_at';
214+
215+
/**
216+
* Registry-injected fields present on (almost) every object but NOT declared in
217+
* `object.fields`, so a dashboard filter targeting one must not be flagged as a
218+
* missing column. Superset of the objectql registry's `applySystemFields`
219+
* (audit columns, ownership, tenant, soft-delete) and spec's `SystemFieldName`.
220+
* Deliberately generous: the cost of over-inclusion is at worst a missed error
221+
* on a `systemFields: false` object (rare); the cost of under-inclusion is a
222+
* false build failure on the ubiquitous `dateRange` → `created_at` default. The
223+
* near-zero-false-positive bias mirrors ADR-0032's field-ref validator.
224+
*/
225+
const SYSTEM_FIELDS = new Set<string>([
226+
'id',
227+
'created_at', 'created_by', 'updated_at', 'updated_by',
228+
'owner_id', 'organization_id', 'tenant_id', 'user_id',
229+
'deleted_at',
230+
]);
231+
232+
interface DashFilterDef {
233+
/** Stable filter name — the key widgets bind against in `filterBindings`. */
234+
name: string;
235+
/** Default target field when a widget declares no explicit binding. */
236+
field: string;
237+
/** Legacy widget-id allow-list; gates the DEFAULT binding only. */
238+
targetWidgets?: string[];
239+
}
240+
241+
/**
242+
* Normalize a dashboard's declared filters into `{ name, field, targetWidgets }`
243+
* defs — the built-in `dateRange` (reserved name) first, then every
244+
* `globalFilters[]` entry named by its `name` (defaulting to `field`). Later
245+
* duplicates win. Mirrors objectui `resolveDashboardFilterDefs`.
246+
*/
247+
function dashboardFilterDefs(dash: AnyRec): DashFilterDef[] {
248+
const byName = new Map<string, DashFilterDef>();
249+
250+
const dateRange = dash.dateRange;
251+
if (dateRange && typeof dateRange === 'object') {
252+
const declared = (dateRange as AnyRec).field;
253+
const field = typeof declared === 'string' && declared ? declared : DATE_RANGE_DEFAULT_FIELD;
254+
byName.set(DATE_RANGE_FILTER_NAME, { name: DATE_RANGE_FILTER_NAME, field });
255+
}
256+
257+
for (const f of asArray(dash.globalFilters)) {
258+
if (typeof f.field !== 'string' || !f.field) continue;
259+
const name = typeof f.name === 'string' && f.name ? f.name : f.field;
260+
const targetWidgets = Array.isArray(f.targetWidgets)
261+
? f.targetWidgets.filter((w): w is string => typeof w === 'string')
262+
: undefined;
263+
byName.set(name, { name, field: f.field, targetWidgets });
264+
}
265+
266+
return [...byName.values()];
267+
}
268+
269+
/**
270+
* Resolve which field of `widget` a filter binds to, or `undefined` when the
271+
* widget is not bound (opted out / not targeted). Precedence mirrors objectui
272+
* `resolveBoundField`: explicit `filterBindings` entry (string re-targets,
273+
* `false` opts out — both win) → legacy `targetWidgets` allow-list → the
274+
* filter's own default `field`. `explicit` distinguishes an author-chosen field
275+
* (a typo they must fix) from the inherited default (which they may opt out of).
276+
*/
277+
function effectiveFilterField(
278+
widget: AnyRec,
279+
def: DashFilterDef,
280+
): { field: string; explicit: boolean } | undefined {
281+
const bindings = widget.filterBindings;
282+
const binding = bindings && typeof bindings === 'object'
283+
? (bindings as AnyRec)[def.name]
284+
: undefined;
285+
if (binding === false) return undefined;
286+
if (typeof binding === 'string' && binding) return { field: binding, explicit: true };
287+
if (def.targetWidgets && def.targetWidgets.length > 0) {
288+
const id = typeof widget.id === 'string' ? widget.id : undefined;
289+
if (!id || !def.targetWidgets.includes(id)) return undefined;
290+
}
291+
return { field: def.field, explicit: false };
292+
}
293+
192294
/**
193295
* Validate every dashboard widget's dataset binding. Returns the list of
194296
* findings (empty = clean). Caller decides how to surface them: `error`
@@ -253,6 +355,9 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] {
253355
const dash = dashboards[i];
254356
const dashName = typeof dash.name === 'string' ? dash.name : `(dashboard ${i})`;
255357
const widgets = Array.isArray(dash.widgets) ? (dash.widgets as AnyRec[]) : [];
358+
// Dashboard-level filters (`dateRange` + `globalFilters`) are broadcast into
359+
// every widget's query (#2501) — resolved once here, checked per widget below.
360+
const dashFilterDefs = dashboardFilterDefs(dash);
256361

257362
for (let j = 0; j < widgets.length; j++) {
258363
const w = widgets[j];
@@ -331,6 +436,49 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] {
331436
// Without a resolved dataset there is nothing to check names against.
332437
if (!dataset) continue;
333438

439+
// ── (a1) dashboard filter fields exist on the widget's object (#3365) ──
440+
// Each dashboard-level filter is ANDed into this widget's analytics query
441+
// (#2501); a filter whose EFFECTIVE field (after `filterBindings`) is not a
442+
// column on the bound dataset object emits SQL like `WHERE close_date …`
443+
// against a table without that column and the widget crashes at query time.
444+
// Errored (not warned): a broken query, not advice. The opt-out is the
445+
// author's own `filterBindings: { <name>: false }`, so no suppression needed.
446+
if (dashFilterDefs.length > 0) {
447+
const datasetObject = typeof dataset.object === 'string' ? dataset.object : undefined;
448+
// Only judge when the bound object's fields are known in THIS stack; an
449+
// object from another installed package is unknowable here — skip rather
450+
// than false-positive (mirrors the measure-aggregate check above).
451+
const objectFields = datasetObject ? objectFieldTypes.get(datasetObject) : undefined;
452+
if (objectFields) {
453+
for (const def of dashFilterDefs) {
454+
const eff = effectiveFilterField(w, def);
455+
if (!eff) continue; // opted out / not targeted → filter never applies
456+
const field = eff.field;
457+
// A relationship path (`account.region`) is resolved by the query
458+
// engine, not a base column, so it can't be checked here — skip it.
459+
if (field.includes('.')) continue;
460+
if (objectFields.has(field) || SYSTEM_FIELDS.has(field)) continue;
461+
push({
462+
severity: 'error',
463+
rule: DASHBOARD_FILTER_FIELD_UNKNOWN,
464+
message: eff.explicit
465+
? `binds dashboard filter \`${def.name}\` to field \`${field}\` ` +
466+
`(via filterBindings), but object \`${datasetObject}\` (dataset "${dsName}") ` +
467+
`has no field \`${field}\`.`
468+
: `inherits dashboard filter \`${def.name}(${field})\`, but object ` +
469+
`\`${datasetObject}\` (dataset "${dsName}") has no field \`${field}\`.`,
470+
hint: eff.explicit
471+
? `Point filterBindings: { ${def.name}: '<field>' } at a field that exists on ` +
472+
`\`${datasetObject}\`, or opt out with filterBindings: { ${def.name}: false }.` +
473+
`${suggest(field, objectFields.keys())} Object fields: ${list(objectFields.keys())}.`
474+
: `Set filterBindings: { ${def.name}: false } on this widget to opt out, or ` +
475+
`re-target to an existing field with filterBindings: { ${def.name}: '<field>' }.` +
476+
`${suggest(field, objectFields.keys())} Object fields: ${list(objectFields.keys())}.`,
477+
});
478+
}
479+
}
480+
}
481+
334482
const dimensionNames = new Set<string>();
335483
for (const d of asArray(dataset.dimensions)) {
336484
if (typeof d.name === 'string') dimensionNames.add(d.name);

0 commit comments

Comments
 (0)