diff --git a/.changeset/10746-joined-report-clears-binding.md b/.changeset/10746-joined-report-clears-binding.md index 239f022732..c2f76dd8ed 100644 --- a/.changeset/10746-joined-report-clears-binding.md +++ b/.changeset/10746-joined-report-clears-binding.md @@ -23,9 +23,16 @@ unbound report's switch stays the one-key patch it always was. `runtimeFilter` a `drilldown` are kept: the joined branch reads both. Switching between two non-joined types keeps the binding; switching away from `joined` restores nothing — the author re-binds. `blocks` is never touched. The clear holds for the patch and for the save that -follows it. The metadata-admin editor rebuilds its draft as the served draft spread over -`layered.effective` (on load, after each save and after publish), and `effective` is the -published layer, so a report whose PUBLISHED version was bound gets those keys back in the -draft after the first draft save until it is published, and a report already saved `joined` -with stale keys is not repaired on load. Both are the host's draft-over-baseline merge, -objectui#10765. +follows it. At this change the metadata-admin editor rebuilds its draft as the served draft +spread over `layered.effective` (on load, after each save and after publish), and +`effective` is the published layer, so a report whose PUBLISHED version was bound gets those +keys back in the draft after the first draft save until it is published, and a report already +saved `joined` with stale keys is not repaired on load. Both are the host's +draft-over-baseline merge, objectui#10765. + +⚠️ **Dated note, 2026-09-27 — that merge has since been removed — objectui#10765.** Later in +this same release the metadata-admin editor takes a served draft as-is instead of spreading +it over the published layer, so the clear holds across the refresh after the save and through +publish. A report already saved `joined` with stale keys is still not repaired on load; the +author deletes them from the JSON tab. The rest of this entry is kept as the reading of this +change; the objectui#10765 entry states what the editors now do. diff --git a/.changeset/10765-draft-merge-deletion.md b/.changeset/10765-draft-merge-deletion.md new file mode 100644 index 0000000000..3c73164569 --- /dev/null +++ b/.changeset/10765-draft-merge-deletion.md @@ -0,0 +1,25 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(app-shell): the metadata editors take a served draft as-is instead of spreading it over the published layer (objectui#10765) + +The metadata-admin editor (`ResourceEditPage`) rebuilt its draft as the served draft spread +over `layered.effective` — on load, after every draft save and after publish — and the Studio +design surface did the same when it opened an app, a surface, an object or a flow, as did the +package OWD overview when it read and re-saved an object's draft. `effective` is the PUBLISHED +layer, and a spread cannot express deletion: a key the author cleared in the draft (the report +inspector's joined clear of `dataset` / `values` / `rows` / `columns` / `chart` / `order`, an OWD +model set back to unset, any key deleted from the JSON tab) was absent from the first draft PUT, +came back into the editor from the published layer on the refresh after that save, invisibly, +and the next save or publish sent it again. + +The spread existed for a partial draft overlay that never reaches these editors: the server +(`metadata-protocol`) stores a `?mode=draft` body raw and its `?state=draft` read returns that +row raw, and every draft writer sends a whole document. So each site now takes the served draft +as-is when there is one and falls back to `effective` (then `code`) only when no draft exists; +the inherited `type` the spread was protecting is already inside the draft. `config.toDraft` +still normalises whatever is taken, now on the post-publish refresh as well. This removes the +reach limit the objectui#10746 entry states — a bound published report getting its cleared +keys back after the first draft save; a report already saved `joined` with stale keys is still +not repaired on load, and the author deletes them from the JSON tab. diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.servedDraftTakenAsIs-10765.test.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.servedDraftTakenAsIs-10765.test.tsx new file mode 100644 index 0000000000..0bcaf427da --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.servedDraftTakenAsIs-10765.test.tsx @@ -0,0 +1,357 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#10765 — the metadata-admin editor takes a served draft AS-IS. It + * never rebuilds its draft as the served draft spread over `layered.effective`. + * + * ## The defect this file keeps fixed + * + * `effective` is the PUBLISHED layer (the server's `getMetaItemLayered` + * composes it from code plus the `state: 'active'` overlay and never reads a + * draft). A spread cannot express deletion: a key the author cleared in the + * draft is absent from the first draft PUT, comes back into the editor from + * the published layer on the refresh after that save — invisibly — and the + * NEXT save sends it again. The named instance is the report inspector's + * joined clear (objectui#10746): a report whose PUBLISHED version was bound + * got `dataset` / `values` / `rows` / `columns` / `chart` / `order` back under + * `type: 'joined'` after the first autosave, and the spec refuses those on a + * joined report. + * + * ## The premise the fix rests on, and why the double below is shaped this way + * + * The spread existed for a "partial draft overlay". No such overlay reaches + * this editor: the server stores a `?mode=draft` body raw (`saveMetaItem` → + * `repo.put(ref, request.item)` → `JSON.stringify(body)`), its `?state=draft` + * read returns that row raw (or `NO_DRAFT` 404), and a publish promotes the + * draft body raw. So the fake client below is a SERVER DOUBLE that stores + * draft rows raw, serves `effective` as the active row only, and promotes on + * publish — the exact shape the real endpoints have. ⚠️ A double that merged + * on the server's behalf would hide the defect on base and prove nothing. + * + * ## What is red on base and what is a control + * + * Red on base (the spread present): the post-save refresh in the report case + * and the load refresh in the generic case both resurrect the cleared keys + * from the published layer. Controls (green on both): an untouched inherited + * `type` survives (it is inside the served draft), an item with no draft + * shows the baseline unchanged, and the first draft save of a never-drafted + * item is the whole edited document. + * + * The editor's draft is read through a stub canvas that prints it, the same + * seam the sibling suites use for the real save door: everything else — the + * page, its load/save/publish refreshes, the real `ReportDefaultInspector` and + * its type picker — is shipping code. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter } from 'react-router-dom'; + +type Row = Record; + +/** The six keys the report inspector clears when the type becomes `joined`. */ +const JOINED_CLEARED = ['dataset', 'values', 'rows', 'columns', 'chart', 'order'] as const; + +/** A matrix report bound the way the Properties tab binds one — the PUBLISHED version. */ +const PUBLISHED_BOUND_REPORT: Row = { + name: 'pipeline', + label: 'Pipeline', + type: 'matrix', + dataset: 'sales_metrics', + values: ['total_amount'], + rows: ['stage'], + columns: ['close_quarter'], + chart: { type: 'bar', xAxis: 'stage', yAxis: 'total_amount' }, + order: [{ by: 'total_amount' }], +}; + +/** A published page carrying a key the pending draft has deleted. */ +const PUBLISHED_PAGE: Row = { + name: 'home', + label: 'Home', + type: 'home', + template: 'default', + description: 'published description — the draft deleted this key', + regions: [{ name: 'main', components: [{ type: 'text', id: 'b1' }] }], +}; + +/** The same page as the author left it in the draft: `description` gone, `type` untouched. */ +const DRAFT_PAGE_WITHOUT_DESCRIPTION: Row = { + name: 'home', + label: 'Home (draft)', + type: 'home', + template: 'default', + regions: [{ name: 'main', components: [{ type: 'text', id: 'b1' }] }], +}; + +/** What `JSON.stringify` puts on the wire and the server stores: own `undefined` keys drop out. */ +const wire = (doc: unknown): Row => JSON.parse(JSON.stringify(doc)) as Row; +const key = (type: string, name: string) => `${type}/${name}`; + +/** + * The server double. `active` is the published overlay row per item, `drafts` + * the pending draft row per item, both stored as the wire body — raw, never + * merged with anything, exactly as `sys_metadata` holds them. + */ +const server = vi.hoisted(() => ({ + active: new Map>(), + drafts: new Map>(), + saves: [] as Array<{ type: string; name: string; body: Record; opts: Record | undefined }>, + publishes: [] as string[], +})); + +const mockClient = vi.hoisted(() => { + const k = (type: string, name: string) => `${type}/${name}`; + const toWire = (doc: unknown) => JSON.parse(JSON.stringify(doc)) as Record; + return { + list: vi.fn(async () => []), + listDrafts: vi.fn(async () => []), + get: vi.fn(async () => null), + references: vi.fn(async () => []), + /** `GET …/layers`: `effective` is the ACTIVE row only — a draft never feeds it. */ + layered: vi.fn(async (type: string, name: string, _opts?: { packageId?: string }) => { + const eff = server.active.get(k(type, name)) ?? null; + return { + code: null, + overlay: eff, + overlayScope: eff ? 'env' : null, + effective: eff, + editable: true, + deletable: true, + resettable: false, + lock: 'none', + }; + }), + /** `GET …?state=draft`: the stored draft row raw (decorated like the real read), else `NO_DRAFT`. */ + getDraft: vi.fn(async (type: string, name: string, _opts?: { packageId?: string }) => { + const row = server.drafts.get(k(type, name)); + if (!row) { + throw Object.assign(new Error(`No pending draft exists for ${type}/${name}.`), { + code: 'NO_DRAFT', + status: 404, + }); + } + return { type, name, item: { ...row, _diagnostics: { valid: true, errors: [] } } }; + }), + /** `PUT …?mode=draft`: the body lands raw in the draft slot; a publish-mode save lands in the active slot. */ + save: vi.fn(async (type: string, name: string, item: unknown, opts?: Record) => { + const body = toWire(item); + server.saves.push({ type, name, body, opts }); + if (opts?.mode === 'draft') server.drafts.set(k(type, name), body); + else server.active.set(k(type, name), body); + return { type, name, item: body }; + }), + /** `POST …/publish`: promote the draft body raw to active and drain the draft row. */ + publish: vi.fn(async (type: string, name: string, _opts?: Record) => { + const row = server.drafts.get(k(type, name)); + if (!row) { + throw Object.assign(new Error(`No pending draft exists for ${type}/${name} — nothing to publish.`), { + code: 'NO_DRAFT', + status: 404, + }); + } + server.active.set(k(type, name), row); + server.drafts.delete(k(type, name)); + server.publishes.push(k(type, name)); + return { success: true, version: 2 }; + }), + reset: vi.fn(async (type: string, name: string, opts?: { state?: string }) => { + if (opts?.state === 'draft') server.drafts.delete(k(type, name)); + else server.active.delete(k(type, name)); + return {}; + }), + }; +}); + +vi.mock('./useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ + entries: [ + { type: 'report', name: 'report', label: 'Report', allowOrgOverride: true, schema: { required: [] } }, + { type: 'page', name: 'page', label: 'Page', allowOrgOverride: true, schema: { required: [] } }, + ], + }), + }; +}); + +import { MetadataResourceEditPage } from './ResourceEditPage'; +import { registerBuiltinInspectors } from './inspectors'; +import { registerMetadataPreview, getMetadataPreview } from './preview-registry'; + +// The REAL default inspectors — `report` resolves to `ReportDefaultInspector`, +// whose type picker is the producer of the joined clear (objectui#10746). +registerBuiltinInspectors(); + +/** + * Canvas stand-in with two jobs: print the editor's draft (the value under + * test — what the load / save / publish refreshes installed), and hand the + * test a way to dirty the draft so the real autosave door fires. + */ +function StubCanvas({ + draft, + onPatch, +}: { + draft: Record; + onPatch?: (patch: Record) => void; +}) { + return ( + <> + 
{JSON.stringify(draft)}
+ + + ); +} + +const realPreviews = { + report: getMetadataPreview('report'), + page: getMetadataPreview('page'), +}; + +beforeEach(() => { + server.active.clear(); + server.drafts.clear(); + server.saves.length = 0; + server.publishes.length = 0; + for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear(); + registerMetadataPreview('report', StubCanvas as never); + registerMetadataPreview('page', StubCanvas as never); +}); + +afterEach(() => { + cleanup(); + if (realPreviews.report) registerMetadataPreview('report', realPreviews.report); + if (realPreviews.page) registerMetadataPreview('page', realPreviews.page); + window.history.replaceState(null, '', '/'); +}); + +function renderEditor(type: string, name: string) { + window.history.replaceState(null, '', `/metadata/${type}/${name}`); + render( + + + , + ); +} + +/** The editor's draft, as the canvas received it. */ +const editorDraft = (): Row => JSON.parse(screen.getByTestId('editor-draft').textContent ?? '{}') as Row; + +const publishButton = () => screen.getAllByRole('button', { name: /^Publish$/ })[0]!; + +/** Let the real autosave door fire (1500 ms debounce) and settle its refresh. */ +async function awaitSaveCount(n: number) { + await waitFor(() => expect(server.saves.length).toBe(n), { timeout: 8000 }); + await waitFor(() => expect(mockClient.getDraft.mock.calls.length).toBeGreaterThanOrEqual(n + 1), { timeout: 8000 }); +} + +describe('MetadataResourceEditPage — a served draft is taken as-is, never spread over `effective` (objectui#10765)', () => { + it('THE TRIAGE PIN: a report over a bound published version, switched to `joined`, keeps the six keys cleared across the save, the refresh after it and the publish', async () => { + server.active.set(key('report', 'pipeline'), wire(PUBLISHED_BOUND_REPORT)); + renderEditor('report', 'pipeline'); + + // The REAL inspector's type picker (objectui#10746's producer). + const picker = await screen.findByRole('combobox', { name: 'Report type' }, { timeout: 8000 }); + await userEvent.click(picker); + await userEvent.click(await screen.findByRole('option', { name: 'Joined' })); + + // ① The first draft PUT is clean — that half is objectui#10746's, pinned there; restated as the control here. + await awaitSaveCount(1); + const firstPut = server.saves[0]!; + expect(firstPut.opts).toMatchObject({ mode: 'draft' }); + expect(firstPut.body.type).toBe('joined'); + for (const k of JOINED_CLEARED) expect(firstPut.body, `first PUT carries no ${k}`).not.toHaveProperty(k); + + // ② THE DEFECT SITE: the refresh after the save. The served draft has no + // six keys; `effective` (the published, BOUND version) still has all six. + // On base the editor spread the draft over `effective` and every key came back. + await waitFor(() => expect(editorDraft().type).toBe('joined'), { timeout: 8000 }); + const afterSave = editorDraft(); + for (const k of JOINED_CLEARED) { + expect(afterSave, `after the post-save refresh the editor draft has no ${k}`).not.toHaveProperty(k); + } + // CONTROL: the inherited fields the spread claimed to protect are inside the served draft. + expect(afterSave).toMatchObject({ name: 'pipeline', label: 'Pipeline', type: 'joined' }); + + // ③ The next save is the one the defect made send them: dirty the draft on + // an unrelated key and let autosave PUT again. + fireEvent.click(screen.getByRole('button', { name: 'patch the label' })); + await awaitSaveCount(2); + const secondPut = server.saves[1]!; + expect(secondPut.body).toMatchObject({ type: 'joined', label: 'Edited in the designer' }); + for (const k of JOINED_CLEARED) expect(secondPut.body, `second PUT carries no ${k}`).not.toHaveProperty(k); + + // ④ Publish: the promoted body is the draft raw, and the post-publish + // refresh shows it — no six keys anywhere. + await waitFor(() => expect(publishButton()).toBeEnabled(), { timeout: 8000 }); + fireEvent.click(publishButton()); + await waitFor(() => expect(server.publishes).toEqual(['report/pipeline']), { timeout: 8000 }); + const published = server.active.get(key('report', 'pipeline'))!; + expect(published).toMatchObject({ type: 'joined', label: 'Edited in the designer' }); + for (const k of JOINED_CLEARED) expect(published, `published body carries no ${k}`).not.toHaveProperty(k); + await waitFor(() => expect(screen.queryAllByRole('button', { name: /^Publish$/ })).toHaveLength(0), { timeout: 8000 }); + const afterPublish = editorDraft(); + expect(afterPublish.type).toBe('joined'); + for (const k of JOINED_CLEARED) expect(afterPublish, `after publish the editor draft has no ${k}`).not.toHaveProperty(k); + }); + + it('GENERIC: a key the pending draft deleted stays deleted on load, across the next save and its refresh, and through publish — while the untouched inherited `type` survives (CONTROL)', async () => { + server.active.set(key('page', 'home'), wire(PUBLISHED_PAGE)); + server.drafts.set(key('page', 'home'), wire(DRAFT_PAGE_WITHOUT_DESCRIPTION)); + renderEditor('page', 'home'); + + // LOAD SITE — red on base: the spread put the published `description` back. + await waitFor(() => expect(editorDraft().label).toBe('Home (draft)'), { timeout: 8000 }); + const loaded = editorDraft(); + expect(loaded).not.toHaveProperty('description'); + // CONTROL: `type` is inside the served draft, so taking the draft as-is keeps it. + expect(loaded.type).toBe('home'); + expect(loaded).toStrictEqual(wire(DRAFT_PAGE_WITHOUT_DESCRIPTION)); + + // SAVE + REFRESH: an unrelated edit, the real autosave, the real refresh. + fireEvent.click(screen.getByRole('button', { name: 'patch the label' })); + await awaitSaveCount(1); + expect(server.saves[0]!.body).not.toHaveProperty('description'); + await waitFor(() => expect(editorDraft().label).toBe('Edited in the designer'), { timeout: 8000 }); + expect(editorDraft()).not.toHaveProperty('description'); + expect(editorDraft().type).toBe('home'); + + // PUBLISH: the promoted body is the draft raw. + await waitFor(() => expect(publishButton()).toBeEnabled(), { timeout: 8000 }); + fireEvent.click(publishButton()); + await waitFor(() => expect(server.publishes).toEqual(['page/home']), { timeout: 8000 }); + expect(server.active.get(key('page', 'home'))).not.toHaveProperty('description'); + await waitFor(() => expect(screen.queryAllByRole('button', { name: /^Publish$/ })).toHaveLength(0), { timeout: 8000 }); + expect(editorDraft()).not.toHaveProperty('description'); + expect(editorDraft().type).toBe('home'); + }); + + it('EDGE / CONTROL: an item with no pending draft shows the published baseline unchanged', async () => { + server.active.set(key('page', 'home'), wire(PUBLISHED_PAGE)); + renderEditor('page', 'home'); + await waitFor(() => expect(editorDraft().label).toBe('Home'), { timeout: 8000 }); + expect(editorDraft()).toStrictEqual(wire(PUBLISHED_PAGE)); + expect(screen.queryAllByRole('button', { name: /^Publish$/ })).toHaveLength(0); + }); + + it('EDGE / CONTROL: the first draft save of a never-drafted item is the WHOLE edited document, and the refresh shows exactly that', async () => { + server.active.set(key('page', 'home'), wire(PUBLISHED_PAGE)); + renderEditor('page', 'home'); + await waitFor(() => expect(editorDraft().label).toBe('Home'), { timeout: 8000 }); + + fireEvent.click(screen.getByRole('button', { name: 'patch the label' })); + await awaitSaveCount(1); + const firstDraft = server.saves[0]!.body; + expect(server.saves[0]!.opts).toMatchObject({ mode: 'draft' }); + expect(firstDraft).toStrictEqual({ ...wire(PUBLISHED_PAGE), label: 'Edited in the designer' }); + await waitFor(() => expect(editorDraft().label).toBe('Edited in the designer'), { timeout: 8000 }); + expect(editorDraft()).toStrictEqual(firstDraft); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index 08e198fe76..fe2a1c5132 100644 --- a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -1044,16 +1044,16 @@ function MetadataResourceEditPageImpl({ // Prefer the pending draft as the editing baseline — the // operator is mid-flight on this item and should see their // own in-progress state, not the last published version. - // A pending draft overlay can carry only the edited fields, so using - // it wholesale would drop inherited fields that were never touched — - // notably `type`, which section-level `visibleOn` predicates depend on - // (ADR-0047 hides Data Context / Layout when `data.type == 'list'`). - // Merge the draft over the effective baseline so those fields survive; - // the draft still wins for anything it does carry. + // A served draft is the WHOLE document (objectui#10765): the server + // stores a `?mode=draft` body raw and its `?state=draft` read returns + // that row raw, and every writer of a draft sends a full document — + // so `type` and every other inherited field are already inside it. + // ⛔ Never spread the draft over `effective`: a spread cannot express + // deletion, so a key the author cleared came back from the PUBLISHED + // layer on every reload and the next save sent it again. The + // baseline is only for an item that has no pending draft at all. const baseline = (lay.effective ?? lay.code ?? {}) as Record; - const rawInitial: Record = draftReal - ? { ...baseline, ...(draftReal as Record) } - : baseline; + const rawInitial: Record = draftReal ?? baseline; // Normalise the wire shape into the editor's draft shape (e.g. // `view` unwraps an expanded ViewItem's `config` into a // `{ list | form }` family key). No-op for types without a hook. @@ -1513,12 +1513,14 @@ function MetadataResourceEditPageImpl({ setLayered(lay); const draftReal = extractDraftBody(draftResp); setHasDraft(!!draftReal); - // Merge the draft over the effective baseline (see the load effect): - // a partial draft overlay must not drop inherited fields like `type`. + // The served draft is the whole document the save just stored (see the + // load effect, objectui#10765): take it as-is. ⛔ Not spread over + // `effective` — that is the PUBLISHED layer, and a spread would bring a + // key this save deleted straight back into the editor, invisibly, for + // the next save to send. The baseline is only for a save that left no + // draft row to read back. const freshBaseline = (lay.effective ?? itemToSave) as Record; - const rawFresh: Record = draftReal - ? { ...freshBaseline, ...(draftReal as Record) } - : freshBaseline; + const rawFresh: Record = draftReal ?? freshBaseline; // Re-normalise the refreshed wire shape so the editor keeps showing // the canonical draft shape after a save (e.g. the backend re-expands // a view into the ViewItem `config` wrapper). @@ -1690,13 +1692,15 @@ function MetadataResourceEditPageImpl({ setLayered(lay); const draftReal = extractDraftBody(draftResp); setHasDraft(!!draftReal); - // Merge the draft over the effective baseline so a partial draft overlay - // doesn't drop inherited fields like `type` (section visibleOn depends - // on it — ADR-0047). + // After a publish the promoted body IS `effective` and the draft row is + // normally gone; a draft that does remain is a whole document and is + // taken as-is (objectui#10765). ⛔ Not spread over `effective`: the + // spread cannot express deletion and would resurrect a published key + // the draft had cleared. const freshBaseline = (lay.effective ?? draft) as Record; - const fresh: Record = draftReal - ? { ...freshBaseline, ...(draftReal as Record) } - : freshBaseline; + const rawFresh: Record = draftReal ?? freshBaseline; + // Same wire-to-editor normalisation the load and save refreshes apply. + const fresh = config.toDraft ? config.toDraft(rawFresh) : rawFresh; setDraft(fresh); draftSnapshotRef.current = fresh; } catch (err: any) { diff --git a/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.servedDraftTakenAsIs-10765.test.tsx b/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.servedDraftTakenAsIs-10765.test.tsx new file mode 100644 index 0000000000..1317d95d54 --- /dev/null +++ b/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.servedDraftTakenAsIs-10765.test.tsx @@ -0,0 +1,103 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#10765 — the package OWD overview reads and re-saves an object's + * pending draft AS-IS. It never spreads the draft over `layered.effective`. + * + * `effective` is the PUBLISHED layer and a spread cannot express deletion, so + * an OWD model the author had set back to unset in the draft (a real authored + * state — `PackageOwdOverviewPanel.test.tsx` pins that clearing a model DROPS + * the key) read as still set from the published row, and the next OWD save + * wrote that published value, plus every other key the draft had deleted, + * back into the object's draft. Same host-level cause as the metadata-admin + * editor's merge; the panel has two sites — the list load that derives each + * row's values and the save that splices the edit into the body — and both + * are pinned here, one assertion each, red on base. + * + * The client is a server double: `effective` is the active row, `getDraft` + * serves the stored draft row raw, `save` records the wire body. + */ + +import '@testing-library/jest-dom/vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import { PackageOwdOverviewPanel } from './PackageOwdOverviewPanel'; + +/** The published object: both OWD models set, plus a description. */ +const PUBLISHED = { + name: 'crm_contact', + label: 'Contact', + sharingModel: 'private', + externalSharingModel: 'private', + description: 'published description — the draft deleted this key', +}; + +/** The pending draft as the author left it: both models unset, description deleted. */ +const SERVED_DRAFT = { + name: 'crm_contact', + label: 'Contact', + _diagnostics: { valid: true, errors: [] }, +}; + +const saved: Array<{ name: string; body: Record; opts: Record | undefined }> = []; + +function makeClient() { + return { + list: async (type: string) => (type === 'object' ? [{ name: PUBLISHED.name, label: PUBLISHED.label }] : []), + listDrafts: async () => [], + layered: async (_t: string, name: string) => ({ + effective: name === PUBLISHED.name ? PUBLISHED : {}, + code: null, + }), + getDraft: async (_t: string, name: string) => + name === PUBLISHED.name ? { type: 'object', name, item: SERVED_DRAFT } : null, + save: async (_t: string, name: string, body: Record, opts?: Record) => { + saved.push({ name, body: JSON.parse(JSON.stringify(body)) as Record, opts }); + return body; + }, + } as any; +} + +beforeEach(() => { + saved.length = 0; + Element.prototype.scrollIntoView = vi.fn(); +}); +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe('PackageOwdOverviewPanel — the pending draft is read and re-saved as-is (objectui#10765)', () => { + it('LIST LOAD: a model the draft cleared reads as unset, not as the published value', async () => { + render(); + await screen.findByTestId('owd-row-crm_contact'); + + const internal = screen.getByTestId('owd-internal-crm_contact') as HTMLSelectElement; + // Red on base: the spread over `effective` showed `private` from the published row. + expect(internal.value).toBe(''); + }); + + it('SAVE: an edit lands on the draft alone — the published model and description the draft deleted do not come back', async () => { + render(); + await screen.findByTestId('owd-row-crm_contact'); + + fireEvent.change(screen.getByTestId('owd-internal-crm_contact'), { target: { value: 'public_read' } }); + fireEvent.click(screen.getByTestId('owd-save')); + + await waitFor(() => expect(saved).toHaveLength(1)); + const { name, body, opts } = saved[0]!; + expect(name).toBe('crm_contact'); + expect(opts).toMatchObject({ mode: 'draft', packageId: 'com.example.showcase' }); + expect(body.sharingModel).toBe('public_read'); + // Red on base: both came back from `effective` through the spread. + expect(body).not.toHaveProperty('externalSharingModel'); + expect(body).not.toHaveProperty('description'); + // The draft's own keys travel; the read decorations do not (objectui#8181). + expect(body.label).toBe('Contact'); + expect(body).not.toHaveProperty('_diagnostics'); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx b/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx index 72fa1eb906..93f20bbdd5 100644 --- a/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx +++ b/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx @@ -39,7 +39,10 @@ import { isExternalWider, deriveMasterObject } from './owd-sharing.js'; import { toast } from 'sonner'; -/** A single object's loaded OWD baseline (already merged over any pending draft). */ +/** + * A single object's loaded OWD values — read from its pending draft as-is when + * one exists, else from its layered baseline (objectui#10765). + */ interface OwdRow { name: string; label: string; @@ -143,8 +146,9 @@ export function PackageOwdOverviewPanel({ // The list row carries `sharingModel` but not reliably `fields` (needed // for the master link), and never the pending-draft value — so read each - // object's merged body (layered ∪ its draft), exactly as the per-object - // Settings tab does, and derive every column from that one source. + // object's body (its pending draft as-is, else its layered baseline), + // exactly as the per-object Settings tab does, and derive every column + // from that one source. const built = await Promise.all( names.map(async (name): Promise => { const [layRaw, draftResp] = await Promise.all([ @@ -154,7 +158,11 @@ export function PackageOwdOverviewPanel({ const lay = (layRaw ?? {}) as { effective?: Record; code?: Record }; const baseline = (lay.effective ?? lay.code ?? {}) as Record; const draftBody = extractDraftBody(draftResp); - const body = draftBody ? { ...baseline, ...draftBody } : baseline; + // The pending draft is the whole document: read it as-is, and the + // published baseline only when no draft exists (objectui#10765). A + // spread over `effective` showed a model the draft had cleared as + // still set, from the published layer. + const body = draftBody ?? baseline; const internal = typeof body.sharingModel === 'string' ? body.sharingModel : ''; return { name, @@ -233,7 +241,7 @@ export function PackageOwdOverviewPanel({ setSaveError(null); try { // Each changed row lands as that object's package-scoped DRAFT: read the - // fresh merged body, apply just the OWD pair (unset → drop the key), and + // fresh body (below), apply just the OWD pair (unset → drop the key), and // save. Identical to the per-object Settings write, N times — publish // then goes through the same package security-domain gate unchanged. for (const s of changed) { @@ -244,7 +252,11 @@ export function PackageOwdOverviewPanel({ const lay = (layRaw ?? {}) as { effective?: Record; code?: Record }; const baseline = (lay.effective ?? lay.code ?? {}) as Record; const draftBody = extractDraftBody(draftResp); - const next: Record = { ...baseline, ...(draftBody ?? {}) }; + // The pending draft is the whole document — the edit lands on it as-is, + // and on the published baseline only when no draft exists + // (objectui#10765): a spread over `effective` would put back every key + // the draft had deleted and save them again as this object's draft. + const next: Record = { ...(draftBody ?? baseline) }; if (s.edit.internal) next.sharingModel = s.edit.internal; else delete next.sharingModel; if (s.edit.external) next.externalSharingModel = s.edit.external; diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.servedDraftTakenAsIs-10765.test.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.servedDraftTakenAsIs-10765.test.tsx new file mode 100644 index 0000000000..4f4a6b0bdf --- /dev/null +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.servedDraftTakenAsIs-10765.test.tsx @@ -0,0 +1,251 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#10765 — the Studio design surface takes a served draft AS-IS at + * every one of its load sites. It never rebuilds a draft as the served draft + * spread over `layered.effective`. + * + * `effective` is the PUBLISHED layer and a spread cannot express deletion: a + * key the author deleted in the draft came back into the design surface from + * the published layer on every open, invisibly, and the next draft save sent + * it again. The metadata-admin editor's three sites are pinned in + * `ResourceEditPage.servedDraftTakenAsIs-10765.test.tsx`; this file pins the + * design surface's FOUR — the app the Interfaces pillar opens, the surface + * leaf it opens under that app, the object the Data pillar opens, and the flow + * the Automations pillar opens. One red-on-base assertion per site, each on a + * key the served draft lacks and the published row carries. ⛔ No per-type + * exception (triage `5853761414`). + * + * The client is a server double: `effective` is the active row, `getDraft` + * serves the stored draft row raw, `save` records the wire body. The canvas + * for `page` and `flow` is a stub that prints the draft it received — the + * value under test — so nothing in the pillars themselves is stubbed. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter } from 'react-router-dom'; + +const PKG = 'com.acme.app'; + +const NAV = [{ id: 'nav_home', type: 'page', label: 'Home', pageName: 'home' }]; + +/** The published app carries a `label`; its pending draft deleted it. */ +const PUBLISHED_APP = { name: 'acme_app', label: 'Acme', navigation: NAV }; +const DRAFT_APP_WITHOUT_LABEL = { name: 'acme_app', navigation: NAV }; + +/** The published page carries a `description`; its pending draft deleted it. */ +const PUBLISHED_PAGE = { name: 'home', label: 'Home', kind: 'blocks', blocks: [], description: 'published description' }; +const DRAFT_PAGE_WITHOUT_DESCRIPTION = { name: 'home', label: 'Home (draft)', kind: 'blocks', blocks: [] }; + +/** The published object has an OWD model set; its pending draft set it back to unset (the key is gone). */ +const FIELDS = [{ name: 'title', label: 'Title', type: 'text' }]; +const PUBLISHED_OBJECT = { name: 'crm_contact', label: 'Contact', sharingModel: 'private', fields: FIELDS }; +const DRAFT_OBJECT_WITHOUT_OWD = { name: 'crm_contact', label: 'Contact', fields: FIELDS }; + +/** The published flow carries a `description`; its pending draft deleted it. */ +const PUBLISHED_FLOW = { name: 'notify_owner', label: 'Notify owner', status: 'active', description: 'published description', nodes: [] }; +const DRAFT_FLOW_WITHOUT_DESCRIPTION = { name: 'notify_owner', label: 'Notify owner', status: 'active', nodes: [] }; + +const server = vi.hoisted(() => ({ + active: new Map>(), + drafts: new Map>(), + saves: [] as Array<{ type: string; name: string; body: Record; opts: Record | undefined }>, +})); + +const mockClient = vi.hoisted(() => { + const k = (type: string, name: string) => `${type}/${name}`; + const toWire = (doc: unknown) => JSON.parse(JSON.stringify(doc)) as Record; + return { + /** The published items of a type — what the rails list. */ + list: vi.fn(async (type: string) => + [...server.active.entries()] + .filter(([key]) => key.startsWith(`${type}/`)) + .map(([, row]) => ({ name: row.name, label: row.label ?? row.name })), + ), + listDrafts: vi.fn(async () => []), + listTypes: vi.fn(async () => ({ entries: [] })), + get: vi.fn(async () => null), + references: vi.fn(async () => []), + /** `GET …/layers`: `effective` is the ACTIVE row only — a draft never feeds it. */ + layered: vi.fn(async (type: string, name: string) => { + const eff = server.active.get(k(type, name)) ?? null; + return { code: null, overlay: eff, overlayScope: eff ? 'env' : null, effective: eff, editable: true, deletable: true, resettable: false, lock: 'none' }; + }), + /** `GET …?state=draft`: the stored draft row raw (decorated like the real read), else `NO_DRAFT`. */ + getDraft: vi.fn(async (type: string, name: string) => { + const row = server.drafts.get(k(type, name)); + if (!row) throw Object.assign(new Error(`No pending draft exists for ${type}/${name}.`), { code: 'NO_DRAFT', status: 404 }); + return { type, name, item: { ...row, _diagnostics: { valid: true, errors: [] } } }; + }), + /** `PUT …?mode=draft`: the body lands raw in the draft slot. */ + save: vi.fn(async (type: string, name: string, item: unknown, opts?: Record) => { + const body = toWire(item); + server.saves.push({ type, name, body, opts }); + if (opts?.mode === 'draft') server.drafts.set(k(type, name), body); + else server.active.set(k(type, name), body); + return { type, name, item: body }; + }), + publish: vi.fn(async () => ({ success: true })), + reset: vi.fn(async () => ({})), + }; +}); + +vi.mock('../metadata-admin/useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ entries: [] }), + }; +}); + +vi.mock('./packages-io', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, fetchPackages: vi.fn(async () => []) }; +}); + +vi.mock('@object-ui/react', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useAdapter: () => dataSource }; +}); + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import { InterfacesPillar, DataPillar, AutomationsPillar } from './StudioDesignSurface'; +import { SurfaceDeepLinkProvider } from './surfaceDeepLinkChannel'; +import { createEmptyDataSource, failOnAbsorbedFetchError } from './__tests__/emptyDataSource'; +import { registerMetadataPreview, getMetadataPreview } from '../metadata-admin/preview-registry'; + +const dataSource = createEmptyDataSource(); +failOnAbsorbedFetchError(); + +// The Automations pillar probes `GET /api/v1/automation/_status` through the +// global `fetch` (no client seam). One double at MODULE scope, never torn down +// — the shape `vitest.setup.network-escape-guard.ts` prescribes, because the +// probe is fire-and-forget and can land after a test body returns. +vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('null', { status: 404, headers: { 'content-type': 'application/json' } })), +); + +/** Canvas stand-in: prints the draft the pillar handed it — the value under test. */ +function StubCanvas({ draft }: { draft: Record }) { + return 
{JSON.stringify(draft)}
; +} + +const realPreviews = { page: getMetadataPreview('page'), flow: getMetadataPreview('flow') }; + +const key = (type: string, name: string) => `${type}/${name}`; +const wire = (doc: unknown) => JSON.parse(JSON.stringify(doc)) as Record; +const surfaceDraft = () => JSON.parse(screen.getByTestId('surface-draft').textContent ?? '{}') as Record; + +beforeEach(() => { + server.active.clear(); + server.drafts.clear(); + server.saves.length = 0; + for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear(); + registerMetadataPreview('page', StubCanvas as never); + registerMetadataPreview('flow', StubCanvas as never); +}); + +afterEach(() => { + cleanup(); + if (realPreviews.page) registerMetadataPreview('page', realPreviews.page); + if (realPreviews.flow) registerMetadataPreview('flow', realPreviews.flow); +}); + +describe('StudioDesignSurface — every load site takes the served draft as-is, never spread over `effective` (objectui#10765)', () => { + it('Interfaces pillar: the APP draft and the SURFACE draft it opens are the served drafts, not the published rows underneath them', async () => { + server.active.set(key('app', 'acme_app'), wire(PUBLISHED_APP)); + server.drafts.set(key('app', 'acme_app'), wire(DRAFT_APP_WITHOUT_LABEL)); + server.active.set(key('page', 'home'), wire(PUBLISHED_PAGE)); + server.drafts.set(key('page', 'home'), wire(DRAFT_PAGE_WITHOUT_DESCRIPTION)); + + render( + + + , + ); + + // APP SITE — red on base: the spread kept the published `label`, so the + // heading read "Acme · Navigation". The draft deleted `label`, and the + // pillar falls back to the app's `name`. + await screen.findByText('acme_app · Navigation', undefined, { timeout: 8000 }); + expect(screen.queryByText('Acme · Navigation')).toBeNull(); + + // SURFACE SITE — red on base: the spread put the published `description` + // back under the draft's own `label`. + await waitFor(() => expect(surfaceDraft().label).toBe('Home (draft)'), { timeout: 8000 }); + expect(surfaceDraft()).not.toHaveProperty('description'); + expect(surfaceDraft()).toStrictEqual(wire(DRAFT_PAGE_WITHOUT_DESCRIPTION)); + }); + + it('Interfaces pillar, CONTROL: with no pending draft the surface shows the published baseline unchanged', async () => { + server.active.set(key('app', 'acme_app'), wire(PUBLISHED_APP)); + server.active.set(key('page', 'home'), wire(PUBLISHED_PAGE)); + + render( + + + , + ); + + await screen.findByText('Acme · Navigation', undefined, { timeout: 8000 }); + await waitFor(() => expect(surfaceDraft().label).toBe('Home'), { timeout: 8000 }); + expect(surfaceDraft()).toStrictEqual(wire(PUBLISHED_PAGE)); + }); + + it('Data pillar: an OWD model the object draft set back to unset reads as unset, not as the published value', async () => { + server.active.set(key('object', 'crm_contact'), wire(PUBLISHED_OBJECT)); + server.drafts.set(key('object', 'crm_contact'), wire(DRAFT_OBJECT_WITHOUT_OWD)); + + render( + + + + + , + ); + + // The first object auto-opens; the Settings panel lives under "Advanced". + const advanced = await screen.findByTestId('data-tabs-advanced', undefined, { timeout: 8000 }); + await userEvent.click(advanced); + await userEvent.click(await screen.findByRole('menuitem', { name: 'Settings' })); + + const internal = (await screen.findByTestId('owd-internal-select', undefined, { timeout: 8000 })) as HTMLSelectElement; + // Red on base: the spread over `effective` showed `private` from the published row. + expect(internal.value).toBe(''); + }); + + it('Automations pillar: the flow draft is the served draft, and the enable switch saves exactly that draft — no published key comes back', async () => { + server.active.set(key('flow', 'notify_owner'), wire(PUBLISHED_FLOW)); + server.drafts.set(key('flow', 'notify_owner'), wire(DRAFT_FLOW_WITHOUT_DESCRIPTION)); + + render( + + + , + ); + + // LOAD SITE — red on base: the spread put the published `description` back. + await waitFor(() => expect(surfaceDraft().name).toBe('notify_owner'), { timeout: 8000 }); + expect(surfaceDraft()).not.toHaveProperty('description'); + expect(surfaceDraft()).toStrictEqual(wire(DRAFT_FLOW_WITHOUT_DESCRIPTION)); + + // The next save is what the defect made carry the key: flip the switch, + // which saves `{ ...draft, status }` at once. + const toggle = await screen.findByRole('switch', undefined, { timeout: 8000 }); + await waitFor(() => expect(toggle).toBeEnabled(), { timeout: 8000 }); + fireEvent.click(toggle); + await waitFor(() => expect(server.saves).toHaveLength(1), { timeout: 8000 }); + const { type, name, body, opts } = server.saves[0]!; + expect([type, name]).toEqual(['flow', 'notify_owner']); + expect(opts).toMatchObject({ mode: 'draft', packageId: PKG }); + expect(body.status).toBe('obsolete'); + expect(body).not.toHaveProperty('description'); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index 469bdf769c..6dc6b4a617 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -1824,7 +1824,10 @@ export function InterfacesPillar({ const lay = layRaw as { effective?: Record; code?: Record }; const eff = (lay.effective ?? lay.code ?? {}) as Record; const appDraftBody = extractDraftBody(appDraftResp); - const body = appDraftBody ? { ...eff, ...appDraftBody } : eff; + // A served draft is the whole document — taken as-is, never spread + // over the published layer (objectui#10765; the rule is stated once + // at `ResourceEditPage`'s load effect). Baseline only when no draft. + const body = appDraftBody ?? eff; if (typeof body.label === 'string' || typeof body.name === 'string') { setAppLabel(String(body.label ?? body.name ?? label)); } @@ -1951,7 +1954,9 @@ export function InterfacesPillar({ (lay as { code?: unknown }).code ?? {}) as Record; const body = extractDraftBody(draftResp); - setDraft(body ? { ...baseline, ...body } : baseline); + // Served draft as-is, baseline only without one (objectui#10765): a + // spread over `effective` resurrects every key the draft deleted. + setDraft(body ?? baseline); setHasDraft(!!body); setIfDirty(false); } catch (e) { @@ -2961,12 +2966,14 @@ export function DataPillar({ const lay = layRaw as { effective?: Record; code?: Record }; const baseline = (lay.effective ?? lay.code ?? {}) as Record; const draftBody = extractDraftBody(draftResp); - setObjDraft(draftBody ? { ...baseline, ...draftBody } : baseline); + // Served draft as-is, baseline only without one (objectui#10765). + setObjDraft(draftBody ?? baseline); setHasDraft(!!draftBody); setHasBaseline(!!(lay.effective ?? lay.code)); // The projection baseline: the object as the SERVER has it. `objDraft` - // below merges the draft on top, which is right for the editor and - // wrong for a `select`. + // above is the pending draft as-is when one exists, which is right for + // the editor and wrong for a `select`: a draft-only field is not a + // column the data API can answer yet (see `gridColumns`). setPublishedFieldNames(new Set(readFields(baseline.fields).entries.map((e) => e.name))); } catch (e) { if (!cancelled) setError(formatMetadataError(e)); @@ -4012,7 +4019,8 @@ export function AutomationsPillar({ const lay = layRaw as { effective?: Record; code?: Record }; const baseline = (lay.effective ?? lay.code ?? {}) as Record; const draftBody = extractDraftBody(draftResp); - setDraft(draftBody ? { ...baseline, ...draftBody } : baseline); + // Served draft as-is, baseline only without one (objectui#10765). + setDraft(draftBody ?? baseline); setHasDraft(!!draftBody); } catch (e) { if (!cancelled) setError(formatMetadataError(e));