diff --git a/.changeset/29212-dashboard-duplicate-filter-stored-listing.md b/.changeset/29212-dashboard-duplicate-filter-stored-listing.md new file mode 100644 index 00000000000..ac5c8556b80 --- /dev/null +++ b/.changeset/29212-dashboard-duplicate-filter-stored-listing.md @@ -0,0 +1,12 @@ +--- +'@objectstack/spec': patch +--- + +fix(spec): `os migrate meta --stored` lists a stored dashboard that declares one global filter key twice + +Clause-②: no +Metadata-protocol: none + +- **Before.** `DashboardSchema` refuses a dashboard whose `globalFilters` repeats a filter key (its `name`, or its `field` when it has no `name`), and the dashboard save door answers 422 `INVALID_METADATA`. A dashboard already stored in that shape still loads with `_diagnostics` reading `valid: false`, but the v18 stored-row preview had no detector for it, so `os migrate meta --stored` named no such row before the upgrade. +- **Now.** The preview lists each stored dashboard that repeats a key, under the TODO id `ui-dashboard-global-filter-duplicate-name-refused` (the refusal's upgrade guide entry), at the second declaration's `name` (or `field`) — the site the parse refuses — naming the key and every index. The detector calls the refinement's own predicate, so a row is listed exactly when the parse refuses it. Nothing rewrites it: which definition keeps the key only the author knows. +- **Not changed.** What any door accepts or refuses. Distinct keys, and distinct names over one `field`, are not listed. diff --git a/packages/metadata-protocol/src/protocol.dashboard-duplicate-global-filter-name-refusal.test.ts b/packages/metadata-protocol/src/protocol.dashboard-duplicate-global-filter-name-refusal.test.ts new file mode 100644 index 00000000000..217d91746b9 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.dashboard-duplicate-global-filter-name-refusal.test.ts @@ -0,0 +1,103 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #29212 — the dashboard save door refuses a dashboard that declares one + * global filter key twice (the #29023 refusal), in every save mode. + * + * The rule lives in the spec parse (`duplicateGlobalFilterNames`, the + * `globalFilters` refinement of `DashboardSchema`), and `saveMetaItem` parses + * with that schema, so `PUT /api/v1/meta/dashboard/X` answers + * `422 INVALID_METADATA` at the second declaration's key and stores nothing, + * with no mode, with `mode: 'publish'` and with `mode: 'draft'` alike. This + * pins that the door keeps reading the refusal; the v18 stored listing + * (`refused-shapes.ts`) names a row already stored in the shape. + */ +import { describe, expect, it } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** The engine surface the repository write path touches (as the container-issue-descent harness). */ +function makeProtocol() { + const rows = new Map>(); + let nextId = 0; + const engine: any = { + async findOne(object: string, query?: EngineFindOneQueryInput) { + assertEngineFindOnePredicate(object, query); return null; }, + async find() { return []; }, + async insert(table: string, data: Record) { + nextId += 1; + if (table === 'sys_metadata') rows.set(`${data.type}|${data.name}`, data); + return { id: `r_${nextId}` }; + }, + async update(_t: string, data: Record, opts?: Record) { + assertEngineUpdateDispatch(data, opts); + return { id: null }; + }, + async delete(_t: string, opts?: Record) { + assertEngineDeleteDispatch(opts); + return { deleted: 0 }; + }, + registry: { registerItem: () => {}, registerObject: () => {} }, + }; + const protocol: any = new ObjectStackProtocolImplementation(engine, () => new Map()); + return { protocol, rows }; +} + +async function rejection(promise: Promise): Promise { + try { + await promise; + } catch (err) { + return err; + } + throw new Error('expected the save to be rejected, but it resolved'); +} + +const MODES = [ + ['no mode', {}], + ['publish', { mode: 'publish' }], + ['draft', { mode: 'draft' }], +] as const; + +const boardWith = (globalFilters: unknown[]) => ({ + name: 'dup_board', + label: 'Dup Board', + widgets: [], + globalFilters, +}); + +describe('#29212 the save door refuses a dashboard that declares one global filter key twice', () => { + for (const [label, mode] of MODES) { + it(`${label}: two filters named \`r\` answer 422 INVALID_METADATA at globalFilters.1.name, and nothing is stored`, async () => { + const { protocol, rows } = makeProtocol(); + + const err = await rejection(protocol.saveMetaItem({ + type: 'dashboard', name: 'dup_board', ...mode, item: boardWith([ + { name: 'r', field: 'region', type: 'text' }, + { name: 'r', field: 'status', type: 'text' }, + ]), + })); + + expect(err.code).toBe('INVALID_METADATA'); + expect(err.status).toBe(422); + expect(err.issues).toEqual([expect.objectContaining({ path: 'globalFilters.1.name', code: 'custom' })]); + expect(err.issues[0].message.startsWith( + "This dashboard declares the global filter 'r' twice in globalFilters: globalFilters[0] and globalFilters[1].", + )).toBe(true); + expect(rows.size).toBe(0); + }); + + it(`${label}: control, the same dashboard with distinct filter names saves`, async () => { + const { protocol, rows } = makeProtocol(); + + const result = await protocol.saveMetaItem({ + type: 'dashboard', name: 'dup_board', ...mode, item: boardWith([ + { name: 'r', field: 'region', type: 'text' }, + { name: 's', field: 'status', type: 'text' }, + ]), + }); + + expect(result.success).toBe(true); + expect(rows.size).toBe(1); + }); + } +}); diff --git a/packages/spec/src/conversions/refused-shapes.test.ts b/packages/spec/src/conversions/refused-shapes.test.ts index eafcd9766b5..e7aede152db 100644 --- a/packages/spec/src/conversions/refused-shapes.test.ts +++ b/packages/spec/src/conversions/refused-shapes.test.ts @@ -7,6 +7,7 @@ import { flowNodeValueTemplateRefusals } from '../automation/flow-value-slot-tem import { ObjectSchema } from '../data/object.zod.js'; import { MIGRATIONS_BY_MAJOR } from '../migrations/registry.js'; import { ComponentPropsMap } from '../ui/component.zod.js'; +import { DashboardSchema } from '../ui/dashboard.zod.js'; import { PageSchema } from '../ui/page.zod.js'; import { ViewDataSchema } from '../ui/view.zod.js'; import { applyConversions, applyConversionsToFlow } from './apply.js'; @@ -71,6 +72,7 @@ describe('the refused-shape list — registered against the migration chain', () 'object-external-binding-managed-datasource-refused', 'visibility-predicate-has-argument-refused', 'object-duplicate-action-name-refused', + 'ui-dashboard-global-filter-duplicate-name-refused', ]); }); @@ -792,3 +794,62 @@ describe('#26725 — a stored object that declares one action name twice is list expect(dupActionIssue(row)).toBeUndefined(); }); }); + +// ── #29212 (the #29023 refusal) ─────────────────────────────────────────── + +const dupBoard = (globalFilters: unknown[]) => ({ name: 'crm_kpi', label: 'KPI', widgets: [], globalFilters }); +/** The dashboard parse's duplicate-key issue, or undefined. */ +const dupFilterIssue = (row: unknown) => { + const parsed = DashboardSchema.safeParse(row); + return parsed.success + ? undefined + : parsed.error.issues.find((i) => i.path[0] === 'globalFilters' && /declares the global filter/.test(i.message)); +}; + +describe('#29212 — a stored dashboard that declares one global filter key twice is listed by dashboard', () => { + it('one TODO at the second declaration\'s name, naming the key and both indexes; the row is not rewritten', () => { + const row = dupBoard([ + { name: 'r', field: 'region', type: 'text' }, + { name: 'r', field: 'status', type: 'text' }, + ]); + const { item, todos } = storedTodos('dashboard', row); + expect(item).toEqual(row); + expect(todos).toHaveLength(1); + const todo = todos[0]!; + expect(todo.conversionId).toBe('ui-dashboard-global-filter-duplicate-name-refused'); + expect(todo.path).toBe('dashboards[0].globalFilters[1].name'); + expect(todo.reason).toMatch(/^No lossless rewrite: which definition keeps the key/); + expect(todo.reason).toContain( + "This dashboard declares the global filter 'r' twice in globalFilters: globalFilters[0] and globalFilters[1].", + ); + expect(todo.reason).not.toMatch(/#\d{3,}/); + expect(JSON.parse(todo.from)).toEqual({ name: 'r', indexes: [0, 1] }); + // The listing and the parse refuse ONE set: the stored row still fails the parse, at the same site. + expect(dupFilterIssue(item)?.path).toEqual(['globalFilters', 1, 'name']); + }); + + it('a key that falls back to `field` is listed at that field, as the parse locates it', () => { + const row = dupBoard([ + { field: 'region', type: 'text' }, + { field: 'region', type: 'select', options: [{ value: 'north', label: 'North' }] }, + { name: 'region', field: 'sales_region', type: 'text' }, + ]); + const { todos } = storedTodos('dashboard', row); + expect(todos).toHaveLength(1); + expect(todos[0]!.path).toBe('dashboards[0].globalFilters[1].field'); + expect(JSON.parse(todos[0]!.from)).toEqual({ name: 'region', indexes: [0, 1, 2] }); + expect(todos[0]!.reason).toContain("the global filter 'region' 3 times"); + expect(dupFilterIssue(row)?.path).toEqual(['globalFilters', 1, 'field']); + }); + + it('distinct keys, and distinct names over one field, are not listed — and the parse accepts them', () => { + for (const filters of [ + [{ name: 'r', field: 'region', type: 'text' }, { name: 's', field: 'status', type: 'text' }], + [{ name: 'region_a', field: 'region', type: 'text' }, { name: 'region_b', field: 'region', type: 'text' }], + ]) { + const row = dupBoard(filters); + expect(storedTodos('dashboard', row).todos).toEqual([]); + expect(DashboardSchema.safeParse(row).success).toBe(true); + } + }); +}); diff --git a/packages/spec/src/conversions/refused-shapes.ts b/packages/spec/src/conversions/refused-shapes.ts index 88279418c28..56057f0aa78 100644 --- a/packages/spec/src/conversions/refused-shapes.ts +++ b/packages/spec/src/conversions/refused-shapes.ts @@ -43,7 +43,9 @@ * `external` binding judge * the object refinement and `defineStack` share * ({@link externalBindingDatasourceRefusal}), and the one-name-per-action - * judge of the object refinement ({@link duplicateActionNames}). A site is listed here exactly + * judge of the object refinement ({@link duplicateActionNames}), and the + * one-key-per-global-filter judge of the dashboard refinement + * ({@link duplicateGlobalFilterNames}). A site is listed here exactly * when that door refuses it. One door lives outside this package: the * page/view `visibleWhen` rule in `@objectstack/lint` judges a `has()` * argument on the engine's AST, which this package cannot parse, so its @@ -68,6 +70,7 @@ import { import { duplicateActionNames } from '../data/duplicate-action-names.js'; import { externalBindingDatasourceRefusal } from '../data/external-binding-datasource.js'; import { ComponentPropsMap } from '../ui/component.zod.js'; +import { duplicateGlobalFilterNames } from '../ui/duplicate-global-filter-names.js'; import { ViewDataSchema } from '../ui/view.zod.js'; import { refusedHasArguments } from './cel-has-argument.js'; import type { ConversionTodoDetail } from './types.js'; @@ -555,6 +558,32 @@ const objectDuplicateActionName: RefusedStoredShape = { }, }; +// ── #29212: a dashboard that declares one global filter key twice (the #29023 refusal) ── + +const dashboardDuplicateGlobalFilterName: RefusedStoredShape = { + id: 'ui-dashboard-global-filter-duplicate-name-refused', + toMajor: 18, + surface: 'dashboard.globalFilters — two or more entries of one dashboard under the same filter key ' + + '(the entry\'s `name`, or its `field` when it has no `name`)', + detect(stack, report) { + mapCollection(stack, 'dashboards', (dashboard, path) => { + for (const duplicate of duplicateGlobalFilterNames(dashboard.globalFilters)) { + report({ + // The site the dashboard parse refuses: the second declaration's `name`, or its `field`. + path: `${path}.globalFilters[${duplicate.indexes[1]}].${duplicate.secondKey}`, + from: JSON.stringify({ name: duplicate.name, indexes: duplicate.indexes }), + reason: 'No lossless rewrite: which definition keeps the key, and what the others are renamed to ' + + '(with every widget `filterBindings` key that means a renamed one), only the author knows. Left as ' + + 'stored, the dashboard still loads with `_diagnostics` reading `valid: false`, and its next save, ' + + 'draft or publish, is refused with 422 INVALID_METADATA. ' + + duplicate.message, + }); + } + return dashboard; + }); + }, +}; + /** * Every refused stored shape, in report order. A new entry is a reviewed edit: * `refused-shapes.test.ts` pins each one. @@ -567,6 +596,7 @@ export const REFUSED_STORED_SHAPES: readonly RefusedStoredShape[] = [ externalBindingOnManagedDatasource, visibilityHasArgument, objectDuplicateActionName, + dashboardDuplicateGlobalFilterName, ]; /** diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index b9cbba70fad..cc386a9671f 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -636,6 +636,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol.dashboard-duplicate-global-filter-name-refusal.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.dashboard-duplicate-global-filter-name-refusal.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.dashboard-duplicate-global-filter-name-refusal.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol.data-door-internal-field-delegation.test.ts", "verb": "findOne",