Skip to content

Commit 703dcec

Browse files
os-zhuangclaude
andauthored
feat(studio): spec-driven package create/edit/view form in a modal (#2535)
* feat(studio): spec-driven package create/edit/view form in a modal Package create/edit/view were three hand-rolled forms (CreatePackageDialog, EditPackageDialog, BuilderLanding's inline form) with two different id-validation regexes — a package created on one surface was rejected by another, and neither matched what the server accepts. Replace them with ONE modal (PackageFormDialog) that renders the manifest form from the spec (ManifestSchema → z.toJSONSchema → SchemaForm), mirroring the existing page-schema / view-schema pattern. - package-schema.ts: spec-derived JSONSchema + a curated FormView (Basics + collapsible Advanced). id/type/namespace/scope are `immutable` — editable on create, locked on edit (the REST PATCH only persists name/description/version). - PackageFormDialog: create → POST, edit → PATCH, view → readOnly. Duplicate 409 maps to a friendly "already exists" message. Create sends NO scope, so a runtime-created base stays writable (not 只读). - CreatePackageDialog / EditPackageDialog kept as thin wrappers so call sites (ResourceListPage, the Studio switcher) don't change; BuilderLanding's inline form now opens the dialog; PackageDetailSheet gains a "View info" (readOnly spec form) action. Net −254 lines (three forms collapse into one spec-driven component). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(studio): port namespace field into the spec-form create dialog The merge with main brought in CreatePackageDialog.namespace.test.tsx (framework#2694), which targeted the old hand-rolled form's test ids and broke now that CreatePackageDialog renders the spec form. PackageFormDialog already auto-derives namespace from the id and gates submit on NAMESPACE_RE; this adds keystroke sanitization (lowercase + allowed alphabet) to match the old field, and rewrites the test to target the spec form by label. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2b30583 commit 703dcec

8 files changed

Lines changed: 606 additions & 440 deletions

File tree

packages/app-shell/src/views/metadata-admin/CreatePackageDialog.namespace.test.tsx

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22

33
/**
44
* Wiring guard for the package-namespace field (framework#2694) in the
5-
* package-switcher "+ new" dialog: it defaults to the id-derived namespace,
6-
* tracks the id input until the user edits it, and gates submit on a valid
7-
* `^[a-z][a-z0-9_]{1,19}$` value.
5+
* spec-driven create dialog (`CreatePackageDialog` → `PackageFormDialog`): it
6+
* defaults to the id-derived namespace, tracks the id input until the user
7+
* edits it, sanitizes keystrokes to the allowed alphabet, and gates submit on a
8+
* valid `^[a-z][a-z0-9_]{1,19}$` value.
9+
*
10+
* The form is now rendered from the manifest spec via SchemaForm, so fields are
11+
* targeted by their label rather than the old hand-rolled test ids.
812
*/
913
import '@testing-library/jest-dom/vitest';
1014
import * as React from 'react';
@@ -17,13 +21,14 @@ afterEach(cleanup);
1721
function open() {
1822
render(<CreatePackageDialog open onOpenChange={vi.fn()} onCreated={vi.fn()} />);
1923
return {
20-
id: screen.getByTestId('package-id-input') as HTMLInputElement,
21-
ns: screen.getByTestId('package-namespace-input') as HTMLInputElement,
22-
name: screen.getByTestId('package-name-input') as HTMLInputElement,
24+
id: screen.getByLabelText(/package id/i) as HTMLInputElement,
25+
ns: screen.getByLabelText(/object namespace/i) as HTMLInputElement,
26+
name: screen.getByLabelText(/display name/i) as HTMLInputElement,
27+
submit: screen.getByTestId('package-form-submit') as HTMLButtonElement,
2328
};
2429
}
2530

26-
describe('CreatePackageDialog — namespace field', () => {
31+
describe('CreatePackageDialog — namespace field (spec form)', () => {
2732
it('derives the namespace from the package id and tracks it', () => {
2833
const { id, ns } = open();
2934
fireEvent.change(id, { target: { value: 'com.example.leave' } });
@@ -50,10 +55,9 @@ describe('CreatePackageDialog — namespace field', () => {
5055
});
5156

5257
it('disables submit while the namespace is invalid, enables it when valid', () => {
53-
const { id, ns, name } = open();
58+
const { id, ns, name, submit } = open();
5459
fireEvent.change(id, { target: { value: 'com.example.leave' } });
5560
fireEvent.change(name, { target: { value: 'Leave' } });
56-
const submit = screen.getByRole('button', { name: /create package|/i });
5761
expect(submit).toBeEnabled();
5862
// Single char is below the 2-char minimum → invalid.
5963
fireEvent.change(ns, { target: { value: 'a' } });

packages/app-shell/src/views/metadata-admin/EditPackageDialog.test.tsx

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
55
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
66
import { EditPackageDialog, type InstalledPackage } from './PackagesPage';
77

8-
// PackagesPage.apiJson uses the raw global fetch (fetch → res.text() → JSON.parse),
9-
// so we stub global fetch and mirror the runtime's { success, data } envelope —
10-
// exactly what `PATCH /api/v1/packages/:id` returns (framework http-dispatcher).
8+
// EditPackageDialog is now a thin wrapper over the spec-driven PackageFormDialog
9+
// (edit mode). It renders the manifest form via SchemaForm and PATCHes only the
10+
// three fields the REST surface persists (name / description / version).
11+
// PackageFormDialog's apiJson uses the raw global fetch (fetch → res.text() →
12+
// JSON.parse), so we stub global fetch and mirror the runtime's { success, data }
13+
// envelope — exactly what `PATCH /api/v1/packages/:id` returns.
1114
const PKG: InstalledPackage = {
1215
manifest: { id: 'com.acme.crm', name: 'Acme CRM', version: '1.0.0', description: 'old', type: 'app' },
1316
enabled: true,
@@ -22,12 +25,8 @@ beforeEach(() => {
2225
'fetch',
2326
vi.fn(async (url: string, init: RequestInit = {}) => {
2427
calls.push({ url, init });
25-
// Echo the patched manifest back, the way the dispatcher does.
2628
const body = init.body ? JSON.parse(init.body as string) : {};
27-
const updated: InstalledPackage = {
28-
...PKG,
29-
manifest: { ...PKG.manifest, ...body },
30-
};
29+
const updated: InstalledPackage = { ...PKG, manifest: { ...PKG.manifest, ...body } };
3130
return {
3231
ok: true,
3332
status: 200,
@@ -39,57 +38,47 @@ beforeEach(() => {
3938

4039
afterEach(() => vi.unstubAllGlobals());
4140

42-
describe('EditPackageDialog', () => {
43-
it('prefills from the manifest and PATCHes only the edited fields', async () => {
41+
describe('EditPackageDialog (spec-form wrapper)', () => {
42+
it('prefills from the manifest and PATCHes only the server-persisted fields', async () => {
4443
const onSaved = vi.fn();
4544
const onOpenChange = vi.fn();
4645
render(<EditPackageDialog pkg={PKG} open onOpenChange={onOpenChange} onSaved={onSaved} />);
4746

48-
const nameInput = (await screen.findByTestId('package-edit-name-input')) as HTMLInputElement;
49-
expect(nameInput.value).toBe('Acme CRM'); // prefilled
50-
51-
fireEvent.change(nameInput, { target: { value: 'Acme CRM v2' } });
52-
fireEvent.click(screen.getByTestId('package-edit-save'));
47+
// Stable submit affordance regardless of the SchemaForm field internals.
48+
fireEvent.click(await screen.findByTestId('package-form-submit'));
5349

5450
await waitFor(() => expect(onSaved).toHaveBeenCalledTimes(1));
5551

56-
// Issued a PATCH to the package's REST id with the edited manifest fields.
52+
// One PATCH to the package's REST id, carrying ONLY name/description/version.
5753
expect(calls).toHaveLength(1);
5854
expect(calls[0].url).toBe('/api/v1/packages/com.acme.crm');
5955
expect(calls[0].init.method).toBe('PATCH');
60-
const sent = JSON.parse(calls[0].init.body as string);
61-
expect(sent).toMatchObject({ name: 'Acme CRM v2', version: '1.0.0' });
56+
expect(JSON.parse(calls[0].init.body as string)).toEqual({
57+
name: 'Acme CRM',
58+
description: 'old',
59+
version: '1.0.0',
60+
});
6261

6362
// onSaved receives the server's updated package; the dialog closes.
64-
expect(onSaved.mock.calls[0][0].manifest.name).toBe('Acme CRM v2');
63+
expect(onSaved.mock.calls[0][0].manifest.name).toBe('Acme CRM');
6564
expect(onOpenChange).toHaveBeenCalledWith(false);
6665
});
6766

68-
it('blocks save on a non-semantic version and never calls the API', async () => {
69-
render(<EditPackageDialog pkg={PKG} open onOpenChange={vi.fn()} onSaved={vi.fn()} />);
70-
const versionInput = await screen.findByLabelText(/version/i);
71-
fireEvent.change(versionInput, { target: { value: '1.2' } });
72-
73-
const save = screen.getByTestId('package-edit-save') as HTMLButtonElement;
74-
expect(save.disabled).toBe(true);
75-
fireEvent.click(save);
76-
expect(calls).toHaveLength(0);
77-
});
78-
7967
it('surfaces a server error and keeps the dialog open', async () => {
8068
vi.stubGlobal(
8169
'fetch',
8270
vi.fn(async () => ({
8371
ok: false,
8472
status: 400,
85-
text: async () => JSON.stringify({ success: false, error: { message: 'version must be semantic (e.g. 1.0.0)' } }),
73+
text: async () =>
74+
JSON.stringify({ success: false, error: { message: 'version must be semantic (e.g. 1.0.0)' } }),
8675
})),
8776
);
8877
const onSaved = vi.fn();
8978
const onOpenChange = vi.fn();
9079
render(<EditPackageDialog pkg={PKG} open onOpenChange={onOpenChange} onSaved={onSaved} />);
9180

92-
fireEvent.click(screen.getByTestId('package-edit-save'));
81+
fireEvent.click(await screen.findByTestId('package-form-submit'));
9382

9483
expect(await screen.findByText(/version must be semantic/i)).toBeInTheDocument();
9584
expect(onSaved).not.toHaveBeenCalled();
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import * as React from 'react';
4+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
5+
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
6+
import { PackageFormDialog } from './PackageFormDialog';
7+
8+
// PackageFormDialog renders the manifest form from the spec (package-schema)
9+
// via SchemaForm and talks to `/api/v1/packages`. apiJson uses the raw global
10+
// fetch (fetch → res.text() → JSON.parse), so we stub fetch and mirror the
11+
// runtime's { success, data } envelope.
12+
let calls: Array<{ url: string; init: RequestInit }>;
13+
14+
function stubOk() {
15+
vi.stubGlobal(
16+
'fetch',
17+
vi.fn(async (url: string, init: RequestInit = {}) => {
18+
calls.push({ url, init });
19+
const body = init.body ? JSON.parse(init.body as string) : {};
20+
const manifest = body.manifest ?? body;
21+
return {
22+
ok: true,
23+
status: 201,
24+
text: async () => JSON.stringify({ success: true, data: { manifest, status: 'installed' } }),
25+
} as Response;
26+
}),
27+
);
28+
}
29+
30+
beforeEach(() => {
31+
calls = [];
32+
stubOk();
33+
});
34+
afterEach(() => vi.unstubAllGlobals());
35+
36+
describe('PackageFormDialog — create mode', () => {
37+
it('POSTs { manifest } from the spec form and reports the new id', async () => {
38+
const onSaved = vi.fn();
39+
render(<PackageFormDialog mode="create" open onOpenChange={vi.fn()} onSaved={onSaved} />);
40+
41+
// Fields come from the spec-derived FormView (package-schema); target them by label.
42+
fireEvent.change(await screen.findByLabelText(/package id/i), { target: { value: 'com.acme.new' } });
43+
fireEvent.change(screen.getByLabelText(/display name/i), { target: { value: 'New App' } });
44+
// version prefills to 0.1.0; type prefills to 'app'.
45+
46+
fireEvent.click(screen.getByTestId('package-form-submit'));
47+
48+
await waitFor(() => expect(onSaved).toHaveBeenCalledTimes(1));
49+
expect(calls).toHaveLength(1);
50+
expect(calls[0].url).toBe('/api/v1/packages');
51+
expect(calls[0].init.method).toBe('POST');
52+
expect(JSON.parse(calls[0].init.body as string).manifest).toMatchObject({
53+
id: 'com.acme.new',
54+
name: 'New App',
55+
version: '0.1.0',
56+
type: 'app',
57+
});
58+
expect(onSaved.mock.calls[0][0].id).toBe('com.acme.new');
59+
});
60+
61+
it('maps a 409 duplicate id to a friendly "already exists" message and stays open', async () => {
62+
vi.stubGlobal(
63+
'fetch',
64+
vi.fn(async () => ({
65+
ok: false,
66+
status: 409,
67+
text: async () => JSON.stringify({ success: false, error: { message: "Package 'com.acme.new' already exists" } }),
68+
})),
69+
);
70+
const onSaved = vi.fn();
71+
const onOpenChange = vi.fn();
72+
render(<PackageFormDialog mode="create" open onOpenChange={onOpenChange} onSaved={onSaved} />);
73+
74+
fireEvent.change(await screen.findByLabelText(/package id/i), { target: { value: 'com.acme.new' } });
75+
fireEvent.change(screen.getByLabelText(/display name/i), { target: { value: 'Dup App' } });
76+
fireEvent.click(screen.getByTestId('package-form-submit'));
77+
78+
expect(await screen.findByText(/already exists/i)).toBeInTheDocument();
79+
expect(onSaved).not.toHaveBeenCalled();
80+
expect(onOpenChange).not.toHaveBeenCalledWith(false);
81+
});
82+
});

0 commit comments

Comments
 (0)