From 51bb8bbeb6570e8fb6eaa2ce0e9ce214efd2d888 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 07:25:34 +0000 Subject: [PATCH 1/3] fix(studio): object writers seeded from a served read drop a picklist-bound field's served options Move `dropServedPicklistOptions` from app-shell to `@object-ui/data-objectstack` (a new export) so the plugin-designer writers can share it, re-point app-shell's two callers, and apply it at the writers that seed an object PUT from a served read: the OWD overview, the Setup fields and objects pages, and `MetadataService.saveFields`. `saveObject` and `saveMetadataItem` read nothing and are left as they are, so an authored picklist + options pair stays the server's loud 422. Claude-Session: https://claude.ai/code/session_01FngvPpdrnhHMdHHq6vwwju Co-authored-by: Claude --- ...ervice.picklistServedOptions-11692.test.ts | 156 +++++++++++++++++ .../app-shell/src/services/MetadataService.ts | 35 +++- .../src/services/builtinComponents.tsx | 5 +- ...Panel.picklistServedOptions-11692.test.tsx | 161 ++++++++++++++++++ .../studio-design/PackageOwdOverviewPanel.tsx | 11 +- .../studio-design/StudioDesignSurface.tsx | 5 +- packages/data-objectstack/src/index.ts | 7 + .../src}/picklist-binding.test.ts | 4 +- .../src}/picklist-binding.ts | 35 +++- ...sPage.picklistServedOptions-11692.test.tsx | 161 ++++++++++++++++++ .../src/MetadataFieldsPage.tsx | 9 +- ...sPage.picklistServedOptions-11692.test.tsx | 145 ++++++++++++++++ .../src/MetadataObjectsPage.tsx | 9 +- 13 files changed, 724 insertions(+), 19 deletions(-) create mode 100644 packages/app-shell/src/services/MetadataService.picklistServedOptions-11692.test.ts create mode 100644 packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.picklistServedOptions-11692.test.tsx rename packages/{app-shell/src/views/metadata-admin => data-objectstack/src}/picklist-binding.test.ts (96%) rename packages/{app-shell/src/views/metadata-admin => data-objectstack/src}/picklist-binding.ts (69%) create mode 100644 packages/plugin-designer/src/MetadataFieldsPage.picklistServedOptions-11692.test.tsx create mode 100644 packages/plugin-designer/src/MetadataObjectsPage.picklistServedOptions-11692.test.tsx diff --git a/packages/app-shell/src/services/MetadataService.picklistServedOptions-11692.test.ts b/packages/app-shell/src/services/MetadataService.picklistServedOptions-11692.test.ts new file mode 100644 index 0000000000..bbacb3f1c6 --- /dev/null +++ b/packages/app-shell/src/services/MetadataService.picklistServedOptions-11692.test.ts @@ -0,0 +1,156 @@ +/** + * 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#11692 — `MetadataService`'s three object writes, and a field that + * names a shared picklist. + * + * The runtime serves a picklist-bound field with `picklist` AND the options it + * resolved from the list; the authoring door refuses the two together for the + * whole object (`422 INVALID_METADATA` at `fields.FIELD.options`). The card's + * "Done when" asks each writer for one of two answers, and this file pins which + * one each method gives: + * + * - `saveFields` SEEDS from a served read (its GET, and the per-field + * carry-over that brings the binding back out), so it drops the served + * `options` on every bound field before its PUT. + * - `saveObject` and `saveMetadataItem` read NOTHING: every key they send is + * the caller's, so they cannot tell a served copy from an authored pair and + * must not strip — the authored pair stays the server's loud refusal, which + * is the authoring check objectui#10202 relies on. Pinned as "no read" plus + * "the pair still reaches the door and is refused". + * + * A real `ObjectStackAdapter` and SDK over a fetch double that serves the + * object and refuses a PUT body the way the authoring door does, so every + * claim is read off the WIRE. + */ + +import { describe, expect, it, vi } from 'vitest'; +import { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import type { DesignerFieldDefinition, ObjectDefinition } from '@object-ui/types'; +import { MetadataService, type FieldMetadataPayload } from './MetadataService'; + +type Row = Record; + +const TIER_OPTIONS = [ + { label: 'Gold', value: 'gold' }, + { label: 'Platinum', value: 'platinum' }, +]; +const STATUS_OPTIONS = [ + { label: 'Open', value: 'open' }, + { label: 'Closed', value: 'closed' }, +]; + +/** The object as the runtime SERVES it. */ +const SERVED: Row = { + name: 'acme_account', + label: 'Account', + fields: { + tier: { type: 'select', label: 'Tier', picklist: 'acme_tier', options: TIER_OPTIONS }, + status: { type: 'select', label: 'Status', options: STATUS_OPTIONS }, + }, +}; + +/** The authoring door's refusal for a field carrying both keys. */ +function doorRefusal(body: Row): string[] { + return Object.entries((body.fields ?? {}) as Record) + .filter(([, def]) => def && def.picklist !== undefined && def.options !== undefined) + .map(([name]) => `fields.${name}.options`); +} + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); +} + +/** A real adapter whose transport serves {@link SERVED} and refuses the pair. */ +function makeAdapter() { + const puts: Array<{ body: Row; status: number }> = []; + let gets = 0; + const adapter = new ObjectStackAdapter({ + baseUrl: 'http://test.local', + fetch: vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + const method = (init?.method ?? 'GET').toUpperCase(); + if (method === 'PUT') { + const body = JSON.parse(String(init?.body ?? '{}')) as Row; + const refused = doorRefusal(body); + const status = refused.length > 0 ? 422 : 200; + puts.push({ body, status }); + return status === 422 + ? json( + { + error: 'failed spec validation', + code: 'INVALID_METADATA', + issues: refused.map((path) => ({ + path, + code: 'custom', + message: '`picklist` and `options` cannot both be declared. Keep `picklist` and delete `options`.', + })), + }, + 422, + ) + : json({ success: true }); + } + gets += 1; + return json({ item: SERVED }); + }) as unknown as typeof fetch, + }); + return { adapter, puts, getCount: () => gets }; +} + +describe('MetadataService — a picklist-bound field and the three object writes (objectui#11692)', () => { + it('saveFields seeds from a served read, so a field list built from that read and left untouched goes out accepted, the bound field without `options`', async () => { + const { adapter, puts, getCount } = makeAdapter(); + // What a field designer holds after loading the served object: both fields, + // the bound one with the resolved options the read handed it. + const designerFields: DesignerFieldDefinition[] = [ + { id: 'tier', name: 'tier', label: 'Tier', type: 'select', options: TIER_OPTIONS }, + { id: 'status', name: 'status', label: 'Status', type: 'select', options: STATUS_OPTIONS }, + ]; + + await new MetadataService(adapter).saveFields('acme_account', designerFields); + + expect(getCount(), 'the served read this writer seeds from').toBeGreaterThan(0); + expect(puts).toHaveLength(1); + const [put] = puts; + expect(put!.status, `refused at ${doorRefusal(put!.body).join(', ')}`).toBe(200); + const fields = put!.body.fields as Record; + // The binding the carry-over brought back stays; its served options do not. + expect(fields.tier).toMatchObject({ type: 'select', label: 'Tier', picklist: 'acme_tier' }); + expect(fields.tier).not.toHaveProperty('options'); + // A field that names no picklist keeps its options exactly. + expect(fields.status!.options).toEqual(STATUS_OPTIONS); + }); + + it('saveObject reads nothing, so it strips nothing: an authored pair reaches the door and comes back refused', async () => { + const { adapter, puts, getCount } = makeAdapter(); + const obj: ObjectDefinition = { id: 'acme_account', name: 'acme_account', label: 'Account', isSystem: false, fieldCount: 1 }; + // The binding is not a declared `FieldMetadataPayload` member, which is the + // point: only a caller holding a wider value can hand one in. + const authored = { name: 'tier', type: 'select', label: 'Tier', picklist: 'acme_tier', options: TIER_OPTIONS }; + + await expect( + new MetadataService(adapter).saveObject(obj, [authored as FieldMetadataPayload]), + ).rejects.toMatchObject({ code: 'INVALID_METADATA', httpStatus: 422 }); + + expect(getCount(), 'saveObject issues no read').toBe(0); + expect(puts).toHaveLength(1); + expect((puts[0]!.body.fields as Record).tier).toMatchObject({ picklist: 'acme_tier', options: TIER_OPTIONS }); + }); + + it('saveMetadataItem reads nothing, so it strips nothing: the pair it is handed reaches the door and comes back refused', async () => { + const { adapter, puts, getCount } = makeAdapter(); + + await expect( + new MetadataService(adapter).saveMetadataItem('object', 'acme_account', SERVED), + ).rejects.toMatchObject({ code: 'INVALID_METADATA', httpStatus: 422 }); + + expect(getCount(), 'saveMetadataItem issues no read').toBe(0); + expect(puts).toHaveLength(1); + expect((puts[0]!.body.fields as Record).tier).toMatchObject({ picklist: 'acme_tier', options: TIER_OPTIONS }); + }); +}); diff --git a/packages/app-shell/src/services/MetadataService.ts b/packages/app-shell/src/services/MetadataService.ts index 0db1066f13..34314eae79 100644 --- a/packages/app-shell/src/services/MetadataService.ts +++ b/packages/app-shell/src/services/MetadataService.ts @@ -19,6 +19,7 @@ import { stripReadDecorations } from '@objectstack/spec/kernel'; import { assertObjectMetadataWritable, + dropServedPicklistOptions, RELATIONSHIP_TYPES_REQUIRING_REFERENCE, viewItemObjectName, type ObjectStackAdapter, @@ -704,6 +705,14 @@ export class MetadataService { * The object binding comes from the body being written, via the same * accessor `listViewOverrides` narrows those rows by — not from a fourth * private copy of "which object is this?". + * + * ⛔ No `dropServedPicklistOptions` here (objectui#11692). This method reads + * nothing: `data` is the caller's, so it cannot tell a served copy of a + * picklist-bound field from an author who wrote `picklist` and `options` + * together, and the second must stay the server's loud 422 with its + * prescription. A caller that seeded `data` from a served object read applies + * the conversion itself. Pinned (no read, the pair still refused) in + * `MetadataService.picklistServedOptions-11692.test.ts`. */ async saveMetadataItem(category: string, name: string, data: Record): Promise { await this.putMetadataItem(category, name, data); @@ -804,6 +813,14 @@ export class MetadataService { * same way — but that builds capability for a path with zero measured * pull and makes this parameter redundant, which then wants retiring on * its own terms (ADR-0049 shape). + * + * ⛔ And, for the same reason it does not fetch, no `dropServedPicklistOptions` + * (objectui#11692): this method reads nothing, so every key it sends is one + * the caller handed it, and it cannot tell a served copy from an authored + * `picklist` + `options` pair — which must stay the server's loud 422. + * `FieldMetadataPayload` declares no `picklist`, so a fresh typed field + * literal cannot carry the binding here at all. Pinned (no read, the pair + * still refused) in `MetadataService.picklistServedOptions-11692.test.ts`. */ async saveObject(obj: ObjectDefinition, existingFields: FieldMetadataPayload[]): Promise { const payload = toObjectPayload(obj, existingFields); @@ -894,6 +911,22 @@ export class MetadataService { * previous entries ride in on the very document this method already * fetched, so `toFieldPayload` merges onto them; see there for the mirror * property that keeps a CLEARED designer property cleared. + * + * …and a fifth, pinned in `MetadataService.picklistServedOptions-11692.test.ts`: + * + * - **A picklist-bound field leaves without its served `options`** + * (objectui#11692). The fetched document is the SERVED one, so a field + * naming a shared picklist carries `picklist` beside the options the + * runtime resolved from the list, and the carry-over above brings the + * binding back out. The authoring door refuses `options` beside + * `picklist` for the WHOLE object, so the body passes through + * `dropServedPicklistOptions` before the PUT. The binding can only have + * come from that read: `DesignerFieldDefinition` declares no `picklist`, + * so this method has no way to author one and none to remove one. A + * bound field's `options` are therefore never half of an authored pair + * here; they are the served copy, or a designer-model value the door + * refuses beside the binding whatever it holds. Fields without a + * `picklist` keep their `options` exactly as before. */ async saveFields(objectName: string, fields: DesignerFieldDefinition[]): Promise { const client = this.adapter.getClient(); @@ -944,7 +977,7 @@ export class MetadataService { fields: toFieldsMap(fields.map((field) => toFieldPayload(field, previousFieldEntry(previousFields, field.name)))), }) as Record; - await this.putMetadataItem('object', objectName, updatedObject); + await this.putMetadataItem('object', objectName, dropServedPicklistOptions(updatedObject)); this.adapter.invalidateCache(`object:${objectName}`); } diff --git a/packages/app-shell/src/services/builtinComponents.tsx b/packages/app-shell/src/services/builtinComponents.tsx index 373b3a8cf3..634332509b 100644 --- a/packages/app-shell/src/services/builtinComponents.tsx +++ b/packages/app-shell/src/services/builtinComponents.tsx @@ -25,7 +25,10 @@ import { isAggregatedViewContainer, viewDisplayType, } from '../views/metadata-admin/view-item-normalize.js'; -import { dropServedPicklistOptions } from '../views/metadata-admin/picklist-binding.js'; +// objectui#11692 - the helper moved beside the metadata client, so the +// plugin-designer writers share it; `@object-ui/data-objectstack` is already on +// the console's eager path (its data source), so this adds no eager chunk. +import { dropServedPicklistOptions } from '@object-ui/data-objectstack'; /* -------------------------------------------------------------------------- */ /* 1) Top-level admin pages — bound to `metadata:directory` + `metadata:resource` */ diff --git a/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.picklistServedOptions-11692.test.tsx b/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.picklistServedOptions-11692.test.tsx new file mode 100644 index 0000000000..d03b4986bd --- /dev/null +++ b/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.picklistServedOptions-11692.test.tsx @@ -0,0 +1,161 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11692 — the package OWD overview saves an object whose fields name a + * shared picklist. + * + * The panel seeds each changed row's PUT from a SERVED object read — the + * pending draft when there is one, else `layered.effective` — and a + * picklist-bound field is served with `picklist` AND the options the runtime + * resolved from the list. The authoring door refuses the two keys together for + * the whole object (`422 INVALID_METADATA` at `fields.FIELD.options`), so an OWD + * edit that never touched a field was refused for one. Measured live on the + * card: Studio → Access → OWD overview, change one object's sharing model, Save. + * + * The client is a real `MetadataClient`; its reads are served by doubles and + * its transport refuses a body the way the authoring door does, so the claim + * is read off the WIRE: the door accepted the body, the bound field went out + * without `options`, and every other field went out byte-identical. + */ + +import '@testing-library/jest-dom/vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; +import { MetadataClient } from '@object-ui/data-objectstack'; + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import { toast } from 'sonner'; +import { PackageOwdOverviewPanel } from './PackageOwdOverviewPanel'; + +type Row = Record; + +const PKG = 'com.example.acme'; + +/** The object as the runtime SERVES it: one bound field, one inline select, one text field. */ +function served(extra: Row = {}): Row { + return { + name: 'acme_account', + label: 'Account', + sharingModel: 'private', + ...extra, + fields: { + tier: { + type: 'select', + label: 'Tier', + picklist: 'acme_tier', + options: [ + { label: 'Gold', value: 'gold' }, + { label: 'Platinum', value: 'platinum' }, + ], + }, + status: { + type: 'select', + label: 'Status', + options: [ + { label: 'Open', value: 'open' }, + { label: 'Closed', value: 'closed' }, + ], + }, + title: { type: 'text', label: 'Title' }, + }, + }; +} + +const SERVED_STATUS = JSON.stringify((served().fields as Record).status); +const SERVED_TITLE = JSON.stringify((served().fields as Record).title); + +/** The authoring door's refusal for a field carrying both keys. */ +function doorRefusal(body: Row): string[] { + return Object.entries((body.fields ?? {}) as Record) + .filter(([, def]) => def && def.picklist !== undefined && def.options !== undefined) + .map(([name]) => `fields.${name}.options`); +} + +const puts: Array<{ url: string; body: Row; status: number }> = []; + +const fetchImpl = vi.fn(async (url: string, init?: RequestInit) => { + if (init?.method === 'PUT') { + const body = JSON.parse(String(init.body)) as Row; + const refused = doorRefusal(body); + const status = refused.length > 0 ? 422 : 200; + puts.push({ url: String(url), body, status }); + return new Response( + JSON.stringify( + status === 422 + ? { + error: 'failed spec validation', + code: 'INVALID_METADATA', + issues: refused.map((path) => ({ + path, + code: 'custom', + message: '`picklist` and `options` cannot both be declared. Keep `picklist` and delete `options`.', + })), + } + : { success: true }, + ), + { status, headers: { 'content-type': 'application/json' } }, + ); + } + return new Response('null', { status: 404, headers: { 'content-type': 'application/json' } }); +}); + +/** A real client over the refusing transport, its reads served by doubles. */ +function makeClient(draft: Row | null): MetadataClient { + const client = new MetadataClient({ baseUrl: 'http://test.local', fetch: fetchImpl as unknown as typeof fetch }); + Object.assign(client, { + list: vi.fn(async (type: string) => (type === 'object' ? [{ name: 'acme_account', label: 'Account' }] : [])), + listDrafts: vi.fn(async () => []), + layered: vi.fn(async () => ({ effective: served(), code: served() })), + getDraft: vi.fn(async () => (draft ? { type: 'object', name: 'acme_account', item: draft } : null)), + }); + return client; +} + +afterEach(() => { + cleanup(); + puts.length = 0; + fetchImpl.mockClear(); + vi.mocked(toast.success).mockClear(); +}); + +async function changeSharingAndSave(client: MetadataClient): Promise { + render(); + await screen.findByTestId('owd-row-acme_account'); + fireEvent.change(screen.getByTestId('owd-internal-acme_account'), { target: { value: 'public_read' } }); + fireEvent.click(screen.getByTestId('owd-save')); + await waitFor(() => expect(puts).toHaveLength(1)); +} + +describe('PackageOwdOverviewPanel — a picklist-bound object saves without its served options (objectui#11692)', () => { + it('no pending draft: the body seeded from `layered.effective` reaches the door accepted, the bound field without `options`', async () => { + await changeSharingAndSave(makeClient(null)); + + const [put] = puts; + expect(put!.status, `refused at ${doorRefusal(put!.body).join(', ')}`).toBe(200); + expect(put!.url).toContain('/meta/object/acme_account'); + const fields = put!.body.fields as Record; + expect(fields.tier).toEqual({ type: 'select', label: 'Tier', picklist: 'acme_tier' }); + // Nothing else is touched: the inline select and the text field go out as served. + expect(JSON.stringify(fields.status)).toBe(SERVED_STATUS); + expect(JSON.stringify(fields.title)).toBe(SERVED_TITLE); + // The control: the OWD edit itself still lands, and the save reports success. + expect(put!.body.sharingModel).toBe('public_read'); + expect(put!.body.label).toBe('Account'); + await waitFor(() => expect(toast.success).toHaveBeenCalledTimes(1)); + }); + + it('a pending draft that serves the bound field resolved: the body seeded from the draft is accepted the same way', async () => { + await changeSharingAndSave(makeClient(served({ description: 'authored in the draft' }))); + + const [put] = puts; + expect(put!.status, `refused at ${doorRefusal(put!.body).join(', ')}`).toBe(200); + const fields = put!.body.fields as Record; + expect(fields.tier).toEqual({ type: 'select', label: 'Tier', picklist: 'acme_tier' }); + expect(JSON.stringify(fields.status)).toBe(SERVED_STATUS); + // The draft's own key rides along: this was seeded from the draft, not the baseline. + expect(put!.body.description).toBe('authored in the draft'); + expect(put!.body.sharingModel).toBe('public_read'); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx b/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx index 9e7c7cff0b..854aabf2f1 100644 --- a/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx +++ b/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.tsx @@ -33,7 +33,9 @@ import type { MetadataClient } from '@object-ui/data-objectstack'; // framework's read decorations in one place. This file used to carry its own // copy that did the unwrap and skipped the strip. // …and `formatMetadataError`, the one metadata-save error reader (objectui#11302). -import { extractDraftBody, formatMetadataError } from '@object-ui/data-objectstack'; +// …and `dropServedPicklistOptions`, the served -> authored conversion of a +// picklist-bound field (objectui#10202, objectui#11692). +import { dropServedPicklistOptions, extractDraftBody, formatMetadataError } from '@object-ui/data-objectstack'; import { t, tFormat, type SupportedLocale } from '../metadata-admin/i18n.js'; import { isExternalWider, deriveMasterObject } from './owd-sharing.js'; import { toast } from 'sonner'; @@ -261,7 +263,12 @@ export function PackageOwdOverviewPanel({ else delete next.sharingModel; if (s.edit.external) next.externalSharingModel = s.edit.external; else delete next.externalSharingModel; - await client.save('object', s.row.name, next, { mode: 'draft', packageId }); + // objectui#11692 — `next` is seeded from a SERVED object read, where a + // picklist-bound field carries `picklist` beside the options the + // runtime resolved from the list; the authoring door refuses the two + // together, for the whole object. So the resolved `options` stay out of + // the body, on every bound field and on nothing else. + await client.save('object', s.row.name, dropServedPicklistOptions(next), { mode: 'draft', packageId }); } toast.success(tFormat('engine.studio.owd.saved', locale, { count: changed.length })); onDraftSaved?.(); diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index 63e8a2d954..09ff292a04 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -21,7 +21,9 @@ import { useAdapter, SchemaRendererProvider } from '@object-ui/react'; // framework's read decorations in one place. This file used to carry its own // copy that did the unwrap and skipped the strip. // …and `formatMetadataError`, the one metadata-save error reader (objectui#11302). -import { extractDraftBody, formatMetadataError } from '@object-ui/data-objectstack'; +// …and `dropServedPicklistOptions`, the served -> authored conversion of a +// picklist-bound field (objectui#10202), moved here by objectui#11692. +import { dropServedPicklistOptions, extractDraftBody, formatMetadataError } from '@object-ui/data-objectstack'; import type { FlowRuntimeState as SpecFlowRuntimeState } from '@objectstack/spec/contracts'; import type { I18nLabel } from '@objectstack/spec/ui'; import { StudioChatDock, type StudioSurfaceLabel } from './StudioAiCopilot.js'; @@ -96,7 +98,6 @@ import { } from '../metadata-admin/inspector-registry.js'; import { getMetadataDefaultInspector } from '../metadata-admin/default-inspector-registry.js'; import { getMetadataResource } from '../metadata-admin/registry.js'; -import { dropServedPicklistOptions } from '../metadata-admin/picklist-binding.js'; import { useMetadataClient, useMetadataTypes } from '../metadata-admin/useMetadata.js'; import { DESIGNER_SURFACE_PARAM, diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index ce97c99364..66a3ed7f38 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -7136,6 +7136,13 @@ export { MetadataClient, readSaveAdvisories } from './metadata-client'; // `getDraft` that produces the envelope, because the unwrap-and-strip is part // of that method's contract rather than a detail of any one view. export { extractDraftBody } from './draft-envelope'; +// objectui#10202 / objectui#11692 - the served -> authored conversion of a +// picklist-bound field, exported for the WRITERS that seed an object PUT from a +// served read (the read answers `picklist` beside the resolved `options`, and the +// authoring door refuses the pair). ⛔ Never applied inside `MetadataClient.save`: +// a door cannot tell a served copy from an authored pair, and the second must +// stay the server's loud refusal. See the module's docblock. +export { dropServedPicklistOptions } from './picklist-binding'; // objectui#11302 - the one reader of a failed metadata save, exported beside the // `MetadataError.issues` it reads so every surface that saves through this // client renders the per-field prescription the same way. diff --git a/packages/app-shell/src/views/metadata-admin/picklist-binding.test.ts b/packages/data-objectstack/src/picklist-binding.test.ts similarity index 96% rename from packages/app-shell/src/views/metadata-admin/picklist-binding.test.ts rename to packages/data-objectstack/src/picklist-binding.test.ts index 6689eea985..2f50e5a913 100644 --- a/packages/app-shell/src/views/metadata-admin/picklist-binding.test.ts +++ b/packages/data-objectstack/src/picklist-binding.test.ts @@ -2,7 +2,9 @@ /** * objectui#10202 — `dropServedPicklistOptions`, the served → authored - * conversion an object designer applies to the body it PUTs. + * conversion a writer that seeded its body from a served object read applies + * to the body it PUTs. Moved here from `@object-ui/app-shell` with the helper + * by objectui#11692, so the writers in `@object-ui/plugin-designer` share it. * * The contract it answers to is read from the installed `@objectstack/spec`, * not restated: `FieldSchema` refuses `picklist` beside `options` (the served diff --git a/packages/app-shell/src/views/metadata-admin/picklist-binding.ts b/packages/data-objectstack/src/picklist-binding.ts similarity index 69% rename from packages/app-shell/src/views/metadata-admin/picklist-binding.ts rename to packages/data-objectstack/src/picklist-binding.ts index 6f6b555104..f791893027 100644 --- a/packages/app-shell/src/views/metadata-admin/picklist-binding.ts +++ b/packages/data-objectstack/src/picklist-binding.ts @@ -2,23 +2,26 @@ /** * The served form of a picklist-bound field, turned back into the authored - * form before an object designer writes it (objectui#10202). + * form before a writer that seeded its body from a served read PUTs it + * (objectui#10202, objectui#11692). * - * ## Why the designer has to do this + * ## Why the writer has to do this * * A select field may name a shared list instead of carrying its own options: * `picklist: 'industry'` (`FieldSchema.picklist`, `@objectstack/spec` * `data/picklist.zod.ts`). The runtime serves such a field with BOTH keys: * `picklist`, still naming the list, and `options`, RESOLVED from that list and * from every `picklistExtensions` entry other packages add to it - * (`PicklistServedFieldSchema`). Every object read the designers seed from - * carries that served form — `GET /meta/object/NAME` and the `effective` layer - * of `GET /meta/object/NAME/layers` alike. + * (`PicklistServedFieldSchema`). Every object read carries that served form — + * the object list `GET /meta/object`, `GET /meta/object/NAME`, and the + * `effective` layer of `GET /meta/object/NAME/layers` alike, because the + * runtime resolves the binding in the one fold all three share. * * The authoring door refuses the two keys together: `FieldSchema` adds an issue * at `fields.FIELD.options` whose prescription is "Keep `picklist` and delete - * `options`". So a designer that writes its seed back unchanged gets - * `422 INVALID_METADATA` for the whole object, whatever the author edited. + * `options`". So a writer that sends its seed back unchanged gets + * `422 INVALID_METADATA` for the whole object, whatever the author edited — + * an OWD batch edit, a relabel, a field reorder, anything. * The resolved list belongs to the picklist, not to the field: the producer of * the PUT drops it (objectstack's producer obligation, pinned server-side in * `protocol-picklist-served-roundtrip.test.ts`). @@ -41,8 +44,22 @@ * * ⛔ Not at the transport. `MetadataClient.save` and the other metadata-write * doors cannot tell a served copy from an author who wrote both keys, and the - * second must stay a loud refusal with its prescription. The designer can tell: - * it seeded its draft from the served read. + * second must stay a loud refusal with its prescription. The writer can tell: + * it seeded its body from the served read. So this is exported for WRITERS — + * the opposite of `assertObjectMetadataWritable`, which is exported for doors — + * and the rule each writer applies is one sentence: a body seeded from a served + * object read goes through this function on its way to the PUT; a body built + * from the author's own input does not. + * + * ## Why it lives in this package (objectui#11692) + * + * The writers that seed from a served read are not all in one package: the + * Studio and metadata-admin surfaces live in `@object-ui/app-shell`, and the + * Setup fields and objects pages in `@object-ui/plugin-designer`, which does + * not depend on `app-shell`. Both depend on this package, which also owns the + * client that serves the read and carries the write — the same reason + * `extractDraftBody` lives here. One function, not one copy per package: a + * second copy is free to drift from the spec's served shape on its own. * * Pure: the input is never mutated, and a document with nothing to drop comes * back BY REFERENCE, so a caller can tell "unchanged" by identity. diff --git a/packages/plugin-designer/src/MetadataFieldsPage.picklistServedOptions-11692.test.tsx b/packages/plugin-designer/src/MetadataFieldsPage.picklistServedOptions-11692.test.tsx new file mode 100644 index 0000000000..f3eca2ae29 --- /dev/null +++ b/packages/plugin-designer/src/MetadataFieldsPage.picklistServedOptions-11692.test.tsx @@ -0,0 +1,161 @@ +/** + * 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#11692 — the Setup fields page saves an object whose fields name a + * shared picklist. + * + * The page reads the object through `MetadataClient.get()` — the SERVED form, + * where a picklist-bound field carries `picklist` AND the options the runtime + * resolved from the list — and writes the whole document back with every + * per-field key carried over. The authoring door refuses `options` beside + * `picklist` for the whole object (`422 INVALID_METADATA` at + * `fields.FIELD.options`), so relabelling an unrelated field was refused. + * + * Both halves of the page's field map are covered: a bound `select` (a type the + * drawer authors, rebuilt through `fromDesignerField`) and a bound `multiselect` + * (a type it only carries, through `carryPreservedField`). + * + * A REAL `MetadataClient` over a fetch double that answers the single-item read + * with the server's envelope and refuses a PUT body the way the authoring door + * does, so the claim is read off the WIRE. `FieldDesigner` is the only other + * double, a prop recorder, as in `MetadataFieldsPage.saveEnvelope.test.tsx`. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, render, waitFor } from '@testing-library/react'; +import { MetadataClient } from '@object-ui/data-objectstack'; +import type { DesignerFieldDefinition } from '@object-ui/types'; + +type Row = Record; + +/** The object as the runtime SERVES it. */ +const SERVED: Row = { + name: 'acme_account', + label: 'Account', + fields: { + title: { type: 'text', label: 'Title' }, + tier: { + type: 'select', + label: 'Tier', + picklist: 'acme_tier', + options: [ + { label: 'Gold', value: 'gold' }, + { label: 'Platinum', value: 'platinum' }, + ], + }, + regions: { + type: 'multiselect', + label: 'Regions', + picklist: 'acme_region', + options: [{ label: 'EU', value: 'eu' }], + }, + status: { + type: 'select', + label: 'Status', + options: [ + { label: 'Open', value: 'open' }, + { label: 'Closed', value: 'closed' }, + ], + }, + }, +}; + +const SERVED_STATUS = JSON.stringify((SERVED.fields as Record).status); + +interface RecordedDesignerProps { + fields: DesignerFieldDefinition[]; + onFieldsChange?: (fields: DesignerFieldDefinition[]) => void; +} + +let designerProps: RecordedDesignerProps | null = null; + +vi.mock('./FieldDesigner', () => ({ + FieldDesigner: (props: RecordedDesignerProps) => { + designerProps = props; + return null; + }, +})); + +import { MetadataFieldsPage } from './MetadataFieldsPage'; + +/** The authoring door's refusal for a field carrying both keys. */ +function doorRefusal(body: Row): string[] { + return Object.entries((body.fields ?? {}) as Record) + .filter(([, def]) => def && def.picklist !== undefined && def.options !== undefined) + .map(([name]) => `fields.${name}.options`); +} + +let puts: Array<{ body: Row; status: number }> = []; + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); +} + +function realClient(): MetadataClient { + return new MetadataClient({ + baseUrl: 'http://localhost:3000', + fetch: (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + if ((init?.method ?? 'GET').toUpperCase() === 'PUT') { + const body = JSON.parse(String(init?.body ?? '{}')) as Row; + const refused = doorRefusal(body); + const status = refused.length > 0 ? 422 : 200; + puts.push({ body, status }); + return status === 422 + ? json( + { + error: 'failed spec validation', + code: 'INVALID_METADATA', + issues: refused.map((path) => ({ + path, + code: 'custom', + message: '`picklist` and `options` cannot both be declared. Keep `picklist` and delete `options`.', + })), + }, + 422, + ) + : json({ success: true, name: 'acme_account' }); + } + if (/\/meta\/object\/acme_account(\?|$)/.test(url)) { + return json({ type: 'object', name: 'acme_account', item: SERVED }); + } + return json({ items: [] }); + }) as unknown as typeof fetch, + }); +} + +beforeEach(() => { + puts = []; + designerProps = null; +}); + +describe('MetadataFieldsPage — a picklist-bound object saves without its served options (objectui#11692)', () => { + it('relabelling an unrelated field: the door accepts the body, no bound field carries `options`, the inline select is byte-identical', async () => { + render(); + await waitFor(() => expect(designerProps?.fields.map((f) => f.name)).toEqual(['title', 'tier', 'status'])); + + const next = designerProps!.fields.map((f) => (f.name === 'title' ? { ...f, label: 'Headline' } : f)); + await act(async () => { + designerProps!.onFieldsChange!(next); + }); + await waitFor(() => expect(puts).toHaveLength(1)); + + const [put] = puts; + expect(put!.status, `refused at ${doorRefusal(put!.body).join(', ')}`).toBe(200); + const fields = put!.body.fields as Record; + // The designable bound field, and the carried-through one: binding kept, served options gone. + expect(fields.tier).toMatchObject({ type: 'select', label: 'Tier', picklist: 'acme_tier' }); + expect(fields.tier).not.toHaveProperty('options'); + expect(fields.regions).toEqual({ type: 'multiselect', label: 'Regions', picklist: 'acme_region' }); + // A field that names no picklist keeps its inline options exactly as served. + expect(JSON.stringify(fields.status!.options)).toBe(JSON.stringify(JSON.parse(SERVED_STATUS).options)); + // The control: the edit itself landed. + expect(fields.title).toMatchObject({ type: 'text', label: 'Headline' }); + }); +}); diff --git a/packages/plugin-designer/src/MetadataFieldsPage.tsx b/packages/plugin-designer/src/MetadataFieldsPage.tsx index 8accb079d8..5f5a5faef7 100644 --- a/packages/plugin-designer/src/MetadataFieldsPage.tsx +++ b/packages/plugin-designer/src/MetadataFieldsPage.tsx @@ -49,6 +49,7 @@ import { retiredFieldKeysFor } from '@object-ui/types/internal/retired-field-key import { MetadataClient, RELATIONSHIP_TYPES_REQUIRING_REFERENCE, + dropServedPicklistOptions, formatMetadataError, type MetadataClientConfig, } from '@object-ui/data-objectstack'; @@ -951,7 +952,13 @@ export function MetadataFieldsPage({ ...state.raw, fields: toFieldsMap(next, prevFields), }; - await client.save('object', objectName, mergedObject); + // objectui#11692 — `state.raw` is the SERVED object, and `carryOver` / + // `carryPreservedField` carry every per-field key it holds, so a + // picklist-bound field leaves with `picklist` AND the options the runtime + // resolved from the list. The authoring door refuses the pair for the + // whole object, whichever field was edited. This page has no options + // editor, so those `options` can only be the served copy: they stay out. + await client.save('object', objectName, dropServedPicklistOptions(mergedObject)); await reload(); } catch (err) { // objectui#11302 - through the ONE metadata-save error reader. A save the diff --git a/packages/plugin-designer/src/MetadataObjectsPage.picklistServedOptions-11692.test.tsx b/packages/plugin-designer/src/MetadataObjectsPage.picklistServedOptions-11692.test.tsx new file mode 100644 index 0000000000..8c3c987a7b --- /dev/null +++ b/packages/plugin-designer/src/MetadataObjectsPage.picklistServedOptions-11692.test.tsx @@ -0,0 +1,145 @@ +/** + * 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#11692 — the Setup objects page saves an object whose fields name a + * shared picklist. + * + * The page lists objects through `MetadataClient.list('object')` — the SERVED + * form, where a picklist-bound field carries `picklist` AND the options the + * runtime resolved from the list (the list read resolves through the same fold + * as the single-item read) — and a manager edit spreads that served document + * into the PUT. The authoring door refuses `options` beside `picklist` for the + * whole object, so relabelling an object was refused for a field the edit + * never touched. + * + * A REAL `MetadataClient` over a fetch double that serves the list and refuses + * a PUT body the way the authoring door does, so the claim is read off the + * WIRE. `ObjectManager` is the only other double, a prop recorder, as in + * `MetadataObjectsPage.specKeyGroup.test.tsx`. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, render, waitFor } from '@testing-library/react'; +import { MetadataClient } from '@object-ui/data-objectstack'; +import type { ObjectDefinition } from '@object-ui/types'; + +type Row = Record; + +/** The object as the runtime SERVES it in the list. */ +const SERVED: Row = { + name: 'acme_account', + label: 'Account', + isSystem: false, + fields: { + title: { type: 'text', label: 'Title' }, + tier: { + type: 'select', + label: 'Tier', + picklist: 'acme_tier', + options: [ + { label: 'Gold', value: 'gold' }, + { label: 'Platinum', value: 'platinum' }, + ], + }, + status: { + type: 'select', + label: 'Status', + options: [ + { label: 'Open', value: 'open' }, + { label: 'Closed', value: 'closed' }, + ], + }, + }, +}; + +interface RecordedManagerProps { + objects: ObjectDefinition[]; + onObjectsChange?: (objects: ObjectDefinition[]) => void; +} + +let managerProps: RecordedManagerProps | null = null; + +vi.mock('./ObjectManager', () => ({ + ObjectManager: (props: RecordedManagerProps) => { + managerProps = props; + return null; + }, +})); + +import { MetadataObjectsPage } from './MetadataObjectsPage'; + +/** The authoring door's refusal for a field carrying both keys. */ +function doorRefusal(body: Row): string[] { + return Object.entries((body.fields ?? {}) as Record) + .filter(([, def]) => def && def.picklist !== undefined && def.options !== undefined) + .map(([name]) => `fields.${name}.options`); +} + +let puts: Array<{ body: Row; status: number }> = []; + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); +} + +function realClient(): MetadataClient { + return new MetadataClient({ + baseUrl: 'http://localhost:3000', + fetch: (async (_input: RequestInfo | URL, init?: RequestInit) => { + if ((init?.method ?? 'GET').toUpperCase() === 'PUT') { + const body = JSON.parse(String(init?.body ?? '{}')) as Row; + const refused = doorRefusal(body); + const status = refused.length > 0 ? 422 : 200; + puts.push({ body, status }); + return status === 422 + ? json( + { + error: 'failed spec validation', + code: 'INVALID_METADATA', + issues: refused.map((path) => ({ + path, + code: 'custom', + message: '`picklist` and `options` cannot both be declared. Keep `picklist` and delete `options`.', + })), + }, + 422, + ) + : json({ success: true }); + } + return json({ items: [SERVED] }); + }) as unknown as typeof fetch, + }); +} + +beforeEach(() => { + puts = []; + managerProps = null; +}); + +describe('MetadataObjectsPage — a picklist-bound object saves without its served options (objectui#11692)', () => { + it('relabelling the object: the door accepts the body, no bound field carries `options`, every other field byte-identical', async () => { + render(); + await waitFor(() => expect(managerProps?.objects).toHaveLength(1)); + + const next = managerProps!.objects.map((o) => ({ ...o, label: 'Customer' })); + await act(async () => { + managerProps!.onObjectsChange!(next); + }); + await waitFor(() => expect(puts).toHaveLength(1)); + + const [put] = puts; + expect(put!.status, `refused at ${doorRefusal(put!.body).join(', ')}`).toBe(200); + const fields = put!.body.fields as Record; + const served = SERVED.fields as Record; + expect(fields.tier).toEqual({ type: 'select', label: 'Tier', picklist: 'acme_tier' }); + expect(JSON.stringify(fields.status)).toBe(JSON.stringify(served.status)); + expect(JSON.stringify(fields.title)).toBe(JSON.stringify(served.title)); + // The control: the relabel itself landed. + expect(put!.body.label).toBe('Customer'); + }); +}); diff --git a/packages/plugin-designer/src/MetadataObjectsPage.tsx b/packages/plugin-designer/src/MetadataObjectsPage.tsx index 5cf52af3d3..fa10f49791 100644 --- a/packages/plugin-designer/src/MetadataObjectsPage.tsx +++ b/packages/plugin-designer/src/MetadataObjectsPage.tsx @@ -35,7 +35,7 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react'; import type { ObjectDefinition } from '@object-ui/types'; -import { MetadataClient, type MetadataClientConfig } from '@object-ui/data-objectstack'; +import { MetadataClient, dropServedPicklistOptions, type MetadataClientConfig } from '@object-ui/data-objectstack'; import { ObjectManager } from './ObjectManager'; /** @@ -254,7 +254,12 @@ export function MetadataObjectsPage({ continue; } try { - await client.save('object', updated.name, merged); + // objectui#11692 — `merged` spreads the SERVED object (`client.list`), + // whose picklist-bound fields carry `picklist` beside the options the + // runtime resolved from the list. The authoring door refuses the pair + // for the whole object, so a relabel here was refused for a field it + // never touched. The resolved `options` stay out of the body. + await client.save('object', updated.name, dropServedPicklistOptions(merged)); } catch (err) { errors.push(`Save ${updated.name}: ${err instanceof Error ? err.message : String(err)}`); } From bc90d2f741ec6168aa9e41766c2ecd78531b12a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 07:33:22 +0000 Subject: [PATCH 2/3] docs(data-objectstack): document dropServedPicklistOptions; changeset for objectui#11692 Claude-Session: https://claude.ai/code/session_01FngvPpdrnhHMdHHq6vwwju Co-authored-by: Claude --- .../11692-writers-drop-served-options.md | 18 +++++++++++ packages/data-objectstack/README.md | 30 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 .changeset/11692-writers-drop-served-options.md diff --git a/.changeset/11692-writers-drop-served-options.md b/.changeset/11692-writers-drop-served-options.md new file mode 100644 index 0000000000..e45c423fb9 --- /dev/null +++ b/.changeset/11692-writers-drop-served-options.md @@ -0,0 +1,18 @@ +--- +'@object-ui/data-objectstack': minor +'@object-ui/app-shell': patch +'@object-ui/plugin-designer': patch +--- + +Objects with a picklist-bound field save again from the OWD overview, the Setup fields and objects pages, and `MetadataService.saveFields` (objectui#11692). + +The runtime serves a field that names a shared picklist (`picklist: 'NAME'`) with the options it resolved from the list. The authoring door refuses `options` beside `picklist`, with `422 INVALID_METADATA` at `fields.FIELD.options`, and the refusal covers the whole object. objectui#10202 fixed the two designers that host the select-field editor. These writers also seed their save from a served object read, so they sent the resolved options back and were refused, whatever had been edited: + +- **Studio → Access → OWD overview.** Changing an object's sharing model and saving was refused. +- **The Setup fields page** (`MetadataFieldsPage`). Editing any field of such an object was refused. +- **The Setup objects page** (`MetadataObjectsPage`). Relabelling such an object was refused. +- **`MetadataService.saveFields`.** A field list built from the served object was refused. + +Each now leaves `options` out of every field that names a picklist, before the PUT. A field without `picklist` keeps its inline `options` unchanged, and no other key is touched. + +`dropServedPicklistOptions` is now exported from `@object-ui/data-objectstack`. It is the served-to-authored conversion these writers apply, and it moves here from `@object-ui/app-shell` (it was not exported there), because `@object-ui/plugin-designer` does not depend on `app-shell`. It is for code that builds an object PUT from a served read. `MetadataClient.save` does not apply it, and neither do `MetadataService.saveObject` and `MetadataService.saveMetadataItem`, which read nothing: a body that pairs `picklist` with `options` without a served read behind it is still refused by the server, with its prescription. diff --git a/packages/data-objectstack/README.md b/packages/data-objectstack/README.md index 26ce37d79f..380cca4984 100644 --- a/packages/data-objectstack/README.md +++ b/packages/data-objectstack/README.md @@ -870,6 +870,36 @@ newlines (`whitespace-pre-line`). `formatMetadataIssue(issue)` is the same one-line format for a single issue, for code that lists several failures, such as a publish response's `failed[]`. +## Saving an Object You Read From the Server + +A select field can name a shared picklist (`picklist: 'industry'`) instead of +carrying its own options. The server *serves* such a field with both keys: +`picklist`, and the `options` it resolved from the list. The server's authoring +check refuses the two together, with `422 INVALID_METADATA` at +`fields.FIELD.options`, and that refuses the whole object. So code that reads an +object, edits it, and saves the whole document back must drop those resolved +options first. `dropServedPicklistOptions` does that: + +```ts +import { MetadataClient, dropServedPicklistOptions } from '@object-ui/data-objectstack'; + +const client = new MetadataClient({ baseUrl: '/api/v1' }); + +const account = await client.get>('object', 'account'); +const edited = { ...account, label: 'Customer' }; +await client.save('object', 'account', dropServedPicklistOptions(edited)); +``` + +It removes `options` from every field that names a picklist, and touches +nothing else: a field without `picklist` keeps its inline `options`, and every +other key is kept. It never mutates its input, and it returns the same object +when there is nothing to remove. + +Apply it only to a body built from a served read. `MetadataClient.save` does +not apply it for you: the client cannot tell a served copy from an author who +wrote `picklist` and `options` together, and that second case should stay the +server's refusal, which says which key to delete. + ## User-Scoped State Adapter In addition to the main `DataSource` adapter, this package ships From 32b8c2ee4a3471f58c9b2313415c8a1827e5b7d8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 08:47:22 +0000 Subject: [PATCH 3/3] fix(app-shell): the embedded-item editor drops a picklist-bound field's served options before the parent PUT `EmbeddedItemEditor.doSave` re-reads the parent object's served `layered.effective`, splices the edited item in and PUTs the whole parent, so an object with a picklist-bound field was refused (422 at `fields.FIELD.options`) whatever item was saved. The object parent now goes through `dropServedPicklistOptions` before the PUT; other parent types are sent as before. Pinned with a 422-refusing transport; changeset prose names the fifth writer. Claude-Session: https://claude.ai/code/session_01FngvPpdrnhHMdHHq6vwwju Co-authored-by: Claude --- .../11692-writers-drop-served-options.md | 3 +- ...ditor.picklistServedOptions-11692.test.tsx | 156 ++++++++++++++++++ .../metadata-admin/EmbeddedItemEditor.tsx | 15 +- 3 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.picklistServedOptions-11692.test.tsx diff --git a/.changeset/11692-writers-drop-served-options.md b/.changeset/11692-writers-drop-served-options.md index e45c423fb9..c08986b18e 100644 --- a/.changeset/11692-writers-drop-served-options.md +++ b/.changeset/11692-writers-drop-served-options.md @@ -4,13 +4,14 @@ '@object-ui/plugin-designer': patch --- -Objects with a picklist-bound field save again from the OWD overview, the Setup fields and objects pages, and `MetadataService.saveFields` (objectui#11692). +Objects with a picklist-bound field save again from the OWD overview, the Setup fields and objects pages, the metadata-admin embedded-item editor, and `MetadataService.saveFields` (objectui#11692). The runtime serves a field that names a shared picklist (`picklist: 'NAME'`) with the options it resolved from the list. The authoring door refuses `options` beside `picklist`, with `422 INVALID_METADATA` at `fields.FIELD.options`, and the refusal covers the whole object. objectui#10202 fixed the two designers that host the select-field editor. These writers also seed their save from a served object read, so they sent the resolved options back and were refused, whatever had been edited: - **Studio → Access → OWD overview.** Changing an object's sharing model and saving was refused. - **The Setup fields page** (`MetadataFieldsPage`). Editing any field of such an object was refused. - **The Setup objects page** (`MetadataObjectsPage`). Relabelling such an object was refused. +- **The metadata-admin embedded-item editor** (`EmbeddedItemEditor`, opened from an object's fields, indexes or validations). Saving any item of such an object was refused. - **`MetadataService.saveFields`.** A field list built from the served object was refused. Each now leaves `options` out of every field that names a picklist, before the PUT. A field without `picklist` keeps its inline `options` unchanged, and no other key is touched. diff --git a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.picklistServedOptions-11692.test.tsx b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.picklistServedOptions-11692.test.tsx new file mode 100644 index 0000000000..fb41ef7376 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.picklistServedOptions-11692.test.tsx @@ -0,0 +1,156 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11692 — the embedded-item editor saves an item of an object whose + * fields name a shared picklist. + * + * `EmbeddedItemEditor` saves an item that lives inside an object (one of its + * `fields`, `indexes` or `validations`) by re-reading the parent's + * `layered.effective` — the SERVED object, where a picklist-bound field carries + * `picklist` AND the options the runtime resolved from the list — splicing the + * item in, and PUTting the whole parent. The authoring door refuses `options` + * beside `picklist` for the whole object (`422 INVALID_METADATA` at + * `fields.FIELD.options`), so saving any item of such an object was refused for + * a field the edit never touched. + * + * The client is a real `MetadataClient`: its `layered` read is served by a + * double, and its transport refuses a PUT body the way the authoring door does, + * so the claim is read off the WIRE. The editor is mounted directly, as its + * sibling suites do. + */ + +import '@testing-library/jest-dom/vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MetadataClient } from '@object-ui/data-objectstack'; + +type Row = Record; + +/** The object as the runtime SERVES it: one bound field, one inline select, one number field. */ +const SERVED: Row = { + name: 'acme_account', + label: 'Account', + fields: { + amount: { type: 'number', label: 'Amount' }, + tier: { + type: 'select', + label: 'Tier', + picklist: 'acme_tier', + options: [ + { label: 'Gold', value: 'gold' }, + { label: 'Platinum', value: 'platinum' }, + ], + }, + status: { + type: 'select', + label: 'Status', + options: [ + { label: 'Open', value: 'open' }, + { label: 'Closed', value: 'closed' }, + ], + }, + }, +}; + +const SERVED_FIELDS = SERVED.fields as Record; + +/** The authoring door's refusal for a field carrying both keys. */ +function doorRefusal(body: Row): string[] { + return Object.entries((body.fields ?? {}) as Record) + .filter(([, def]) => def && def.picklist !== undefined && def.options !== undefined) + .map(([name]) => `fields.${name}.options`); +} + +const puts: Array<{ url: string; body: Row; status: number }> = []; + +const fetchImpl = vi.fn(async (url: string, init?: RequestInit) => { + if (init?.method === 'PUT') { + const body = JSON.parse(String(init.body)) as Row; + const refused = doorRefusal(body); + const status = refused.length > 0 ? 422 : 200; + puts.push({ url: String(url), body, status }); + return new Response( + JSON.stringify( + status === 422 + ? { + error: 'failed spec validation', + code: 'INVALID_METADATA', + issues: refused.map((path) => ({ + path, + code: 'custom', + message: '`picklist` and `options` cannot both be declared. Keep `picklist` and delete `options`.', + })), + } + : { success: true }, + ), + { status, headers: { 'content-type': 'application/json' } }, + ); + } + return new Response('null', { status: 404, headers: { 'content-type': 'application/json' } }); +}); + +const client = new MetadataClient({ baseUrl: 'http://test.local', fetch: fetchImpl as unknown as typeof fetch }); +Object.assign(client, { + layered: vi.fn(async () => ({ effective: SERVED, code: SERVED, overlay: null, overlayScope: null })), +}); + +const FIELD_SCHEMA = { + type: 'object', + properties: { label: { type: 'string', title: 'Label' } }, +} satisfies Record; + +vi.mock('./useMetadata', () => ({ + useMetadataClient: () => client, + useMetadataTypes: () => ({ + loading: false, + error: null, + entries: [{ type: 'field', label: 'Field', allowOrgOverride: true, schema: FIELD_SCHEMA }], + }), +})); + +import { EmbeddedItemEditor } from './EmbeddedItemEditor'; + +afterEach(() => { + cleanup(); + puts.length = 0; + fetchImpl.mockClear(); +}); + +async function saveItem(itemName: string): Promise<{ url: string; body: Row; status: number }> { + render( + , + ); + fireEvent.click(screen.getByRole('button', { name: /save into object/i })); + await waitFor(() => expect(puts).toHaveLength(1)); + return puts[0]!; +} + +describe('EmbeddedItemEditor — a picklist-bound object saves without its served options (objectui#11692)', () => { + it('saving another item (`amount`): the door accepts the parent, the bound field without `options`, the inline select byte-identical', async () => { + const put = await saveItem('amount'); + + expect(put.status, `refused at ${doorRefusal(put.body).join(', ')}`).toBe(200); + expect(put.url).toContain('/meta/object/acme_account'); + const fields = put.body.fields as Record; + expect(fields.tier).toEqual({ type: 'select', label: 'Tier', picklist: 'acme_tier' }); + expect(JSON.stringify(fields.status)).toBe(JSON.stringify(SERVED_FIELDS.status)); + // The control: the edited item itself is in the body. + expect(fields.amount).toEqual({ type: 'number', label: 'Amount' }); + }); + + it('saving the bound field itself, untouched: the door accepts the parent, and the item keeps its binding without the served options', async () => { + const put = await saveItem('tier'); + + expect(put.status, `refused at ${doorRefusal(put.body).join(', ')}`).toBe(200); + const fields = put.body.fields as Record; + expect(fields.tier).toEqual({ type: 'select', label: 'Tier', picklist: 'acme_tier' }); + expect(JSON.stringify(fields.status)).toBe(JSON.stringify(SERVED_FIELDS.status)); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx index 12afd2ccd3..59bb5cf17b 100644 --- a/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx +++ b/packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx @@ -38,6 +38,8 @@ import { useMetadataClient, useMetadataTypes } from './useMetadata.js'; 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'; export interface EmbeddedItemEditorProps { parentType: string; @@ -117,8 +119,17 @@ export function EmbeddedItemEditor({ // 2. Splice modified item back into the parent collection. const updated = spliceEmbedded(parent, embeddedPath, itemName, draft); - // 3. PUT the parent. - await client.save(parentType, parentName, updated); + // 3. PUT the parent. objectui#11692 — an object parent was seeded from + // the SERVED read above, where a picklist-bound field carries `picklist` + // 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, + ); setSavedAt(Date.now()); onSaved?.(draft); } catch (err: any) {