fix(app-shell): store only the patch in a view personalization overlay (#5233)

.changeset/view-overlay-write-patch-only-5233.md

@@ -0,0 +1,33 @@
+---
+'@object-ui/app-shell': patch
+---
+
+A view personalization overlay now stores **only the patch** — a column drag no
+longer freezes the source view's filter into it (objectui#5233).
+
+`ObjectView`'s `persistViewPatch` wrote `{ ...baseViewDef, ...patch }`, so an
+overlay written by a mere sort/density/column change copied the view's *current
+effective* `filter` — along with its `columns`, `label`, `type`, `isDefault` —
+into the stored row. The display merge is `{ ...source, ...override }`, so that
+copy then outranked the source view forever: an admin edited the view's filter
+and every user who had once resized a column silently kept the old filter, with
+nothing anywhere reporting it.
+
+The write now goes through the exported seam `buildPersistedViewBody`. For a
+**system view's overlay** the body is the patch plus `viewKind` when the active
+tab carries one — identity, not content, and the same line the read side already
+draws (`VIEW_OVERLAY_IDENTITY_KEYS`); `object`, `name` and the overlay marker are
+stamped by the adapter. A **saved view's own row** is deliberately still written
+whole: for it the body *is* the view, there is no source underneath it for a
+copied key to shadow, and the write is a whole-document PUT — narrowing it would
+delete the user's view rather than narrow it.
+
+This is the write half of the maintainer's 2026-08-12 ruling (objectstack#7494).
+It was blocked until `columnState` was admitted to the view-metadata surface as
+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.

packages/app-shell/src/views/ObjectView.overlayFilterRecovery.test.tsx

@@ -147,8 +147,10 @@ describe('sanitizeViewOverride — what must NOT change', () => {
     });
 
     it('handles the legacy runtime TRIPLE shape a source view may declare', () => {
-        // `persistViewPatch` copies the whole view body into the overlay, so an
-        // overlay's `filter` can be triples rather than spec rule objects.
+        // `persistViewPatch` USED TO copy the whole view body into the overlay
+        // (objectui#5233 narrowed it to the patch), so an overlay's `filter` can
+        // be triples rather than spec rule objects — on any row written before
+        // that landed, and on a saved view's own body.
         expect(sanitizeViewOverride({ name: 'v', filter: [['status', '=', '']] }))
             .toEqual({ name: 'v' });
         const real = { name: 'v', filter: [['status', 'not_in', ['archived']]] };

packages/app-shell/src/views/ObjectView.setDefaultViewIdentity.test.tsx

@@ -36,10 +36,13 @@
  * FIRST, so any `id` key inside the merged body or the stored override REPLACED
  * it. Both of those are stored metadata documents that really do carry one:
  *
- *   A. `persistViewPatch(viewDef.id, viewDef, patch)` writes the whole tab
+ *   A. `persistViewPatch(viewDef.id, viewDef, patch)` WROTE the whole tab
  *      object — its `id` included — back through `updateViewConfig`, which
  *      stores it under `name: viewId`. `listViewOverrides` reads that row back
  *      and `loadViewOverrides` hands it to the tab list, where its `id` won.
+ *      (objectui#5233 narrowed the overlay branch of that write to the patch,
+ *      so no NEW overlay carries an `id` — every row written before it did,
+ *      which is what this divergence is still about.)
  *   B. A duplicated view copies its source artifact's `id` verbatim into the
  *      view body; `MetadataProvider.applyViewItem` spreads that body into
  *      `objectDef.listViews[<key>]`, so the entry carries the foreign `id`.
@@ -148,8 +151,10 @@ describe('the identity seam — ONE spelling for a view row id (#4211)', () => {
 
 describe('divergence A — a stored override row carrying a foreign `id` (#4211)', () => {
   /**
-   * The row `persistViewPatch` → `updateViewConfig` leaves behind: the whole tab
-   * object, `id` included, stored under `name: <viewId>`. Its `id` was written
+   * The row `persistViewPatch` → `updateViewConfig` USED TO leave behind: the
+   * whole tab object, `id` included, stored under `name: <viewId>`. Since
+   * objectui#5233 an overlay is written as the patch alone, so this is a row
+   * already at rest rather than one a new write produces. Its `id` was written
    * under an earlier tab-id spelling (objectui#3770 re-keyed the default list),
    * so from then on the stored `id` and the row's `name` disagree — and the
    * stored one used to win.

packages/app-shell/src/views/ObjectView.tsx

@@ -325,7 +325,10 @@ export function defaultListColumnsFromObject(
  * Filter entries are matched in both at-rest shapes: spec `ViewFilterRule`
  * objects (`{ field, operator, value }`, what the fold writes) and the legacy
  * runtime triple `[field, operator, value]` a source view may declare and which
- * `persistViewPatch` copies into the overlay along with the rest of the view.
+ * `persistViewPatch` USED TO copy into the overlay along with the rest of the
+ * view — it now stores the patch alone (objectui#5233,
+ * {@link buildPersistedViewBody}), so a triple can only reach this pass from a
+ * row written before that landed, or from a saved view's own body.
  *
  * Whether a matched entry's VALUE counts as supplied is asked of the builder's
  * own {@link isFilterValueComplete}, not of a local predicate (objectstack#8815,
@@ -458,8 +461,11 @@ export async function loadViewOverrides(
  * They disagreed because the metadata side spelled identity `{ id: key,
  * ...body, ...override }` — `id` FIRST, so an `id` key inside the stored body
  * or the stored override silently replaced it. Both are stored documents that
- * really do carry one: `persistViewPatch` writes the whole tab object (its `id`
- * included) back through `updateViewConfig`, and a duplicated view copies its
+ * really do carry one: `persistViewPatch` WROTE the whole tab object (its `id`
+ * included) back through `updateViewConfig` — since objectui#5233 it writes the
+ * patch alone for an overlay ({@link buildPersistedViewBody}) but still the
+ * whole body for a saved view's own row, and every row written before that
+ * landed still carries an `id` — and a duplicated view copies its
  * source artifact's `id` verbatim. `viewEntry` stamps identity LAST at all
  * three sites, so the tab id is now a property of the key the caller looked the
  * view up by, and can never be a property of the data.
@@ -642,6 +648,88 @@ export function dispatchViewPatches(
     );
 }
 
+/**
+ * The body a toolbar toggle persists — **the patch only, for an overlay**
+ * (objectui#5233).
+ *
+ * `persistViewPatch` used to send `{ ...baseViewDef, ...patch }`, so an overlay
+ * written by a mere column drag copied the view's CURRENT effective `filter` —
+ * and its `columns`, `label`, `type`, `isDefault` … — into the stored row. The
+ * display merge is `{ ...source, ...override }`, so that copy then outranked
+ * the source view forever: an admin edited the view's filter and every user who
+ * had once resized a column kept the old one, with nothing reporting it.
+ *
+ * Ruled by the maintainer on 2026-08-12 (objectstack#7494, comment
+ * `5261754173`): 「**`persistViewPatch` 只存 patch,不存 merged base** —— 独立小
+ * 修,现在做:它把写入时的有效 filter 冻进 overlay,导致源视图后续的 filter 变更
+ * 到不了带 overlay 的用户,这与 per-user 之争无关,是纯粹的存储形状错误。」
+ *
+ * ## Why the two branches are not one
+ *
+ * `updateViewConfig` persists through `client.meta.saveItem`, which is a **PUT
+ * of the whole document** — the adapter's own `updateView` reads the current row
+ * and merges onto it precisely because `saveItem` does not ("there is nothing to
+ * merge onto. Fail loudly instead of emitting the partial write"). So what this
+ * function returns is what the row BECOMES, not what is layered onto it:
+ *
+ * - **`isSavedView: false`** — a code-defined *system* view. The row is a
+ *   personalization OVERLAY laid on top of a definition that lives elsewhere;
+ *   nothing in it but the patch is an opinion the user expressed. Store the
+ *   patch. The source view is then free to change under it, which is the card.
+ * - **`isSavedView: true`** — a genuinely user-created *saved* view. The row IS
+ *   the view; there is no source underneath it to shadow, so the ruled harm
+ *   cannot arise here. A patch-only PUT would not narrow this row, it would
+ *   **delete the user's view definition** — its `config`/`columns`/`filter`/
+ *   `label` are not a frozen copy of anything, they are the view. This branch
+ *   therefore still carries the body, byte-identical to the pre-fix write.
+ *
+ * The same `isSavedViewId` classification already decides the switcher's
+ * readonly flag, its five mutating handlers, and whether `updateViewConfig`
+ * stamps the overlay marker — so a row cannot be a saved view for one of them
+ * and an overlay for another.
+ *
+ * ## What the overlay branch keeps besides the patch
+ *
+ * `viewKind` only, and only when the active tab carries one — **identity, not
+ * content**, the same line `VIEW_OVERLAY_IDENTITY_KEYS`
+ * (`@object-ui/data-objectstack`) draws on the read side, so the write now
+ * produces exactly the shape the reader would keep. `object`, `name` and the
+ * overlay marker are stamped by `updateViewConfig` itself; `label` is
+ * deliberately NOT kept (the platform inherits it from the shadowed registry
+ * entry — a stored one is a snapshot of the source view's label at write time,
+ * which is the very class of frozen key this narrowing exists to stop).
+ *
+ * The patch is written verbatim rather than filtered through
+ * `VIEW_OVERLAY_OWNED_KEYS`: every call site passes exactly the one key the
+ * user just changed, and a filter here would SILENTLY drop a sixth key someone
+ * later persists. The ratchet in `ObjectView.overlayPatchOnly.test.ts` is what
+ * makes that drift loud instead.
+ *
+ * ## Consequence for rows written before this shipped
+ *
+ * They are fat at rest and stay that way until touched — already harmless on
+ * read since PR #5272 narrowed the merge (`sanitizeViewOverride` →
+ * `narrowPersonalizationOverlay`). Because this write is a full-document PUT,
+ * the next toolbar toggle on such a view **replaces** the fat row with the thin
+ * one. So the issue's three dispositions land as: tolerate on read (shipped),
+ * strip on next write (here), no migration — and both halves are pinned.
+ *
+ * Extracted from `persistViewPatch` so the write shape is assertable without
+ * mounting the view, the same reason `buildViewTabs`, `setDefaultViewPatches`
+ * and `reorderViewPatches` above are exported.
+ */
+export function buildPersistedViewBody(
+    baseViewDef: Record<string, any> | null | undefined,
+    patch: Record<string, any>,
+    opts: { isSavedView: boolean },
+): Record<string, any> {
+    if (opts.isSavedView) return { ...(baseViewDef || {}), ...patch };
+    const viewKind = (baseViewDef as any)?.viewKind;
+    // Identity is stamped LAST for the same reason `updateViewConfig` stamps
+    // `object`/`name`/the marker last: nothing in the payload can shadow it.
+    return viewKind === undefined ? { ...patch } : { ...patch, viewKind };
+}
+
 export function ObjectView({ dataSource, objects, onEdit, externalRefreshKey }: any) {
     const { objectName } = useParams();
     const { t } = useObjectTranslation();
@@ -763,11 +851,21 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
                 // merely toggled its density (objectui#4227 follow-up,
                 // PM review on PR #4713).
                 const targetIsSavedView = isSavedViewId(savedViewsRef.current, viewIdLocal);
+                // objectui#5233 — for a system view's personalization overlay
+                // this is the PATCH ONLY: the row used to be written as
+                // `{ ...baseViewDef, ...merged }`, which froze the source
+                // view's effective `filter` (and `columns`, `label`, `type`,
+                // `isDefault` …) into it as of the drag. See
+                // `buildPersistedViewBody` for why the saved-view branch is
+                // deliberately NOT narrowed — its row IS the view, and
+                // `saveItem` is a whole-document PUT.
                 Promise.resolve(
-                    dataSource.updateViewConfig(objectName, viewIdLocal, {
-                        ...baseViewDef,
-                        ...merged,
-                    }, { isSavedView: targetIsSavedView })
+                    dataSource.updateViewConfig(
+                        objectName,
+                        viewIdLocal,
+                        buildPersistedViewBody(baseViewDef, merged, { isSavedView: targetIsSavedView }),
+                        { isSavedView: targetIsSavedView },
+                    )
                 ).catch((err: any) => {
                     console.error('[ObjectView] Failed to persist view config:', err);
                 });

packages/data-objectstack/src/index.ts

@@ -1334,18 +1334,37 @@ function isPersonalizationOverlayRow(item: any, spec: any): boolean {
  * production writer of these rows — read off the tree rather than recalled:
  * `rowHeight` (the density toggle, spec-canonical since #2890), `sort`,
  * `hiddenFields`, `columnState` and `inlineEdit`. Nothing else in such a row
- * is an opinion the user expressed; it is a COPY of the source view as it
- * stood at write time, because `persistViewPatch` sends
- * `{ ...baseViewDef, ...patch }` and this adapter persists what it is given.
+ * is an opinion the user expressed; anything else it carries is a COPY of the
+ * source view as it stood at write time, because `persistViewPatch` USED TO
+ * send `{ ...baseViewDef, ...patch }` and this adapter persists what it is
+ * given.
  *
- * That copy is the defect the maintainer ruled on (objectstack#7494, comment
- * 5261754173): an overlay written by a mere column drag freezes the view's
+ * That copy was the defect the maintainer ruled on (objectstack#7494, comment
+ * 5261754173): an overlay written by a mere column drag froze the view's
  * effective `filter` — and its `columns`, `label`, `type`, `isDefault` … — as
  * of that moment, and because the display merge is `{ ...source, ...override }`
- * the frozen copy SHADOWS the source view forever. An admin then edits the
- * view's filter and every user who once resized a column keeps the old one,
+ * the frozen copy SHADOWED the source view forever. An admin then edited the
+ * view's filter and every user who once resized a column kept the old one,
  * with nothing anywhere reporting it.
  *
+ * Both halves of that ruling have now landed, and this adapter's behaviour is
+ * unchanged by either — it still persists what it is given:
+ *
+ * - **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.
+ * - **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
+ *   `@objectstack/spec` 17.1.0): a *system view's* overlay is now written as
+ *   the patch alone, so no new row freezes anything, and because the write is
+ *   a whole-document PUT the next toggle also strips an old fat row. A *saved
+ *   view's* own row is deliberately still written whole — for it the body IS
+ *   the view, not a copy of one.
+ *
+ * A fat row is therefore a legacy shape, not a shape this product still
+ * produces; the list below is still what a reader is allowed to trust from one.
+ *
  * ⛔ Do not grow this list to make some other key "stick" through an overlay.
  * A key that belongs to the view belongs in the view; the overlay is a patch,
  * and a patch that carries the whole document is what this list exists to

packages/data-objectstack/src/listViewOverrides.test.ts

@@ -79,7 +79,7 @@ function makeDS(meta: any) {
 }
 
 /**
- * What ObjectView's `persistViewPatch` actually sends on a density toggle +
+ * What ObjectView's `persistViewPatch` USED TO send on a density toggle +
  * column sort: the view's raw config plus the personalization state. The
  * adapter stamps `object` / `name` on top (and the framework's overlay heals
  * the same identity fields onto rows that lack them — objectstack#2555).

packages/data-objectstack/src/listViews.test.ts

@@ -109,7 +109,7 @@ describe('ObjectStackDataSource.listViews', () => {
   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
-      // copy of the system view's body (`persistViewPatch` spreads the whole
+      // copy of the system view's body (`persistViewPatch` USED TO spread the whole
       // active tab into the write).
       const override = {
         name: 'crm_lead.default', object: 'crm_lead', type: 'grid',

packages/data-objectstack/src/viewOverlayMarker.test.ts

@@ -94,8 +94,9 @@ describe('updateViewConfig stamps the overlay marker (objectui#4227)', () => {
     const { meta } = makeMetaStore();
     const ds = makeDS(meta);
 
-    // `persistViewPatch` spreads the whole active tab into the write; nothing
-    // in that spread should be able to defeat the marker.
+    // `persistViewPatch` USED TO spread the whole active tab into the write
+    // (objectui#5233 narrowed the overlay branch to the patch); nothing a
+    // caller sends — spread or patch — may defeat the marker.
     await ds.updateViewConfig('crm_lead', 'crm_lead.default', { _isOverride: false, rowHeight: 40 } as any);
 
     expect(meta.saveItem).toHaveBeenCalledWith(

packages/data-objectstack/src/viewOverlayPatchOnly.test.ts

@@ -10,11 +10,14 @@
  * objectui#5233 — an overlay row is a PATCH, and only the keys it owns may
  * reach the view it is merged over.
  *
- * `ObjectView.persistViewPatch` sends `{ ...baseViewDef, ...patch }` and this
- * adapter persists what it is given, so a row written by a column drag carries
- * the whole source view — `filter` included — as of that moment. The display
- * merge is `{ ...source, ...override }`, which hands that stale copy authority
- * over the source declaration it was copied from.
+ * `ObjectView.persistViewPatch` USED TO send `{ ...baseViewDef, ...patch }` and
+ * this adapter persists what it is given, so a row written by a column drag
+ * carried the whole source view — `filter` included — as of that moment. The
+ * display merge is `{ ...source, ...override }`, which handed that stale copy
+ * authority over the source declaration it was copied from. The write half has
+ * since landed (`buildPersistedViewBody` in app-shell's `ObjectView`), so a fat
+ * row is now a LEGACY shape — which is exactly what the fixtures below model,
+ * and why this read-side policy still has work to do.
  *
  * {@link narrowPersonalizationOverlay} is the read-side half: the policy for
  * WHAT an overlay may contribute, owned by this package because this package
@@ -78,7 +81,9 @@ function makeDS(meta: any) {
   return ds;
 }
 
-/** What `persistViewPatch` sends for a column drag: the whole tab + one key. */
+/** What `persistViewPatch` USED TO send for a column drag: the whole tab + one
+ *  key. Rows in this shape are still at rest on every install that predates the
+ *  write-side narrowing, which is why this file keeps modelling them. */
 const FAT_WRITE = {
   name: 'crm_lead.default',
   label: 'All Leads',