From a5ae60f4f5f61ef40a11627646f3929745534838 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 06:48:02 +0000 Subject: [PATCH] fix(studio): gate edit affordances on read-only packages, live-sync field API name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P1 findings from the Studio package-create UX dogfood (objectstack-ai/framework#2615, mirrored as #2258): - Read-only packages accepted edit gestures client-side and only failed at save time. Thread package writability (from the same fetchPackages() the PackageSwitcher already uses) down into the Data pillar and hide/disable "Add field" (toolbar button, grid header "+", form-designer "+") and "New object" when the package isn't writable. Purely a courtesy gate — the server's writable_package_required check remains the authority. - A new field's API name never followed its label before first save, so a field relabeled to "Status" kept its auto-generated name (e.g. field_2) forever. Sync the name from the label live, per keystroke, while the name is still auto-generated (either the type-based or nextFieldName() field_N scheme) and un-customised — mirroring the existing object/app identifier behavior, using the same keystroke-safe toFieldNameLoose() slug. --- .../src/views/metadata-admin/i18n.ts | 2 + .../inspectors/ObjectFieldInspector.test.tsx | 54 +++++++++++++++ .../inspectors/ObjectFieldInspector.tsx | 53 +++++++++------ .../studio-design/ObjectFormDesigner.tsx | 20 +++--- .../studio-design/StudioDesignSurface.tsx | 67 ++++++++++++++++--- 5 files changed, 159 insertions(+), 37 deletions(-) diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index 3920a891b..a813655cb 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -935,6 +935,7 @@ const ENGINE_STRINGS_EN: Record = { 'engine.studio.data.labelPlaceholder': 'Display name (e.g. Repair Ticket)', 'engine.studio.data.idPlaceholder': 'Identifier (e.g. repair_ticket)', 'engine.studio.data.newObject': 'New object', + 'engine.studio.data.readOnlyPackage': 'This package is read-only — switch to or create a writable package to add objects', 'engine.studio.data.firstObjectTitle': 'Start with your first object', 'engine.studio.data.firstObjectHint': 'Objects are your app’s data foundation (e.g. “Orders”, “Customers”). Enter a display name and identifier at the bottom-left to create one; then design its fields, forms, and automations, and publish once at the end.', @@ -1807,6 +1808,7 @@ const ENGINE_STRINGS_ZH: Record = { 'engine.studio.data.labelPlaceholder': '显示名(如:报修工单)', 'engine.studio.data.idPlaceholder': '标识符(如:repair_ticket)', 'engine.studio.data.newObject': '新建对象', + 'engine.studio.data.readOnlyPackage': '此包为只读——请切换到或新建一个可写的包后再添加对象', 'engine.studio.data.firstObjectTitle': '从第一个对象开始', 'engine.studio.data.firstObjectHint': '对象是应用的数据基座(如「订单」「客户」)。在左下角输入显示名与标识符即可创建;之后为它设计字段、表单与自动化,最后一次发布。', diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.test.tsx index 5ee69eee9..3da4b35ff 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.test.tsx @@ -1,5 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +import * as React from 'react'; import { describe, it, expect, vi, afterEach } from 'vitest'; import { render, screen, fireEvent, cleanup } from '@testing-library/react'; @@ -85,6 +86,59 @@ describe('ObjectFieldInspector — duplicate field', () => { }); }); +describe('ObjectFieldInspector — label derives API name until customised', () => { + it('syncs field_ (the nextFieldName() auto-name) to the label live, per keystroke', () => { + const { onPatch, onSelectionChange } = renderField( + { field_2: { type: 'text', label: '' } }, + 'field_2', + ); + fireEvent.change(controlFor('Label'), { target: { value: 'Status' } }); + const patch = onPatch.mock.calls.at(-1)![0]; + expect(Object.keys(patch.fields)).toEqual(['status']); + expect(patch.fields.status).toMatchObject({ label: 'Status' }); + expect(onSelectionChange).toHaveBeenCalledWith( + expect.objectContaining({ kind: 'field', id: 'status' }), + ); + }); + + it('stops deriving once the API name has been hand-edited', () => { + // Simulate the real parent, which feeds each onPatch back into `draft` — + // a single static-props render (like the other cases here) can't observe + // this, since the second edit needs the FIRST edit's rename reflected in + // `entry.name` before it decides whether the name is still auto-generated. + function Stateful() { + const [fields, setFields] = React.useState>>({ + field_2: { type: 'text', label: '' }, + }); + return ( + setFields(patch.fields)} + onClearSelection={() => {}} + onSelectionChange={() => {}} + readOnly={false} + locale={'en-US'} + /> + ); + } + render(); + fireEvent.change(controlFor('API name'), { target: { value: 'ticket_status' } }); + fireEvent.change(controlFor('Label'), { target: { value: 'Status' } }); + expect(controlFor('API name')).toHaveValue('ticket_status'); + }); + + it('leaves an already-meaningful name untouched when the label changes', () => { + const { onPatch } = renderField({ priority: { type: 'text', label: 'Priority' } }, 'priority'); + fireEvent.change(controlFor('Label'), { target: { value: 'Urgency' } }); + const patch = onPatch.mock.calls.at(-1)![0]; + expect(Object.keys(patch.fields)).toEqual(['priority']); + expect(patch.fields.priority.label).toBe('Urgency'); + }); +}); + describe('ObjectFieldInspector — default value', () => { it('commits a text default for a text field', () => { const { onPatch } = renderField({ note: { type: 'text', label: 'Note' } }, 'note'); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx index 55c9bb606..bd47680bb 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx @@ -22,7 +22,6 @@ */ import * as React from 'react'; -import { slugify } from '../createDerive'; import type { MetadataInspectorProps } from '../inspector-registry'; import { MetadataClient } from '@object-ui/data-objectstack'; import { useMetadataClient } from '../useMetadata'; @@ -195,25 +194,30 @@ export function ObjectFieldInspector({ onSelectionChange?.({ kind: 'field', id: nextName, label: String(def.label ?? nextName) }); }; - // Derive the API name from the label (on blur, so we use the complete - // string — not per keystroke, which would churn the field key) while the - // name is still an auto-generated default and the user hasn't customised it. - // Mirrors the object Name behaviour; slugify() returns '' for non-Latin - // labels, in which case the unique default name is kept. - const maybeDeriveName = (label: string) => { - if (readOnly) return; + // Derive the API name from the label live, per keystroke — with + // toFieldNameLoose (prefix-stable, unlike slugify which trims trailing + // underscores and would fight mid-word typing) — while the name is still + // an auto-generated default and the user hasn't customised it. Mirrors the + // object/app Name behaviour. toFieldNameLoose returns '' for non-Latin + // labels, in which case the unique default name is kept. Pure — the + // caller applies the label and (if any) the derived name in one write, + // since two separate writeView() calls from the same stale `entry` closure + // would have the second clobber the first. + const deriveNameFor = (label: string): string | null => { + if (readOnly) return null; const base = type === 'select' ? 'status' : type; const isAutoName = entry.name === base || - (entry.name.startsWith(`${base}_`) && /^\d+$/.test(entry.name.slice(base.length + 1))); - if (!isAutoName) return; - const derived = slugify(label); - if (!derived || derived === entry.name) return; - if (view.entries.some((e, i) => i !== idx && e.name === derived)) return; - const nextEntries = [...view.entries]; - nextEntries[idx] = { ...entry, name: derived }; - writeView({ shape: view.shape, entries: nextEntries }); - onSelectionChange?.({ kind: 'field', id: derived, label: String(def.label ?? derived) }); + (entry.name.startsWith(`${base}_`) && /^\d+$/.test(entry.name.slice(base.length + 1))) || + // Freshly added fields are named by nextFieldName() as `field_` + // (StudioDesignSurface.tsx), independent of the field's type — match + // that scheme too, or a type-typed rename right after add never derives. + /^field_\d+$/.test(entry.name); + if (!isAutoName) return null; + const derived = toFieldNameLoose(label); + if (!derived || derived === entry.name) return null; + if (view.entries.some((e, i) => i !== idx && e.name === derived)) return null; + return derived; }; const removeField = () => { @@ -319,8 +323,19 @@ export function ObjectFieldInspector({ patchDef({ label: v })} - onBlur={maybeDeriveName} + onCommit={(v) => { + const derivedName = deriveNameFor(v); + const nextEntries = [...view.entries]; + nextEntries[idx] = { + ...entry, + name: derivedName ?? entry.name, + def: { ...def, label: v }, + }; + writeView({ shape: view.shape, entries: nextEntries }); + if (derivedName) { + onSelectionChange?.({ kind: 'field', id: derivedName, label: v }); + } + }} disabled={readOnly} testId="field-label-input" /> diff --git a/packages/app-shell/src/views/studio-design/ObjectFormDesigner.tsx b/packages/app-shell/src/views/studio-design/ObjectFormDesigner.tsx index 52159791a..0d62f2d11 100644 --- a/packages/app-shell/src/views/studio-design/ObjectFormDesigner.tsx +++ b/packages/app-shell/src/views/studio-design/ObjectFormDesigner.tsx @@ -69,8 +69,8 @@ export interface ObjectFormDesignerProps { selectedField?: string | null; /** Select a field → opens the shared field inspector. */ onSelectField: (name: string) => void; - /** Append a new field (reuses the pillar's add-field). */ - onAddField: () => void; + /** Append a new field (reuses the pillar's add-field). Omit to hide the button — e.g. a read-only package. */ + onAddField?: () => void; } /** A faithful, non-interactive preview of a field's control (by type). */ @@ -423,13 +423,15 @@ export function ObjectFormDesigner({ > 添加分组 - + {onAddField && ( + + )} { + let cancelled = false; + fetchPackages() + .then((pkgs) => { + if (cancelled) return; + const current = pkgs.find((p) => p.id === packageId); + setPackageWritable(current ? current.writable : true); + }) + .catch(() => { + if (!cancelled) setPackageWritable(true); + }); + return () => { + cancelled = true; + }; + }, [packageId]); + // Package-level publish (ADR-0033/0037/0048): edits accumulate as per-item // drafts STAMPED with this package (each save passes packageId → the draft row's // sys_metadata.package_id). Publishing promotes exactly THIS package's drafts in @@ -606,7 +629,12 @@ export function StudioDesignSurface({ aiSlot }: StudioDesignSurfaceProps): React
{tab === 'data' ? ( - + ) : tab === 'automations' ? ( ) : tab === 'access' ? ( @@ -1350,10 +1378,20 @@ function DataPillar({ packageId, publishNonce = 0, onDraftSaved, + writable = true, }: { packageId: string; publishNonce?: number; onDraftSaved?: () => void; + /** + * Courtesy client-side gate (ADR-0057 D10) mirroring the server's + * writable_package_required check — a read-only (code/installed) package + * rejects authoring writes late, at save time, so hide/disable the "Add + * field" and "New object" affordances up front instead of letting the user + * mutate the canvas and then hit a save-time error. The server remains the + * authority; this only sets expectations. + */ + writable?: boolean; }): React.ReactElement { const client = useMetadataClient(); const adapter = useAdapter(); @@ -1419,7 +1457,7 @@ function DataPillar({ setCurrent((c) => c ?? items[0] ?? null); // First-run: an empty writable package opens the creator right away — // the first thing to do here is make an object, so put the inputs up. - if (items.length === 0) setCreating(true); + if (items.length === 0 && writable) setCreating(true); } catch (e) { if (!cancelled) setError(formatMetadataError(e)); } finally { @@ -1429,7 +1467,7 @@ function DataPillar({ return () => { cancelled = true; }; - }, [client, packageId]); + }, [client, packageId, writable]); React.useEffect(() => { if (!current) return; @@ -1478,14 +1516,17 @@ function DataPillar({ }, []); // "+ add field": append a fresh text field and select it for editing in the panel. + // Guarded by `writable` in addition to being hidden — belt-and-suspenders, + // since it's also wired through GridFieldAuthoringProvider/ObjectFormDesigner. const addField = React.useCallback(() => { + if (!writable) return; const view = readFields(objDraft.fields); const name = nextFieldName(view.entries.map((e) => e.name)); view.entries.push(newField(name, 'text', t('engine.studio.data.newFieldLabel', locale))); setObjDraft((d) => ({ ...d, fields: writeFields(view) })); setDirty(true); setFieldSel({ kind: 'field', id: name }); - }, [objDraft]); + }, [objDraft, writable]); // "+ new object": create a fresh object as a DRAFT in this package (runtime // create — same path the classic Studio editor uses), seeded with one text @@ -1493,6 +1534,7 @@ function DataPillar({ // until the package publish, so we land on 表单·布局 — the metadata-level // surface that never fires data SQL. const doCreateObject = React.useCallback(async () => { + if (!writable) return; const label = newLabel.trim(); const name = toFieldName(newName.trim() || label); if (!label || !name || name === 'field') return; // CJK label → identifier must be typed @@ -1520,7 +1562,7 @@ function DataPillar({ } finally { setCreateBusy(false); } - }, [newLabel, newName, objects, client, packageId, onDraftSaved]); + }, [newLabel, newName, objects, client, packageId, onDraftSaved, writable]); const doSave = React.useCallback(async () => { if (!current) return; @@ -1640,7 +1682,14 @@ function DataPillar({ ))}
- {creating ? ( + {!writable ? ( +
+ {t('engine.studio.data.newObject', locale)} +
+ ) : creating ? (
- {(viewMode === 'grid' || viewMode === 'form') && ( + {(viewMode === 'grid' || viewMode === 'form') && writable && (