diff --git a/.changeset/10210-overlay-marker-only.md b/.changeset/10210-overlay-marker-only.md new file mode 100644 index 0000000000..9b7639d969 --- /dev/null +++ b/.changeset/10210-overlay-marker-only.md @@ -0,0 +1,19 @@ +--- +'@object-ui/data-objectstack': patch +--- + +Views that an earlier "Edit view config → Save" made read-only recover on read, and keep +their edits (objectui#10210). + +A view row now counts as a personalization overlay only when it carries the `_isOverride` +marker. The view list used to treat a flat row carrying `viewKind: 'list'` as an overlay +too, a shape guess kept for toolbar rows written before the marker existed. A config save +on a code-defined view stored exactly that shape, so the view dropped out of the saved +views and its tab turned read-only, for good once the draft was published. With the guess +retired, such a row reads back as the saved view it is: its tab is editable again and shows +what the save stored. Nothing is rewritten at rest; the next read is enough. + +⚠️ This exposes one class of row: overlay rows written before the marker +(objectui#4227, closed 2026-08-15) and never touched since. They have the same shape, so +they now also read as plain rows, and their frozen label, columns and filter copy covers +the code definition again. No deployment is named as holding one. diff --git a/.changeset/10210-view-config-save-envelope.md b/.changeset/10210-view-config-save-envelope.md index 5f71e13557..aee1e60c82 100644 --- a/.changeset/10210-view-config-save-envelope.md +++ b/.changeset/10210-view-config-save-envelope.md @@ -6,8 +6,8 @@ Saving a view's config no longer turns the view read-only (objectui#10210). "Edit view config → Save" on a code-defined view used to store the flat view the panel edits. The platform then copies `viewKind: 'list'` onto that row from the -code definition it shadows, and a flat row carrying `viewKind` is the shape the list -reader treats as a personalization overlay: the view dropped out of the saved views, +code definition it shadows, and a flat row carrying `viewKind` was the shape the list +reader treated as a personalization overlay: the view dropped out of the saved views, every mutating entry vanished from its tab menu except "Manage all views…", and publishing the draft made that permanent. @@ -24,9 +24,7 @@ saves it back to the same view. Before, a stored envelope lost the view's identi the way into the panel and the next Save wrote nothing. A change saved before this release is still stored flat and still resumes. -⚠️ Views that an earlier save already made read-only are **not** repaired by this -release: their stored rows cannot be told apart, by shape, from older toolbar -personalization rows, so they keep reading back as read-only and their tab menu -still offers no way back. Deleting such a view's stored customization through the -metadata API resets it to its code definition and drops the edits that save made; an -in-product repair is a separate decision. +Views that an earlier save already made read-only are repaired on read, edits kept, +by the `@object-ui/data-objectstack` change for objectui#10210: the list reader no +longer treats that shape as an overlay unless the row carries the `_isOverride` +marker. diff --git a/.changeset/view-overlay-write-patch-only-5233.md b/.changeset/view-overlay-write-patch-only-5233.md index 427d2ae8a1..fec2936669 100644 --- a/.changeset/view-overlay-write-patch-only-5233.md +++ b/.changeset/view-overlay-write-patch-only-5233.md @@ -28,6 +28,8 @@ an explicitly runtime-only overlay key (objectstack#9933, released in `@objectstack/spec` 17.1.0) — before that a `columnState`-only patch was refused `422 INVALID_METADATA`, and the fat copy was the only thing supplying a recognized key. The read half (`narrowPersonalizationOverlay`) shipped earlier -and stays: rows written before this land are still tolerated on read, and because -the write replaces the whole document, the next toolbar toggle also strips such a -row at rest. No migration. +and stays: rows written before this land are still tolerated on read when they +carry the overlay marker (a row older than the marker is not narrowed — see the +`@object-ui/data-objectstack` change for objectui#10210), and because the write +replaces the whole document, the next toolbar toggle also strips such a row at +rest. No migration. diff --git a/packages/app-shell/src/views/ObjectView.overlayPatchOnly.test.ts b/packages/app-shell/src/views/ObjectView.overlayPatchOnly.test.ts index 15271e6ff0..9e00797e1b 100644 --- a/packages/app-shell/src/views/ObjectView.overlayPatchOnly.test.ts +++ b/packages/app-shell/src/views/ObjectView.overlayPatchOnly.test.ts @@ -435,6 +435,14 @@ describe('objectui#5233 — rows written BEFORE this fix (the disposition, pinne * frozen body behaves correctly on the very next page load, without its * user having to touch that view again and without an operator running * anything. + * + * One exception is ruled (objectui#10210, ruling B, comment 5824008636): + * a row written before the `_isOverride` marker existed carries no + * marker, and the marker is now the only thing that makes a row an + * overlay. The shape guess that used to narrow it too is retired — an + * "Edit view config → Save" wrote the same shape and the guess turned the + * user's own view read-only. The case that pinned the old narrowing is + * rewritten below to pin the exposure the ruling accepts, not deleted. */ it('a row stored in the old shape stops shadowing the source on the NEXT READ — no write, no migration', async () => { const { meta, rows } = makeMetaStore(); @@ -460,13 +468,17 @@ describe('objectui#5233 — rows written BEFORE this fix (the disposition, pinne expect(meta.saveItem).not.toHaveBeenCalled(); }); - it('a PRE-MARKER legacy row is narrowed by the same predicate listViews() excludes it by', async () => { + it('a PRE-MARKER legacy row is no longer narrowed: its frozen copy covers the source again (objectui#10210 ruling B, the accepted exposure)', async () => { const { meta, rows } = makeMetaStore(); const ds = makeAdapter(meta); // Written before `_isOverride` existed (objectui#4227): flat body, and // a `viewKind` only the platform's registry-backed identity heal can - // have put there. + // have put there. This case used to assert that the retired shape + // guess narrowed it, so the admin's edit won. Under ruling B the row is + // a plain row: the ruling names exactly this — an overlay written + // before the marker and never touched since, whose frozen label, + // columns and filter copy covers the code definition again. rows.set(`view::${VIEW_ID}`, { ...SOURCE_VIEW_AT_WRITE_TIME, object: OBJECT_NAME, @@ -476,7 +488,9 @@ describe('objectui#5233 — rows written BEFORE this fix (the disposition, pinne const tab = await tabAfterAdminEdit(ds); - expect(tab.filter).toEqual(SOURCE_VIEW_AFTER_ADMIN_EDIT.filter); + expect(tab.filter).toEqual(SOURCE_VIEW_AT_WRITE_TIME.filter); + expect(tab.columns).toEqual(SOURCE_VIEW_AT_WRITE_TIME.columns); + expect(tab.label).toBe(SOURCE_VIEW_AT_WRITE_TIME.label); expect(tab.sort).toEqual([{ field: 'created_at', order: 'desc' }]); }); }); diff --git a/packages/app-shell/src/views/ObjectView.overrideMasquerade.test.ts b/packages/app-shell/src/views/ObjectView.overrideMasquerade.test.ts index 111aa7da17..af899afbd9 100644 --- a/packages/app-shell/src/views/ObjectView.overrideMasquerade.test.ts +++ b/packages/app-shell/src/views/ObjectView.overrideMasquerade.test.ts @@ -26,8 +26,17 @@ * companion fixture that DOES include one, run through the REAL production * pipeline: the adapter's `listViews()` (the actual fix) feeding * `buildViewTabs` / `isSavedViewId` (the actual consumers), exactly as - * `ObjectView`'s own effect normalizes them (mirrored from - * ObjectView.tsx:967-978). + * `ObjectView`'s own effect normalizes them (mirrored from the `normalized` + * map in `ObjectView.tsx`'s `listViews(objectName, { previewDrafts })` effect). + * + * objectui#10210, ruling B (comment 5824008636): only the `_isOverride` marker + * makes a row an overlay. This file used to pin a second, shape-based layer as + * well — an unmarked flat row with a server-backfilled `viewKind` excluded and + * kept read-only. That guess is retired: "Edit view config → Save" wrote the + * same shape before PR #10332, and the guess turned the user's own view + * read-only for good once published. The case that pinned it is rewritten + * below, through the same real pipeline, to pin the ruled behaviour — not + * deleted. */ import { describe, it, expect, vi } from 'vitest'; @@ -43,7 +52,7 @@ const DEFINED_VIEWS = { const fallbackTab = () => ({ id: 'all', label: 'All records', type: 'grid', columns: [] }); -/** `ObjectView.tsx`'s own `savedViews` normalization (ObjectView.tsx:967-978), verbatim. */ +/** `ObjectView.tsx`'s own `savedViews` normalization (the `normalized` map in its `listViews` effect), verbatim. */ function normalizeSavedViews(rows: any[]) { return rows.map((sv: any) => ({ ...sv, @@ -81,13 +90,6 @@ describe('a system view stays readonly even with a personalization row (objectui rowHeight: 40, _isOverride: true, }, ], - [ - 'legacy unmarked row (viewKind backfilled server-side, pre-marker writes)', - { - name: 'crm_lead.default', object: 'crm_lead', viewKind: 'list', - label: 'All Leads', type: 'grid', rowHeight: 40, - }, - ], ])('%s: excluded from savedViews, tab stays readonly, guard refuses', async (_label, overrideRow) => { const ds = makeAdapterWithItems([overrideRow]); @@ -113,6 +115,43 @@ describe('a system view stays readonly even with a personalization row (objectui expect(isMutable(savedViews, 'crm_lead.default')).toBe(false); }); + it('an UNMARKED flat row with a backfilled viewKind is a saved view: the tab is editable and shows its edits (objectui#10210 ruling B)', async () => { + // This row was the second `it.each` case above, asserting the retired + // shape guess: excluded, tab read-only, guard refusing. Rewritten, not + // deleted. The fixture is now the row a pre-PR-#10332 config save left at + // rest (the flat panel draft, `viewKind`/`object` inherited server-side), + // which is the same shape as a pre-marker toolbar overlay; under ruling B + // both read as the plain row they are stored as. + const flatConfigSave = { + label: 'All Leads EDITED', type: 'grid', columns: ['name', 'status'], + name: 'crm_lead.default', isDefault: false, id: 'crm_lead.default', + viewKind: 'list', object: 'crm_lead', + }; + const ds = makeAdapterWithItems([flatConfigSave]); + + const rawSavedViews = await ds.listViews(OBJECT_NAME); + expect(rawSavedViews.map((v: any) => v.name)).toEqual(['crm_lead.default']); + + const savedViews = normalizeSavedViews(rawSavedViews); + const tabs = buildViewTabs({ + definedViews: DEFINED_VIEWS, + primary: undefined, + primaryId: undefined, + savedViews, + viewOverrides: {}, + fallbackTab, + }); + + expect(tabs.map((t) => t.id)).toEqual(['crm_lead.default']); + // The edits the save stored are what the tab shows. + expect(tabs[0].label).toBe('All Leads EDITED'); + expect(tabs[0].columns).toEqual(['name', 'status']); + // Render-time gate (`isSystem = !saved`, `readonly: isSystem`) lifts, and + // the predicate all five mutating handlers short-circuit on now admits it. + expect(isReadonlyTab(savedViews, 'crm_lead.default')).toBe(false); + expect(isMutable(savedViews, 'crm_lead.default')).toBe(true); + }); + it('positive control: a genuinely created saved view stays fully manageable, even reusing a system-view-shaped label', async () => { // A real save (createView / the ADR-0034 seam) is always a nested // ViewItem record — the shape `listViews()` must keep letting through. diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 0b7bc68a9e..0d32bac42e 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -2700,7 +2700,9 @@ export function viewItemObjectName(item: any): string | undefined { /** * The explicit discriminant {@link ObjectStackAdapter.updateViewConfig} stamps * on the rows it writes for a **system**-view target, and - * {@link ObjectStackAdapter.listViews} excludes on read (objectui#4227). + * {@link ObjectStackAdapter.listViews} excludes on read (objectui#4227). It is + * the ONLY thing that classifies a row as an overlay (objectui#10210, ruling B — + * see {@link isPersonalizationOverlayRow}). * * `updateViewConfig` has exactly ONE production caller — `ObjectView`'s * `persistViewPatch`, invoked only for the toolbar-driven density / sort / @@ -2735,64 +2737,44 @@ export function viewItemObjectName(item: any): string | undefined { */ const VIEW_OVERLAY_MARKER = '_isOverride' as const; -/** - * Best-effort classification of a `view` row {@link ObjectStackAdapter.listViews} - * reads back from BEFORE {@link VIEW_OVERLAY_MARKER} existed (objectui#4227) — - * a legacy personalization row written by an older `updateViewConfig` carries - * no discriminant at all. - * - * Measured against the actual write paths, not guessed: - * - * - A genuine saved view is always created with a NESTED `config` — the - * ViewItem-record shape `{name, object, viewKind, config}` (app-shell's - * `viewEnvelope`, and this adapter's own {@link ObjectStackAdapter.createView} - * `fullSpec`). `viewKind` lives OUTSIDE `config` on that shape. - * - A personalization overlay (`updateViewConfig`) is always FLAT — its - * fields sit at the top level, never wrapped in `config`. - * - * `viewKind` on a FLAT row is therefore never something objectui itself - * authors: the only way it gets there is the platform's own server-side - * identity inheritance (`viewIdentityPatch`, `@objectstack/metadata-protocol` - * #2555 / #7741), which fires ONLY when the write's `name` resolves against a - * REGISTRY-backed (i.e. system, code-defined) view. A runtime-created saved - * view has no registry entry to inherit from, so its row — even flattened by - * a later toolbar toggle — never gains a `viewKind`. So "flat body + a - * `viewKind`" is a reliable signature of "override on a system view", while a - * flat row with NO `viewKind` is left alone — exactly the shape the existing - * legacy-bare-spec pin relies on staying a saved view (`listViews.test.ts` — - * "keeps legacy bare specs without a viewKind (saved/list views)"). - * - * Deliberately does NOT try to catch every legacy override: a row the - * CURRENT `persistViewPatch` writes (pre-marker) also copies the system - * view's full body — `type`/`columns`/`data` — into the override, and *that* - * shape is structurally indistinguishable from an untouched saved view's own - * body without this `viewKind` signal or the new marker above. Those rows - * self-heal on their NEXT write (which carries the marker); until then this - * predicate is a best-effort net over the realistic current-state case, not a - * guarantee for every possible legacy row. See the PR description for the - * measured readings this was decided against. - */ -function isLegacyOverlayRow(item: any, spec: any): boolean { - // A ViewItem record (nested `config`) is never an overlay row, regardless - // of what else it carries. - if (spec && spec.config && typeof spec.config === 'object') return false; - const viewKind = item?.viewKind ?? spec?.viewKind; - // 'form' rows are already dropped upstream by the FORM_FAMILY filter before - // this runs; a bare 'list' here is what a system-view override looks like. - return viewKind === 'list'; -} - /** * Whether a `view` row {@link ObjectStackAdapter.listViews} enumerated is a - * personalization overlay rather than a saved view — the marker (new writes) - * or the best-effort legacy shape (pre-marker writes). Both layers are - * needed: excluding only the marker would leave every row written before - * this fix still masquerading as a saved view (objectui#4227). + * personalization overlay rather than a saved view: the row carries + * {@link VIEW_OVERLAY_MARKER}, on the item or on its `{list: …}` body. Nothing + * else classifies a row. Both {@link ObjectStackAdapter.listViews} and + * {@link narrowPersonalizationOverlay} ask this one predicate, so a row cannot + * be a saved view for one reader and an overlay for the other. + * + * ## The shape guess is retired (objectui#10210, ruling B) + * + * This predicate used to answer `true` by SHAPE as well: a flat row (no nested + * `config`) carrying `viewKind: 'list'`, the identity the platform's + * `viewIdentityPatch` inherits onto a write addressed to a code-defined view. + * That net was cast for toolbar overlays written before the marker existed + * (objectui#4227). "Edit view config → Save" then wrote the same shape: the + * flat panel draft, with `viewKind` inherited server-side. The net dropped the + * user's own view out of `listViews()`, the tab was stamped read-only, and + * publishing made that permanent. PR #10332 fixed that save going forward; the + * rows it had already written stayed caught. The card measured both + * populations shape-identical, so no narrower shape test exists. + * + * The maintainer ruled B (objectui#10210, ruling comment 5824008636): an overlay + * is a row carrying the marker, nothing else. Both consequences are accepted + * and pinned in `viewOverlayMarkerOnly-10210.test.ts`: + * + * - a row an earlier config save left flat reads as the saved view it is, so + * the view heals on read and keeps its edits; + * - an overlay row written BEFORE the marker (objectui#4227, closed 2026-08-15) + * and never touched since also reads as a plain row, so its frozen `label`, + * `columns` and `filter` copy covers the code definition again. No + * deployment is named as holding one. + * + * ⛔ Do not bring a shape test back to win the second population back: a shape + * that another writer can produce is not a discriminant. The marker is one, and + * the write side stamps it ({@link ObjectStackAdapter.updateViewConfig}). */ function isPersonalizationOverlayRow(item: any, spec: any): boolean { - if (item?.[VIEW_OVERLAY_MARKER] === true) return true; - if (spec?.[VIEW_OVERLAY_MARKER] === true) return true; - return isLegacyOverlayRow(item, spec); + return item?.[VIEW_OVERLAY_MARKER] === true || spec?.[VIEW_OVERLAY_MARKER] === true; } /** @@ -2820,7 +2802,9 @@ function isPersonalizationOverlayRow(item: any, spec: any): boolean { * * - **read** (PR #5272, {@link narrowPersonalizationOverlay}): the consumer * that MERGES an overlay over a source view contributes only these keys, so - * every already-stored fat row stops shadowing its source. + * every already-stored fat row that carries the marker stops shadowing its + * source. A fat row written before the marker is no longer narrowed + * (objectui#10210, ruling B — see {@link isPersonalizationOverlayRow}). * - **write** (objectui#5233, `buildPersistedViewBody` in app-shell's * `ObjectView`, unblocked by `columnState`'s admission to the view-metadata * surface as a runtime-only overlay key — objectstack#9933, released in @@ -2874,8 +2858,8 @@ const VIEW_OVERLAY_IDENTITY_KEYS = Object.freeze([ * {@link VIEW_OVERLAY_OWNED_KEYS}). Of the three dispositions the issue names * for them — strip on next write, migrate, tolerate on read — this is the * third, chosen deliberately and stated here rather than left implicit, - * because it is the only one that is already true for every existing row the - * moment it ships: strip-on-next-write heals a row only when its user happens + * because it is the only one that is already true for every existing marked + * row the moment it ships: strip-on-next-write heals a row only when its user happens * to touch that view again (and leaves the frozen filter live until then), * and a migration needs a runner this product does not have for `sys_metadata` * rows an operator may not even know exist. What the issue forbids is SILENT @@ -2896,7 +2880,9 @@ const VIEW_OVERLAY_IDENTITY_KEYS = Object.freeze([ * IS the view, and every key on it is an opinion its author expressed. * Classification is {@link isPersonalizationOverlayRow}, the same predicate * {@link ObjectStackAdapter.listViews} excludes rows by, so a row cannot be a - * saved view for one reader and an overlay for the other. + * saved view for one reader and an overlay for the other. It reads the marker + * only (objectui#10210, ruling B): an unmarked row is returned by reference + * whatever its shape, including a fat row written before the marker existed. */ export function narrowPersonalizationOverlay(row: T): T { if (!row || typeof row !== 'object' || Array.isArray(row)) return row; @@ -5453,8 +5439,9 @@ export class ObjectStackAdapter implements DataSource { // inlineEdit — written by `updateViewConfig`) are NOT saved views: // returning one here is what let a system view's override row read // back as user-created and gain Rename/Delete/Set-default/Pin - // (objectui#4227). Marked rows and the best-effort legacy shape are - // both excluded — see {@link isPersonalizationOverlayRow}. + // (objectui#4227). Only a row carrying the marker is excluded: the + // shape guess that also caught unmarked flat rows is retired + // (objectui#10210, ruling B) — see {@link isPersonalizationOverlayRow}. if (isPersonalizationOverlayRow(v, spec)) return false; return true; }).map((v: any) => { diff --git a/packages/data-objectstack/src/listViews.test.ts b/packages/data-objectstack/src/listViews.test.ts index 12f65a9344..d03ddfdc91 100644 --- a/packages/data-objectstack/src/listViews.test.ts +++ b/packages/data-objectstack/src/listViews.test.ts @@ -106,6 +106,13 @@ describe('ObjectStackDataSource.listViews', () => { // ── objectui#4227 — a personalization override must never read back as a // saved view (which is what let a system view gain Rename/Delete/ // Set-default/Pin just because someone toggled its density). ────────── + // + // objectui#10210, ruling B (comment 5824008636): the `_isOverride` marker + // is the ONLY thing that classifies a row as an overlay. The shape guess + // (flat body + `viewKind: 'list'`) that also excluded pre-marker rows is + // retired: an "Edit view config → Save" wrote that same shape, and the + // guess turned the user's own view read-only. The case below that pinned + // the guess is rewritten to pin the ruled rule, not deleted. describe('excludes personalization overlays (objectui#4227)', () => { it('excludes a row carrying the explicit write-side marker', async () => { // Exactly what `updateViewConfig` writes today: the marker plus a full @@ -121,18 +128,23 @@ describe('ObjectStackDataSource.listViews', () => { expect(views).toEqual([]); }); - it('excludes a legacy (unmarked) override whose viewKind was backfilled server-side', async () => { + it('returns an UNMARKED flat row whose viewKind was backfilled server-side — only the marker classifies (objectui#10210 ruling B)', async () => { // #7741/#2555: the platform inherits `viewKind`/`object`/`label` from - // the REGISTRY baseline for a personalization PUT against a real - // system view — so a pre-marker row targeting `crm_lead.default` - // still carries `viewKind: 'list'` even though objectui never sent it. - const legacyOverride = { + // the REGISTRY baseline for a PUT against a code-defined view — so a + // pre-marker row targeting `crm_lead.default` carries `viewKind: 'list'` + // even though objectui never sent it. This case used to assert that the + // row was excluded by that shape. A config save before PR #10332 stored + // the same shape, and the two cannot be told apart by it, so under + // ruling B the row reads back as the plain row it is stored as: the + // repair for a config-save row, the accepted exposure for a pre-marker + // overlay (`viewOverlayMarkerOnly-10210.test.ts` pins both). + const unmarkedFlatRow = { name: 'crm_lead.default', object: 'crm_lead', viewKind: 'list', label: 'All', type: 'grid', rowHeight: 40, }; - const ds = makeDS([legacyOverride]); + const ds = makeDS([unmarkedFlatRow]); const views = await ds.listViews('crm_lead'); - expect(views).toEqual([]); + expect(views).toEqual([unmarkedFlatRow]); }); it('a marked row is excluded even without the legacy viewKind signal', async () => { diff --git a/packages/data-objectstack/src/viewOverlayMarker.test.ts b/packages/data-objectstack/src/viewOverlayMarker.test.ts index 0da9059526..7fd2fde62c 100644 --- a/packages/data-objectstack/src/viewOverlayMarker.test.ts +++ b/packages/data-objectstack/src/viewOverlayMarker.test.ts @@ -20,11 +20,20 @@ * * 1. `updateViewConfig` — the ONLY production writer of personalization * rows — stamps an explicit `_isOverride` marker on every row it saves. - * 2. `listViews()` excludes any row carrying that marker, AND (for rows - * already persisted before this fix shipped) a best-effort legacy - * shape: flat body + a `viewKind` the platform can only have backfilled - * from a REGISTRY baseline (objectstack#2555 / #7741) — which a - * genuine runtime-created saved view never has. + * 2. `listViews()` excludes any row carrying that marker — and, since + * objectui#10210, ONLY such a row. + * + * The second layer used to be wider. For rows persisted before the marker + * shipped it also excluded a best-effort legacy SHAPE: a flat body plus a + * `viewKind` the platform backfills from a REGISTRY baseline (objectstack#2555 / + * #7741). That guess was retired by the maintainer's ruling B on objectui#10210 + * (comment 5824008636): "Edit view config → Save" on a code-defined view wrote + * the same shape, so the guess dropped the user's own view out of `listViews()` + * and turned it read-only for good once published. An overlay is now a row + * carrying `_isOverride`, nothing else. Every case in this file already turned + * on the marker rather than on the shape (measured: all of them stay green + * with the guess removed), so this rewrite is to this header only; the retired + * shape is pinned as ruled in `viewOverlayMarkerOnly-10210.test.ts`. * * `listViewOverrides` (the batch personalization reader `ObjectView` merges * for DISPLAY) is a separate, unchanged consumer of the same rows — it is diff --git a/packages/data-objectstack/src/viewOverlayMarkerOnly-10210.test.ts b/packages/data-objectstack/src/viewOverlayMarkerOnly-10210.test.ts new file mode 100644 index 0000000000..8188cfe8a2 --- /dev/null +++ b/packages/data-objectstack/src/viewOverlayMarkerOnly-10210.test.ts @@ -0,0 +1,236 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#10210, ruling B (comment 5824008636): a view row is a + * personalization overlay when it carries `_isOverride`, and for no other + * reason. + * + * The retired alternative was a SHAPE guess: a flat row (no nested `config`) + * carrying `viewKind: 'list'`. The platform puts that `viewKind` on any write + * addressed to a code-defined view (`viewIdentityPatch`, inherited from the + * registry entry the row shadows), so two populations end up in that shape: + * + * - a view an "Edit view config → Save" stored before PR #10332, when the save + * wrote the flat panel draft. The guess read it as an overlay, so the view + * dropped out of `listViews()`, its tab was stamped read-only, and publishing + * made that permanent; + * - a toolbar overlay written before the marker existed (objectui#4227, closed + * 2026-08-15) and never touched since, carrying a frozen copy of the view. + * + * The card measured the two as structurally identical. The ruling takes the + * marker as the only discriminant and accepts both consequences, pinned here + * through a store that inherits identity the way the platform's write door + * does: + * + * 1. the broken config-save row heals on read — it is a saved view again, and + * its edits are kept; + * 2. the pre-marker overlay also reads as a plain row, so its frozen label, + * columns and filter cover the code definition again. No deployment is + * named as holding one. + * + * The control at the end is the same shape carrying the marker: it is still + * excluded and still narrowed, so what moved is the shape guess and nothing + * else. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackAdapter, narrowPersonalizationOverlay } from './index'; + +const OBJECT_NAME = 'crm_lead'; +const VIEW_ID = 'crm_lead.all'; + +/** The code-defined view as the registry serves it, untouched. */ +const REGISTRY: Record> = { + [VIEW_ID]: { + name: VIEW_ID, + object: OBJECT_NAME, + viewKind: 'list', + label: 'Everything', + config: { type: 'grid', columns: [{ field: 'name' }] }, + }, +}; + +/** + * What "Edit view config → Save" sent before PR #10332: the flat panel draft, + * as captured from a live run against an `@objectstack` 17.4.0 backend and + * recorded on objectui#10210. The label and the filter are the user's edits. + */ +const FLAT_CONFIG_SAVE = { + label: 'Everything EDITED', + type: 'grid', + columns: [{ field: 'name' }, { field: 'status' }], + filter: [{ field: 'status', operator: 'equals', value: 'open' }], + name: VIEW_ID, + isDefault: false, + id: VIEW_ID, +}; + +/** The code-defined view as it stood when a pre-marker toolbar toggle copied it. */ +const DEFINITION_AT_OVERLAY_TIME = { + label: 'Everything', + type: 'grid', + columns: [{ field: 'name' }], + filter: [{ field: 'status', operator: 'equals', value: 'open' }], +}; + +/** The same view after its code definition moved on. */ +const DEFINITION_NOW = { + label: 'Everything (active)', + type: 'grid', + columns: [{ field: 'name' }, { field: 'owner' }], + filter: [{ field: 'status', operator: 'equals', value: 'qualified' }], +}; + +/** + * What `updateViewConfig` stored before the marker existed (objectui#4227): + * the whole active tab plus the one key the toggle changed, and no marker. + * Today's `updateViewConfig` stamps the marker, so this row can only be put in + * the store directly — it is the state of an install that has carried it since. + */ +const PRE_MARKER_OVERLAY_WRITE = { + ...DEFINITION_AT_OVERLAY_TIME, + name: VIEW_ID, + rowHeight: 'compact', +}; + +/** + * A `sys_metadata`-shaped store standing in for the platform's `view` write + * door in the one way this card depends on: a write addressed to a + * code-defined view inherits `viewKind` / `object` / `label` from the registry + * entry it shadows, when the body does not carry them. + */ +function makePlatformStore() { + const rows = new Map(Object.entries(REGISTRY).map(([k, v]) => [k, { ...v }])); + const meta = { + getItems: vi.fn(async () => ({ type: 'view', items: [...rows.values()] })), + getItem: vi.fn(async (_type: string, name: string) => ({ type: 'view', name, item: rows.get(name) })), + saveItem: vi.fn(async (_type: string, name: string, item: any) => { + const baseline = REGISTRY[name]; + const stored = { ...item }; + for (const key of ['viewKind', 'object', 'label'] as const) { + if (stored[key] === undefined && baseline?.[key] !== undefined) stored[key] = baseline[key]; + } + rows.set(name, stored); + return { success: true, item: stored }; + }), + }; + return { meta, rows }; +} + +function makeAdapter(meta: any) { + const ds: any = new ObjectStackAdapter({ + baseUrl: 'http://test.local', + fetch: vi.fn(async () => + new Response(JSON.stringify({ success: true, data: { capabilities: {}, routes: {} } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + })), + }); + ds.connected = true; + ds.connectionState = 'connected'; + ds.client = { meta }; + return ds; +} + +/** An adapter whose `?preview=draft` metadata route answers with `items`. */ +function makeDraftPreviewAdapter(items: any[]) { + const ds: any = new ObjectStackAdapter({ + baseUrl: 'http://test.local', + fetch: vi.fn(async (input: RequestInfo | URL) => { + const body = String(input).includes('/meta/view') ? { items } : { success: true, data: {} }; + return new Response(JSON.stringify(body), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }), + }); + ds.connected = true; + ds.connectionState = 'connected'; + ds.client = { meta: { getItems: vi.fn(async () => ({ items: [] })) } }; + return ds; +} + +/** The row the store holds for `VIEW_ID` after `body` lands on it. */ +async function storedAfter(body: Record) { + const { meta, rows } = makePlatformStore(); + await meta.saveItem('view', VIEW_ID, body); + return { ds: makeAdapter(meta), row: rows.get(VIEW_ID) }; +} + +describe('objectui#10210 ruling B — a view broken by an earlier config save heals on read', () => { + it('listViews() returns it as a saved view, with the edits the save stored', async () => { + const { ds, row } = await storedAfter(FLAT_CONFIG_SAVE); + // Precondition — the row at rest is the flat, unmarked shape the card + // measured: {label,type,columns,name,isDefault,id,viewKind,object}, plus + // `filter` because this draft edited one. + expect(Object.keys(row).sort()).toEqual( + ['columns', 'filter', 'id', 'isDefault', 'label', 'name', 'object', 'type', 'viewKind'], + ); + expect(row.viewKind).toBe('list'); + + const views: any[] = await ds.listViews(OBJECT_NAME); + // The tab's read-only stamp is `!saved`, and `saved` is found by this + // name — so a view in this list is an editable tab again. + const healed = views.find((v) => v.name === VIEW_ID); + expect(healed).toBeDefined(); + expect(healed.label).toBe(FLAT_CONFIG_SAVE.label); + expect(healed.columns).toEqual(FLAT_CONFIG_SAVE.columns); + expect(healed.filter).toEqual(FLAT_CONFIG_SAVE.filter); + }); + + it('the draft-preview read — where the card saw the menu collapse to one entry — returns it too', async () => { + const { row } = await storedAfter(FLAT_CONFIG_SAVE); + const ds = makeDraftPreviewAdapter([{ ...row, _draft: true }]); + const views: any[] = await ds.listViews(OBJECT_NAME, { previewDrafts: true }); + expect(views.map((v) => v.name)).toEqual([VIEW_ID]); + expect(views[0]).toMatchObject({ label: FLAT_CONFIG_SAVE.label, _draft: true }); + }); + + it('read as a patch it keeps every edit — nothing is narrowed away', async () => { + const { row } = await storedAfter(FLAT_CONFIG_SAVE); + const asPatch: any = narrowPersonalizationOverlay(row); + expect(asPatch).toBe(row); + const merged = { ...DEFINITION_NOW, ...asPatch }; + expect(merged.label).toBe(FLAT_CONFIG_SAVE.label); + expect(merged.filter).toEqual(FLAT_CONFIG_SAVE.filter); + }); +}); + +describe('objectui#10210 ruling B — the accepted exposure: a pre-marker overlay never touched since', () => { + it('reads back from listViews() as a plain row, carrying its frozen copy', async () => { + const { ds, row } = await storedAfter(PRE_MARKER_OVERLAY_WRITE); + expect(row.viewKind).toBe('list'); + const views: any[] = await ds.listViews(OBJECT_NAME); + expect(views.find((v) => v.name === VIEW_ID)).toMatchObject({ + label: DEFINITION_AT_OVERLAY_TIME.label, + columns: DEFINITION_AT_OVERLAY_TIME.columns, + filter: DEFINITION_AT_OVERLAY_TIME.filter, + rowHeight: 'compact', + }); + }); + + it('merged over the current code definition, its frozen label, columns and filter cover it again', async () => { + const { row } = await storedAfter(PRE_MARKER_OVERLAY_WRITE); + const merged = { ...DEFINITION_NOW, ...narrowPersonalizationOverlay(row) }; + expect(merged.label).toBe(DEFINITION_AT_OVERLAY_TIME.label); + expect(merged.columns).toEqual(DEFINITION_AT_OVERLAY_TIME.columns); + expect(merged.filter).toEqual(DEFINITION_AT_OVERLAY_TIME.filter); + }); +}); + +describe('objectui#10210 ruling B — control: the marker still classifies', () => { + it('the same shape carrying `_isOverride` is excluded from listViews() and narrowed to its patch', async () => { + const { ds, row } = await storedAfter({ ...PRE_MARKER_OVERLAY_WRITE, _isOverride: true }); + expect(await ds.listViews(OBJECT_NAME)).toEqual([]); + const merged = { ...DEFINITION_NOW, ...narrowPersonalizationOverlay(row) }; + expect(merged.filter).toEqual(DEFINITION_NOW.filter); + expect(merged.label).toBe(DEFINITION_NOW.label); + expect(merged.rowHeight).toBe('compact'); + }); +}); diff --git a/packages/data-objectstack/src/viewOverlayPatchOnly.test.ts b/packages/data-objectstack/src/viewOverlayPatchOnly.test.ts index 74b30e3bfa..22882383b7 100644 --- a/packages/data-objectstack/src/viewOverlayPatchOnly.test.ts +++ b/packages/data-objectstack/src/viewOverlayPatchOnly.test.ts @@ -32,6 +32,14 @@ * same document"), and `InterfaceListPage` hydrates a hollow view out of it. * A row is narrowed where it is read AS A PATCH, not where it is read as a row * — and that boundary is pinned below. + * + * WHICH rows are narrowed is the `_isOverride` marker's call alone + * (objectui#10210, ruling B, comment 5824008636). A flat row carrying an + * inherited `viewKind` used to be narrowed by shape too; that guess is retired, + * because an "Edit view config → Save" wrote the same shape and the guess + * turned the user's own view read-only. The case below that pinned the + * narrowing of a pre-marker row is rewritten to pin the ruled behaviour, not + * deleted. */ import { describe, it, expect, vi } from 'vitest'; @@ -145,16 +153,18 @@ describe('narrowPersonalizationOverlay — what an overlay may contribute', () = ); }); - it('narrows a PRE-MARKER legacy row — flat body plus an inherited `viewKind`', () => { - // objectui#4227's best-effort signature: only the platform's registry-backed - // identity heal can put `viewKind` on a flat row, so this is an override on - // a system view. Same predicate `listViews()` excludes the row by, so a row - // cannot be an overlay for one reader and a saved view for the other. - const narrowed: any = narrowPersonalizationOverlay({ - ...FAT_WRITE, object: 'crm_lead', viewKind: 'list', - }); - expect('filter' in narrowed).toBe(false); - expect(narrowed.columnState).toEqual({ order: ['status', 'name'] }); + it('leaves a PRE-MARKER row whole — flat body plus an inherited `viewKind`, no marker (objectui#10210 ruling B)', () => { + // This case used to pin objectui#4227's best-effort signature, which + // narrowed this row as an override on a system view. Ruling B retires it: + // only the marker makes a row an overlay. For this row that is the one + // exposure the ruling accepts — an overlay written before the marker and + // never touched since keeps its frozen copy, so its `filter` reaches the + // merge. Same predicate `listViews()` classifies by, so the row is a plain + // row for both readers. + const preMarkerRow = { ...FAT_WRITE, object: 'crm_lead', viewKind: 'list' }; + const read: any = narrowPersonalizationOverlay(preMarkerRow); + expect(read).toBe(preMarkerRow); + expect(read.filter).toEqual(FAT_WRITE.filter); }); it("returns a saved view's own body BY REFERENCE — untouched", () => {