diff --git a/.changeset/dashboard-strict.md b/.changeset/dashboard-strict.md new file mode 100644 index 0000000000..47dca5dab2 --- /dev/null +++ b/.changeset/dashboard-strict.md @@ -0,0 +1,25 @@ +--- +'@objectstack/spec': minor +--- + +The dashboard's header, filter bar and root reject unknown keys — the posture its widget was rescued from three releases ago. + +`DashboardWidgetSchema` has been `.strict()` since the ADR-0021 cutover, and its error map states the reason in its own words: undeclared keys "were dropped silently before strict validation, shipping inert metadata". Everything *around* the widget kept the posture the widget was rescued from — the header, the header actions, the global filters and their option sources, the date range, and the dashboard root itself. + +Closed with `strictObject`, so each now names its surface, echoes the offending key, and suggests the nearest declared one. The aliases are the vocabulary of a dashboard: `charts`/`components`/`cards`/`tiles` → `widgets`, `filters` → `globalFilters`, `refresh`/`autoRefresh`/`pollInterval` → `refreshInterval`, `dateFilter`/`timeRange` → `dateRange`. + +Three wrong-layer keys get a prescription rather than a rename, because a rename would be wrong: + +- **`layout` on the dashboard.** It reads like a template selector; there isn't one. Each *widget* carries `layout: { x, y, w, h }`, and a widget with none is auto-flowed into the grid. +- **`subtitle` on the header.** The header renders the dashboard's own `label`/`description` — there is no separate header copy, only `showTitle`/`showDescription` to toggle them. +- **`filterBindings` on a global filter.** The binding runs the other way: a *widget* maps this filter's `name` to one of its own fields, or `false` to opt out. + +Deliberately left open: `DashboardWidgetOptionsSchema` stays `passthrough`. It is the renderer-extras escape hatch by design — presentation settings the renderer understands are none of the spec's business — and the four keys in it that *do* reach the analytics query are already declared explicitly (framework#3588). Closing it would break the escape hatch to fix a problem that was already fixed the right way. + +Also unchanged: the widget's bespoke `strictWidgetAnalyticsError`, which carries the pre-ADR-0021 inline-analytics and objectui-internal prescriptions. It works and is tested; converging it onto `strictObject` (which would add "did you mean" suggestions on top of those prescriptions) is a follow-up, not a prerequisite. + +Registered types closed at the top level: **23 of 25**. Still open: `action`, `view` — and they are the last two, so the unknown-key *warning* layer is down to two covered roots. When both close it has nothing left to warn about at a root, which is the campaign finishing rather than the layer breaking; the test says so in place rather than being deleted. + +One thing surfaced while re-pointing a test at `view`: a single unknown key on a view reports **twice**, because `view` is a union (container | ViewItem | overlay) and the walk emits one finding per strip-mode variant the key lands in. Recorded in the test rather than dodged by picking a non-union collection; it becomes moot when `view` closes. + +Authoring impact: a key none of these shapes declares is now rejected instead of silently discarded — it was already being ignored, so no working dashboard changes. diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts index eb21e71afa..4adf1e357c 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.test.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -54,11 +54,17 @@ describe('coverage derivation (#3786 — no third hand-written list)', () => { // `page.regions[0].zzz` all `safeParse` to failure — and the check is worth // repeating on the next one, because a broken walk and a successful // graduation shrink this count identically. - expect(lintables.length).toBeGreaterThanOrEqual(3); + // 3 → 2 when `dashboard` closed; `dashboard.zzz` was confirmed rejected by + // the parse first, same as the batch before it. + expect(lintables.length).toBeGreaterThanOrEqual(2); // `view` matters doubly: it is a UNION (container | ViewItem | overlay), so // its presence pins the union half of the posture logic — a regression that // silently dropped unions would shrink coverage without failing the count. - for (const expected of ['dashboard', 'action', 'view']) { + // When `view` and `action` close, this whole layer has nothing left to warn + // about at a ROOT, which is the campaign finishing rather than the lint + // breaking — at that point assert the empty set deliberately, do not delete + // the test. + for (const expected of ['action', 'view']) { expect(lintableTypes, `expected '${expected}' to be lint-covered`).toContain(expected); } }); @@ -163,20 +169,26 @@ describe('the #4148 behaviours survive the generalization', () => { }); it('reports across collections in one walk, with per-type surfaces', () => { - // `page`/`agent` (batch 6a) and `field` (batch 6b) used to appear here too; - // each closed, so the parse rejects and the lint correctly stays quiet — - // the hand-off this whole layer was built to make. Two collections still - // participate, so this keeps proving per-type surfacing rather than + // `page`/`agent` (6a), `field` (6b) and `dashboard` (6c) used to appear + // here; each closed, so the parse rejects and the lint correctly stays + // quiet — the hand-off this whole layer was built to make. Two collections + // still participate so this keeps proving per-type surfacing rather than // degenerating into a single-collection test: `object` reports a NESTED - // strip site under a CLOSED root (the #4522 behaviour), and `dashboard` - // reports at its root. + // strip site under a CLOSED root (the #4522 behaviour), and `view` — the + // last open root, and the union case — reports at its own. const findings = lintUnknownAuthoringKeys({ objects: [{ name: 'a', label: 'A', actions: [{ name: 'act', zzz: 1 }] }], - dashboards: [{ name: 'd', label: 'D', zzz: 1 }], + views: [{ name: 'v', object: 'a', zzz: 1 }], }); - expect(findings.map((f) => `${f.surface}:${f.path}`).sort()).toEqual([ - 'dashboard:dashboards.d.zzz', + // Deduped deliberately: `view` is a union (container | ViewItem | overlay) + // and the walk emits one finding per strip-mode variant the key lands in, + // so a single authored key reports twice. That is noise an author would + // see, but it is the union walk's behaviour and not this test's subject — + // and it becomes moot when `view` closes. Left recorded rather than papered + // over by picking a non-union collection. + expect([...new Set(findings.map((f) => `${f.surface}:${f.path}`))].sort()).toEqual([ 'object:objects.a.actions.0.zzz', + 'view:views.v.zzz', ]); }); diff --git a/packages/spec/src/kernel/metadata-type-schemas.test.ts b/packages/spec/src/kernel/metadata-type-schemas.test.ts index 84a47c0d50..e5ff7ef7e5 100644 --- a/packages/spec/src/kernel/metadata-type-schemas.test.ts +++ b/packages/spec/src/kernel/metadata-type-schemas.test.ts @@ -218,7 +218,7 @@ describe('registered metadata types', () => { * type fails this suite until the list shrinks, so the list cannot outlive the * debt and start exempting types that no longer need exempting. */ -const STILL_STRIP = new Set(['action', 'dashboard', 'view']); +const STILL_STRIP = new Set(['action', 'view']); /** The registered schema's own top-level posture: `.strict()` sets a `never` catchall. */ function topLevelPosture(schema: unknown, depth = 0): 'strict' | 'strip' | null { @@ -285,7 +285,7 @@ describe('#4001 — registered-type closure is derived, not tallied', () => { it('reports the campaign number so a reader never has to count', () => { const closed = types.filter((t) => !STILL_STRIP.has(t)); expect(closed.length + STILL_STRIP.size).toBe(types.length); - expect(closed.length).toBe(22); + expect(closed.length).toBe(23); expect(types.length).toBe(25); }); }); diff --git a/packages/spec/src/ui/dashboard.zod.ts b/packages/spec/src/ui/dashboard.zod.ts index 72b08440f2..d626cb74ec 100644 --- a/packages/spec/src/ui/dashboard.zod.ts +++ b/packages/spec/src/ui/dashboard.zod.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { ProtectionSchema } from '../shared/protection.zod'; import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; +import { strictObject } from '../shared/strict-object'; import { FilterConditionSchema } from '../data/filter.zod'; import { DateGranularity } from '../data/query.zod'; import { ChartTypeSchema, ChartConfigSchema } from './chart.zod'; @@ -44,11 +45,28 @@ export const WidgetColorVariantSchema = lazySchema(() => z.enum([ */ export const WidgetActionTypeSchema = lazySchema(() => ActionType.describe('Widget action type')); +/** + * Shared history for this file (#4001). + * + * `DashboardWidgetSchema` has been strict since the ADR-0021 cutover, and its + * error map says why in its own words: undeclared keys "were dropped silently + * before strict validation, shipping inert metadata". Everything AROUND the + * widget — the header, the filter bar, the dashboard itself — kept the posture + * the widget was rescued from. + */ +const DASHBOARD_HISTORY = + 'Until #4001 closed this shape these were dropped silently — the dashboard still rendered, ' + + 'without whatever the key was meant to configure.'; + /** * Dashboard Header Action Schema * An action button displayed in the dashboard header area. */ -export const DashboardHeaderActionSchema = lazySchema(() => z.object({ +export const DashboardHeaderActionSchema = lazySchema(() => strictObject({ + surface: 'this dashboard header action', + history: DASHBOARD_HISTORY, + aliases: { title: 'label', text: 'label', name: 'label', url: 'actionUrl', href: 'actionUrl', link: 'actionUrl', target: 'actionUrl', type: 'actionType', kind: 'actionType' }, +}, { /** Action label */ label: I18nLabelSchema.describe('Action button label'), @@ -66,7 +84,16 @@ export const DashboardHeaderActionSchema = lazySchema(() => z.object({ * Dashboard Header Schema * Structured header configuration for the dashboard. */ -export const DashboardHeaderSchema = lazySchema(() => z.object({ +export const DashboardHeaderSchema = lazySchema(() => strictObject({ + surface: 'this dashboard header', + history: DASHBOARD_HISTORY, + aliases: { title: 'showTitle', description: 'showDescription', buttons: 'actions', links: 'actions' }, + guidance: { + // The header shows the DASHBOARD's own label/description — it does not + // carry copy of its own, which is the mistake `title:`/`subtitle:` here is. + subtitle: 'the header renders the dashboard\'s own `label`/`description` — there is no separate header copy; toggle them with `showTitle`/`showDescription`', + }, +}, { /** Whether to show the dashboard title in the header */ showTitle: z.boolean().default(true).describe('Show dashboard title in header'), @@ -339,7 +366,11 @@ export const DashboardWidgetSchema = lazySchema(() => z.object({ * Dynamic options binding for global filters. * Allows dropdown options to be fetched from an object at runtime. */ -export const GlobalFilterOptionsFromSchema = lazySchema(() => z.object({ +export const GlobalFilterOptionsFromSchema = lazySchema(() => strictObject({ + surface: 'this dynamic filter option source', + history: DASHBOARD_HISTORY, + aliases: { objectName: 'object', from: 'object', source: 'object', value: 'valueField', label: 'labelField', text: 'labelField', where: 'filter', criteria: 'filter' }, +}, { /** Source object name to fetch options from */ object: z.string().describe('Source object name'), @@ -357,7 +388,16 @@ export const GlobalFilterOptionsFromSchema = lazySchema(() => z.object({ * Global Filter Schema * Defines a single global filter control for the dashboard filter bar. */ -export const GlobalFilterSchema = lazySchema(() => z.object({ +export const GlobalFilterSchema = lazySchema(() => strictObject({ + surface: 'this global filter', + history: DASHBOARD_HISTORY, + aliases: { key: 'name', variable: 'name', fieldName: 'field', title: 'label', inputType: 'type', control: 'type', choices: 'options', values: 'options', source: 'optionsFrom', dataSource: 'optionsFrom', optionsSource: 'optionsFrom', default: 'defaultValue', widgets: 'targetWidgets', appliesTo: 'targetWidgets' }, + guidance: { + // The binding runs the other way: a widget opts in/out via `filterBindings`. + // `targetWidgets` exists, but only for `scope: 'widget'`. + filterBindings: 'a widget declares its own binding — `filterBindings` lives on the WIDGET and maps this filter\'s `name` to one of that widget\'s fields (or `false` to opt out)', + }, +}, { /** * Stable filter name (framework#2501) — the dashboard-variable key under * which the filter's value is published (readable in widget expressions as @@ -377,7 +417,11 @@ export const GlobalFilterSchema = lazySchema(() => z.object({ type: z.enum(['text', 'select', 'date', 'number', 'lookup']).optional().describe('Filter input type'), /** Static options for select/lookup filters */ - options: z.array(z.object({ + options: z.array(strictObject({ + surface: 'this filter option', + history: DASHBOARD_HISTORY, + aliases: { key: 'value', id: 'value', text: 'label', title: 'label', name: 'label' }, + }, { value: z.union([z.string(), z.number(), z.boolean()]).describe('Option value'), label: I18nLabelSchema, })).optional().describe('Static filter options'), @@ -424,7 +468,25 @@ export const GlobalFilterSchema = lazySchema(() => z.object({ * ] * } */ -export const DashboardSchema = lazySchema(() => z.object({ +export const DashboardSchema = lazySchema(() => strictObject({ + surface: 'this dashboard', + history: DASHBOARD_HISTORY, + aliases: { + title: 'label', displayName: 'label', + charts: 'widgets', components: 'widgets', cards: 'widgets', tiles: 'widgets', + filters: 'globalFilters', globalFilter: 'globalFilters', + grid: 'columns', columnCount: 'columns', + spacing: 'gap', + refresh: 'refreshInterval', autoRefresh: 'refreshInterval', pollInterval: 'refreshInterval', + dateFilter: 'dateRange', timeRange: 'dateRange', + }, + guidance: { + // Widget layout is per-widget; a dashboard-level `layout` reads like a + // template selector, which does not exist here. + layout: 'a dashboard has no layout template — each widget carries its own `layout: { x, y, w, h }`, and a widget with none is auto-flowed into the grid', + theme: 'dashboards do not carry a theme — presentation-only widget settings go under that widget\'s `options`', + }, +}, { /** Machine name */ name: SnakeCaseIdentifierSchema.describe('Dashboard unique name'), @@ -450,7 +512,11 @@ export const DashboardSchema = lazySchema(() => z.object({ refreshInterval: z.number().optional().describe('Auto-refresh interval in seconds'), /** Dashboard Date Range (Global time filter) */ - dateRange: z.object({ + dateRange: strictObject({ + surface: 'this dashboard date range', + history: DASHBOARD_HISTORY, + aliases: { dateField: 'field', fieldName: 'field', preset: 'defaultRange', range: 'defaultRange', default: 'defaultRange', allowCustom: 'allowCustomRange', custom: 'allowCustomRange' }, + }, { field: z.string().optional().describe('Default date field name for time-based filtering'), defaultRange: z.enum(['today', 'yesterday', 'this_week', 'last_week', 'this_month', 'last_month', 'this_quarter', 'last_quarter', 'this_year', 'last_year', 'last_7_days', 'last_30_days', 'last_90_days', 'custom']).default('this_month').describe('Default date range preset'), allowCustomRange: z.boolean().default(true).describe('Allow users to pick a custom date range'),