diff --git a/.changeset/12810-seed-placeholder.md b/.changeset/12810-seed-placeholder.md new file mode 100644 index 0000000000..617ad5b744 --- /dev/null +++ b/.changeset/12810-seed-placeholder.md @@ -0,0 +1,5 @@ +--- +'@object-ui/app-shell': patch +--- + +Metadata designer create flows no longer save the spec create seed's placeholder identity. The seed's `label` and its placeholder object binding are taken from the author or not at all, so a dataset created without a base object is no longer saved bound to the seed's placeholder object (whose reads were then refused). A placeholder the type requires and the author left unset — a dataset's base object, or a required label — now refuses Save in the browser with a message naming it, and no request is sent. The seed's structural defaults (for example a dataset's `count` measure) are still carried. diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.seedPlaceholder-12810.test.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.seedPlaceholder-12810.test.tsx new file mode 100644 index 0000000000..6ecaf6a000 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.seedPlaceholder-12810.test.tsx @@ -0,0 +1,152 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#12810 — a dataset created with no base object is refused at the + * client; it is never saved bound to the spec seed's placeholder object. + * + * The REAL page, with the real dataset designer (preview + curated inspector), + * over a REAL `MetadataClient` whose transport is an in-memory server that + * records every `PUT`. The `/meta/types` entry carries what the live server + * serves for `dataset`: the spec's create seed (`getMetadataCreateSeed`) and + * the dataset JSON schema (its `required` list). + * + * (a) no base object picked → Save is refused with a stated reason, and no + * `PUT` is sent; + * (b) a base object picked → the body carries the picked object and the + * author's label, and no placeholder anywhere. + * + * That the seed's structural defaults survive `buildCreateModeBody` is pinned + * in `createBody.seedPlaceholders-12810.test.ts` (here the designer's own + * object picker resets `measures` when the object first changes). + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { z } from 'zod'; +import { getMetadataCreateSeed, getMetadataTypeSchema } from '@objectstack/spec/kernel'; +import { MetadataClient } from '@object-ui/data-objectstack'; + +const server = { + puts: [] as Array<{ name: string; body: Record }>, +}; + +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 meta = url.pathname.match(/^\/api\/v1\/meta\/(.+)$/); + if (!meta) return json(200, []); + const seg = meta[1].split('/').map(decodeURIComponent); + if (seg[0] === '_drafts') return json(200, { drafts: [] }); + if (seg.length === 1) return json(200, { items: [] }); + if (method === 'PUT') { + server.puts.push({ name: seg[1]!, body: JSON.parse(String(init?.body ?? '{}')) as Record }); + return json(200, { success: true, version: 'hmac-sha256:v1', seq: 1, state: 'draft', message: `Saved ${seg[0]}/${seg[1]}` }); + } + return json(404, { error: { code: 'NO_DRAFT', message: 'absent' } }); +} + +const client = vi.hoisted(() => ({ current: null as unknown })); + +vi.mock('./useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => client.current, + useMetadataTypes: () => ({ + loading: false, + entries: [ + { + type: 'dataset', + label: 'Dataset', + domain: 'analytics', + allowEnvironmentOverlay: true, + allowRuntimeCreate: true, + schema: DATASET_JSON_SCHEMA, + createSeed: getMetadataCreateSeed('dataset'), + }, + ], + }), + }; +}); + +import { MetadataResourceEditPage } from './ResourceEditPage'; +import { registerDefaultMetadataSchemas } from './default-schemas'; +import { registerBuiltinPreviews } from './previews'; +import { registerBuiltinInspectors } from './inspectors'; + +const DATASET_JSON_SCHEMA = z.toJSONSchema(getMetadataTypeSchema('dataset') as z.ZodType, { + io: 'input', + unrepresentable: 'any', +}) as Record; +/** The spec seed's placeholder object, read from the seed (never restated). */ +const PLACEHOLDER = (getMetadataCreateSeed('dataset') as { object: string }).object; + +client.current = new MetadataClient({ baseUrl: '', fetch: vi.fn(serve) as unknown as typeof fetch }); + +registerDefaultMetadataSchemas(); +registerBuiltinPreviews(); +registerBuiltinInspectors(); + +beforeEach(() => { + server.puts = []; + window.localStorage.setItem('metadata-admin:autosave', '0'); +}); +afterEach(() => { + cleanup(); + window.localStorage.removeItem('metadata-admin:autosave'); +}); + +const saveButton = () => screen.getByRole('button', { name: /Save \(⌘S\)/ }); + +/** Open a new dataset, type its Label (which derives the Name). */ +async function openNewDataset(label: string) { + render( + + + } /> + } /> + + , + ); + const labelBox = await screen.findByRole('textbox', { name: 'Label' }, { timeout: 8000 }); + fireEvent.change(labelBox, { target: { value: label } }); + fireEvent.blur(labelBox); + await waitFor(() => expect(screen.getByRole('textbox', { name: 'Name' })).toHaveValue('probe_ds')); +} + +describe('dataset create with no base object (objectui#12810)', () => { + it('(a) Save is refused at the client, naming the unset base object, and no PUT is sent', async () => { + expect(DATASET_JSON_SCHEMA.required).toContain('object'); + await openNewDataset('Probe Ds'); + fireEvent.click(saveButton()); + const alert = await screen.findByText(/before saving: a new item is never saved with the placeholder/, {}, { timeout: 8000 }); + expect(alert.textContent).toContain('object'); + // Give any stray save a chance to land before reading the server's log. + await new Promise((r) => setTimeout(r, 50)); + expect(server.puts).toEqual([]); + expect(screen.queryByTestId('landed')).toBeNull(); + }); + + it('(b) with a base object picked, the body carries the picked object', async () => { + await openNewDataset('Probe Ds'); + const baseObject = screen.getByRole('combobox', { name: 'Base object' }); + fireEvent.click(baseObject); + const search = await screen.findByPlaceholderText(/Search objects/i); + fireEvent.change(search, { target: { value: 'crm_account' } }); + fireEvent.keyDown(search, { key: 'Enter' }); + await waitFor(() => expect(baseObject).toHaveTextContent('crm_account')); + fireEvent.click(saveButton()); + await waitFor(() => expect(server.puts).toHaveLength(1), { timeout: 8000 }); + const body = server.puts[0]!.body; + expect(server.puts[0]!.name).toBe('probe_ds'); + expect(body.object).toBe('crm_account'); + expect(body.label).toBe('Probe Ds'); + expect(JSON.stringify(body)).not.toContain(PLACEHOLDER); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index 1c6efaabe8..5a513af3bf 100644 --- a/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -137,7 +137,7 @@ import { useMetadataLocale, t, tFormat, translateMetadataType, translateValidati import { JsonSourceEditor } from './JsonSourceEditor.js'; import { validateMetadataDraft, hasClientValidator, type DraftMode } from './clientValidation.js'; import { describeIssuePath, issueSlicePath, issueSliceFingerprint } from './issuePath.js'; -import { buildCreateModeBody } from './createBody.js'; +import { buildCreateModeBody, unsetSeedPlaceholders } from './createBody.js'; import { errorCodeIs, errorCodeIsAnyOf } from '@object-ui/types'; import { useCanAuthorMetadata, useCanReadAuthoringTrail } from '../../hooks/useCanAuthorMetadata.js'; import { environmentScopeSuffix, isEnvironmentScope, useWalledCatalogReadOnly } from './catalog-scope.js'; @@ -1657,6 +1657,28 @@ function MetadataResourceEditPageImpl({ setSaving(false); return; } + // objectui#12810 — a create never saves the seed's placeholder identity + // (`buildCreateModeBody` leaves it out). One the author left unset that + // the type requires — a dataset's base object — is refused HERE, named, + // with no request sent, as an empty name is above. + const unsetPlaceholders = createMode + ? unsetSeedPlaceholders( + config, + itemToSave as Record, + specCreateSeed, + (entry?.schema as { required?: unknown } | undefined)?.required, + identityField, + ) + : []; + if (unsetPlaceholders.length > 0) { + setError( + tFormat('engine.validation.seedPlaceholderUnset', locale, { + fields: unsetPlaceholders.map((key) => `“${labelForIssuePath(key)}”`).join(', '), + }), + ); + setSaving(false); + return; + } // Save lands in the draft buffer — the runtime keeps serving the // last published version until the operator clicks Publish. The // backend defaults to publish mode for backward-compatibility, so diff --git a/packages/app-shell/src/views/metadata-admin/createBody.seedPlaceholders-12810.test.ts b/packages/app-shell/src/views/metadata-admin/createBody.seedPlaceholders-12810.test.ts new file mode 100644 index 0000000000..ad6a41c469 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/createBody.seedPlaceholders-12810.test.ts @@ -0,0 +1,184 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#12810 — the enumeration pin over every spec create seed. + * + * The spec's create-seed module says each seed carries placeholder identity + * (`name` / `label` / object binding) and that "a create flow overrides those + * from user input". For EVERY entry `listMetadataCreateSeedTypes()` lists, this + * file asserts that each placeholder identity the seed carries is either + * overridden from user input by `buildCreateModeBody` (the author's value, or + * none — never the seed's) or refused at save (`unsetSeedPlaceholders`, for a + * key the type requires; an empty `name` has the save path's own refusal). + * + * `PLACEHOLDERS` below is the per-type status listing. A seed added later, or + * a seed that gains a placeholder, turns the first test red until it is + * listed here, and the rest then judge it. A placeholder nested where + * `buildCreateModeBody` cannot take it out (it only drops top-level keys) + * turns the second test red. + * + * The placeholder values are read from the spec (`getMetadataCreateSeed`), + * never restated. `required` is read from the type's spec schema as JSON + * schema, the shape the live `/meta/types` entry serves. + */ + +import { beforeAll, describe, expect, it } from 'vitest'; +import { z } from 'zod'; +import { + getMetadataCreateSeed, + getMetadataTypeSchema, + listMetadataCreateSeedTypes, +} from '@objectstack/spec/kernel'; +import { + buildCreateModeBody, + seedPlaceholderKeys, + specSeedPlaceholderObjects, + unsetSeedPlaceholders, +} from './createBody'; +import { registerBuiltinAnchors } from './anchors'; +import { registerDefaultMetadataSchemas } from './default-schemas'; +import { resolveResourceConfig } from './registry'; + +/** + * Per seed type: its placeholder identity keys, and what happens to each when + * the author leaves it unset — `refused` (the save is refused at the client), + * `name` (the save path's empty-name refusal), `omitted` (not required by the + * type: saved without it), or `built` (the type's `createBuildBody` builds the + * body from the draft alone; the seed is not read). + */ +const PLACEHOLDERS: Record> = { + action: { name: 'name', label: 'refused' }, + agent: { name: 'name', label: 'refused' }, + dashboard: { name: 'name', label: 'refused' }, + dataset: { name: 'name', label: 'refused', object: 'refused' }, + email_template: { name: 'name', label: 'refused' }, + flow: { name: 'name', label: 'refused' }, + hook: { name: 'name', label: 'omitted', object: 'refused' }, + mapping: { name: 'name', label: 'omitted', targetObject: 'refused' }, + object: { name: 'name', label: 'omitted' }, + page: { name: 'name', label: 'refused', object: 'omitted' }, + permission: { name: 'name', label: 'omitted' }, + skill: { name: 'name', label: 'refused' }, + tool: { name: 'name', label: 'refused' }, + translation: { name: 'name', label: 'omitted' }, + view: { name: 'built', object: 'built', label: 'built' }, +}; + +const seedOf = (type: string) => getMetadataCreateSeed(type) as Record; + +function requiredOf(type: string): string[] { + const json = z.toJSONSchema(getMetadataTypeSchema(type) as z.ZodType, { + io: 'input', + unrepresentable: 'any', + }) as { required?: string[] }; + return json.required ?? []; +} + +/** Every path in `value` whose leaf is one of the spec's placeholder objects. */ +function placeholderObjectPaths(value: unknown, path: string[] = []): string[] { + const objects = specSeedPlaceholderObjects(); + if (typeof value === 'string') { + // A qualified name built from the placeholder (`.new_view`). + const head = value.split('.')[0]!; + return objects.has(value) || objects.has(head) ? [path.join('.')] : []; + } + if (Array.isArray(value)) return value.flatMap((v, i) => placeholderObjectPaths(v, [...path, String(i)])); + if (value && typeof value === 'object') { + return Object.entries(value).flatMap(([k, v]) => placeholderObjectPaths(v, [...path, k])); + } + return []; +} + +/** The draft a create form starts from, as the page builds it. */ +function pristineDraft(type: string): Record { + const config = resolveResourceConfig(type); + return { ...(config.createDefaults ?? {}), [config.identityField ?? 'name']: '' }; +} + +beforeAll(() => { + registerDefaultMetadataSchemas(); + registerBuiltinAnchors(); +}); + +describe('every spec create seed: placeholder identity is overridden or refused (objectui#12810)', () => { + it('the spec placeholder object is read from the seeds, and every seed type is listed with its placeholders', () => { + expect(specSeedPlaceholderObjects().size).toBe(1); + const types = listMetadataCreateSeedTypes(); + expect(types.length).toBeGreaterThan(0); + const listed = Object.fromEntries(types.map((type) => [type, seedPlaceholderKeys(seedOf(type)).sort()])); + const expected = Object.fromEntries( + Object.entries(PLACEHOLDERS).map(([type, keys]) => [type, Object.keys(keys).sort()]), + ); + expect(listed).toEqual(expected); + }); + + it('a placeholder object sits only where the create flow takes it out', () => { + for (const type of listMetadataCreateSeedTypes()) { + const seed = seedOf(type); + const config = resolveResourceConfig(type); + const topLevel = new Set(seedPlaceholderKeys(seed)); + for (const path of placeholderObjectPaths(seed)) { + const handled = !!config.createBuildBody || topLevel.has(path); + expect({ type, path, handled }).toEqual({ type, path, handled: true }); + } + } + }); + + it.each(Object.keys(PLACEHOLDERS))( + '%s: an author who sets nothing gets no seed placeholder in the body, and a required one is refused', + (type) => { + const seed = seedOf(type); + const config = resolveResourceConfig(type); + const identityField = config.identityField ?? 'name'; + const draft = pristineDraft(type); + const body = buildCreateModeBody(config, draft, seed); + + // No placeholder value survives, anywhere in the body. + expect(placeholderObjectPaths(body)).toEqual([]); + for (const key of Object.keys(PLACEHOLDERS[type]!)) { + if (typeof seed[key] === 'string' && seed[key] !== '') { + expect({ type, key, value: body[key] }).not.toEqual({ type, key, value: seed[key] }); + } + } + + const refused = unsetSeedPlaceholders(config, body, seed, requiredOf(type), identityField); + for (const [key, fate] of Object.entries(PLACEHOLDERS[type]!)) { + if (fate === 'refused') expect(refused).toContain(key); + else expect(refused).not.toContain(key); + if (fate === 'name') expect(body[key]).toBe(''); + if (fate === 'omitted') expect(body).not.toHaveProperty(key); + } + }, + ); + + it.each(Object.keys(PLACEHOLDERS))( + '%s (control): the author\'s values are carried, nothing is refused, and the seed\'s structural defaults are kept', + (type) => { + const seed = seedOf(type); + const config = resolveResourceConfig(type); + const identityField = config.identityField ?? 'name'; + const authored: Record = { ...pristineDraft(type) }; + for (const key of Object.keys(PLACEHOLDERS[type]!)) { + authored[key] = key === 'label' ? 'Probe Item' : key === 'name' ? 'probe_item' : 'crm_account'; + } + const body = buildCreateModeBody(config, authored, seed); + expect(unsetSeedPlaceholders(config, body, seed, requiredOf(type), identityField)).toEqual([]); + expect(placeholderObjectPaths(body)).toEqual([]); + if (config.createBuildBody) return; + for (const key of Object.keys(PLACEHOLDERS[type]!)) expect(body[key]).toBe(authored[key]); + // Every non-placeholder seed default the draft does not set is kept. + for (const [key, value] of Object.entries(seed)) { + if (key in PLACEHOLDERS[type]! || key in authored) continue; + expect({ type, key, value: body[key] }).toEqual({ type, key, value }); + } + }, + ); + + it('dataset: the seed\'s `count` measure is kept while its placeholder binding is not', () => { + const seed = seedOf('dataset'); + const body = buildCreateModeBody(resolveResourceConfig('dataset'), { name: 'probe_ds', label: 'Probe Ds' }, seed); + expect(body.measures).toEqual(seed.measures); + expect(body.dimensions).toEqual(seed.dimensions); + expect(body).not.toHaveProperty('object'); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/createBody.ts b/packages/app-shell/src/views/metadata-admin/createBody.ts index 2e5390c108..3384b667c0 100644 --- a/packages/app-shell/src/views/metadata-admin/createBody.ts +++ b/packages/app-shell/src/views/metadata-admin/createBody.ts @@ -6,8 +6,62 @@ * LICENSE file in the root directory of this source tree. */ +import { getMetadataCreateSeed, listMetadataCreateSeedTypes } from '@objectstack/spec/kernel'; import type { MetadataResourceConfig } from './registry.js'; +/** + * The placeholder identity keys every spec create seed fills: the seed module's + * header names them (`name` / `label` / object binding) and says "a create flow + * overrides those from user input". The object binding is found by VALUE, see + * {@link specSeedPlaceholderObjects}. + */ +const SEED_IDENTITY_KEYS: ReadonlySet = new Set(['name', 'label']); + +let placeholderObjects: ReadonlySet | undefined; + +/** + * The object name(s) the spec's create seeds bind to: their placeholder object. + * + * Read from the seeds themselves (`getMetadataCreateSeed`), never restated + * here. The spec keeps its `PLACEHOLDER_OBJECT` constant module-private, and + * every seed that binds an object holds it at `object`, so the set of those + * values IS that constant. A seed that binds it under another key + * (`mapping.targetObject`) is then found by its value in + * {@link seedPlaceholderKeys}. + */ +export function specSeedPlaceholderObjects(): ReadonlySet { + if (!placeholderObjects) { + const values = new Set(); + for (const type of listMetadataCreateSeedTypes()) { + const seed = getMetadataCreateSeed(type); + const bound = seed && typeof seed === 'object' ? (seed as Record).object : undefined; + if (typeof bound === 'string' && bound) values.add(bound); + } + placeholderObjects = values; + } + return placeholderObjects; +} + +/** + * The top-level keys of a create seed that hold placeholder identity: `name`, + * `label`, and every key whose value is the spec's placeholder object (the + * object binding). These are the values a create flow must take from the + * author, never from the seed. + */ +export function seedPlaceholderKeys(seed: Record | undefined): string[] { + if (!seed) return []; + const objects = specSeedPlaceholderObjects(); + return Object.keys(seed).filter((key) => { + if (SEED_IDENTITY_KEYS.has(key)) return true; + const value = seed[key]; + return typeof value === 'string' && objects.has(value); + }); +} + +function isBlank(value: unknown): boolean { + return value === undefined || value === null || (typeof value === 'string' && value.trim() === ''); +} + /** * Build the create-mode save body for a new metadata item. * @@ -21,7 +75,15 @@ import type { MetadataResourceConfig } from './registry.js'; * server provides no seed (older server, or canvas-create types whose shape is * built interactively). * - * User-supplied draft values always win over the seed's placeholders. + * Only the seed's STRUCTURAL defaults are carried (`measures`, `widgets`, the + * required `type`/`body`, …). Its placeholder identity + * ({@link seedPlaceholderKeys}) is not: those values come from the author's + * draft or not at all, so a create never saves the seed's `label` or its + * placeholder object binding (objectui#12810: a dataset with no base object + * saved `object: `, and every read of it was then refused). + * A placeholder the author left unset and the type requires refuses the save + * — see {@link unsetSeedPlaceholders}. + * * `createBuildBody` (dynamic identity, e.g. a view's qualified name) still takes * precedence — it incorporates user input the static seed cannot. */ @@ -33,5 +95,30 @@ export function buildCreateModeBody( if (config.createBuildBody) { return config.createBuildBody(draft) as Record; } - return { ...(specCreateSeed ?? config.createDefaults ?? {}), ...draft }; + const base = { ...(specCreateSeed ?? config.createDefaults ?? {}) }; + for (const key of seedPlaceholderKeys(specCreateSeed)) delete base[key]; + return { ...base, ...draft }; +} + +/** + * The seed placeholder keys a create body still lacks a value for, where the + * type requires one — the save refuses these at the client, before any + * request, and names them. + * + * `required` is the LIVE server schema's `required` list (the `/meta/types` + * entry), so this gate is never stricter than the server that judges the body + * (the skew `clientValidation.ts` guards against). The identity field is left + * out: an empty name already has its own refusal in the save path. + */ +export function unsetSeedPlaceholders( + config: Pick, + body: Record, + specCreateSeed: Record | undefined, + required: unknown, + identityField: string, +): string[] { + if (config.createBuildBody || !Array.isArray(required)) return []; + return seedPlaceholderKeys(specCreateSeed).filter( + (key) => key !== identityField && required.includes(key) && isBlank(body[key]), + ); } diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index c1304ac5f0..380ddc0002 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -2243,6 +2243,9 @@ const ENGINE_STRINGS_EN: Record = { 'engine.validation.serverRefused': 'The server refused this draft — fix {issue} before saving.', 'engine.validation.failedOne': 'Validation failed (1 issue).', 'engine.validation.nameRequired': 'A name is required.', + // objectui#12810 — a create that leaves a required seed placeholder unset + // (a dataset's base object) is refused before any request. + 'engine.validation.seedPlaceholderUnset': 'Set {fields} before saving: a new item is never saved with the placeholder its template starts from.', 'engine.embedded.saved': 'Saved.', 'engine.embedded.readOnlyParent': 'The parent type is read-only — saving will still attempt a PUT and may be refused by the server.', 'engine.embedded.saveIntoParent': 'Save into {parentType}', @@ -5617,6 +5620,8 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.validation.serverRefused': '服务端已拒绝此草稿 —— 请先修复 {issue} 再保存。', 'engine.validation.failedOne': '校验失败（1 个问题）。', 'engine.validation.nameRequired': '名称必填。', + // objectui#12810 — 见英文表同名条目。 + 'engine.validation.seedPlaceholderUnset': '请先设置 {fields} 再保存：新建项不会带着模板的占位值保存。', 'engine.embedded.saved': '已保存。', 'engine.embedded.readOnlyParent': '父类型为只读；仍会尝试 PUT 保存，但服务器可能拒绝。', 'engine.embedded.saveIntoParent': '保存到 {parentType}',