Skip to content

Commit 31a608e

Browse files
fix(app-shell): nav page picker offers a draft page only when its draft body can be a nav target (#26968)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8PBqR3JqcpQdxvP362pdfwm)_ Fixes #26743 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio Interfaces 导航项的「页面」选择器不再列出草稿态的记录页;草稿页按其草稿正文的 `type` 判定,与已发布页同一规则(绑定对象的草稿动作同理)。 - 为什么改:`_drafts` 行只带元数据类型 `page`,不带页面自身的 `type`,原代码对草稿行一律放行,选中草稿记录页、随应用发布后导航项打不开。 - 风险与回滚:只改 Studio 选择器的候选列表;每个仅草稿的页面/动作多一次草稿读取;读不到正文时仍列出(与无 `type` 的已发布行一致)。回滚即 revert 本 PR。 - 先看:`StudioDesignSurface.tsx` 的 `isDraftNavTarget` 与 `usePackageNavTargets` 中草稿循环;再看新测试 `StudioDesignSurface.navPageDrafts-26743.test.tsx`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/StudioDesignSurface.navPageDrafts-26743.test.tsx`,预期 6 个测试全过。 - 需要你做的:无,席位落地。 ## What changes - `usePackageNavTargets` (Interfaces nav item inspector's target picker): a draft-only row of a type whose `isNavTargetRow` verdict reads a body key (`page`: the body's `type`; `action`: `objectName`) is now judged on its draft body, read with `client.getDraft` and unwrapped by `extractDraftBody`, the one draft-envelope reader. A draft record page is no longer offered; a draft static page still is. - A body that cannot be read (no draft served, a failed read) keeps the row, as a published row with no `type` is kept: only a confirmed non-target is dropped. Other target types (`dashboard`, `report`, `doc`) read no body. - In-place fix named here: the same draft loop also offered a draft action bound to an object, which the nav cannot run (it runs global actions only). Same defect class, same line, same gate families; covered by one test. ## Why The `_drafts` header (`ListDraftsResponseSchema`) carries the metadata type name and the draft's `label`, never the body's own `type`, so the old loop could not filter and added every draft-only row. Fixed client-side with the existing draft read; no spec or server change. ## Tests New `StudioDesignSurface.navPageDrafts-26743.test.tsx` (6 tests): a draft record page is not offered; a draft static page is offered; a published record page is still not offered; CONTROL an unreadable draft body is still offered; a draft object-bound action is not offered, a draft global action is; only body-keyed types read drafts, published rows are never re-read. <details><summary>Ablation (commit ca23ec4)</summary> `node scripts/ablation-replace.mjs` replaced the anchor `if (verdicts[i]) byName.set(row.name, row);` with an always-true condition: anchor 1 -> 0, blob 8a30d22 -> e63569e6a119. Result: `a draft record page is not offered` and `a draft action bound to an object is not offered, a draft global action is` failed (2 failed, 4 passed). Restored: blob == HEAD (8a30d22), `git diff HEAD` empty. No dist leg: the test imports the component by relative path. </details> ## Verification All local readings at head `ca23ec41` (base `aa00362f`). <details><summary>Local gates</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 24 derived families, all run, every one exit 0; `--ran` reconciliation: 24 derived, 24 run, 0 NOT-MEASURED, 0 UNRUN. - `node scripts/check-sdui-manifest.mjs` exit 0 (and `--self-test` exit 0). - `pnpm --filter @object-ui/app-shell run type-check` exit 0. - `pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/ packages/app-shell/src/views/metadata-admin/inspectors/`: Test Files 429 passed, Tests 4289 passed, 1 skipped. - `pnpm --dir ui run check:new-line-citations`: 0 new citations, exit 0. - `node scripts/check-nul-bytes.mjs` exit 0. </details> Changeset: none. `@object-ui/app-shell` is `private: true`, so nothing publishes; `skip-changeset` is declared with this PR. ## Acceptance notes - Each draft-only page or action costs one `?state=draft` read when the picker opens; `getDraft` asks the shared `_drafts` ledger read first, so no extra ledger request is sent. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes (CI). - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset (nothing publishes here), and both declaration lines are filled in. - [x] 6. `packages/spec` 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. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbYChiSg5184uQo2r5x)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent fd246c2 commit 31a608e

2 files changed

Lines changed: 241 additions & 4 deletions

File tree

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack-ai/objectstack#26743 — the Interfaces nav editor's target picker
5+
* offers a draft-only page only when its draft body can be a nav target.
6+
*
7+
* A `page` nav entry opens a static page URL with no record id, so a record
8+
* page is never a valid target (`isStaticPageOption`). The picker filtered the
9+
* published rows by the body's `type`, but added every draft-only row
10+
* unfiltered: a `_drafts` header names the metadata type (`page`), not the
11+
* body's `type`. A draft record page was offered, and picking it gave a dead
12+
* entry once the app was published. The picker now reads a draft-only row's
13+
* draft body and judges it as it judges a published row:
14+
*
15+
* - a draft record page is not offered;
16+
* - a draft static page is offered;
17+
* - a published record page is still not offered;
18+
* - a draft action bound to an object is not offered (the nav runs global
19+
* actions only), a draft global action is;
20+
* - CONTROL: a draft whose body cannot be read is still offered, as a row
21+
* with no `type` is.
22+
*/
23+
24+
import '@testing-library/jest-dom/vitest';
25+
import * as React from 'react';
26+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
27+
import { cleanup, fireEvent, render, screen, within } from '@testing-library/react';
28+
import { MetadataCtx } from '@object-ui/react';
29+
30+
const PKG = 'com.acme.app';
31+
32+
/** Published pages, as `client.list('page')` serves them: bodies, with `type`. */
33+
const PUBLISHED_PAGES = [
34+
{ name: 'ops_home', label: 'Ops home', type: 'app' },
35+
{ name: 'ticket_detail', label: 'Ticket detail', type: 'record' },
36+
];
37+
38+
/** Draft bodies the `?state=draft` read serves, keyed `type:name`. */
39+
const DRAFT_BODIES: Record<string, Record<string, unknown> | undefined> = {
40+
'page:case_record': { name: 'case_record', label: 'Case record', type: 'record' },
41+
'page:case_board': { name: 'case_board', label: 'Case board', type: 'list' },
42+
'action:close_case': { name: 'close_case', label: 'Close case', objectName: 'repairs_case' },
43+
'action:open_inbox': { name: 'open_inbox', label: 'Open inbox' },
44+
};
45+
46+
const mockClient = vi.hoisted(() => ({
47+
list: vi.fn(async (): Promise<unknown[]> => []),
48+
// The `_drafts` headers: metadata type and name, never the body's `type`.
49+
listDrafts: vi.fn(async (options?: { packageId?: string; type?: string }): Promise<unknown[]> => {
50+
if (options?.type === 'page') {
51+
return [
52+
{ type: 'page', name: 'case_record', packageId: 'com.acme.app', label: 'Case record' },
53+
{ type: 'page', name: 'case_board', packageId: 'com.acme.app', label: 'Case board' },
54+
{ type: 'page', name: 'case_unread', packageId: 'com.acme.app', label: 'Case unread' },
55+
];
56+
}
57+
if (options?.type === 'action') {
58+
return [
59+
{ type: 'action', name: 'close_case', packageId: 'com.acme.app', label: 'Close case' },
60+
{ type: 'action', name: 'open_inbox', packageId: 'com.acme.app', label: 'Open inbox' },
61+
];
62+
}
63+
return [];
64+
}),
65+
getDraft: vi.fn(async (): Promise<unknown> => null),
66+
get: vi.fn(async () => undefined),
67+
withPreviewDrafts: vi.fn((): unknown => mockClient),
68+
}));
69+
70+
vi.mock('../metadata-admin/useMetadata', async (importOriginal) => {
71+
const mod = await importOriginal<typeof import('../metadata-admin/useMetadata')>();
72+
return {
73+
...mod,
74+
useMetadataClient: () => mockClient,
75+
useMetadataTypes: () => ({ loading: false, error: null, entries: [] }),
76+
};
77+
});
78+
79+
vi.mock('@object-ui/react', async (importOriginal) => {
80+
const mod = await importOriginal<typeof import('@object-ui/react')>();
81+
return { ...mod, useAdapter: () => ({}) };
82+
});
83+
84+
vi.mock('sonner', () => ({
85+
toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), dismiss: vi.fn() },
86+
Toaster: () => null,
87+
}));
88+
89+
vi.mock('../../components/SuggestedBindingsPanel', () => ({ SuggestedBindingsPanel: () => null }));
90+
vi.mock('../metadata-admin/AccessExplainPanel', () => ({ AccessExplainPanel: () => null }));
91+
vi.mock('./StudioAiCopilot', () => ({ StudioChatDock: () => null }));
92+
vi.mock('../../preview/DraftChangesPanel', () => ({ DraftChangesPanel: () => null }));
93+
94+
import { StudioNavItemInspector } from './StudioDesignSurface';
95+
96+
window.matchMedia = ((query: string) => ({
97+
matches: false,
98+
media: query,
99+
onchange: null,
100+
addEventListener: () => {},
101+
removeEventListener: () => {},
102+
addListener: () => {},
103+
removeListener: () => {},
104+
dispatchEvent: () => false,
105+
})) as unknown as typeof window.matchMedia;
106+
(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver =
107+
(globalThis as unknown as { ResizeObserver?: unknown }).ResizeObserver ??
108+
class {
109+
observe() {}
110+
unobserve() {}
111+
disconnect() {}
112+
};
113+
114+
const METADATA = {
115+
apps: [],
116+
objects: [],
117+
dashboards: [],
118+
reports: [],
119+
pages: [],
120+
loading: false,
121+
error: null,
122+
refresh: async () => {},
123+
invalidate: () => {},
124+
ensureType: async () => [],
125+
getItem: async () => null,
126+
getItemsByType: () => [],
127+
getTypeStatus: () => 'ready' as const,
128+
};
129+
130+
beforeEach(() => {
131+
for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear();
132+
mockClient.list.mockImplementation(async (type?: unknown) => (type === 'page' ? PUBLISHED_PAGES : []));
133+
mockClient.getDraft.mockImplementation(async (type?: unknown, name?: unknown) => {
134+
if (name === 'case_unread') throw new Error('draft read failed');
135+
const body = DRAFT_BODIES[`${String(type)}:${String(name)}`];
136+
// The served envelope, decorated as the framework serves it.
137+
return body ? { type, name, item: { ...body, _draft: true } } : null;
138+
});
139+
});
140+
141+
afterEach(() => {
142+
cleanup();
143+
});
144+
145+
async function openPicker(navigation: unknown[], name: string) {
146+
render(
147+
<MetadataCtx.Provider value={METADATA as never}>
148+
<StudioNavItemInspector
149+
navId="navigation[0]"
150+
appDraft={{ navigation }}
151+
objects={[]}
152+
packageId={PKG}
153+
onNavPatch={vi.fn()}
154+
onClear={vi.fn()}
155+
/>
156+
</MetadataCtx.Provider>,
157+
);
158+
fireEvent.keyDown(screen.getByRole('combobox', { name }), { key: 'ArrowDown' });
159+
return screen.findByRole('listbox');
160+
}
161+
162+
describe('objectstack-ai/objectstack#26743 — the nav page picker judges a draft-only page on its draft body', () => {
163+
it('a draft record page is not offered', async () => {
164+
const listbox = await openPicker([{ id: 'nav_page', type: 'page' }], 'Page');
165+
// Wait for the draft static page, read in the same pass, before asserting an absence.
166+
await within(listbox).findByRole('option', { name: 'Case board (case_board)' });
167+
expect(within(listbox).queryByRole('option', { name: /case_record/ })).toBeNull();
168+
});
169+
170+
it('a draft static page is offered', async () => {
171+
const listbox = await openPicker([{ id: 'nav_page', type: 'page' }], 'Page');
172+
expect(await within(listbox).findByRole('option', { name: 'Case board (case_board)' })).toBeInTheDocument();
173+
});
174+
175+
it('a published record page is still not offered, a published static page is', async () => {
176+
const listbox = await openPicker([{ id: 'nav_page', type: 'page' }], 'Page');
177+
expect(await within(listbox).findByRole('option', { name: 'Ops home (ops_home)' })).toBeInTheDocument();
178+
expect(within(listbox).queryByRole('option', { name: /ticket_detail/ })).toBeNull();
179+
});
180+
181+
it('CONTROL: a draft whose body cannot be read is still offered', async () => {
182+
const listbox = await openPicker([{ id: 'nav_page', type: 'page' }], 'Page');
183+
expect(await within(listbox).findByRole('option', { name: 'Case unread (case_unread)' })).toBeInTheDocument();
184+
});
185+
186+
it('a draft action bound to an object is not offered, a draft global action is', async () => {
187+
const listbox = await openPicker([{ id: 'nav_action', type: 'action' }], 'Action');
188+
expect(await within(listbox).findByRole('option', { name: 'Open inbox (open_inbox)' })).toBeInTheDocument();
189+
expect(within(listbox).queryByRole('option', { name: /close_case/ })).toBeNull();
190+
});
191+
192+
it('a draft body is read only for a type whose verdict reads the body', async () => {
193+
await openPicker([{ id: 'nav_page', type: 'page' }], 'Page');
194+
await screen.findByRole('option', { name: 'Case board (case_board)' });
195+
expect(mockClient.getDraft).toHaveBeenCalledWith('page', 'case_record', { packageId: PKG });
196+
// A published row is never re-read as a draft.
197+
expect(mockClient.getDraft).not.toHaveBeenCalledWith('page', 'ops_home', expect.anything());
198+
});
199+
});

‎ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2980,16 +2980,45 @@ const NAV_TARGET_KEY = {
29802980
* Whether a listed row can be a nav target. A record page needs a record id a
29812981
* `page` entry cannot pass (`isStaticPageOption`), and an action bound to an
29822982
* object is not addressable from the nav, which runs GLOBAL actions only
2983-
* (`useNavActionDispatch`). Only a CONFIRMED one is left out: a draft header
2984-
* carries no body, so a draft-only row is kept, as `isStaticPageOption` keeps
2985-
* a row with no `type`.
2983+
* (`useNavActionDispatch`). Only a CONFIRMED one is left out, as
2984+
* `isStaticPageOption` keeps a row with no `type`.
2985+
*
2986+
* objectstack-ai/objectstack#26743 — a `_drafts` header names the METADATA
2987+
* type (`page`), never the body's own `type` (`record`), so a draft-only row
2988+
* is judged on its draft body instead ({@link NAV_TARGET_BODY_KEYED}). A draft
2989+
* record page is still a record page once it is published with the app.
29862990
*/
29872991
function isNavTargetRow(metaType: string, row: Record<string, unknown>): boolean {
29882992
if (metaType === 'page') return isStaticPageOption(row as { type?: string });
29892993
if (metaType === 'action') return !row.objectName;
29902994
return true;
29912995
}
29922996

2997+
/** The types whose {@link isNavTargetRow} verdict reads a body key, so a draft-only row's body is read for it. */
2998+
const NAV_TARGET_BODY_KEYED: ReadonlySet<string> = new Set(['page', 'action']);
2999+
3000+
/**
3001+
* Whether a draft-only row can be a nav target, judged on its draft body. A
3002+
* body that cannot be read (no draft served, a failed read) leaves the row in,
3003+
* as a published row with no `type` is left in: only a confirmed non-target
3004+
* is dropped.
3005+
*/
3006+
async function isDraftNavTarget(
3007+
client: MetadataClient,
3008+
metaType: string,
3009+
name: string,
3010+
packageId: string,
3011+
): Promise<boolean> {
3012+
if (!NAV_TARGET_BODY_KEYED.has(metaType)) return true;
3013+
let body: Record<string, unknown> | null;
3014+
try {
3015+
body = extractDraftBody(await client.getDraft(metaType, name, { packageId }));
3016+
} catch {
3017+
return true;
3018+
}
3019+
return !body || isNavTargetRow(metaType, body);
3020+
}
3021+
29933022
/**
29943023
* The package's items of `metaType`, published ∪ draft by name (the published
29953024
* row wins, for its label), as the pillar's object list is read
@@ -3021,10 +3050,19 @@ function usePackageNavTargets(
30213050
// objectui#11862 — a draft-only target carries the label its `_drafts`
30223051
// row serves (objectstack-ai/objectstack#22200), so the picker offers
30233052
// it by that label, as it offers a published one.
3053+
const draftOnly: NavTargetRow[] = [];
30243054
for (const draft of (drafts ?? []) as Array<{ name?: string | null; label?: I18nLabel | null }>) {
30253055
const name = typeof draft?.name === 'string' ? draft.name : '';
3026-
if (name && !byName.has(name)) byName.set(name, { name, label: draft.label ?? undefined });
3056+
if (name && !byName.has(name) && !draftOnly.some((row) => row.name === name)) {
3057+
draftOnly.push({ name, label: draft.label ?? undefined });
3058+
}
30273059
}
3060+
const verdicts = await Promise.all(
3061+
draftOnly.map((row) => isDraftNavTarget(client, metaType, row.name, packageId)),
3062+
);
3063+
draftOnly.forEach((row, i) => {
3064+
if (verdicts[i]) byName.set(row.name, row);
3065+
});
30283066
} catch {
30293067
/* non-fatal — the picker just stays empty */
30303068
}

0 commit comments

Comments
 (0)