Skip to content

Commit 3fd0ba1

Browse files
fix(app-shell): creating an item or opening one from Pending changes asks before it drops a held Studio edit (#26927)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8KPvkcu6BzA4jz9XAcq4ErH)_ Fixes #26845 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio 的 Data、Automations、Interfaces 支柱有暂存未发的编辑时,点「新建对象/流程/仪表盘等」会先弹出同一句离开确认;从 Pending changes 里打开同一支柱的另一个对象也先问。取消则对话框不打开、暂存编辑保留。 - 为什么改:#26839 只拦了离开支柱、左侧栏切换和刷新;新建和 Pending changes 跳转同样会换掉暂存所在的缓冲区,之前一点就静默丢失。分诊裁决是「两条路径都走同一个离开守卫」。 - 风险与回滚:只多问一句,复用 `confirmDropUnsavedWork` 与原有文案,不改保存、发布和暂存判定。有问题直接 revert 本 PR。 - 先看:`StudioDesignSurface.tsx` 中 DataPillar 挪到 `objHeld` 之后的 live request effect(唯一的行为改动点),然后三处新建入口(`openCreate`、`openCreateObject`、`openCreateFlow`),最后 `StudioDesignSurface.heldDataCreate-26845.test.tsx`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/StudioDesignSurface.heldDataCreate-26845.test.tsx`,预期 3 个测试全绿。 - 需要你做的:无,席位落地。 ## What changes Base `94e29cd1`. One shared guard, now read by the two departures #26705 left open: - **Create while held.** Each pillar asks `confirmDropUnsavedWork` before its New dialog opens, because a created object, flow or Interfaces item is opened at once (`setCurrent` in `doCreateObject`, `doCreateFlow` and `doCreateSurface`), and that load replaces the one buffer the held edit lives in. Asking before the dialog means a decline costs nothing and the create itself never asks again. - Interfaces: `openCreate` (the "+" menu's New dashboard / report / page) reads `leafHeld`. - Data: a new `openCreateObject`, declared below `objHeld`, wraps `openCreateDialog` for the rail's New and the empty state's New (the empty state can hold nothing; it goes through the same door so every create asks the same way). - Automations: `openCreateFlow` reads `flowHeld`. - **Same-pillar Pending changes request.** Measured: the Data pillar is the only subscriber of the live surface channel (`useRequestedSurface`), and the sheet's security block is the only producer, so a same-pillar request is always an object request applied by the Data pillar. Its apply effect moves below `objHeld` and asks, like the rail, only when the requested object is not the open one. Declined, the request id is spent (as before, at most once) and the held edit stays. A request for the open object asks nothing. - **Cross-pillar request:** unchanged. `requestSurface` already asks through `confirmLeavePillar` (which reads `heldEdit` since #26839) and vetoes on a decline; the Data pillar it lands on holds nothing, so its apply asks nothing more. Pinned below, not changed. - Unchanged by design: how a held edit is judged and saved, Publish gating, and the question's wording (`engine.edit.unsavedLeaveConfirm`, no new i18n key). The Access pillar is not touched. ## Why Triage first grade on the card, verbatim: "**p2, the remainder of #26705** (PR #26839): two more departures still drop a held pillar edit without asking: creating a new object, flow or Interfaces item, and opening another item from Pending changes. Route both through the same departure guard." ## Tests Three new files, one per pillar, each through the real `StudioDesignSurface` on the real route shape, with the held edit produced as #26839's files produce it (a Picklist with no option, a Notify step with no Title, a Metric widget with no dataset). The Data and Automations files replace the mocked Pending changes sheet with a stand-in that asks the host for an object through `useSurfaceNavigator`, as the sheet's security block does. - `StudioDesignSurface.heldDataCreate-26845.test.tsx` (3): New object asks, Cancel opens no dialog and keeps the held field, OK opens the dialog and the create (Ticket, stored as `acme_ticket`) asks nothing more; a Pending changes request for another object asks, Cancel keeps the held field, a request for the open object asks nothing, OK opens the other object; CONTROL with nothing held, New object and a request never ask. - `StudioDesignSurface.heldFlowCreate-26845.test.tsx` (3): New flow asks, Cancel opens no dialog and keeps the held step, OK opens the dialog; a cross-pillar request (an object, from Automations while a step is held) asks exactly once, Cancel stays on Automations with the held step, OK opens the Data pillar on that object with no second ask; CONTROL with nothing held. - `StudioDesignSurface.heldWidgetCreate-26845.test.tsx` (2): New dashboard asks, Cancel opens no dialog and keeps the held widget, OK opens the dialog; CONTROL with nothing held. <details><summary>Runs (head 3a6924e)</summary> - New files: 3 files, 8/8 passed (lock VERDICT command-exit 0; tests 3.64 s of wall in total). - Every test file under `ui/packages` that imports `StudioDesignSurface`, `DataPillar`, `AutomationsPillar` or `InterfacesPillar` (161 files, `DataPillar.surfaceRequest.test.tsx` and the three #26839 files among them): 161/161 files, 1179/1179 tests passed, lock VERDICT command-exit 0. - `pnpm --filter @object-ui/app-shell run type-check` (both projects, tests included): exit 0. - eslint on the four files: 0 errors; `StudioDesignSurface.tsx` carries 16 warnings, as on the base (the moved effect's `set-state-in-effect` warning moves with it; the count is unchanged), the three tests none. </details> <details><summary>Ablation (one-shot, through scripts/ablation-replace.mjs --delete; each leg restored to the HEAD blob with an empty git diff HEAD)</summary> - Leg 1, drop the Interfaces create guard (`leafHeld`): `heldWidgetCreate` 1 of 2 red, the create case only. - Leg 2, drop the Data create guard (`openCreateObject`): `heldDataCreate` 1 of 3 red, the New object case only. - Leg 3, drop the Data request guard (in the apply effect): `heldDataCreate` 1 of 3 red, the Pending changes case only. - Leg 4, drop the Automations create guard (`flowHeld`): `heldFlowCreate` 1 of 3 red, the New flow case only. Its cross-pillar case pins the existing surface guard and stays green, as intended. </details> ## Acceptance notes - The Interfaces "+" menu's other kinds (link, group, and the rest) add a nav entry and open nav editing without changing the open leaf, so they do not replace the leaf buffer and are not guarded here. - Only the Data pillar applies a same-pillar live request today; the Automations and Access pillars do not subscribe to the channel, so a same-pillar request there opens nothing and drops nothing. - `origin/main` gained `57820085` (#26856, metadata-protocol only) after the base; it touches none of these files or the derived gate inputs, so it was not merged in. - `@object-ui/app-shell` is `private: true`, so no changeset. ## Verification Gates derived by `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `3a6924e3`: 24 families, all exit 0, reconciled with `--ran` (24 run, 0 NOT-MEASURED, 0 UNRUN). `pnpm --dir ui exec vitest run scripts/` first queued out of the lock (exit 99, not run) while the 161-file run held it, then ran in full under the same slot: 178 files passed, 2 skipped, exit 0. `pnpm check:new-line-citations` (ui): 0 new citations. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (Not touched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. (eslint on the four files: 0 errors.) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. (Publishes nothing: `@object-ui/app-shell` is private.) - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (Not touched.) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English, apart from the maintainer brief above. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbZTZSxa584VWULEuMZ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 216a39e commit 3fd0ba1

4 files changed

Lines changed: 744 additions & 15 deletions

File tree

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,237 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#26845 — the two departures objectstack#26705 left open in the
5+
* Data pillar: creating an object, and opening another object from the
6+
* Pending changes sheet. Both load another object into the one buffer a held
7+
* edit lives in, so both ask the shared departure question first.
8+
*
9+
* New object asks before its dialog opens, so Cancel costs nothing and the
10+
* create itself asks nothing. A Pending changes request for another object
11+
* asks before the pillar applies it; one for the open object drops nothing
12+
* and asks nothing.
13+
*
14+
* Rendered through the REAL `StudioDesignSurface` on the real route shape, as
15+
* in `StudioDesignSurface.heldDataLeave-26705.test.tsx`; the sheet is a stand-in
16+
* that asks the host for an object exactly as its security block does
17+
* (`useSurfaceNavigator`).
18+
*/
19+
20+
import '@testing-library/jest-dom/vitest';
21+
import * as React from 'react';
22+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
23+
import { render, screen, fireEvent, cleanup, waitFor, within } from '@testing-library/react';
24+
import { MemoryRouter, Route, Routes } from 'react-router-dom';
25+
26+
const PKG = 'com.acme.app';
27+
28+
const OBJECTS: Record<string, Record<string, unknown>> = {
29+
acme_task: { name: 'acme_task', label: 'Task', fields: [{ name: 'title', label: 'Title', type: 'text' }] },
30+
acme_note: { name: 'acme_note', label: 'Note', fields: [{ name: 'body', label: 'Body', type: 'text' }] },
31+
};
32+
33+
const mockClient = vi.hoisted(() => ({
34+
list: vi.fn(async (_type: string) => [] as unknown[]),
35+
listDrafts: vi.fn(async () => []),
36+
listTypes: vi.fn(async () => ({ entries: [] })),
37+
get: vi.fn(async () => null),
38+
references: vi.fn(async () => []),
39+
layered: vi.fn(async (_type: string, _name: string) => ({ code: null, overlay: null, overlayScope: null, effective: null as unknown })),
40+
getDraft: vi.fn(async () => null),
41+
save: vi.fn(async () => ({ success: true })),
42+
publishDraft: vi.fn(async () => ({ success: true })),
43+
publishPackageDrafts: vi.fn(async () => ({ success: true, failed: [] })),
44+
publish: vi.fn(async () => ({ success: true })),
45+
reset: vi.fn(async () => ({})),
46+
}));
47+
48+
vi.mock('../metadata-admin/useMetadata', async (importOriginal) => {
49+
const mod = await importOriginal<typeof import('../metadata-admin/useMetadata')>();
50+
return { ...mod, useMetadataClient: () => mockClient, useMetadataTypes: () => ({ entries: [] }) };
51+
});
52+
53+
vi.mock('./packages-io', async (importOriginal) => {
54+
const mod = await importOriginal<typeof import('./packages-io')>();
55+
return {
56+
...mod,
57+
fetchPackages: vi.fn(async () => [{ id: 'com.acme.app', name: 'Acme', writable: true, namespace: 'acme' }]),
58+
};
59+
});
60+
61+
vi.mock('@object-ui/react', async (importOriginal) => {
62+
const mod = await importOriginal<typeof import('@object-ui/react')>();
63+
return { ...mod, useAdapter: () => dataSource };
64+
});
65+
66+
vi.mock('./StudioAiCopilot', () => ({ StudioChatDock: () => null }));
67+
// Stands in for the Pending changes sheet: its security block asks the host to
68+
// open the object a draft names, through the surface channel.
69+
vi.mock('../../preview/DraftChangesPanel', async () => {
70+
const { createElement } = await import('react');
71+
const { useSurfaceNavigator } = await import('./surfaceDeepLinkChannel');
72+
return {
73+
DraftChangesPanel: () => {
74+
const openSurface = useSurfaceNavigator();
75+
return createElement(
76+
'div',
77+
null,
78+
['acme_task', 'acme_note'].map((name) =>
79+
createElement(
80+
'button',
81+
{ key: name, type: 'button', onClick: () => openSurface?.({ type: 'object', name }) },
82+
`Pending object/${name}`,
83+
),
84+
),
85+
);
86+
},
87+
};
88+
});
89+
90+
import { StudioDesignSurface } from './StudioDesignSurface';
91+
import { addFieldFromFormToolbar } from './__tests__/dataPillarAddField';
92+
import { createEmptyDataSource } from './__tests__/emptyDataSource';
93+
import { registerBuiltinInspectors } from '../metadata-admin/inspectors';
94+
import { t } from '../metadata-admin/i18n';
95+
96+
const dataSource = createEmptyDataSource();
97+
registerBuiltinInspectors();
98+
99+
vi.stubGlobal(
100+
'fetch',
101+
vi.fn(async () => new Response('null', { status: 404, headers: { 'content-type': 'application/json' } })),
102+
);
103+
104+
window.matchMedia = ((query: string) => ({
105+
matches: false,
106+
media: query,
107+
onchange: null,
108+
addEventListener: () => {},
109+
removeEventListener: () => {},
110+
addListener: () => {},
111+
removeListener: () => {},
112+
dispatchEvent: () => false,
113+
})) as unknown as typeof window.matchMedia;
114+
(globalThis as { ResizeObserver?: unknown }).ResizeObserver =
115+
(globalThis as { ResizeObserver?: unknown }).ResizeObserver ??
116+
class {
117+
observe() {}
118+
unobserve() {}
119+
disconnect() {}
120+
};
121+
122+
// This jsdom environment ships no window.confirm — install a mock outright.
123+
let confirmSpy: ReturnType<typeof vi.fn>;
124+
const LEAVE_QUESTION = t('engine.edit.unsavedLeaveConfirm', 'en');
125+
126+
beforeEach(() => {
127+
for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear();
128+
mockClient.list.mockImplementation(async (type: string) =>
129+
type === 'object' ? Object.values(OBJECTS).map((o) => ({ name: o.name, label: o.label, _packageId: PKG })) : [],
130+
);
131+
mockClient.layered.mockImplementation(async (type: string, name: string) => ({
132+
code: null,
133+
overlay: null,
134+
overlayScope: null,
135+
effective: type === 'object' && OBJECTS[name] ? JSON.parse(JSON.stringify(OBJECTS[name])) : null,
136+
}));
137+
confirmSpy = vi.fn();
138+
window.confirm = confirmSpy as unknown as typeof window.confirm;
139+
});
140+
141+
afterEach(() => {
142+
cleanup();
143+
localStorage.clear();
144+
window.history.replaceState(null, '', '/');
145+
});
146+
147+
/** The object the header names: its api-name chip. */
148+
async function openObjectIs(name: string): Promise<void> {
149+
await waitFor(
150+
() => expect(within(screen.getByTestId('data-object-header')).getByTestId('studio-item-api-name')).toHaveTextContent(name),
151+
{ timeout: 8000 },
152+
);
153+
}
154+
155+
async function openTask(): Promise<void> {
156+
// The shell's link guard reads the window's URL to skip a link to the page
157+
// already shown; `BrowserRouter` keeps the two equal, so this does too.
158+
window.history.replaceState(null, '', `/studio/${PKG}/data`);
159+
render(
160+
<MemoryRouter initialEntries={[`/studio/${PKG}/data`]}>
161+
<Routes>
162+
<Route path="/studio/:packageId/:tab" element={<StudioDesignSurface />} />
163+
</Routes>
164+
</MemoryRouter>,
165+
);
166+
await openObjectIs('acme_task');
167+
}
168+
169+
/** The card's repro: add a Picklist "Priority" with no option yet; the autosave holds it. */
170+
async function holdAPicklist(): Promise<void> {
171+
fireEvent.click(await screen.findByRole('button', { name: 'Form' }));
172+
await addFieldFromFormToolbar(undefined, { label: 'Priority', type: /^Picklist$/ });
173+
await screen.findByTestId('studio-held', undefined, { timeout: 8000 });
174+
}
175+
176+
const newObject = () => fireEvent.click(within(screen.getByTestId('studio-rail-data')).getByTestId('studio-rail-add'));
177+
const pendingRequest = (name: string) => fireEvent.click(screen.getByRole('button', { name: `Pending object/${name}` }));
178+
179+
describe('a held Data field guards create and Pending changes (objectstack#26845)', () => {
180+
it('New object asks; Cancel opens no dialog and keeps the held field, OK opens it and the create asks nothing', async () => {
181+
await openTask();
182+
await holdAPicklist();
183+
184+
confirmSpy.mockReturnValueOnce(false);
185+
newObject();
186+
expect(confirmSpy).toHaveBeenCalledTimes(1);
187+
expect(confirmSpy).toHaveBeenCalledWith(LEAVE_QUESTION);
188+
expect(screen.queryByRole('dialog')).toBeNull();
189+
expect(screen.getByTestId('studio-held')).toBeInTheDocument();
190+
191+
confirmSpy.mockReturnValueOnce(true);
192+
newObject();
193+
expect(confirmSpy).toHaveBeenCalledTimes(2);
194+
const dialog = await screen.findByRole('dialog');
195+
fireEvent.change(within(dialog).getByLabelText(t('engine.studio.data.nameLabel', 'en')), {
196+
target: { value: 'Ticket' },
197+
});
198+
fireEvent.click(within(dialog).getByRole('button', { name: t('engine.studio.createDraft', 'en') }));
199+
await openObjectIs('acme_ticket');
200+
expect(confirmSpy).toHaveBeenCalledTimes(2);
201+
});
202+
203+
it('a Pending changes request for another object asks; Cancel keeps the held field, OK opens the object', async () => {
204+
await openTask();
205+
await holdAPicklist();
206+
207+
confirmSpy.mockReturnValueOnce(false);
208+
pendingRequest('acme_note');
209+
await waitFor(() => expect(confirmSpy).toHaveBeenCalledTimes(1));
210+
expect(confirmSpy).toHaveBeenCalledWith(LEAVE_QUESTION);
211+
await openObjectIs('acme_task');
212+
expect(screen.getByTestId('studio-held')).toBeInTheDocument();
213+
214+
// A request for the open object drops nothing, so it asks nothing.
215+
pendingRequest('acme_task');
216+
await openObjectIs('acme_task');
217+
expect(screen.getByTestId('studio-held')).toBeInTheDocument();
218+
expect(confirmSpy).toHaveBeenCalledTimes(1);
219+
220+
confirmSpy.mockReturnValueOnce(true);
221+
pendingRequest('acme_note');
222+
await waitFor(() => expect(confirmSpy).toHaveBeenCalledTimes(2));
223+
await openObjectIs('acme_note');
224+
expect(screen.queryByTestId('studio-held')).toBeNull();
225+
});
226+
227+
// CONTROL: nothing held, nothing asked.
228+
it('with no edit held, New object and a Pending changes request never ask', async () => {
229+
await openTask();
230+
231+
pendingRequest('acme_note');
232+
await openObjectIs('acme_note');
233+
newObject();
234+
await screen.findByRole('dialog');
235+
expect(confirmSpy).not.toHaveBeenCalled();
236+
});
237+
});

0 commit comments

Comments
 (0)