From 0ae4f06bfe3c8fb8182428607ee82e9453f43140 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 21 Jul 2026 13:45:09 +0000 Subject: [PATCH] feat(lint): validate dashboard filter field-existence at build time (#3365) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: { : 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. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01BDsV6nCuuRPWbJhqkNBQzF --- .../dashboard-filter-field-validation.md | 23 +++ .../getting-started/validating-metadata.mdx | 10 ++ packages/lint/README.md | 7 +- packages/lint/src/index.ts | 1 + .../lint/src/validate-widget-bindings.test.ts | 133 ++++++++++++++++ packages/lint/src/validate-widget-bindings.ts | 148 ++++++++++++++++++ 6 files changed, 320 insertions(+), 2 deletions(-) create mode 100644 .changeset/dashboard-filter-field-validation.md diff --git a/.changeset/dashboard-filter-field-validation.md b/.changeset/dashboard-filter-field-validation.md new file mode 100644 index 0000000000..1867620c98 --- /dev/null +++ b/.changeset/dashboard-filter-field-validation.md @@ -0,0 +1,23 @@ +--- +"@objectstack/lint": minor +--- + +Validate dashboard filter field-existence at build time (extend ADR-0021, #3365). + +`validateWidgetBindings` now checks that every dashboard-level filter (`dateRange` ++ each `globalFilters[]`) resolves to a real field on each bound widget's dataset +object. Since #2501 wired these filters into every widget's analytics query, a +filter field absent on a widget's object — e.g. a `dateRange` bound to +`close_date` inherited by an account/contact widget over a different object — +emitted invalid SQL (`no such column: close_date`) and crashed the widget at +render time. That build-decidable invariant previously escaped `os validate` / +`os build` and failed only when a user opened the dashboard. + +It now fails the build (new rule `dashboard-filter-field-unknown`) with a message +naming the dashboard, widget, filter, field, and object, unless the widget opts +out via `filterBindings: { : false }` or re-targets to an existing field — +mirroring the field-existence invariant ADR-0032 enforces for CEL references. +Effective-field resolution matches the runtime (`filterBindings` re-target / +opt-out, 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. diff --git a/content/docs/getting-started/validating-metadata.mdx b/content/docs/getting-started/validating-metadata.mdx index aa7b8e26de..416bb8b27e 100644 --- a/content/docs/getting-started/validating-metadata.mdx +++ b/content/docs/getting-started/validating-metadata.mdx @@ -48,6 +48,16 @@ If a name doesn't resolve, the chart renders **empty** — no error (ADR-0021). `os validate` resolves every binding against the declared datasets and fails on a dangling one. +The same gate also checks **dashboard-level filter fields**. A dashboard's +`dateRange` and each `globalFilters[]` entry are broadcast into every widget's +analytics query, so a filter field that doesn't exist on a bound widget's object +emits invalid SQL (`no such column: …`) and crashes that widget at render time. +`os validate` fails when a filter's effective field — after any per-widget +`filterBindings` re-target — is absent on the widget's dataset object, naming the +dashboard, widget, filter, field, and object. Opt a widget out with +`filterBindings: { : false }`, or re-target the filter to one of that +object's own fields. + ## The one gate, two entry points `os validate` and `os build` (alias of `os compile`) run the **same** validator: diff --git a/packages/lint/README.md b/packages/lint/README.md index 3089c43379..a7a85b0a76 100644 --- a/packages/lint/README.md +++ b/packages/lint/README.md @@ -14,8 +14,11 @@ It never depends on a runtime and is never bundled into a frontend. ## API - `validateWidgetBindings(stack)` — dashboard widget → dataset → measure/dimension - reference integrity, chart-config bindings, and measure-aggregation coherence - (e.g. SUM of a `percent` field is meaningless). + reference integrity, chart-config bindings, measure-aggregation coherence + (e.g. SUM of a `percent` field is meaningless), and dashboard-level filter + field existence — every `dateRange` / `globalFilters[]` field (after any + `filterBindings` re-target) must exist on each bound widget's object, else the + broadcast filter emits invalid SQL and the widget crashes at render (#3365). - `validateStackExpressions(stack)` — CEL/predicate validity for field conditionals, sharing rules, action `visible`/`disabled`, lifecycle hooks (ADR-0032). diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 336cd13684..dbdb8451c0 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -20,6 +20,7 @@ export { CHART_CONFIG_MISSING, TABLE_COUNT_ONLY, MEASURE_AGGREGATE_INCOHERENT, + DASHBOARD_FILTER_FIELD_UNKNOWN, } from './validate-widget-bindings.js'; export type { WidgetBindingFinding, WidgetBindingSeverity } from './validate-widget-bindings.js'; diff --git a/packages/lint/src/validate-widget-bindings.test.ts b/packages/lint/src/validate-widget-bindings.test.ts index 8226098bea..b179be51d8 100644 --- a/packages/lint/src/validate-widget-bindings.test.ts +++ b/packages/lint/src/validate-widget-bindings.test.ts @@ -10,6 +10,7 @@ import { MEASURE_AGGREGATE_INCOHERENT, WIDGET_LEGACY_ANALYTICS_SHAPE, WIDGET_LEGACY_ANALYTICS_UNRENDERABLE, + DASHBOARD_FILTER_FIELD_UNKNOWN, } from './validate-widget-bindings.js'; /** The downstream repro from issue #1719 — dataset with a count AND a sum @@ -483,3 +484,135 @@ describe('validateWidgetBindings — legacy analytics shape (#1878/#1894)', () = expect(legacyOnly(validateWidgetBindings(stack))).toHaveLength(1); }); }); + +describe('validateWidgetBindings (dashboard-filter-field-unknown, issue #3365)', () => { + const only = (findings: ReturnType) => + findings.filter((f) => f.rule === DASHBOARD_FILTER_FIELD_UNKNOWN); + + /** + * The #3365 repro: a dashboard `dateRange` bound to `close_date` (which lives + * only on the opportunity object) inherited by a widget over `crm_account`. + * `dash` overrides the dashboard tail (dateRange/globalFilters); `widget` + * overrides the single account widget. + */ + function stack(dash: Record = {}, widget: Record = {}) { + return { + objects: [ + { name: 'crm_account', fields: [ + { name: 'name', type: 'text' }, + { name: 'industry', type: 'select' }, + { name: 'renewal_date', type: 'date' }, + ] }, + { name: 'crm_opportunity', fields: [ + { name: 'name', type: 'text' }, + { name: 'close_date', type: 'date' }, + ] }, + ], + datasets: [ + { name: 'account_metrics', object: 'crm_account', + dimensions: [{ name: 'industry', field: 'industry' }], + measures: [{ name: 'account_count', aggregate: 'count' }] }, + ], + dashboards: [{ + name: 'executive_dashboard', + label: 'Executive', + dateRange: { field: 'close_date', defaultRange: 'this_quarter' }, + widgets: [{ + id: 'total_accounts', type: 'metric', + dataset: 'account_metrics', values: ['account_count'], + ...widget, + }], + ...dash, + }], + }; + } + + it('errors on the repro: an inherited dateRange field absent on the widget object', () => { + const findings = only(validateWidgetBindings(stack())); + expect(findings).toHaveLength(1); + const f = findings[0]; + expect(f.severity).toBe('error'); + // names the dashboard, widget, filter, field, and object (acceptance criteria) + expect(f.where).toContain('executive_dashboard'); + expect(f.where).toContain('total_accounts'); + expect(f.message).toContain('dateRange'); + expect(f.message).toContain('close_date'); + expect(f.message).toContain('crm_account'); + expect(f.hint).toContain('filterBindings: { dateRange: false }'); + expect(f.path).toBe('dashboards[0].widgets[0]'); + }); + + it('passes when the widget opts out via filterBindings: { dateRange: false }', () => { + expect(only(validateWidgetBindings(stack({}, { filterBindings: { dateRange: false } })))).toHaveLength(0); + }); + + it('passes when the widget re-targets to an existing field', () => { + expect(only(validateWidgetBindings(stack({}, { filterBindings: { dateRange: 'renewal_date' } })))).toHaveLength(0); + }); + + it('errors (explicit wording) when a re-target names a non-existent field', () => { + const findings = only(validateWidgetBindings(stack({}, { filterBindings: { dateRange: 'closed_date' } }))); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('via filterBindings'); + expect(findings[0].message).toContain('closed_date'); + }); + + it('passes when the inherited field exists on the object', () => { + expect(only(validateWidgetBindings(stack({ dateRange: { field: 'renewal_date' } })))).toHaveLength(0); + }); + + it('does not false-positive on the created_at system field (bare dateRange default)', () => { + // dateRange with no `field` defaults to `created_at`, a registry-injected + // system field never present in `object.fields`. + expect(only(validateWidgetBindings(stack({ dateRange: { defaultRange: 'this_month' } })))).toHaveLength(0); + }); + + it('checks globalFilters[] fields too (name defaults to field)', () => { + const findings = only(validateWidgetBindings(stack({ + dateRange: undefined, + globalFilters: [{ field: 'region', type: 'select' }], + }))); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('region'); + expect(findings[0].message).toContain('crm_account'); + }); + + it('a globalFilter opt-out uses the filter name (custom name honoured)', () => { + // custom `name` becomes the filterBindings key, not the raw field. + expect(only(validateWidgetBindings(stack( + { dateRange: undefined, globalFilters: [{ name: 'sales_region', field: 'region', type: 'select' }] }, + { filterBindings: { sales_region: false } }, + )))).toHaveLength(0); + }); + + it('a targetWidgets allow-list gates the default binding (unlisted widget is unbound)', () => { + // `region` targets only some_other_widget, so total_accounts never inherits + // it — even though crm_account has no `region`. + expect(only(validateWidgetBindings(stack({ + dateRange: undefined, + globalFilters: [{ field: 'region', type: 'select', targetWidgets: ['some_other_widget'] }], + })))).toHaveLength(0); + }); + + it('skips a relationship-path filter field (dotted paths are engine-resolved)', () => { + expect(only(validateWidgetBindings(stack({ dateRange: { field: 'account.region' } })))).toHaveLength(0); + }); + + it('cannot judge — and never false-positives — when the object is not in the stack', () => { + const s = stack(); + delete (s as { objects?: unknown }).objects; + expect(only(validateWidgetBindings(s))).toHaveLength(0); + }); + + it('the error is NOT suppressible via suppressWarnings', () => { + const findings = only(validateWidgetBindings(stack({}, { + suppressWarnings: [DASHBOARD_FILTER_FIELD_UNKNOWN], + }))); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + }); + + it('is silent on dashboards with no dashboard-level filters', () => { + expect(only(validateWidgetBindings(stack({ dateRange: undefined })))).toHaveLength(0); + }); +}); diff --git a/packages/lint/src/validate-widget-bindings.ts b/packages/lint/src/validate-widget-bindings.ts index 8f9f1401da..3cabad6942 100644 --- a/packages/lint/src/validate-widget-bindings.ts +++ b/packages/lint/src/validate-widget-bindings.ts @@ -34,6 +34,16 @@ import { isIncoherentAggregate } from '@objectstack/spec/data'; * renders nothing. Errored (not warned) so this class of authoring mistake — * very often an AI emitting a removed shape — fails the build instead of * shipping a blank widget past human review. + * - `dashboard-filter-field-unknown` (#3365) — a dashboard-level filter + * (`dateRange` or a `globalFilters[]` entry) is wired into EVERY widget's + * analytics query (#2501), but its EFFECTIVE field (after any `filterBindings` + * re-target) does not exist on a bound widget's dataset object. The widget's + * query then references a non-existent column and crashes at render time + * (`no such column …`) — a build-decidable invariant that previously escaped + * the static gate and failed only when a user opened the dashboard. A widget + * opts out with `filterBindings: { : false }` or re-targets to a real + * field. This is the same field-existence invariant ADR-0032 enforces for + * CEL formula / sharing-rule references, applied to dashboard filter fields. * * Advisory rules — severity `warning`, build stays green: * @@ -72,6 +82,7 @@ export const TABLE_COUNT_ONLY = 'table-count-only'; export const MEASURE_AGGREGATE_INCOHERENT = 'measure-aggregate-incoherent'; export const WIDGET_LEGACY_ANALYTICS_SHAPE = 'widget-legacy-analytics-shape'; export const WIDGET_LEGACY_ANALYTICS_UNRENDERABLE = 'widget-legacy-analytics-unrenderable'; +export const DASHBOARD_FILTER_FIELD_UNKNOWN = 'dashboard-filter-field-unknown'; /** * Pre-ADR-0021 inline-analytics keys. The single-form cutover replaced them @@ -189,6 +200,97 @@ function list(names: Iterable): string { return arr.length > 0 ? arr.join(', ') : '(none)'; } +// ── dashboard-filter field-existence (#3365) ───────────────────────────────── + +/** Reserved filter name for the dashboard's built-in date range (#2501). */ +const DATE_RANGE_FILTER_NAME = 'dateRange'; +/** + * Default field of the built-in date range when `dateRange.field` is omitted. + * MUST track objectui `dashboard-filters.ts` `DATE_RANGE_DEFAULT_FIELD` — the + * runtime this check shadows. `created_at` is a registry-injected system field + * (below), so a bare `dateRange` never false-positives. + */ +const DATE_RANGE_DEFAULT_FIELD = 'created_at'; + +/** + * Registry-injected fields present on (almost) every object but NOT declared in + * `object.fields`, so a dashboard filter targeting one must not be flagged as a + * missing column. Superset of the objectql registry's `applySystemFields` + * (audit columns, ownership, tenant, soft-delete) and spec's `SystemFieldName`. + * Deliberately generous: the cost of over-inclusion is at worst a missed error + * on a `systemFields: false` object (rare); the cost of under-inclusion is a + * false build failure on the ubiquitous `dateRange` → `created_at` default. The + * near-zero-false-positive bias mirrors ADR-0032's field-ref validator. + */ +const SYSTEM_FIELDS = new Set([ + 'id', + 'created_at', 'created_by', 'updated_at', 'updated_by', + 'owner_id', 'organization_id', 'tenant_id', 'user_id', + 'deleted_at', +]); + +interface DashFilterDef { + /** Stable filter name — the key widgets bind against in `filterBindings`. */ + name: string; + /** Default target field when a widget declares no explicit binding. */ + field: string; + /** Legacy widget-id allow-list; gates the DEFAULT binding only. */ + targetWidgets?: string[]; +} + +/** + * Normalize a dashboard's declared filters into `{ name, field, targetWidgets }` + * defs — the built-in `dateRange` (reserved name) first, then every + * `globalFilters[]` entry named by its `name` (defaulting to `field`). Later + * duplicates win. Mirrors objectui `resolveDashboardFilterDefs`. + */ +function dashboardFilterDefs(dash: AnyRec): DashFilterDef[] { + const byName = new Map(); + + const dateRange = dash.dateRange; + if (dateRange && typeof dateRange === 'object') { + const declared = (dateRange as AnyRec).field; + const field = typeof declared === 'string' && declared ? declared : DATE_RANGE_DEFAULT_FIELD; + byName.set(DATE_RANGE_FILTER_NAME, { name: DATE_RANGE_FILTER_NAME, field }); + } + + for (const f of asArray(dash.globalFilters)) { + if (typeof f.field !== 'string' || !f.field) continue; + const name = typeof f.name === 'string' && f.name ? f.name : f.field; + const targetWidgets = Array.isArray(f.targetWidgets) + ? f.targetWidgets.filter((w): w is string => typeof w === 'string') + : undefined; + byName.set(name, { name, field: f.field, targetWidgets }); + } + + return [...byName.values()]; +} + +/** + * Resolve which field of `widget` a filter binds to, or `undefined` when the + * widget is not bound (opted out / not targeted). Precedence mirrors objectui + * `resolveBoundField`: explicit `filterBindings` entry (string re-targets, + * `false` opts out — both win) → legacy `targetWidgets` allow-list → the + * filter's own default `field`. `explicit` distinguishes an author-chosen field + * (a typo they must fix) from the inherited default (which they may opt out of). + */ +function effectiveFilterField( + widget: AnyRec, + def: DashFilterDef, +): { field: string; explicit: boolean } | undefined { + const bindings = widget.filterBindings; + const binding = bindings && typeof bindings === 'object' + ? (bindings as AnyRec)[def.name] + : undefined; + if (binding === false) return undefined; + if (typeof binding === 'string' && binding) return { field: binding, explicit: true }; + if (def.targetWidgets && def.targetWidgets.length > 0) { + const id = typeof widget.id === 'string' ? widget.id : undefined; + if (!id || !def.targetWidgets.includes(id)) return undefined; + } + return { field: def.field, explicit: false }; +} + /** * Validate every dashboard widget's dataset binding. Returns the list of * findings (empty = clean). Caller decides how to surface them: `error` @@ -253,6 +355,9 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { const dash = dashboards[i]; const dashName = typeof dash.name === 'string' ? dash.name : `(dashboard ${i})`; const widgets = Array.isArray(dash.widgets) ? (dash.widgets as AnyRec[]) : []; + // Dashboard-level filters (`dateRange` + `globalFilters`) are broadcast into + // every widget's query (#2501) — resolved once here, checked per widget below. + const dashFilterDefs = dashboardFilterDefs(dash); for (let j = 0; j < widgets.length; j++) { const w = widgets[j]; @@ -331,6 +436,49 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { // Without a resolved dataset there is nothing to check names against. if (!dataset) continue; + // ── (a1) dashboard filter fields exist on the widget's object (#3365) ── + // Each dashboard-level filter is ANDed into this widget's analytics query + // (#2501); a filter whose EFFECTIVE field (after `filterBindings`) is not a + // column on the bound dataset object emits SQL like `WHERE close_date …` + // against a table without that column and the widget crashes at query time. + // Errored (not warned): a broken query, not advice. The opt-out is the + // author's own `filterBindings: { : false }`, so no suppression needed. + if (dashFilterDefs.length > 0) { + const datasetObject = typeof dataset.object === 'string' ? dataset.object : undefined; + // Only judge when the bound object's fields are known in THIS stack; an + // object from another installed package is unknowable here — skip rather + // than false-positive (mirrors the measure-aggregate check above). + const objectFields = datasetObject ? objectFieldTypes.get(datasetObject) : undefined; + if (objectFields) { + for (const def of dashFilterDefs) { + const eff = effectiveFilterField(w, def); + if (!eff) continue; // opted out / not targeted → filter never applies + const field = eff.field; + // A relationship path (`account.region`) is resolved by the query + // engine, not a base column, so it can't be checked here — skip it. + if (field.includes('.')) continue; + if (objectFields.has(field) || SYSTEM_FIELDS.has(field)) continue; + push({ + severity: 'error', + rule: DASHBOARD_FILTER_FIELD_UNKNOWN, + message: eff.explicit + ? `binds dashboard filter \`${def.name}\` to field \`${field}\` ` + + `(via filterBindings), but object \`${datasetObject}\` (dataset "${dsName}") ` + + `has no field \`${field}\`.` + : `inherits dashboard filter \`${def.name}(${field})\`, but object ` + + `\`${datasetObject}\` (dataset "${dsName}") has no field \`${field}\`.`, + hint: eff.explicit + ? `Point filterBindings: { ${def.name}: '' } at a field that exists on ` + + `\`${datasetObject}\`, or opt out with filterBindings: { ${def.name}: false }.` + + `${suggest(field, objectFields.keys())} Object fields: ${list(objectFields.keys())}.` + : `Set filterBindings: { ${def.name}: false } on this widget to opt out, or ` + + `re-target to an existing field with filterBindings: { ${def.name}: '' }.` + + `${suggest(field, objectFields.keys())} Object fields: ${list(objectFields.keys())}.`, + }); + } + } + } + const dimensionNames = new Set(); for (const d of asArray(dataset.dimensions)) { if (typeof d.name === 'string') dimensionNames.add(d.name);