diff --git a/.changeset/dashboard-filter-dotted-seam.md b/.changeset/dashboard-filter-dotted-seam.md new file mode 100644 index 0000000000..6346322743 --- /dev/null +++ b/.changeset/dashboard-filter-dotted-seam.md @@ -0,0 +1,41 @@ +--- +'@objectstack/lint': patch +--- + +lint: `dashboard-filter-field-unknown` resolves dotted dashboard-filter fields on the object graph, and answers system columns per object + +A dashboard-level filter (`dateRange`, or a `globalFilters[]` entry) is ANDed into +**every** widget's analytics query, so its effective field — after any +`filterBindings` re-target — has to resolve on each bound widget's dataset object. +The rule that enforces that shipped with two holes, and this closes both by +migrating the check onto the shared `resolveFieldPath` / `joinablePrefixes` seam +the widget's own `filter` keys already use one position over. + +- **Dotted paths are no longer skipped.** The branch carried + `if (field.includes('.')) continue;`, accurate when nothing in the package could + walk relationship hops and false since the object-graph seam landed. A filter + re-targeted to `account.signed_at` was unjudged whether or not `account` existed, + whether or not `signed_at` existed on it, and whether or not `account` was + declared in the dataset's `include`. It is now walked hop by hop, and a miss + names **which** hop failed. +- **System columns are resolved per object, not through the flat union.** The old + test was `objectFields.has(field) || SYSTEM_FIELDS.has(field)`, which answers + "could this be a system column *anywhere*". On an `ownership: 'none'` object the + platform injects no `owner_id`, and on `systemFields: { audit: false }` no + `created_at` — both were answered as resolvable and are now reported. + +New error id **`dashboard-filter-field-not-included`**: the effective field +resolves, but its relationship prefix is not declared in the bound dataset's +`include`, so ADR-0021 compiles no join and the column is out of the broadcast +query's reach. It mirrors `widget-filter-field-not-included` one level down, and is +its own id because the fix is a different edit (declare the join, versus point the +filter at something real). + +This **narrows the accept set of a shipped gating rule**. Both new answers are +error-tier, so like the rule's other errors they fail `os validate` / `os build` +and the runtime publish gate for `dashboard` writes. Measured over the shipped +dashboard corpus — the three example apps plus the platform's own +`system_overview` — the change is 0 findings before and 0 after; the +`dashboard-filter-field-unprovisioned` warning is unchanged and now travels with +the verdict, so it answers a dotted path landing on an ADR-0015 `external` object +too. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index d1cb4e62a3..9f89bdcc55 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -24,6 +24,10 @@ export { WIDGET_LEGACY_ANALYTICS_UNRENDERABLE, DASHBOARD_FILTER_FIELD_UNKNOWN, DASHBOARD_FILTER_FIELD_UNPROVISIONED, + // [#14275] The `include` clause on a DASHBOARD-level filter's effective + // field, now that it is resolved on the object graph rather than skipped + // whenever it was dotted. + DASHBOARD_FILTER_FIELD_NOT_INCLUDED, // [#14148] The widget's OWN two references, at the same site: the keys of its // presentation-scope `filter` (resolved on the #14105 object-graph seam, with // the ADR-0021 `include` clause its `runtimeFilter` really is subject to) and diff --git a/packages/lint/src/system-fields-consumers.test.ts b/packages/lint/src/system-fields-consumers.test.ts index bb1e7cb8a8..7fb37c2ea6 100644 --- a/packages/lint/src/system-fields-consumers.test.ts +++ b/packages/lint/src/system-fields-consumers.test.ts @@ -451,12 +451,15 @@ const LEDGER: Record = { 'prevent. The blank-column consequence belongs to the surface that RENDERS the anchor ' + '(validate-page-field-bindings, #8340), not to the bundle that names it.', }, - 'validate-widget-bindings.ts': { - kind: 'rule', - reach: ['direct'], - asksProvenance: true, - why: 'Blanket .has read site. Provenance wired by #8340.', - }, + // [#14275] `validate-widget-bindings.ts` LEFT the population. Its one blanket + // `.has` read site was the `dashboard-filter-field-unknown` branch, which now + // resolves through `resolveFieldPath` and therefore through the PER-OBJECT + // `injectedColumnsFor` instead — the distinction this ledger's own header + // calls "the two differ exactly where it matters". Its #8116 obligation did + // not lapse with the row: the rule still asks the provenance question, now + // gated by the verdict's `injected` marker. Recorded here rather than by a + // silent deletion because a shrinking population is the one direction this + // census cannot distinguish from a broken analyzer. }; // ── Fixtures: a module set whose right answer is known ──────────────────── diff --git a/packages/lint/src/validate-widget-bindings.test.ts b/packages/lint/src/validate-widget-bindings.test.ts index 13411a57fb..71d9d45084 100644 --- a/packages/lint/src/validate-widget-bindings.test.ts +++ b/packages/lint/src/validate-widget-bindings.test.ts @@ -13,6 +13,7 @@ import { WIDGET_LEGACY_ANALYTICS_UNRENDERABLE, DASHBOARD_FILTER_FIELD_UNKNOWN, DASHBOARD_FILTER_FIELD_UNPROVISIONED, + DASHBOARD_FILTER_FIELD_NOT_INCLUDED, WIDGET_FILTER_FIELD_UNKNOWN, WIDGET_FILTER_FIELD_NOT_INCLUDED, WIDGET_SORTBY_UNSELECTED, @@ -627,8 +628,20 @@ describe('validateWidgetBindings (dashboard-filter-field-unknown, issue #3365)', })))).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('RESOLVES a relationship-path filter field instead of skipping it (#14275)', () => { + // REWRITTEN, not deleted. This case pinned the branch's + // `if (field.includes('.')) continue;`, whose comment claimed a dotted path + // "can't be checked here". That was accurate when nothing in this package + // could walk hops and became false when `resolveFieldPath` landed + // (#14267 for #14105); the assertion kept passing precisely because the + // rule had stopped asking the question. `account` is not a field on + // `crm_account`, so the head hop is a real miss and is now named as one. + const findings = only(validateWidgetBindings(stack({ dateRange: { field: 'account.region' } }))); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].message).toContain('account.region'); + expect(findings[0].message).toContain('traverses "account"'); + expect(findings[0].message).toContain('crm_account'); }); it('cannot judge — and never false-positives — when the object is not in the stack', () => { @@ -765,6 +778,304 @@ describe('validateWidgetBindings (dashboard-filter-field-unprovisioned, issue #8 delete (s as { objects?: unknown }).objects; expect(only(validateWidgetBindings(s))).toHaveLength(0); }); + + it('[#14275] the anchor is looked up on the object the LEAF landed on, not the base', () => { + // The #8340 question now rides the migrated branch, so it travels with the + // verdict: a DOTTED filter path ending on an ADR-0015 external object is + // answered, which the pre-#14275 branch could not do at all (it skipped + // every dotted field before reaching the provenance test). + const federated = { + objects: [ + { + name: 'crm_order', + fields: [ + { name: 'total', type: 'number' }, + { name: 'customer', type: 'lookup', reference: 'ext_customer' }, + ], + }, + { + name: 'ext_customer', + external: { remoteName: 'customers' }, + fields: [{ name: 'email', type: 'text' }], + }, + ], + datasets: [{ + name: 'order_metrics', object: 'crm_order', include: ['customer'], + measures: [{ name: 'order_count', aggregate: 'count' }], + }], + dashboards: [{ + name: 'orders', label: 'Orders', + globalFilters: [{ field: 'customer.owner_id', type: 'select' }], + widgets: [{ id: 'total_orders', type: 'metric', dataset: 'order_metrics', values: ['order_count'] }], + }], + }; + const findings = only(validateWidgetBindings(federated)); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + // The cause names ext_customer — the LEAF's object — not crm_order. + expect(findings[0].message).toContain('ext_customer'); + expect(findings[0].message).toContain('external object (ADR-0015)'); + expect(unknownOnly(validateWidgetBindings(federated))).toHaveLength(0); + }); +}); + +// ── [#14275] The DASHBOARD-level filter, on the object-graph seam ──────────── + +/** + * The card's repro shape: a dashboard `dateRange` re-targeted onto a + * relationship path (`account.signed_at`), broadcast into a widget whose + * dataset joins `account`. Every knob the two gaps turn is exposed: + * `dash`/`widget` for the binding, `datasetOverrides` for the `include` clause, + * `objectOverrides` for the per-object injected set. + */ +function dashboardDottedStack( + dash: Record = {}, + widget: Record = {}, + datasetOverrides: Record = {}, + objectOverrides: Record = {}, +) { + return { + objects: [ + { + name: 'crm_deal', + fields: [ + { name: 'stage', type: 'select' }, + { name: 'amount', type: 'number' }, + { name: 'account', type: 'lookup', reference: 'crm_account' }, + ], + ...objectOverrides, + }, + { + name: 'crm_account', + fields: [ + { name: 'name', type: 'text' }, + { name: 'signed_at', type: 'date' }, + ], + }, + ], + datasets: [{ + name: 'deal_metrics', + object: 'crm_deal', + include: ['account'], + dimensions: [{ name: 'stage', field: 'stage' }], + measures: [{ name: 'deal_count', aggregate: 'count' }], + ...datasetOverrides, + }], + dashboards: [{ + name: 'pipeline_health', + label: 'Pipeline', + dateRange: { field: 'account.signed_at', defaultRange: 'this_quarter' }, + widgets: [{ + id: 'open_deals', type: 'metric', + dataset: 'deal_metrics', values: ['deal_count'], + ...widget, + }], + ...dash, + }], + }; +} + +describe('dashboard-filter-field-unknown — gap 1: dotted paths are resolved (#14275)', () => { + const unknown = (fs: ReturnType) => + fs.filter((f) => f.rule === DASHBOARD_FILTER_FIELD_UNKNOWN); + + it('is silent on the clean shape — a dotted path through a declared include', () => { + expect(validateWidgetBindings(dashboardDottedStack())).toEqual([]); + }); + + it('errors when a HOP names nothing on the bound object', () => { + const findings = unknown(validateWidgetBindings( + dashboardDottedStack({ dateRange: { field: 'accont.signed_at' } }), + )); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.severity).toBe('error'); + // Names the dashboard, the widget, the filter, the path and WHICH hop failed. + expect(f.where).toBe('dashboard "pipeline_health" › widget "open_deals"'); + expect(f.message).toContain('dateRange'); + expect(f.message).toContain('traverses "accont"'); + expect(f.message).toContain('crm_deal'); + expect(f.message).toContain('Did you mean "account"?'); + expect(f.path).toBe('dashboards[0].widgets[0]'); + }); + + it('errors when the LEAF names nothing on the object the hop landed on', () => { + const findings = unknown(validateWidgetBindings( + dashboardDottedStack({ dateRange: { field: 'account.signd_at' } }), + )); + expect(findings).toHaveLength(1); + // The verdict travelled: the miss is reported against crm_ACCOUNT, the + // object the leaf lives on, not against the dataset's base object. + expect(findings[0].message).toContain('crm_account'); + expect(findings[0].hint).toContain('signed_at'); + }); + + it('errors when a hop is not a relationship at all', () => { + const findings = unknown(validateWidgetBindings( + dashboardDottedStack({ dateRange: { field: 'amount.total' } }), + )); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('not a relationship'); + }); + + it('carries the explicit wording when filterBindings re-targets onto a bad path', () => { + const findings = unknown(validateWidgetBindings(dashboardDottedStack( + {}, + { filterBindings: { dateRange: 'accont.signed_at' } }, + ))); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('via filterBindings'); + expect(findings[0].message).toContain('accont.signed_at'); + }); + + it('is silent when the widget opts a bad dotted filter out entirely', () => { + expect(validateWidgetBindings(dashboardDottedStack( + { dateRange: { field: 'accont.signed_at' } }, + { filterBindings: { dateRange: false } }, + ))).toEqual([]); + }); + + it('takes the injected-hop skip — a path THROUGH a registry column is unknowable', () => { + // `owner_id` is injected and IS a lookup at the registry, but its target is + // registry-owned and invisible here. Reporting it would be the false + // positive skip 3 exists to prevent (ADR-0072 D1). + expect(validateWidgetBindings( + dashboardDottedStack({ dateRange: { field: 'owner_id.name' } }), + )).toEqual([]); + }); + + it('takes skip 2 — an object with no readable field map is never reported', () => { + const s = dashboardDottedStack({ dateRange: { field: 'accont.signed_at' } }); + delete (s.objects[0] as { fields?: unknown }).fields; + expect(unknown(validateWidgetBindings(s))).toHaveLength(0); + }); +}); + +describe('dashboard-filter-field-not-included — the include clause (#14275)', () => { + const notIncluded = (fs: ReturnType) => + fs.filter((f) => f.rule === DASHBOARD_FILTER_FIELD_NOT_INCLUDED); + + it('errors when a RESOLVABLE dotted path traverses an undeclared relationship', () => { + const findings = notIncluded(validateWidgetBindings( + dashboardDottedStack({}, {}, { include: [] }), + )); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.severity).toBe('error'); + expect(f.message).toContain('account'); + expect(f.message).toContain('deal_metrics'); + // The consequence that makes this a p2 rather than the widget-level twin: + // the filter reaches every widget on the board. + expect(f.message).toContain('EVERY bound widget'); + expect(f.hint).toContain('(none)'); + }); + + it('is silent for a bare base column — it needs no join', () => { + expect(notIncluded(validateWidgetBindings( + dashboardDottedStack({ dateRange: { field: 'stage' } }, {}, { include: [] }), + ))).toHaveLength(0); + }); + + it('does not double-report: an unresolvable path yields the existence finding only', () => { + const all = validateWidgetBindings( + dashboardDottedStack({ dateRange: { field: 'account.signd_at' } }, {}, { include: [] }), + ); + expect(all.filter((f) => f.rule === DASHBOARD_FILTER_FIELD_UNKNOWN)).toHaveLength(1); + expect(notIncluded(all)).toHaveLength(0); + }); + + it('honours the implicit-prefix rule — declaring "a.b" includes "a"', () => { + expect(notIncluded(validateWidgetBindings( + dashboardDottedStack({}, {}, { include: ['account.owner'] }), + ))).toHaveLength(0); + }); + + it('is NOT suppressible — it describes a query the analytics service cannot satisfy', () => { + expect(notIncluded(validateWidgetBindings(dashboardDottedStack( + {}, { suppressWarnings: [DASHBOARD_FILTER_FIELD_NOT_INCLUDED] }, { include: [] }, + )))).toHaveLength(1); + }); +}); + +describe('dashboard-filter-field-unknown — gap 2: the PER-OBJECT injected set (#14275)', () => { + const unknown = (fs: ReturnType) => + fs.filter((f) => f.rule === DASHBOARD_FILTER_FIELD_UNKNOWN); + + /** A bare `owner_id` dashboard filter, on the object knob that decides it. */ + const ownerFilter = (objectOverrides: Record) => + dashboardDottedStack({ dateRange: undefined, globalFilters: [{ field: 'owner_id', type: 'select' }] }, + {}, {}, objectOverrides); + + it("reports owner_id on an `ownership: 'none'` object — the union answered this resolvable", () => { + // The card's gap 2, stated as its consequence: the platform injects NO + // `owner_id` here, so the broadcast filter emits `WHERE owner_id = …` + // against a table without that column. The object-independent + // `SYSTEM_FIELDS` union said "could be a system column anywhere" and the + // rule stayed silent; `injectedColumnsFor` answers per object. + const findings = unknown(validateWidgetBindings(ownerFilter({ ownership: 'none' }))); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].message).toContain('owner_id'); + expect(findings[0].message).toContain('crm_deal'); + }); + + it('stays silent on the twin where the platform DOES inject it (mutation: drop `ownership`)', () => { + // Breaks the other half of the derivation — without it the test above + // would pass for a rule that simply flags every injected column. + expect(unknown(validateWidgetBindings(ownerFilter({})))).toHaveLength(0); + }); + + it("reports created_at on an object that opts out of the audit family", () => { + // The same gap on the built-in date range's DEFAULT field: a bare + // `dateRange` lands on `created_at`, which `systemFields: { audit: false }` + // withholds. + const s = dashboardDottedStack( + { dateRange: { defaultRange: 'this_month' } }, {}, {}, + { systemFields: { audit: false } }, + ); + const findings = unknown(validateWidgetBindings(s)); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('created_at'); + }); + + it('stays silent on the same bare dateRange where the audit family IS injected', () => { + expect(unknown(validateWidgetBindings( + dashboardDottedStack({ dateRange: { defaultRange: 'this_month' } }), + ))).toHaveLength(0); + }); +}); + +/** + * [#14275] Both new answers gate `validate` AND `build`, pinned end-to-end + * rather than inferred from the registry entry — `validateWidgetBindings` is + * `commands: ALL`, and nothing else in this file would notice if that were + * narrowed later. The dotted existence miss and the include miss are both + * error-tier, so they also join the #7529 runtime publish gate as members of + * the same "this board cannot render" reference-integrity class. + */ +describe('#14275 acceptance — the migrated branch gates `validate` AND `build`', () => { + const missing = dashboardDottedStack({ dateRange: { field: 'accont.signed_at' } }); + const unjoined = dashboardDottedStack({}, {}, { include: [] }); + + for (const command of ['validate', 'build'] as const) { + it(`a dotted dashboard filter that does not resolve fails \`${command}\``, () => { + const { errors } = splitBySeverity(runAuthoringRules(command, { normalized: missing })); + expect(errors.map((f) => f.rule)).toContain(DASHBOARD_FILTER_FIELD_UNKNOWN); + }); + + it(`a resolvable-but-unjoined dotted dashboard filter fails \`${command}\``, () => { + const { errors } = splitBySeverity(runAuthoringRules(command, { normalized: unjoined })); + expect(errors.map((f) => f.rule)).toContain(DASHBOARD_FILTER_FIELD_NOT_INCLUDED); + }); + + it(`the clean shape passes \`${command}\``, () => { + const { errors } = splitBySeverity(runAuthoringRules(command, { normalized: dashboardDottedStack() })); + const mine = errors.filter((f) => [ + DASHBOARD_FILTER_FIELD_UNKNOWN, DASHBOARD_FILTER_FIELD_NOT_INCLUDED, + ].includes(f.rule)); + expect(mine).toEqual([]); + }); + } }); // ── [#14148] The widget's OWN filter keys and options.sortBy ───────────────── diff --git a/packages/lint/src/validate-widget-bindings.ts b/packages/lint/src/validate-widget-bindings.ts index 4c3379d74c..d7cb997e8a 100644 --- a/packages/lint/src/validate-widget-bindings.ts +++ b/packages/lint/src/validate-widget-bindings.ts @@ -13,7 +13,6 @@ import { type ObjectGraph, } from './object-graph.js'; import { - SYSTEM_FIELDS, indexUnprovisionedAnchors, unprovisionedAnchorCause, unprovisionedAnchorHint, @@ -54,13 +53,26 @@ import { * - `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 + * re-target) does not resolve 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. + * Resolution is {@link resolveFieldPath}'s (#14275), so a DOTTED effective + * field (`account.signed_at`) is walked hop by hop rather than skipped, and a + * bare name is judged against the object's OWN injected columns rather than + * the object-independent `SYSTEM_FIELDS` union — see "What #14275 closed" + * below for why each half was a hole rather than a nicety. + * - `dashboard-filter-field-not-included` (#14275) — the effective field + * RESOLVES, but its relationship prefix is not declared in the bound + * dataset's `include`, so ADR-0021 compiles no join for it and the column is + * out of the broadcast query's reach. The `include` half of the same two + * clauses `widget-filter-field-not-included` applies to the widget's own + * `filter`, at the position one level up — a separate id because the fix is a + * different edit (declare the join, versus point the filter somewhere real) + * and because the family keeps one id per class. * * Advisory rules — severity `warning`, build stays green: * @@ -135,14 +147,40 @@ import { * are derived from `include`, and `assertDeclared` refuses an undeclared * relationship path). So a dotted key here is judged on the same two clauses * `validate-dataset-references.ts` applies one level down — existence, then - * joinability — rather than skipped. That decision is deliberate and recorded - * rather than implicit: `dashboard-filter-field-unknown` above SKIPS a dotted - * field (`field.includes('.')`), which was correct when nothing in this package - * could walk hops, and a third silent pass-through would have reproduced the - * very card this rule closes. The runtime is NOT a backstop for it either — - * `assertDeclared` runs over `dimensions` and `measures` only, never over - * `runtimeFilter` — so nothing between the author and the empty tile asks this - * question. + * joinability — rather than skipped. The runtime is NOT a backstop for it + * either — `assertDeclared` runs over `dimensions` and `measures` only, never + * over `runtimeFilter` — so nothing between the author and the empty tile asks + * this question. + * + * ### What #14275 closed, one position up + * + * `dashboard-filter-field-unknown` (a1) shipped with the two limitations the + * paragraph above used to record as deliberate, and #14275 closed both by + * migrating that branch onto this same seam: + * + * 1. it SKIPPED every dotted effective field (`if (field.includes('.')) + * continue;`), which was accurate when nothing in this package could walk + * hops and false once {@link resolveFieldPath} landed. The consequence was + * the sharper one: a dashboard filter is broadcast to EVERY widget on the + * board, so a single unjudged `filterBindings: { dateRange: + * 'account.signed_at' }` degraded the whole dashboard rather than one tile + * — unjudged whether or not `account` existed, whether or not `signed_at` + * existed on it, and whether or not `account` was in the dataset's + * `include`; + * 2. it resolved bare names against the object-independent `SYSTEM_FIELDS` + * union, which answers "could this be a system column ANYWHERE". The two + * differ exactly where it matters: on `ownership: 'none'` the platform + * injects no `owner_id`, so the union answered a real defect as + * resolvable. Both positions now take skip 3 per object, through + * {@link resolveFieldPath}'s use of `injectedColumnsFor`. + * + * The #8340 provenance question rides on the migrated branch unchanged, and is + * now GATED by the verdict's `injected` marker rather than by union membership: + * a leaf that resolved because it is injected is the only leaf whose anchor can + * be unprovisioned (an author-DECLARED column of the same name is the author's, + * #7859), and the marker states that fact where the flat union could not. It + * also travels: the anchor is looked up on the object the LEAF landed on, so a + * dotted filter path ending on an ADR-0015 `external` object is answered too. * * Resolution is {@link resolveFieldPath}'s and its `unknowable` verdicts are * never reported (ADR-0072 D1), so the three skips every field-existence rule @@ -168,6 +206,11 @@ 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'; export const DASHBOARD_FILTER_FIELD_UNPROVISIONED = 'dashboard-filter-field-unprovisioned'; +/** + * [#14275] A dashboard filter's effective field resolves, but its relationship + * prefix is not declared in the bound dataset's `include`. + */ +export const DASHBOARD_FILTER_FIELD_NOT_INCLUDED = 'dashboard-filter-field-not-included'; /** [#14148] A key of the widget's OWN `filter` that resolves to no column. */ export const WIDGET_FILTER_FIELD_UNKNOWN = 'widget-filter-field-unknown'; /** [#14148] A widget filter key whose relationship prefix is not in `include`. */ @@ -310,9 +353,11 @@ 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 - * (the package-shared `SYSTEM_FIELDS`, `system-fields.ts`), so a bare - * `dateRange` never false-positives. + * runtime this check shadows. `created_at` is a registry-injected system field, + * resolved PER OBJECT through `injectedColumnsFor` (#14275), so a bare + * `dateRange` never false-positives on an object that gets the audit family — + * and IS reported on one that opts out of it (`systemFields: { audit: false }`), + * where the broadcast filter really does address a column that is not there. */ const DATE_RANGE_DEFAULT_FIELD = 'created_at'; @@ -568,66 +613,138 @@ export function validateWidgetBindings(stack: AnyRec): WidgetBindingFinding[] { // 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; - // [#8340] The provenance half of the same question, for THIS object. - const anchors = datasetObject ? unprovisionedAnchors.get(datasetObject) : undefined; - if (objectFields && datasetObject) { + // [#14275] Skips 1 and 2, taken once for the whole widget — exactly as + // the (a2) limb below does, off the SAME index. An object this stack + // does not define, or one with no readable field map, is unknowable + // here; resolving against it would turn one unjudgeable base binding + // into a finding per dashboard filter. + const base = datasetObject ? graph.get(datasetObject) : undefined; + if (datasetObject && base) { + // ADR-0021 joins ONLY what `include` declares, and the dashboard + // filter lands in the same `runtimeFilter` slot the widget's own + // filter does — so the second clause is the same clause. + const included = joinablePrefixes(dataset.include); 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)) { - // The name RESOLVES — the existence error above is right to stay - // silent, and `SYSTEM_FIELDS` keeps that decision (#8340 adds a - // question, it does not replace the membership test). But a - // resolvable anchor on a federated object has no column behind - // it, so ANDing it into the widget's analytics query silently - // narrows the result to nothing instead of crashing. Warning, - // not error, on #4330's cost asymmetry: this pass cannot see the - // remote table, only that the PLATFORM provisions no storage. - if (anchors?.has(field)) { + // [#14275] The effective field is resolved on the object graph — a + // dotted `account.signed_at` hop by hop, a bare name against THIS + // object's authored + injected columns. Unjudgeable verdicts are + // never reported (ADR-0072 D1). + const verdict = resolveFieldPath(graph, datasetObject, field); + if (isUnjudgeable(verdict) || !verdict) continue; + + // How this widget came to carry the filter — the half an author + // acts on, and the reason the two message shapes differ: an + // explicit `filterBindings` target is a typo they must fix, an + // inherited default is one they may opt out of. + const provenance = eff.explicit + ? `binds dashboard filter \`${def.name}\` to field \`${field}\` (via filterBindings), but ` + : `inherits dashboard filter \`${def.name}(${field})\`, but `; + + if (verdict.kind !== 'ok') { + // A DOTTED miss is reported through the shared account, which + // names WHICH hop failed — a bare "has no field `a.b`" would send + // the author looking for a column nobody wrote. A bare name keeps + // #3365's shipped wording verbatim: it is the whole existing + // population, and this PR narrows the accept set rather than + // re-wording what already fires. + const account = field.includes('.') + ? describeFieldPathVerdict(verdict, field, 'the effective field') + : undefined; + if (account) { push({ - severity: 'warning', - rule: DASHBOARD_FILTER_FIELD_UNPROVISIONED, + severity: 'error', + rule: DASHBOARD_FILTER_FIELD_UNKNOWN, + // #2501 is the fan-out this sentence describes; the id stays + // in the comment, never in the string an author reads. message: - (eff.explicit - ? `binds dashboard filter \`${def.name}\` to field \`${field}\` (via filterBindings), but ` - : `inherits dashboard filter \`${def.name}(${field})\`, but `) + - `${unprovisionedAnchorCause(datasetObject, field)}. The filter is ANDed ` + - `into this widget's analytics query (#2501), so it can never match a real value — ` + - `on SQLite it silently degrades to constant-false and the widget renders empty ` + - `(HTTP 200, zero rows, no error).`, + `${provenance}${account.message} The filter is ANDed into EVERY bound ` + + `widget's analytics query, so the query addresses a column that ` + + `does not exist.`, hint: - `${unprovisionedAnchorHint(datasetObject, field)} A widget can also opt out ` + - `with filterBindings: { ${def.name}: false }. Suppress with ` + - `suppressWarnings: ['${DASHBOARD_FILTER_FIELD_UNPROVISIONED}'] if the remote schema ` + - `resolves it some other way.`, + `Point filterBindings: { ${def.name}: '' } at a field that resolves on ` + + `\`${datasetObject}\` — or at a \`relationship[.relationship].field\` path whose ` + + `prefix is declared in dataset "${dsName}"'s \`include\` — or opt out with ` + + `filterBindings: { ${def.name}: false }. ${account.detail}`, }); + 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, base.names)} Object fields: ${list(base.names)}.` + : `Set filterBindings: { ${def.name}: false } on this widget to opt out, or ` + + `re-target to an existing field with filterBindings: { ${def.name}: '' }.` + + `${suggest(field, base.names)} Object fields: ${list(base.names)}.`, + }); continue; } + + // [#14275] The field RESOLVES. Second clause: a dotted path is only + // in this query's reach if its relationship prefix is declared — + // `assertDeclared` never sees a `runtimeFilter`, so there is no + // runtime door in front of this one either. + const cut = field.lastIndexOf('.'); + if (cut >= 0) { + const prefix = field.slice(0, cut); + if (!included.has(prefix)) { + push({ + severity: 'error', + rule: DASHBOARD_FILTER_FIELD_NOT_INCLUDED, + // Same #2501 fan-out as above — id in the comment, not the + // message: `#NNNN` resolves for nobody downstream of here. + message: + `${provenance}its relationship prefix "${prefix}" is not declared in dataset ` + + `"${dsName}"'s \`include\` — and ADR-0021 joins ONLY declared paths, so no join ` + + `is compiled and the column is out of this query's reach. The filter is ANDed ` + + `into EVERY bound widget's analytics query, so the whole board renders ` + + `empty, not one tile.`, + hint: + `Add "${prefix}" to dataset "${dsName}"'s include (declaring "a.b" implicitly ` + + `includes "a"), filter on a field of "${datasetObject}" itself, or opt out with ` + + `filterBindings: { ${def.name}: false }. Declared include paths: ` + + `${included.size > 0 ? [...included].sort().join(', ') : '(none)'}.`, + }); + continue; + } + } + + // [#8340] The provenance half, gated by the verdict's `injected` + // marker (#14275): only a leaf that resolved BECAUSE it is injected + // can be an anchor with no storage — an author-declared column of + // the same name is one they vouch for (#7859). Looked up on the + // object the LEAF landed on, so a dotted path ending on an ADR-0015 + // `external` object is answered too. Warning, not error, on #4330's + // cost asymmetry: this pass cannot see the remote table, only that + // the PLATFORM provisions no storage. + if (!verdict.injected) continue; + const leafObject = verdict.object; + const leafField = verdict.field; + if (!unprovisionedAnchors.get(leafObject)?.has(leafField)) 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())}.`, + severity: 'warning', + rule: DASHBOARD_FILTER_FIELD_UNPROVISIONED, + message: + `${provenance}${unprovisionedAnchorCause(leafObject, leafField)}. The filter is ANDed ` + + `into this widget's analytics query (#2501), so it can never match a real value — ` + + `on SQLite it silently degrades to constant-false and the widget renders empty ` + + `(HTTP 200, zero rows, no error).`, + hint: + `${unprovisionedAnchorHint(leafObject, leafField)} A widget can also opt out ` + + `with filterBindings: { ${def.name}: false }. Suppress with ` + + `suppressWarnings: ['${DASHBOARD_FILTER_FIELD_UNPROVISIONED}'] if the remote schema ` + + `resolves it some other way.`, }); } }