Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/dashboard-filter-field-validation.md
Original file line number Diff line number Diff line change
@@ -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: { <name>: 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.
10 changes: 10 additions & 0 deletions content/docs/getting-started/validating-metadata.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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: { <name>: 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:
Expand Down
7 changes: 5 additions & 2 deletions packages/lint/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
1 change: 1 addition & 0 deletions packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
133 changes: 133 additions & 0 deletions packages/lint/src/validate-widget-bindings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<typeof validateWidgetBindings>) =>
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<string, unknown> = {}, widget: Record<string, unknown> = {}) {
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);
});
});
148 changes: 148 additions & 0 deletions packages/lint/src/validate-widget-bindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: { <name>: 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:
*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -189,6 +200,97 @@ function list(names: Iterable<string>): 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<string>([
'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<string, DashFilterDef>();

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`
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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: { <name>: 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}: '<field>' } 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}: '<field>' }.` +
`${suggest(field, objectFields.keys())} Object fields: ${list(objectFields.keys())}.`,
});
}
}
}

const dimensionNames = new Set<string>();
for (const d of asArray(dataset.dimensions)) {
if (typeof d.name === 'string') dimensionNames.add(d.name);
Expand Down