From ca23ec4179bac0aef894bab833a3571da5ab811c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 22:53:20 +0000 Subject: [PATCH] fix(app-shell): nav target picker judges a draft-only page on its draft body A `_drafts` header names the metadata type, never the page body's own `type`, so the Interfaces nav page picker offered a draft record page and picking it gave a dead entry once the app was published. A draft-only row of a type whose verdict reads the body (page, action) is now judged on its draft body, as a published row is; an unreadable body keeps the row. Claude-Session: https://claude.ai/code/session_01JQwxbYChiSg5184uQo2r5x Co-authored-by: Claude --- ...DesignSurface.navPageDrafts-26743.test.tsx | 199 ++++++++++++++++++ .../studio-design/StudioDesignSurface.tsx | 46 +++- 2 files changed, 241 insertions(+), 4 deletions(-) create mode 100644 ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.navPageDrafts-26743.test.tsx diff --git a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.navPageDrafts-26743.test.tsx b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.navPageDrafts-26743.test.tsx new file mode 100644 index 00000000000..3ef3dec26e9 --- /dev/null +++ b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.navPageDrafts-26743.test.tsx @@ -0,0 +1,199 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack-ai/objectstack#26743 — the Interfaces nav editor's target picker + * offers a draft-only page only when its draft body can be a nav target. + * + * A `page` nav entry opens a static page URL with no record id, so a record + * page is never a valid target (`isStaticPageOption`). The picker filtered the + * published rows by the body's `type`, but added every draft-only row + * unfiltered: a `_drafts` header names the metadata type (`page`), not the + * body's `type`. A draft record page was offered, and picking it gave a dead + * entry once the app was published. The picker now reads a draft-only row's + * draft body and judges it as it judges a published row: + * + * - a draft record page is not offered; + * - a draft static page is offered; + * - a published record page is still not offered; + * - a draft action bound to an object is not offered (the nav runs global + * actions only), a draft global action is; + * - CONTROL: a draft whose body cannot be read is still offered, as a row + * with no `type` is. + */ + +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, within } from '@testing-library/react'; +import { MetadataCtx } from '@object-ui/react'; + +const PKG = 'com.acme.app'; + +/** Published pages, as `client.list('page')` serves them: bodies, with `type`. */ +const PUBLISHED_PAGES = [ + { name: 'ops_home', label: 'Ops home', type: 'app' }, + { name: 'ticket_detail', label: 'Ticket detail', type: 'record' }, +]; + +/** Draft bodies the `?state=draft` read serves, keyed `type:name`. */ +const DRAFT_BODIES: Record | undefined> = { + 'page:case_record': { name: 'case_record', label: 'Case record', type: 'record' }, + 'page:case_board': { name: 'case_board', label: 'Case board', type: 'list' }, + 'action:close_case': { name: 'close_case', label: 'Close case', objectName: 'repairs_case' }, + 'action:open_inbox': { name: 'open_inbox', label: 'Open inbox' }, +}; + +const mockClient = vi.hoisted(() => ({ + list: vi.fn(async (): Promise => []), + // The `_drafts` headers: metadata type and name, never the body's `type`. + listDrafts: vi.fn(async (options?: { packageId?: string; type?: string }): Promise => { + if (options?.type === 'page') { + return [ + { type: 'page', name: 'case_record', packageId: 'com.acme.app', label: 'Case record' }, + { type: 'page', name: 'case_board', packageId: 'com.acme.app', label: 'Case board' }, + { type: 'page', name: 'case_unread', packageId: 'com.acme.app', label: 'Case unread' }, + ]; + } + if (options?.type === 'action') { + return [ + { type: 'action', name: 'close_case', packageId: 'com.acme.app', label: 'Close case' }, + { type: 'action', name: 'open_inbox', packageId: 'com.acme.app', label: 'Open inbox' }, + ]; + } + return []; + }), + getDraft: vi.fn(async (): Promise => null), + get: vi.fn(async () => undefined), + withPreviewDrafts: vi.fn((): unknown => mockClient), +})); + +vi.mock('../metadata-admin/useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ loading: false, error: null, entries: [] }), + }; +}); + +vi.mock('@object-ui/react', async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, useAdapter: () => ({}) }; +}); + +vi.mock('sonner', () => ({ + toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), dismiss: vi.fn() }, + Toaster: () => null, +})); + +vi.mock('../../components/SuggestedBindingsPanel', () => ({ SuggestedBindingsPanel: () => null })); +vi.mock('../metadata-admin/AccessExplainPanel', () => ({ AccessExplainPanel: () => null })); +vi.mock('./StudioAiCopilot', () => ({ StudioChatDock: () => null })); +vi.mock('../../preview/DraftChangesPanel', () => ({ DraftChangesPanel: () => null })); + +import { StudioNavItemInspector } from './StudioDesignSurface'; + +window.matchMedia = ((query: string) => ({ + matches: false, + media: query, + onchange: null, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + dispatchEvent: () => false, +})) as unknown as typeof window.matchMedia; +(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver = + (globalThis as unknown as { ResizeObserver?: unknown }).ResizeObserver ?? + class { + observe() {} + unobserve() {} + disconnect() {} + }; + +const METADATA = { + apps: [], + objects: [], + dashboards: [], + reports: [], + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], + getTypeStatus: () => 'ready' as const, +}; + +beforeEach(() => { + for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear(); + mockClient.list.mockImplementation(async (type?: unknown) => (type === 'page' ? PUBLISHED_PAGES : [])); + mockClient.getDraft.mockImplementation(async (type?: unknown, name?: unknown) => { + if (name === 'case_unread') throw new Error('draft read failed'); + const body = DRAFT_BODIES[`${String(type)}:${String(name)}`]; + // The served envelope, decorated as the framework serves it. + return body ? { type, name, item: { ...body, _draft: true } } : null; + }); +}); + +afterEach(() => { + cleanup(); +}); + +async function openPicker(navigation: unknown[], name: string) { + render( + + + , + ); + fireEvent.keyDown(screen.getByRole('combobox', { name }), { key: 'ArrowDown' }); + return screen.findByRole('listbox'); +} + +describe('objectstack-ai/objectstack#26743 — the nav page picker judges a draft-only page on its draft body', () => { + it('a draft record page is not offered', async () => { + const listbox = await openPicker([{ id: 'nav_page', type: 'page' }], 'Page'); + // Wait for the draft static page, read in the same pass, before asserting an absence. + await within(listbox).findByRole('option', { name: 'Case board (case_board)' }); + expect(within(listbox).queryByRole('option', { name: /case_record/ })).toBeNull(); + }); + + it('a draft static page is offered', async () => { + const listbox = await openPicker([{ id: 'nav_page', type: 'page' }], 'Page'); + expect(await within(listbox).findByRole('option', { name: 'Case board (case_board)' })).toBeInTheDocument(); + }); + + it('a published record page is still not offered, a published static page is', async () => { + const listbox = await openPicker([{ id: 'nav_page', type: 'page' }], 'Page'); + expect(await within(listbox).findByRole('option', { name: 'Ops home (ops_home)' })).toBeInTheDocument(); + expect(within(listbox).queryByRole('option', { name: /ticket_detail/ })).toBeNull(); + }); + + it('CONTROL: a draft whose body cannot be read is still offered', async () => { + const listbox = await openPicker([{ id: 'nav_page', type: 'page' }], 'Page'); + expect(await within(listbox).findByRole('option', { name: 'Case unread (case_unread)' })).toBeInTheDocument(); + }); + + it('a draft action bound to an object is not offered, a draft global action is', async () => { + const listbox = await openPicker([{ id: 'nav_action', type: 'action' }], 'Action'); + expect(await within(listbox).findByRole('option', { name: 'Open inbox (open_inbox)' })).toBeInTheDocument(); + expect(within(listbox).queryByRole('option', { name: /close_case/ })).toBeNull(); + }); + + it('a draft body is read only for a type whose verdict reads the body', async () => { + await openPicker([{ id: 'nav_page', type: 'page' }], 'Page'); + await screen.findByRole('option', { name: 'Case board (case_board)' }); + expect(mockClient.getDraft).toHaveBeenCalledWith('page', 'case_record', { packageId: PKG }); + // A published row is never re-read as a draft. + expect(mockClient.getDraft).not.toHaveBeenCalledWith('page', 'ops_home', expect.anything()); + }); +}); diff --git a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index 2ed269061bd..8a30d22bfe3 100644 --- a/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -2980,9 +2980,13 @@ const NAV_TARGET_KEY = { * Whether a listed row can be a nav target. A record page needs a record id a * `page` entry cannot pass (`isStaticPageOption`), and an action bound to an * object is not addressable from the nav, which runs GLOBAL actions only - * (`useNavActionDispatch`). Only a CONFIRMED one is left out: a draft header - * carries no body, so a draft-only row is kept, as `isStaticPageOption` keeps - * a row with no `type`. + * (`useNavActionDispatch`). Only a CONFIRMED one is left out, as + * `isStaticPageOption` keeps a row with no `type`. + * + * objectstack-ai/objectstack#26743 — a `_drafts` header names the METADATA + * type (`page`), never the body's own `type` (`record`), so a draft-only row + * is judged on its draft body instead ({@link NAV_TARGET_BODY_KEYED}). A draft + * record page is still a record page once it is published with the app. */ function isNavTargetRow(metaType: string, row: Record): boolean { if (metaType === 'page') return isStaticPageOption(row as { type?: string }); @@ -2990,6 +2994,31 @@ function isNavTargetRow(metaType: string, row: Record): boolean return true; } +/** The types whose {@link isNavTargetRow} verdict reads a body key, so a draft-only row's body is read for it. */ +const NAV_TARGET_BODY_KEYED: ReadonlySet = new Set(['page', 'action']); + +/** + * Whether a draft-only row can be a nav target, judged on its draft body. A + * body that cannot be read (no draft served, a failed read) leaves the row in, + * as a published row with no `type` is left in: only a confirmed non-target + * is dropped. + */ +async function isDraftNavTarget( + client: MetadataClient, + metaType: string, + name: string, + packageId: string, +): Promise { + if (!NAV_TARGET_BODY_KEYED.has(metaType)) return true; + let body: Record | null; + try { + body = extractDraftBody(await client.getDraft(metaType, name, { packageId })); + } catch { + return true; + } + return !body || isNavTargetRow(metaType, body); +} + /** * The package's items of `metaType`, published ∪ draft by name (the published * row wins, for its label), as the pillar's object list is read @@ -3021,10 +3050,19 @@ function usePackageNavTargets( // objectui#11862 — a draft-only target carries the label its `_drafts` // row serves (objectstack-ai/objectstack#22200), so the picker offers // it by that label, as it offers a published one. + const draftOnly: NavTargetRow[] = []; for (const draft of (drafts ?? []) as Array<{ name?: string | null; label?: I18nLabel | null }>) { const name = typeof draft?.name === 'string' ? draft.name : ''; - if (name && !byName.has(name)) byName.set(name, { name, label: draft.label ?? undefined }); + if (name && !byName.has(name) && !draftOnly.some((row) => row.name === name)) { + draftOnly.push({ name, label: draft.label ?? undefined }); + } } + const verdicts = await Promise.all( + draftOnly.map((row) => isDraftNavTarget(client, metaType, row.name, packageId)), + ); + draftOnly.forEach((row, i) => { + if (verdicts[i]) byName.set(row.name, row); + }); } catch { /* non-fatal — the picker just stays empty */ }