diff --git a/.changeset/view-conversions-reach-all-three-spellings.md b/.changeset/view-conversions-reach-all-three-spellings.md new file mode 100644 index 0000000000..d1acd6c691 --- /dev/null +++ b/.changeset/view-conversions-reach-all-three-spellings.md @@ -0,0 +1,13 @@ +--- +'@objectstack/spec': patch +--- + +Every view-family conversion now reaches all three persisted `view` spellings. + +`ViewMetadataSchema` accepts three body shapes and all three land in `sys_metadata` rows — the `defineView` container (`list`/`listViews`/`form`/`formViews`), the standalone ViewItem record (`{ viewKind, config }`), and the flattened runtime overlay (a raw ListView/FormView config at the top level plus its `object` + `viewKind` binding). Every view-family conversion walked only the container keys, so for the other two spellings the whole chain replayed by `applyConversionsToStoredItem('view', row)` was a no-op: a row written under an older protocol kept its historical shape while the conversion layer reported it canonicalized, and the rehydration parse then refused exactly what had never been rewritten. + +A new shared walker (`mapViewPayloads` in `conversions/walk.ts`) discriminates the three spellings using `ViewMetadataSchema`'s own discriminators — `viewKind` plus a `config` object for a record, the container slots for a container, `viewKind` with those slots absent for a flattened overlay — and hands each conversion the list/form payload wherever it lives, labelled with its family. All five view-family conversions adopt it: `view-visibleOn-to-visibleWhen`, `view-inert-keys-removed`, `view-list-passthrough-keys-removed`, `view-export-options-pdf-removed` and `form-view-option-default-removed`. + +The family label is load-bearing rather than informational: these conversions are shape-scoped, and two of them strip a key that is inert on one family and live on the other (`aria` is retired on a form and live on a list, `data` the reverse), so a walk that could not tell the two apart would delete live keys. + +No authoring surface moves and no accept set changes — this is data-at-rest canonicalization catching up to shapes the schema already ruled on. Container behaviour, including every notice path, is unchanged. diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index c7f5f91d10..ac23e348ce 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -22,6 +22,7 @@ import { mapFlowNodes, mapPageComponents, mapPages, + mapViewPayloads, renameConfigKey, renameKey, } from './walk.js'; @@ -428,8 +429,9 @@ function renameVisibilityAlias( * * The conditional-visibility predicate is unified under the canonical * `visibleWhen` across all layers. Applies to form sections and (recursively - * nested) form fields in every `views[].form` / `views[].formViews.*` - * container. **Live window**: the protocol-15 loader accepts the deprecated + * nested) form fields in every FORM payload {@link mapViewPayloads} reaches — + * `views[].form` / `views[].formViews.*`, a ViewItem record's `config`, and a + * flattened form overlay's top level (#13031). **Live window**: the protocol-15 loader accepts the deprecated * key (the zod schemas also normalize it at parse — this entry makes the * acceptance *declared, loud, and expiring* per ADR-0087 D2, and will * graduate into the step-16 chain when the alias is removed). @@ -482,22 +484,9 @@ const viewVisibleOnToVisibleWhen: MetadataConversion = { return dict; }; - return mapCollection(stack, 'views', (view, path) => { - let next = view; - const form = mapForm(next.form, `${path}.form`); - if (form !== next.form) next = { ...next, form }; - const formViews = next.formViews; - if (formViews && typeof formViews === 'object' && !Array.isArray(formViews)) { - let fvChanged = false; - const nextViews: Record = {}; - for (const [name, fv] of Object.entries(formViews as Record)) { - const mapped = mapForm(fv, `${path}.formViews.${name}`); - if (mapped !== fv) fvChanged = true; - nextViews[name] = mapped; - } - if (fvChanged) next = { ...next, formViews: nextViews }; - } - return next; + return mapViewPayloads(stack, (payload, kind, path) => { + if (kind !== 'form') return payload; + return mapForm(payload, path) as Dict; }); }, fixture: { @@ -2218,34 +2207,12 @@ const viewInertKeysRemoved: MetadataConversion = { // NOT 'data': the sweep's removal attempt was refuted by the build — // defineForm writes data.provider='schema' on every metadata form. const FORM_KEYS = ['defaultSort', 'aria'] as const; - return mapCollection(stack, 'views', (view, path) => { - let touched = false; - const next: Record = { ...view }; - const fix = (keys: readonly string[], sub: Record, subPath: string) => { - const cleaned = stripKeys(sub, keys, emit, subPath); - if (cleaned !== sub) { touched = true; return cleaned; } - return sub; - }; - for (const [slot, keys] of [['list', LIST_KEYS], ['form', FORM_KEYS]] as const) { - const v = next[slot]; - if (v && typeof v === 'object' && !Array.isArray(v)) next[slot] = fix(keys, v as Record, `${path}.${slot}`); - } - for (const [slot, keys] of [['listViews', LIST_KEYS], ['formViews', FORM_KEYS]] as const) { - const named = next[slot]; - if (named && typeof named === 'object' && !Array.isArray(named)) { - const rebuilt: Record = { ...(named as Record) }; - let subTouched = false; - for (const [name, v] of Object.entries(rebuilt)) { - if (v && typeof v === 'object' && !Array.isArray(v)) { - const cleaned = stripKeys(v as Record, keys, emit, `${path}.${slot}.${name}`); - if (cleaned !== v) { rebuilt[name] = cleaned; subTouched = true; } - } - } - if (subTouched) { next[slot] = rebuilt; touched = true; } - } - } - return touched ? next : view; - }); + // The per-family key sets are why {@link mapViewPayloads} labels every + // payload: `aria` is retired on a form and LIVE on a list, `data` the + // reverse, so a walk that could not tell the two apart would delete a live + // key on whichever family it guessed wrong. + return mapViewPayloads(stack, (payload, kind, path) => + stripKeys(payload, kind === 'list' ? LIST_KEYS : FORM_KEYS, emit, path)); }, fixture: { before: { @@ -2282,28 +2249,8 @@ const viewListPassthroughKeysRemoved: MetadataConversion = { summary: "view list keys removed (#7176): 'striped'/'bordered'/'virtualScroll' — every measured reader copied the key forward and none applied it (pass-through-only; ADR-0049 enforce-or-remove)", apply(stack, emit) { const LIST_KEYS = ['striped', 'bordered', 'virtualScroll'] as const; - return mapCollection(stack, 'views', (view, path) => { - let touched = false; - const next: Record = { ...view }; - const list = next.list; - if (list && typeof list === 'object' && !Array.isArray(list)) { - const cleaned = stripKeys(list as Record, LIST_KEYS, emit, `${path}.list`); - if (cleaned !== list) { next.list = cleaned; touched = true; } - } - const named = next.listViews; - if (named && typeof named === 'object' && !Array.isArray(named)) { - const rebuilt: Record = { ...(named as Record) }; - let subTouched = false; - for (const [name, lv] of Object.entries(rebuilt)) { - if (lv && typeof lv === 'object' && !Array.isArray(lv)) { - const cleaned = stripKeys(lv as Record, LIST_KEYS, emit, `${path}.listViews.${name}`); - if (cleaned !== lv) { rebuilt[name] = cleaned; subTouched = true; } - } - } - if (subTouched) { next.listViews = rebuilt; touched = true; } - } - return touched ? next : view; - }); + return mapViewPayloads(stack, (payload, kind, path) => + kind === 'list' ? stripKeys(payload, LIST_KEYS, emit, path) : payload); }, fixture: { before: { @@ -2388,23 +2335,8 @@ const viewExportOptionsPdfRemoved: MetadataConversion = { } return slot; }; - return mapCollection(stack, 'views', (view, path) => { - let touched = false; - const next: Record = { ...view }; - const list = stripPdf(next.list, `${path}.list`); - if (list !== next.list) { next.list = list; touched = true; } - const named = next.listViews; - if (named && typeof named === 'object' && !Array.isArray(named)) { - const rebuilt: Record = { ...(named as Record) }; - let subTouched = false; - for (const [name, lv] of Object.entries(rebuilt)) { - const cleaned = stripPdf(lv, `${path}.listViews.${name}`); - if (cleaned !== lv) { rebuilt[name] = cleaned; subTouched = true; } - } - if (subTouched) { next.listViews = rebuilt; touched = true; } - } - return touched ? next : view; - }); + return mapViewPayloads(stack, (payload, kind, path) => + kind === 'list' ? (stripPdf(payload, path) as Dict) : payload); }, fixture: { before: { @@ -8210,8 +8142,9 @@ const permissionAllowRestorePurgeRemoved: MetadataConversion = { * sources, a pure lossless delete (it never had an effect on this surface to * lose). * - * Walks the same containers as `view-visibleOn-to-visibleWhen`: `views[].form` - * and `views[].formViews.*`, through `sections[]`/`groups[]` and top-level + * Walks the same payloads as `view-visibleOn-to-visibleWhen` — every FORM + * payload {@link mapViewPayloads} reaches, in all three persisted spellings + * (#13031) — through `sections[]`/`groups[]` and top-level * `fields[]`, recursing into nested `fields` (composite/repeater/record rows * carry their own option lists). Only the exact key `default` is stripped — * the alias spellings `isDefault`/`selected` were never accepted on this @@ -8281,23 +8214,8 @@ const formViewOptionDefaultRemoved: MetadataConversion = { if (fields !== dict.fields) dict = { ...dict, fields }; return dict; }; - return mapCollection(stack, 'views', (view, path) => { - let next = view; - const form = mapForm(next.form, `${path}.form`); - if (form !== next.form) next = { ...next, form }; - const formViews = next.formViews; - if (isDict(formViews)) { - let fvChanged = false; - const nextViews: Record = {}; - for (const [name, fv] of Object.entries(formViews)) { - const mapped = mapForm(fv, `${path}.formViews.${name}`); - if (mapped !== fv) fvChanged = true; - nextViews[name] = mapped; - } - if (fvChanged) next = { ...next, formViews: nextViews }; - } - return next; - }); + return mapViewPayloads(stack, (payload, kind, path) => + kind === 'form' ? (mapForm(payload, path) as Dict) : payload); }, fixture: { before: { diff --git a/packages/spec/src/conversions/view-spelling-walk.test.ts b/packages/spec/src/conversions/view-spelling-walk.test.ts new file mode 100644 index 0000000000..9e694656ca --- /dev/null +++ b/packages/spec/src/conversions/view-spelling-walk.test.ts @@ -0,0 +1,432 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Every view-family conversion reaches all THREE persisted `view` spellings. + * + * `ViewMetadataSchema` (`ui/view.zod.ts`) accepts three body shapes, and all + * three land in `sys_metadata` rows: the `defineView` **container** + * (`{ list, listViews, form, formViews }`), the standalone **ViewItem record** + * (`{ viewKind, config }`), and the **flattened runtime overlay** (a raw + * ListView/FormView config at the top level plus its `object`/`viewKind` + * binding). `applyConversionsToStoredItem('view', row)` replays the full chain + * over every one of them — data at rest is the "perpetual consumer arriving + * late" (`stored.ts` module doc). + * + * Before the shared walker, every view-family conversion read only the + * container keys, so for the other two spellings the whole chain was a no-op: a + * row persisted under an old protocol kept its historical shape while the + * conversion layer claimed to have canonicalized it, and the rehydration parse + * then refused exactly what was never rewritten. Same structural gap the LINT + * walk closed one layer over (a standalone ViewItem's nested `config.*` judged + * by no list-view field rule). + * + * So the assertion these cases make over and over is the **parity** one: + * whatever the table does to a payload in a container slot, it must do to the + * identical payload under `config`, and to the identical payload flattened at + * the top level. Every conversion is exercised in all three, because the gap + * was family-wide rather than specific to any entry. + */ + +import { describe, expect, it } from 'vitest'; + +import { applyConversionsToStoredItem } from './stored.js'; +import type { ConversionNotice } from './types.js'; + +type Dict = Record; + +/** Replay the full stored chain over one `view` row, collecting its notices. */ +function convertViewRow(row: Dict): { out: Dict; notices: ConversionNotice[] } { + const notices: ConversionNotice[] = []; + const out = applyConversionsToStoredItem('view', row, { onNotice: (n) => notices.push(n) }); + return { out: out as Dict, notices }; +} + +/** Notice paths for one conversion id, in emission order. */ +function pathsFor(notices: ConversionNotice[], conversionId: string): string[] { + return notices.filter((n) => n.conversionId === conversionId).map((n) => n.path); +} + +/** + * The three persisted spellings of ONE list payload, as + * {@link applyConversionsToStoredItem} receives them. + * + * The flattened overlay carries `object` + `viewKind` because #7741 made that + * pair REQUIRED on both inline arms — the exact pair the object-bound read + * paths (`GET /meta/view?object=`, the view switcher) filter on. A fixture + * without them would pin a body no read path could ever serve. + */ +const listSpellings = (payload: Dict) => ({ + container: { object: 'crm_lead', list: { ...payload } }, + containerNamed: { object: 'crm_lead', listViews: { all: { ...payload } } }, + record: { name: 'crm_lead.all', object: 'crm_lead', viewKind: 'list', config: { ...payload } }, + flattened: { name: 'crm_lead.all', object: 'crm_lead', viewKind: 'list', ...payload }, +}); + +/** The three persisted spellings of ONE form payload. */ +const formSpellings = (payload: Dict) => ({ + container: { object: 'crm_lead', form: { ...payload } }, + containerNamed: { object: 'crm_lead', formViews: { quick: { ...payload } } }, + record: { name: 'crm_lead.edit', object: 'crm_lead', viewKind: 'form', config: { ...payload } }, + flattened: { name: 'crm_lead.edit', object: 'crm_lead', viewKind: 'form', ...payload }, +}); + +describe('view conversions reach the ViewItem-record spelling ({ viewKind, config })', () => { + it('renames `visibleOn` inside a record `config` exactly as inside a container `form`', () => { + const payload = { + type: 'simple', + sections: [{ + label: 'Details', + visibleOn: "record.status == 'open'", + fields: ['name', { field: 'priority', visibleOn: "record.priority != ''" }], + }], + }; + const spellings = formSpellings(payload); + + const container = convertViewRow(spellings.container); + const record = convertViewRow(spellings.record); + + // The container leg is the reference behaviour — unchanged by this fix. + const containerSection = (container.out.form as Dict).sections as Dict[]; + expect(containerSection[0]!.visibleWhen).toBe("record.status == 'open'"); + expect('visibleOn' in containerSection[0]!).toBe(false); + + // Parity: the identical payload under `config` converts identically. + const recordSection = (record.out.config as Dict).sections as Dict[]; + expect(recordSection[0]!.visibleWhen).toBe("record.status == 'open'"); + expect('visibleOn' in recordSection[0]!).toBe(false); + expect((recordSection[0]!.fields as Dict[])[1]!.visibleWhen).toBe("record.priority != ''"); + + // The notice names the site the author has to edit — `config`, not `form`. + expect(pathsFor(record.notices, 'view-visibleOn-to-visibleWhen')).toEqual([ + 'views[0].config.sections[0].visibleWhen', + 'views[0].config.sections[0].fields[1].visibleWhen', + ]); + }); + + it('strips the retired `pdf` export format from a record `config`', () => { + const { out, notices } = convertViewRow( + listSpellings({ type: 'grid', columns: ['name'], exportOptions: ['xlsx', 'pdf'] }).record, + ); + expect((out.config as Dict).exportOptions).toEqual(['xlsx']); + expect(pathsFor(notices, 'view-export-options-pdf-removed')).toEqual([ + 'views[0].config.exportOptions', + ]); + }); + + it('strips `pdf` from the OBJECT export spelling under a record `config`', () => { + const { out } = convertViewRow( + listSpellings({ + type: 'grid', + columns: ['name'], + exportOptions: { formats: ['csv', 'pdf'], maxRecords: 100 }, + }).record, + ); + // The surviving sibling keys stay — the strip is the value, not the block. + expect((out.config as Dict).exportOptions).toEqual({ formats: ['csv'], maxRecords: 100 }); + }); + + it('strips the inert LIST keys from a `viewKind: list` record, and only those', () => { + const { out, notices } = convertViewRow( + listSpellings({ + type: 'grid', + columns: ['name'], + responsive: { sm: {} }, + performance: { lazyLoad: true }, + aria: { label: 'Leads' }, // list `aria` is LIVE — must survive + }).record, + ); + const config = out.config as Dict; + expect('responsive' in config).toBe(false); + expect('performance' in config).toBe(false); + expect(config.aria).toEqual({ label: 'Leads' }); + expect(pathsFor(notices, 'view-inert-keys-removed').sort()).toEqual([ + 'views[0].config.performance', + 'views[0].config.responsive', + ]); + }); + + it('strips the inert FORM keys from a `viewKind: form` record, and only those', () => { + const { out } = convertViewRow( + formSpellings({ + type: 'simple', + data: { provider: 'object', object: 'crm_lead' }, // form `data` is LIVE + defaultSort: [{ field: 'created_at', order: 'desc' }], + aria: { label: 'Lead form' }, + }).record, + ); + const config = out.config as Dict; + expect('defaultSort' in config).toBe(false); + expect('aria' in config).toBe(false); + expect(config.data).toEqual({ provider: 'object', object: 'crm_lead' }); + }); + + it('is SHAPE-scoped across the record arms: a form key is not stripped from a list record', () => { + // `aria` is inert on a FORM and live on a LIST. Reading `viewKind` wrong + // (or ignoring it and stripping both key sets) would delete a live key. + const { out } = convertViewRow( + listSpellings({ type: 'grid', columns: ['name'], aria: { label: 'Leads' } }).record, + ); + expect((out.config as Dict).aria).toEqual({ label: 'Leads' }); + }); + + it('strips the pass-through list keys from a record `config`', () => { + const { out, notices } = convertViewRow( + listSpellings({ + type: 'grid', + columns: ['name'], + resizable: true, // live — survives + striped: true, + bordered: true, + virtualScroll: true, + }).record, + ); + const config = out.config as Dict; + expect(config.resizable).toBe(true); + expect(Object.keys(config).sort()).toEqual(['columns', 'resizable', 'type']); + expect(pathsFor(notices, 'view-list-passthrough-keys-removed')).toHaveLength(3); + }); + + it('strips the per-option `default` inside a record `config`, nested rows included', () => { + const { out, notices } = convertViewRow( + formSpellings({ + type: 'simple', + sections: [{ + label: 'Details', + fields: [ + { field: 'status', type: 'select', options: [{ label: 'Open', value: 'open', default: true }] }, + { + field: 'meta', + type: 'composite', + fields: [{ + field: 'priority', + type: 'radio', + options: [{ label: 'High', value: 'high', default: true }], + }], + }, + ], + }], + }).record, + ); + const sections = (out.config as Dict).sections as Dict[]; + const fields = sections[0]!.fields as Dict[]; + expect((fields[0]!.options as Dict[])[0]).toEqual({ label: 'Open', value: 'open' }); + expect(((fields[1]!.fields as Dict[])[0]!.options as Dict[])[0]) + .toEqual({ label: 'High', value: 'high' }); + expect(pathsFor(notices, 'form-view-option-default-removed')).toEqual([ + 'views[0].config.sections[0].fields[0].options[0].default', + 'views[0].config.sections[0].fields[1].fields[0].options[0].default', + ]); + }); + + it('leaves the record identity fields alone while converting its `config`', () => { + const { out } = convertViewRow( + listSpellings({ type: 'grid', columns: ['name'], striped: true }).record, + ); + expect(out.name).toBe('crm_lead.all'); + expect(out.object).toBe('crm_lead'); + expect(out.viewKind).toBe('list'); + }); +}); + +describe('view conversions reach the flattened-overlay spelling (payload at top level)', () => { + it('renames `visibleOn` at the top level of a flattened FORM overlay', () => { + const { out, notices } = convertViewRow( + formSpellings({ + type: 'simple', + sections: [{ label: 'Details', visibleOn: "record.status == 'open'" }], + }).flattened, + ); + const sections = out.sections as Dict[]; + expect(sections[0]!.visibleWhen).toBe("record.status == 'open'"); + expect('visibleOn' in sections[0]!).toBe(false); + expect(pathsFor(notices, 'view-visibleOn-to-visibleWhen')).toEqual([ + 'views[0].sections[0].visibleWhen', + ]); + }); + + it('strips the retired `pdf` export format from a flattened LIST overlay', () => { + const { out, notices } = convertViewRow( + listSpellings({ type: 'grid', columns: ['name'], exportOptions: ['xlsx', 'pdf'] }).flattened, + ); + expect(out.exportOptions).toEqual(['xlsx']); + expect(pathsFor(notices, 'view-export-options-pdf-removed')).toEqual([ + 'views[0].exportOptions', + ]); + }); + + it('strips the inert LIST keys from a flattened LIST overlay', () => { + const { out, notices } = convertViewRow( + listSpellings({ + type: 'grid', + columns: ['name'], + responsive: { sm: {} }, + performance: { lazyLoad: true }, + aria: { label: 'Leads' }, + }).flattened, + ); + expect('responsive' in out).toBe(false); + expect('performance' in out).toBe(false); + expect(out.aria).toEqual({ label: 'Leads' }); + expect(pathsFor(notices, 'view-inert-keys-removed').sort()).toEqual([ + 'views[0].performance', + 'views[0].responsive', + ]); + }); + + it('strips the inert FORM keys from a flattened FORM overlay', () => { + const { out } = convertViewRow( + formSpellings({ + type: 'simple', + data: { provider: 'object', object: 'crm_lead' }, + defaultSort: [{ field: 'created_at', order: 'desc' }], + aria: { label: 'Lead form' }, + }).flattened, + ); + expect('defaultSort' in out).toBe(false); + expect('aria' in out).toBe(false); + expect(out.data).toEqual({ provider: 'object', object: 'crm_lead' }); + }); + + it('strips the pass-through list keys from a flattened LIST overlay', () => { + const { out, notices } = convertViewRow( + listSpellings({ + type: 'grid', + columns: ['name'], + striped: true, + bordered: true, + virtualScroll: true, + }).flattened, + ); + expect('striped' in out).toBe(false); + expect('bordered' in out).toBe(false); + expect('virtualScroll' in out).toBe(false); + expect(pathsFor(notices, 'view-list-passthrough-keys-removed').sort()).toEqual([ + 'views[0].bordered', + 'views[0].striped', + 'views[0].virtualScroll', + ]); + }); + + it('strips the per-option `default` inside a flattened FORM overlay', () => { + const { out, notices } = convertViewRow( + formSpellings({ + type: 'simple', + fields: [{ + field: 'channel', + type: 'select', + options: [{ label: 'Email', value: 'email', default: true }, { label: 'Phone', value: 'phone' }], + }], + }).flattened, + ); + const options = (out.fields as Dict[])[0]!.options as Dict[]; + expect(options).toEqual([{ label: 'Email', value: 'email' }, { label: 'Phone', value: 'phone' }]); + expect(pathsFor(notices, 'form-view-option-default-removed')).toEqual([ + 'views[0].fields[0].options[0].default', + ]); + }); + + /** + * The flattened overlay is the one spelling where the payload and the row's + * identity share a dict, so the walker hands a conversion a dict that also + * holds `name`/`object`/`viewKind`/`columnState`/… . None of the keys any + * view-family conversion strips collides with that identity set — this pins + * that, so a later conversion whose key DOES collide fails here rather than + * silently deleting a row's binding. + */ + it('never touches the overlay identity/round-trip fields', () => { + const { out } = convertViewRow({ + name: 'crm_lead.all', + object: 'crm_lead', + viewKind: 'list', + label: 'All leads', + isDefault: true, + order: 3, + scope: 'user', + owner: 'usr_1', + columnState: { order: ['name'], widths: { name: 120 } }, + type: 'grid', + columns: ['name'], + striped: true, + }); + expect(out.name).toBe('crm_lead.all'); + expect(out.object).toBe('crm_lead'); + expect(out.viewKind).toBe('list'); + expect(out.label).toBe('All leads'); + expect(out.isDefault).toBe(true); + expect(out.order).toBe(3); + expect(out.scope).toBe('user'); + expect(out.owner).toBe('usr_1'); + expect(out.columnState).toEqual({ order: ['name'], widths: { name: 120 } }); + expect('striped' in out).toBe(false); // the one key that IS retired + }); +}); + +describe('the container spelling is unchanged, and the three agree', () => { + it('converts a container `list`, a named `listViews` entry, a record and an overlay alike', () => { + const payload = { type: 'grid', columns: ['name'], striped: true, exportOptions: ['xlsx', 'pdf'] }; + const s = listSpellings(payload); + + const fromContainer = convertViewRow(s.container).out.list as Dict; + const fromNamed = (convertViewRow(s.containerNamed).out.listViews as Dict).all as Dict; + const fromRecord = convertViewRow(s.record).out.config as Dict; + const flattened = convertViewRow(s.flattened).out; + + const expected = { type: 'grid', columns: ['name'], exportOptions: ['xlsx'] }; + expect(fromContainer).toEqual(expected); + expect(fromNamed).toEqual(expected); + expect(fromRecord).toEqual(expected); + // The overlay's payload is its top level, so compare after dropping identity. + const { name: _n, object: _o, viewKind: _k, ...overlayPayload } = flattened; + expect(overlayPayload).toEqual(expected); + }); + + it('converts a container `form` and a `viewKind: form` record alike', () => { + const payload = { + type: 'simple', + sections: [{ label: 'Details', visibleOn: 'record.open', fields: [] }], + aria: { label: 'Lead form' }, + }; + const s = formSpellings(payload); + const expected = { type: 'simple', sections: [{ label: 'Details', visibleWhen: 'record.open', fields: [] }] }; + + expect(convertViewRow(s.container).out.form).toEqual(expected); + expect((convertViewRow(s.containerNamed).out.formViews as Dict).quick).toEqual(expected); + expect(convertViewRow(s.record).out.config).toEqual(expected); + }); +}); + +describe('the walker recognizes shapes rather than guessing at them', () => { + it('leaves a body that is none of the three spellings untouched', () => { + // No `viewKind`, no container slot — nothing positively identifies a view + // payload here, so the chain must not invent one and start stripping. + const row = { name: 'mystery', object: 'crm_lead', striped: true, aria: { label: 'x' } }; + const { out, notices } = convertViewRow(row); + expect(out).toEqual(row); + expect(notices).toHaveLength(0); + }); + + it('treats a record whose `config` is not a dict as no payload at all', () => { + // `config: null` matches no `ViewMetadataSchema` member (the record arm + // requires a config object; both overlay arms guard `config: undefined`). + // A malformed body gets no rewrite — it is not silently re-read as a + // flattened overlay whose top level would then be stripped. + const row = { name: 'broken', object: 'crm_lead', viewKind: 'list', config: null, striped: true }; + const { out, notices } = convertViewRow(row); + expect(out).toEqual(row); + expect(notices).toHaveLength(0); + }); + + it('is idempotent: replaying the chain over an already-converted row is a no-op', () => { + const once = convertViewRow( + listSpellings({ type: 'grid', columns: ['name'], striped: true, exportOptions: ['xlsx', 'pdf'] }).record, + ); + const twice = convertViewRow(once.out); + expect(twice.out).toEqual(once.out); + expect(twice.notices).toHaveLength(0); + }); + + it('returns the SAME row reference when nothing converts (copy-on-write)', () => { + const row = { name: 'clean', object: 'crm_lead', viewKind: 'list', config: { type: 'grid', columns: ['name'] } }; + expect(applyConversionsToStoredItem('view', row)).toBe(row); + }); +}); diff --git a/packages/spec/src/conversions/walk.ts b/packages/spec/src/conversions/walk.ts index 2dc3bdddd8..08f2dc95fe 100644 --- a/packages/spec/src/conversions/walk.ts +++ b/packages/spec/src/conversions/walk.ts @@ -436,6 +436,142 @@ export function mapCollection( return { ...stack, [key]: next }; } +/** + * Which view family one payload belongs to — the `viewKind` discriminator, and + * for a container the slot the payload sits in. + * + * Load-bearing rather than informational: the view conversions are + * SHAPE-SCOPED, and two of them strip a key that is inert on one family and + * LIVE on the other (`aria` is retired on a form and live on a list, `data` the + * reverse). A walker that handed every payload to every conversion would delete + * live keys, so every payload arrives labelled. + */ +export type ViewPayloadKind = 'list' | 'form'; + +/** + * The container slots a `view` body aggregates its payloads under — the four + * `ViewSchema` keys, paired with the family each holds. + * + * Also the container DISCRIMINATOR: `containerHasAView` (`ui/view.zod.ts`) asks + * exactly "is at least one of these present?", and `ViewMetadataSchema`'s two + * flattened-overlay arms guard all four to `undefined` for the same reason. + */ +const VIEW_CONTAINER_SLOTS = [ + { key: 'list', named: false, kind: 'list' }, + { key: 'form', named: false, kind: 'form' }, + { key: 'listViews', named: true, kind: 'list' }, + { key: 'formViews', named: true, kind: 'form' }, +] as const; + +/** + * Immutably map every list/form view payload in `stack.views[]`, in **all three + * persisted spellings** of a `view` metadata body. + * + * `mapper` receives one payload dict, the family it belongs to, and its path, + * and returns the same reference (no change) or a new dict. Every container on + * the way — the stack, `views`, an item, its `listViews`/`formViews` record — + * is copied only when a descendant actually changed: {@link mapCollection}'s + * contract, one level further in. + * + * **Why this is centralized (#13031).** `ViewMetadataSchema` accepts three body + * shapes and all three land in `sys_metadata` rows, but every view-family + * conversion was written against the container alone — so for a stored ViewItem + * record or a flattened overlay the whole chain was a no-op, while + * `applyConversionsToStoredItem`'s documented policy (data at rest is "the + * perpetual consumer arriving late") promised it had run. A row written under an + * old protocol therefore kept its historical shape and the rehydration parse + * then refused exactly what was never rewritten: the "row that once worked + * breaks with no author in the loop" case the stored pass exists to prevent. + * Same structural gap the LINT walk closed one layer over — a standalone + * ViewItem's nested `config.*` judged by neither list-view field rule — and the + * same remedy `region-slots.ts` applied to flow regions: one table, one walk, + * so the reach cannot differ per conversion. + * + * **The three spellings, discriminated by the schema's own discriminators** — + * never by heuristics, and in `ViewMetadataSchema`'s own member order, so the + * walk and the parse cannot disagree about what a body is: + * + * 1. **ViewItem record** — `viewKind` names the family and `config` holds the + * payload (`{ name, object, viewKind, config }`). This is member 1's + * discriminated union: `viewKind` picks the arm, `config` is validated + * against ListView/FormView. Requiring `config` to be a dict is that arm's + * own requirement: a body with a non-object `config` matches no member at + * all (the record arm needs an object, both overlay arms guard + * `config: undefined`), so it is malformed and gets no rewrite rather than + * being re-read as some other spelling. + * 2. **Container** — at least one of {@link VIEW_CONTAINER_SLOTS}, the payload + * in each present slot. Member 2, whose `containerHasAView` refinement is + * that same test. + * 3. **Flattened overlay** — the payload IS the top-level body, with + * `viewKind` naming the family alongside the `object` binding. Members 3 + * and 4 are `ListViewSchema`/`FormViewSchema` extended with those identity + * fields, so "the top level is the payload" is what the schema declares, + * not an inference: which family it is comes from `viewKind`, which #7741 + * made REQUIRED on both arms precisely because the object-bound read paths + * match on `object` + `viewKind`. The mapper therefore sees a dict that + * also carries the row's identity keys; no key any view conversion strips + * collides with that set, and `view-spelling-walk.test.ts` pins it so a + * later conversion whose key DOES collide fails loudly instead of deleting + * a row's binding. + * + * A body matching none of the three passes through untouched — this walker + * never manufactures shape, the same discipline `applyConversions` states for + * the table as a whole. + */ +export function mapViewPayloads( + stack: Dict, + mapper: (payload: Dict, kind: ViewPayloadKind, path: string) => Dict, +): Dict { + return mapCollection(stack, 'views', (view, path) => { + const kind = view.viewKind; + const isKind = kind === 'list' || kind === 'form'; + + // 1. ViewItem record — the payload hangs off `config`, family from `viewKind`. + if (isKind && isDict(view.config)) { + const mapped = mapper(view.config, kind, `${path}.config`); + return mapped === view.config ? view : { ...view, config: mapped }; + } + + // 2. Container — a payload per present slot, family from the slot itself. + // Selected by the PRESENCE of a slot, not by whether one converted: a + // container in which nothing changed is still a container, and must not + // fall through to have its top level walked as an overlay as well. + if (VIEW_CONTAINER_SLOTS.some(({ key }) => view[key] !== undefined)) { + let next = view; + for (const { key, named, kind: slotKind } of VIEW_CONTAINER_SLOTS) { + const slot = next[key]; + if (!isDict(slot)) continue; + if (!named) { + const mapped = mapper(slot, slotKind, `${path}.${key}`); + if (mapped !== slot) next = { ...next, [key]: mapped }; + continue; + } + let namedChanged = false; + const rebuilt: Dict = { ...slot }; + for (const [name, entry] of Object.entries(slot)) { + if (!isDict(entry)) continue; + const mapped = mapper(entry, slotKind, `${path}.${key}.${name}`); + if (mapped === entry) continue; + rebuilt[name] = mapped; + namedChanged = true; + } + if (namedChanged) next = { ...next, [key]: rebuilt }; + } + return next; + } + + // 3. Flattened overlay — the body IS the payload. Reached only once the + // container slots are known absent (the guard those two arms declare), + // and only when `config` is absent too: a present-but-malformed `config` + // is case 1's business, not a licence to strip this row's top level. + if (isKind && view.config === undefined) { + return mapper(view, kind, path); + } + + return view; + }); +} + /** * Rename `dict[from]` → `dict[to]`, immutably. Returns `null` when there is * nothing to do — the caller keeps the original reference and emits no notice.