diff --git a/.changeset/12027-embedded-draft-save.md b/.changeset/12027-embedded-draft-save.md new file mode 100644 index 0000000000..278d774d74 --- /dev/null +++ b/.changeset/12027-embedded-draft-save.md @@ -0,0 +1,14 @@ +--- +'@object-ui/app-shell': patch +--- + +The embedded item editor ("Save into object", opened from a metadata item's Related drawer) now saves into the parent's draft (objectui#12027). + +It saves an item such as `object.fields.amount` by writing the whole parent back. It used to base that write on the parent's published version and send it in publish mode. For a parent that exists only as a draft, there is no published version: the layered read answers 404 by design, and the client reads that as every layer empty. So the save sent a publish-mode write of a one-field stub, without the draft's name, label or other fields, and then showed "Saved.". + +- **A parent with a pending draft:** the save reads that draft, splices the item into it, and writes it back as a draft (`mode=draft`). Every other field the draft holds is kept, and nothing is written live. This covers a draft-only parent and a published parent whose draft has moved on. +- **A published parent with no draft:** the save reads the published body and writes it as before. +- **A parent with neither:** the save is refused with an error ("Failed to load TYPE/NAME: (not found)"), and nothing is sent. +- A failed draft read is the save's error, and nothing is sent. A refused draft save still marks the sub-form field its issues name. + +Nothing is added to the package entry: no export, prop, type member or language-pack key. The refusal reuses existing strings from the metadata-admin designer's own table. diff --git a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.draftSave-12027.test.tsx b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.draftSave-12027.test.tsx new file mode 100644 index 0000000000..d28769dc69 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.draftSave-12027.test.tsx @@ -0,0 +1,269 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#12027 — the embedded item editor saves into the parent's DRAFT. + * + * `EmbeddedItemEditor` edits an item that lives inside a parent body + * (`object.fields.amount`) and saves it by writing the whole parent back. It + * used to base that write on the parent's published layers and send it in + * publish mode. For a parent that exists only as a draft, `/layers` answers 404 + * by design (objectstack-ai/objectstack#22397), the client resolves that as + * every layer `null`, and the save sent a publish-mode PUT of a one-field stub, + * then said "Saved.". + * + * The REAL editor over a REAL `MetadataClient`, whose transport is an + * in-memory server answering as the framework does: the pending-drafts ledger, + * `?state=draft` (a decorated draft), `/layers` (404 for a parent with no + * published row), and a PUT that stores into the draft row under `?mode=draft` + * and into the published row otherwise. + * + * Pinned: + * - a draft-only parent's edit lands in its draft, every other field kept, and + * no publish-mode request is sent; + * - a published parent with a pending draft: the edit lands in the draft, the + * draft's own edits are kept, and the published row is untouched; + * - CONTROL: a published parent with no draft saves as it did before this card + * (one publish-mode PUT of the effective body with the item spliced in); + * - a parent with neither a draft nor a published version is refused with an + * error state: nothing is PUT, and no "Saved."; + * - a draft-read failure is the save's error, and nothing is PUT; + * - a 422 on a draft-mode save still lands its issue on the sub-form field. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MetadataClient } from '@object-ui/data-objectstack'; + +const FIELD_SCHEMA = { + type: 'object', + properties: { + label: { type: 'string', title: 'Label' }, + description: { type: 'string', title: 'Description' }, + }, +} satisfies Record; + +type Row = Record; + +const server = { + active: new Map(), + drafts: new Map(), + requests: [] as Array<{ method: string; path: string; search: string; status: number }>, + puts: [] as Array<{ search: string; body: Row }>, + /** Rows whose `?state=draft` read fails with a 5xx. */ + failDraftRead: new Set(), + /** Rows whose draft-mode PUT is refused with this issue. */ + refuseDraftPut: new Map(), +}; + +const json = (status: number, body: unknown) => + new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } }); + +async function serve(input: RequestInfo | URL, init?: RequestInit): Promise { + const method = (init?.method ?? 'GET').toUpperCase(); + const url = new URL(String(input), 'http://console.test'); + const q = url.searchParams; + let status = 200; + let body: unknown = []; + const meta = url.pathname.match(/^\/api\/v1\/meta\/(.+)$/); + if (meta) { + const seg = meta[1].split('/').map(decodeURIComponent); + const key = `${seg[0]}/${seg[1]}`; + if (seg[0] === '_drafts') { + body = { drafts: [...server.drafts.keys()].map((k) => ({ type: k.split('/')[0], name: k.split('/')[1], packageId: null })) }; + } else if (seg.length === 3 && seg[2] === 'layers') { + const row = server.active.get(key); + if (row) body = { code: null, overlay: row, overlayScope: 'env', effective: row }; + else [status, body] = [404, { error: { code: 'NOT_FOUND', message: 'absent' } }]; + } else if (seg.length === 2 && method === 'PUT') { + const item = JSON.parse(String(init?.body ?? '{}')) as Row; + server.puts.push({ search: url.search, body: item }); + const draftMode = q.get('mode') === 'draft'; + const refusal = draftMode ? server.refuseDraftPut.get(key) : undefined; + if (refusal) { + [status, body] = [422, { + error: `[invalid_metadata] ${key} failed spec validation: 1 issue — ${refusal.path} [custom]`, + code: 'INVALID_METADATA', + issues: [{ ...refusal, code: 'custom' }], + }]; + } else { + if (draftMode) server.drafts.set(key, item); + else server.active.set(key, item); + body = { type: seg[0], name: seg[1], state: draftMode ? 'draft' : 'active' }; + } + } else if (seg.length === 2) { + const draftRead = q.get('state') === 'draft'; + const row = (draftRead ? server.drafts : server.active).get(key); + if (draftRead && server.failDraftRead.has(key)) { + [status, body] = [500, { error: { code: 'INTERNAL_ERROR', message: 'draft store unavailable' } }]; + } else if (row) { + // A served draft is DECORATED (`_draft`); the editor must strip it. + body = { type: seg[0], name: seg[1], item: draftRead ? { ...row, _draft: true } : row }; + } else { + [status, body] = [404, { error: { code: draftRead ? 'NO_DRAFT' : 'NOT_FOUND', message: 'absent' } }]; + } + } + } + server.requests.push({ method, path: url.pathname, search: url.search, status }); + return json(status, body); +} + +const client = new MetadataClient({ baseUrl: '', fetch: vi.fn(serve) as unknown as typeof fetch }); + +vi.mock('./useMetadata', () => ({ + useMetadataClient: () => client, + useMetadataTypes: () => ({ + loading: false, + error: null, + entries: [{ type: 'field', label: 'Field', allowOrgOverride: true, schema: FIELD_SCHEMA }], + }), +})); + +import { EmbeddedItemEditor } from './EmbeddedItemEditor'; + +/** The draft an author is mid-flight on: never published. */ +const DRAFT_ORDER: Row = { + name: 'sales_order', + label: 'Sales Order', + description: 'Orders taken by the field team', + fields: { + amount: { type: 'number', label: 'Amount' }, + region: { type: 'text', label: 'Region' }, + }, +}; + +/** The published version, and a pending draft that has moved past it. */ +const PUBLISHED_ORDER: Row = { + name: 'sales_order', + label: 'Sales Order', + fields: { amount: { type: 'number', label: 'Amount' } }, +}; +const PENDING_ORDER: Row = { + name: 'sales_order', + label: 'Sales Orders (renamed in the draft)', + fields: { + amount: { type: 'number', label: 'Amount' }, + channel: { type: 'text', label: 'Channel' }, + }, +}; + +const EDITED_AMOUNT = { type: 'number', label: 'Order amount' }; + +beforeEach(() => { + server.active.clear(); + server.drafts.clear(); + server.requests = []; + server.puts = []; + server.failDraftRead.clear(); + server.refuseDraftPut.clear(); +}); +afterEach(cleanup); + +async function settle() { + await act(async () => { + for (let i = 0; i < 20; i++) await new Promise((r) => setTimeout(r, 0)); + }); +} + +function editAmountLabelAndSave() { + render( + , + ); + fireEvent.change(screen.getByRole('textbox', { name: 'Label' }), { target: { value: 'Order amount' } }); + fireEvent.click(saveButton()); +} + +const saveButton = () => screen.getByRole('button', { name: /save into object/i }); +const publishModePuts = () => server.puts.filter((p) => !new URLSearchParams(p.search).has('mode')); + +describe('EmbeddedItemEditor — an embedded edit saves into the parent\'s draft (objectui#12027)', () => { + it('a draft-only parent: the edit lands in its draft with every other field kept, and no publish-mode request is sent', async () => { + server.drafts.set('object/sales_order', DRAFT_ORDER); + editAmountLabelAndSave(); + + expect(await screen.findByText('Saved.')).toBeInTheDocument(); + expect(server.puts).toHaveLength(1); + expect(server.puts[0].search).toBe('?mode=draft'); + expect(publishModePuts()).toEqual([]); + // The whole draft, the one item replaced, and the read decoration gone. + expect(server.puts[0].body).toEqual({ + ...DRAFT_ORDER, + fields: { ...(DRAFT_ORDER.fields as Row), amount: EDITED_AMOUNT }, + }); + expect(server.active.has('object/sales_order')).toBe(false); + }); + + it('a published parent with a pending draft: the edit lands in the draft, the draft\'s own edits are kept, the published row is untouched', async () => { + server.active.set('object/sales_order', PUBLISHED_ORDER); + server.drafts.set('object/sales_order', PENDING_ORDER); + editAmountLabelAndSave(); + + expect(await screen.findByText('Saved.')).toBeInTheDocument(); + expect(publishModePuts()).toEqual([]); + expect(server.drafts.get('object/sales_order')).toEqual({ + ...PENDING_ORDER, + fields: { ...(PENDING_ORDER.fields as Row), amount: EDITED_AMOUNT }, + }); + expect(server.active.get('object/sales_order')).toEqual(PUBLISHED_ORDER); + }); + + it('CONTROL: a published parent with no draft saves as before — one publish-mode PUT of the effective body with the item spliced in', async () => { + server.active.set('object/sales_order', PUBLISHED_ORDER); + editAmountLabelAndSave(); + + expect(await screen.findByText('Saved.')).toBeInTheDocument(); + expect(server.puts).toHaveLength(1); + expect(server.puts[0].search).toBe(''); + expect(server.puts[0].body).toEqual({ + ...PUBLISHED_ORDER, + fields: { amount: EDITED_AMOUNT }, + }); + expect(server.drafts.has('object/sales_order')).toBe(false); + }); + + it('a parent with neither a draft nor a published version is refused with an error state: nothing is PUT, and no "Saved."', async () => { + editAmountLabelAndSave(); + + expect(await screen.findByText('Failed to load object/sales_order: (not found)')).toBeInTheDocument(); + await waitFor(() => expect(saveButton()).not.toBeDisabled()); + await settle(); + expect(server.puts).toEqual([]); + expect(screen.queryByText('Saved.')).toBeNull(); + }); + + it('a draft-read failure is the save\'s error, and nothing is PUT', async () => { + server.active.set('object/sales_order', PUBLISHED_ORDER); + server.drafts.set('object/sales_order', PENDING_ORDER); + server.failDraftRead.add('object/sales_order'); + editAmountLabelAndSave(); + + expect(await screen.findByText('draft store unavailable')).toBeInTheDocument(); + await waitFor(() => expect(saveButton()).not.toBeDisabled()); + expect(server.puts).toEqual([]); + expect(screen.queryByText('Saved.')).toBeNull(); + }); + + it('a 422 on the draft-mode save lands its issue on the sub-form field', async () => { + server.drafts.set('object/sales_order', DRAFT_ORDER); + server.refuseDraftPut.set('object/sales_order', { + path: 'fields.amount.label', + message: 'A field label must not be empty', + }); + editAmountLabelAndSave(); + + expect(await screen.findByText('Validation failed (1 issue).')).toBeInTheDocument(); + expect(screen.getByText('A field label must not be empty')).toBeInTheDocument(); + expect(server.puts).toHaveLength(1); + expect(server.puts[0].search).toBe('?mode=draft'); + expect(screen.queryByText('Saved.')).toBeNull(); + expect(server.drafts.get('object/sales_order')).toEqual(DRAFT_ORDER); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.layersRead-11799.test.tsx b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.layersRead-11799.test.tsx index 52dfe7302b..8e0d2a2646 100644 --- a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.layersRead-11799.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.layersRead-11799.test.tsx @@ -23,12 +23,12 @@ * - CONTROL: a failed `/layers` read (a 5xx) is the save's error, and nothing * is PUT. * - * Not covered, said here so it does not read as covered: what a SAVE does for - * a draft-only parent. `MetadataClient.layered()` resolves the 404 as an - * envelope with every layer null, so the read itself raises nothing; the save - * then splices the item into an empty body and PUTs the parent in publish - * mode. That PUT is outside objectui#11799 (its claim excludes it), and it is - * not pinned here, so this file does not hold that body in place. + * Not covered here: what a SAVE does for a draft-only parent. That is + * objectui#12027's write side, pinned in + * `EmbeddedItemEditor.draftSave-12027.test.tsx`: the save reads the parent's + * pending draft first and writes it back in draft mode, so it sends no + * `/layers` read for such a parent at all. Both controls below have no draft, + * so they still reach `/layers`. */ import '@testing-library/jest-dom/vitest'; diff --git a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.refusalIssues-11379.test.tsx b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.refusalIssues-11379.test.tsx index 1086b09698..e8d2c662c5 100644 --- a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.refusalIssues-11379.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.refusalIssues-11379.test.tsx @@ -35,6 +35,8 @@ const mocks = vi.hoisted(() => ({ save: vi.fn() })); vi.mock('./useMetadata', () => ({ useMetadataClient: () => ({ + // No pending draft: the save reads the published parent (objectui#12027). + getDraft: async () => null, layered: async () => ({ effective: { name: 'sales_order', fields: { amount: { type: 'number', label: 'Amount' } } }, code: null, diff --git a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx index 8a76783ea9..062bbc87bb 100644 --- a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx +++ b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx @@ -6,11 +6,12 @@ * * Embedded items don't have their own HTTP endpoint (`PUT /meta/field/email` * does NOT exist for object-scoped fields) — so we: - * 1. Re-fetch the parent's effective body. - * 2. Render a SchemaForm using the registered sub-type's schema / form + * 1. Render a SchemaForm using the registered sub-type's schema / form * (e.g. `field` for `object.fields`). - * 3. On save: deep-clone the parent, splice the modified item back - * under `parent..`, and PUT the parent. + * 2. On save: re-read the parent (its pending draft when one exists, else + * its published body), splice the modified item back under + * `parent..`, and PUT the parent — into its + * draft when the base was the draft (objectui#12027). * * If the sub-type isn't registered (e.g. `index` has no `editAs`), we * fall back to a raw-JSON editor so users can still hand-edit and save. @@ -39,7 +40,8 @@ import type { FormViewSpec } from './form-spec.js'; import { useMetadataLocale, t, tFormat, translateValidationMessage } from './i18n.js'; import { errorCodeIsAnyOf } from '@object-ui/types'; // objectui#11692 - the served -> authored conversion of a picklist-bound field. -import { dropServedPicklistOptions } from '@object-ui/data-objectstack'; +// objectui#12027 - the parent's pending draft, unwrapped and stripped. +import { dropServedPicklistOptions, extractDraftBody } from '@object-ui/data-objectstack'; export interface EmbeddedItemEditorProps { parentType: string; @@ -110,13 +112,43 @@ export function EmbeddedItemEditor({ setError(null); setIssues([]); try { - // 1. Re-fetch parent to avoid clobbering concurrent edits. - const layered = await client.layered>( - parentType, - parentName, + // 1. Re-read the parent at save time, to avoid clobbering concurrent + // edits. objectui#12027 — its pending DRAFT is the base when one exists, + // and the save then goes back into that draft (`mode: 'draft'`): the + // author is mid-flight on the parent, and an item editor never writes + // live behind their back. A served draft is the whole document + // (objectui#10765), so every other field the draft holds rides along. + // A draft-read failure is the save's error, never "no draft": guessing + // there is none would send a publish-mode write over a pending one. + const parentDraft = extractDraftBody( + await client.getDraft(parentType, parentName), ); - const parent = - (layered.effective ?? layered.code ?? {}) as Record; + let parent: Record; + if (parentDraft) { + parent = parentDraft; + } else { + // No draft: the published version, as before. `/layers` answers 404 + // for a parent that was never published (objectstack-ai/objectstack#22397), + // which the client resolves as every layer `null`. ⛔ That absence is + // never a body: an empty base would PUT a stub of the one item over + // the parent. With nothing readable, the save is refused. + const layered = await client.layered>( + parentType, + parentName, + ); + const published = layered.effective ?? layered.code; + if (!published) { + setError( + tFormat('engine.edit.loadFailed', locale, { + type: parentType, + name: parentName, + message: t('engine.form.notFound', locale), + }), + ); + return; + } + parent = published; + } // 2. Splice modified item back into the parent collection. const updated = spliceEmbedded(parent, embeddedPath, itemName, draft); @@ -126,12 +158,16 @@ export function EmbeddedItemEditor({ // beside the options the runtime resolved from the list; the authoring // door refuses the pair for the whole object, whichever item was edited. // So the resolved `options` stay out of every bound field, and nothing - // else is touched. Other parent types are sent exactly as before. - await client.save( - parentType, - parentName, - parentType === 'object' ? dropServedPicklistOptions(updated) : updated, - ); + // else is touched. Other parent types are sent exactly as before. A + // draft-based save stays a draft; a published parent with no pending + // draft saves as it always has. + const body = + parentType === 'object' ? dropServedPicklistOptions(updated) : updated; + if (parentDraft) { + await client.save(parentType, parentName, body, { mode: 'draft' }); + } else { + await client.save(parentType, parentName, body); + } setSavedAt(Date.now()); onSaved?.(draft); } catch (err: any) {