From 9736e8d20297fa0c9b9beed25bd564bf0e9923e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 00:03:06 +0000 Subject: [PATCH] fix(data-objectstack): classify a view overlay by its `_isOverride` marker only (objectui#10210, ruling B) Retire `isLegacyOverlayRow`, the shape guess that also treated a flat view row carrying `viewKind: 'list'` as a personalization overlay. An "Edit view config -> Save" on a code-defined view stored that same shape before PR #10332, so `listViews()` dropped the user's own view, the tab was stamped read-only, and publishing made it permanent. Under the maintainer's ruling B (objectui#10210, comment 5824008636) the marker is the only discriminant: such rows heal on read with their edits, and the one exposed class (overlay rows written before the marker and never touched since) is named in the changeset and pinned. The pins that asserted the guess are rewritten with the reason, not deleted: `listViews`, `viewOverlayPatchOnly` (and the header of `viewOverlayMarker`), plus the two app-shell pins that ran the real adapter through the same guess. The two pending changesets this makes false are corrected in place. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01BP8CMtACxTdLjqR6rhd33C --- .changeset/10210-overlay-marker-only.md | 19 ++ .changeset/10210-view-config-save-envelope.md | 14 +- .../view-overlay-write-patch-only-5233.md | 8 +- .../views/ObjectView.overlayPatchOnly.test.ts | 20 +- .../ObjectView.overrideMasquerade.test.ts | 59 ++++- packages/data-objectstack/src/index.ts | 109 ++++---- .../data-objectstack/src/listViews.test.ts | 26 +- .../src/viewOverlayMarker.test.ts | 19 +- .../src/viewOverlayMarkerOnly-10210.test.ts | 236 ++++++++++++++++++ .../src/viewOverlayPatchOnly.test.ts | 30 ++- 10 files changed, 433 insertions(+), 107 deletions(-) create mode 100644 .changeset/10210-overlay-marker-only.md create mode 100644 packages/data-objectstack/src/viewOverlayMarkerOnly-10210.test.ts 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", () => {