From 9f4cd3a534ace58005c67934bd69501f91dc326a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 03:40:08 +0000 Subject: [PATCH 1/7] fix(app-shell): a refused metadata write on the view, report and draft surfaces is said, and the view-config panel stays dirty Every metadata write under packages/app-shell/src/views now raises a refusal through toast.error with formatMetadataError: the view-config panel's edit Save, pin, reorder, the toolbar toggles' updateViewConfig, the report editor's Save, and the draft bar's Publish and Discard. ViewConfigPanel clears isDirty and bumps savedSignal only after the save lands. A census over every write call under the directory keys each site to a ledger row with mechanical evidence, so a new write site with no row is red. Claude-Session: https://claude.ai/code/session_01FjqrwXPfSMkSfkKYDSRkN2 Co-authored-by: Claude --- packages/app-shell/src/views/ObjectView.tsx | 53 +- ...ObjectView.viewWriteRefusal-11583.test.tsx | 456 ++++++++++++++++++ .../ReportView.saveRefusal-11583.test.tsx | 161 +++++++ packages/app-shell/src/views/ReportView.tsx | 11 +- .../RuntimeDraftBar.refusal-11583.test.tsx | 137 ++++++ .../app-shell/src/views/RuntimeDraftBar.tsx | 22 +- ...ViewConfigPanel.saveOutcome-11583.test.tsx | 151 ++++++ .../app-shell/src/views/ViewConfigPanel.tsx | 45 +- .../views/writeRefusalCensus-11583.test.ts | 385 +++++++++++++++ packages/i18n/src/locales/ar.ts | 4 + packages/i18n/src/locales/de.ts | 4 + packages/i18n/src/locales/en.ts | 6 + packages/i18n/src/locales/es.ts | 4 + packages/i18n/src/locales/fr.ts | 4 + packages/i18n/src/locales/ja.ts | 4 + packages/i18n/src/locales/ko.ts | 4 + packages/i18n/src/locales/pt.ts | 4 + packages/i18n/src/locales/ru.ts | 4 + packages/i18n/src/locales/zh.ts | 4 + 19 files changed, 1441 insertions(+), 22 deletions(-) create mode 100644 packages/app-shell/src/views/ObjectView.viewWriteRefusal-11583.test.tsx create mode 100644 packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx create mode 100644 packages/app-shell/src/views/RuntimeDraftBar.refusal-11583.test.tsx create mode 100644 packages/app-shell/src/views/ViewConfigPanel.saveOutcome-11583.test.tsx create mode 100644 packages/app-shell/src/views/writeRefusalCensus-11583.test.ts diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 2f4764a69a..e24f9c5392 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -1482,6 +1482,14 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co return; } console.error('[ObjectView] Failed to persist view config:', err); + // objectui#11583: every other refusal is said too, with the + // door's message. The client gate above answers only for a + // session whose capabilities were reported; an unreported + // one passes it, and the server's 403 lands here. + toast.error(t('form.saveError'), { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); }); }, 300); }, @@ -1512,27 +1520,42 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co * folds through it. What is gone is the automatic write, not the fold. */ - const handleViewConfigSave = useCallback((draft: Record) => { + /** + * The view-config panel's edit Save. Resolves whether the draft was staged, + * so the panel stays dirty, and announces no draft, on anything but `true` + * (objectui#11583). + */ + const handleViewConfigSave = useCallback(async (draft: Record): Promise => { setViewDraft(draft); setRefreshKey(k => k + 1); // ADR-0034: stage a per-item draft via the metadata seam; an explicit // Publish (RuntimeDraftBar) promotes it + records a version. const vid = draft.id; - if (metadataClient && vid) { + if (!metadataClient || !vid) { + console.warn('[ViewConfigPanel] Cannot persist view config: missing metadataClient or viewId.'); + return false; + } + try { // `dataSource` + `objectName` let the seam drop this object's view // cache keys (#4373) — the adapter owns which keys those are. - persistRuntimeMetadata('view', vid, buildViewConfigSaveBody(objectName, draft), { + await persistRuntimeMetadata('view', vid, buildViewConfigSaveBody(objectName, draft), { metadataClient, dataSource, objectName, - }).catch((err: any) => { - console.error('[ViewConfigPanel] Failed to persist view config:', err); }); - } else { - console.warn('[ViewConfigPanel] Cannot persist view config: missing metadataClient or viewId.'); + return true; + } catch (err) { + console.error('[ViewConfigPanel] Failed to persist view config:', err); + // objectui#11583: a refused save is SAID, with the door's own + // message, as the Create View doors do (objectui#11578). + toast.error(t('form.saveError'), { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); + return false; } - }, [metadataClient, dataSource, objectName]); + }, [metadataClient, dataSource, objectName, t]); /** * Create a new view: the Create View dialog's door and the view-config @@ -2278,6 +2301,11 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co setRefreshKey(k => k + 1); } catch (err) { console.error('[ViewTabBar] Failed to pin view:', err); + // objectui#11583: a refused pin is said, with the door's message. + toast.error(t('form.saveError'), { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); } }, [dataSource, objectName, isSavedView, t]); @@ -2331,10 +2359,17 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co await Promise.all(updates); } catch (err) { console.error('[ViewTabBar] Failed to reorder views:', err); + // objectui#11583: the new order still shows from this + // browser's copy, but other sessions read the server's, so a + // refused write is said, with the door's message. + toast.error(t('form.saveError'), { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); } } setRefreshKey(k => k + 1); - }, [dataSource, savedViews, objectName]); + }, [dataSource, savedViews, objectName, t]); const handleConfigView = useCallback((vid: string) => { // System (metadata-defined) views are read-only — opening the diff --git a/packages/app-shell/src/views/ObjectView.viewWriteRefusal-11583.test.tsx b/packages/app-shell/src/views/ObjectView.viewWriteRefusal-11583.test.tsx new file mode 100644 index 0000000000..0cdee047fc --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.viewWriteRefusal-11583.test.tsx @@ -0,0 +1,456 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11583: every metadata write the object page makes says a refusal, + * and the view-config panel never reports a refused save as saved. + * + * ## The defects this pins + * + * objectui#11578 closed the two Create View doors. This card is that family's + * closing card, and these are the object page's remaining write sites: + * + * - `handleViewConfigSave` (the view-config panel's edit Save) caught a refused + * `persistRuntimeMetadata` with `console.error` only. Measured by the + * objectui#11578 dev with a client whose save answers 403: no `toast.error`, + * no `toast.success`. And `ViewConfigPanel.handleSave` cleared `isDirty` + * and bumped `savedSignal` before the save settled, so the panel looked + * saved, raised the "unpublished changes" indicator, and disabled Save, so + * the refused edit could not be retried. + * - `handlePinView` caught a refused `updateView` with `console.error` only. + * - `handleReorderViews` caught a refused `sortOrder` write with + * `console.error` only. + * - `persistViewPatch` (the toolbar toggles' `updateViewConfig`) said only the + * client-side permission gate's refusal; every other refusal, a server 403 + * included, was `console.error` only. + * + * Rename, set-as-default and delete already said a refusal; they are pinned + * here as controls, so the census in `writeRefusalCensus-11583.test.ts` can + * name a pin for every surfaced row. + * + * ## What runs + * + * The real `ObjectView`, the real `ViewConfigPanel` and its real + * `RuntimeDraftBar`, and a real `@object-ui/data-objectstack` `MetadataClient` + * whose fetch answers the view PUT with the refusal under test, so the error + * `handleViewConfigSave` catches is the one the client's own parser builds + * from the wire. Three seams are stubbed, each to reach a door without + * driving a third-party widget: the panel's spec inspector (one button makes + * an edit), `ManageViewsDialog` (buttons call the handlers the page hands it) + * and `ListView` (its schema is captured, so a toolbar toggle's callback can + * be called). + * + * Direction, written before the run: on the unmodified tree the refused cases + * of the panel, pin, reorder and toolbar toggle go RED (no `toast.error`; the + * panel's Save disables and the indicator shows), and every control stays + * GREEN, since it pins behaviour this change does not move. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, act, screen, fireEvent, waitFor, within } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { MetadataClient, ViewConfigPermissionDeniedError } from '@object-ui/data-objectstack'; +import { toast } from 'sonner'; + +vi.mock('@object-ui/permissions', async (importOriginal) => { + const actual = await importOriginal(); + // Stable identities: `ListView` names `perms` in its fetch dependencies. + const perms = { + check: () => ({ allowed: true }), + checkField: () => true, + getFieldPermissions: () => [], + getRowFilter: () => undefined, + getObjectApiOperations: () => undefined, + roles: [], + isLoaded: false, + hasCapabilities: () => true, + can: () => true, + cannot: () => false, + }; + const fieldPerms = { canRead: () => true, canWrite: () => true, permissions: [] }; + return { ...actual, usePermissions: () => perms, useFieldPermissions: () => fieldPerms }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada' }, activeOrganization: null }), + // The config panel and the view-management handlers are admin-only. + useWorkspaceAdminStatus: () => ({ isAdmin: true, isResolved: true }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + useRealtimeSubscription: () => ({ lastMessage: null }), + useConflictResolution: () => ({ hasConflicts: false, resolveAllConflicts: () => {} }), +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); +vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null })); + +/** The list schema the object page hands down: captured, not rendered. */ +let listSchema: any = null; +vi.mock('@object-ui/plugin-list', async (importOriginal) => ({ + ...(await importOriginal()), + ListView: (props: any) => { + listSchema = props.schema; + return null; + }, +})); + +// The panel's spec inspector, reduced to one edit: a new label. +vi.mock('./metadata-admin/inspectors/ViewVariantInspector', () => ({ + ViewVariantInspector: ({ onPatch }: any) => ( + + ), +})); + +const OBJECT_NAME = 'crm_deal'; +const SYSTEM_ID = `${OBJECT_NAME}.all`; +const SAVED_ID = `${OBJECT_NAME}.pipeline`; + +// The view-management dialog, reduced to the handlers the page hands it. +vi.mock('@object-ui/plugin-view', async (importOriginal) => ({ + ...(await importOriginal()), + ManageViewsDialog: (props: any) => ( +
+ + + + + + +
+ ), +})); + +/** What the view PUT answers in this test: a refusal, or a 2xx. */ +let putAnswer: () => Response; +const puts: string[] = []; + +const json = (body: unknown, status: number) => + new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); + +/** The dispatcher's ADR-0112 refusal envelope for a view the spec gate refused. */ +const INVALID_METADATA = () => + json( + { + success: false, + error: { + code: 'INVALID_METADATA', + message: 'The view failed spec validation: 1 issue (columns [custom])', + details: { + code: 'INVALID_METADATA', + issues: [{ path: 'columns', message: 'Column "ghost_field" is not a field of crm_deal', code: 'custom' }], + }, + }, + }, + 422, + ); +/** A permission refusal: a message and no structured issues. */ +const PERMISSION_DENIED = () => + json( + { success: false, error: { code: 'PERMISSION_DENIED', message: 'Saving views requires the Manage Metadata permission' } }, + 403, + ); + +function wire() { + return vi.fn(async (input: string, init?: RequestInit) => { + const path = new URL(input, 'http://localhost').pathname; + if (init?.method === 'PUT' && path.includes('/meta/view/')) { + puts.push(path); + return putAnswer(); + } + // No draft is pending on any view: the bar's draft read answers 404. + if (path.includes('/meta/view/')) return json({ success: false }, 404); + if (path.endsWith('/meta/dataset')) return json([], 200); + return json({ data: [] }, 200); + }); +} +let client: MetadataClient; + +vi.mock('./metadata-admin/useMetadata', async (importOriginal) => ({ + ...(await importOriginal>()), + useMetadataClient: () => client, +})); + +import { ObjectView, type ConsoleObjectViewProps } from './ObjectView'; +import { ExpressionProvider } from '../providers/ExpressionProvider'; + +const FIELDS = { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + amount: { type: 'number', label: 'Amount' }, +}; +/** The saved view: an overlay row the adapter's `listViews` serves. */ +const SAVED_ROW = { name: SAVED_ID, object: OBJECT_NAME, label: 'Pipeline', type: 'grid', columns: ['name'] }; + +/** A refusal as the adapter's `updateView` / `updateViewConfig` raise it. */ +const refusal = (message: string) => Object.assign(new Error(message), { status: 403 }); + +function makeDataSource(writes: Record = {}) { + return { + find: vi.fn(async () => ({ data: [], total: 0 })), + findOne: vi.fn(async () => null), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + getObjectSchema: vi.fn(async () => ({ name: OBJECT_NAME, fields: {} })), + listViews: vi.fn(async () => [SAVED_ROW]), + updateView: vi.fn(async () => ({})), + updateViewConfig: vi.fn(async () => ({})), + deleteView: vi.fn(async () => ({})), + ...writes, + } as unknown as ConsoleObjectViewProps['dataSource']; +} + +const settle = () => act(() => new Promise((resolve) => setTimeout(resolve, 300))); + +/** Mount the object page on the saved view, and wait for its tab. */ +async function mountOnSavedView(dataSource: ConsoleObjectViewProps['dataSource']) { + render( + + + + {}} + /> + } + /> + + + , + ); + await settle(); + // The saved view is a tab once `listViews` has answered. + await screen.findByText('Pipeline'); +} + +/** The one `toast.error` call's description, as text. */ +const errorDescription = (call = 0) => { + const [, options] = vi.mocked(toast.error).mock.calls[call] as [unknown, { description?: unknown } | undefined]; + return String(options?.description); +}; + +const saveButton = () => screen.getByTestId('view-config-save') as HTMLButtonElement; + +/** Open the config panel on the saved view, edit it, and press Save. */ +async function editAndSave() { + fireEvent.click(screen.getByTestId('stub-config')); + await screen.findByTestId('stub-inspector-edit'); + fireEvent.click(screen.getByTestId('stub-inspector-edit')); + await waitFor(() => expect(saveButton().disabled).toBe(false)); + await act(async () => { + fireEvent.click(saveButton()); + }); + await waitFor(() => expect(puts).toHaveLength(1)); +} + +beforeEach(() => { + cleanup(); + puts.length = 0; + listSchema = null; + client = new MetadataClient({ baseUrl: 'http://localhost', fetch: wire() as unknown as typeof fetch }); + vi.stubGlobal('fetch', vi.fn(async () => json({ data: [] }, 200))); + vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + // Unmount before the real fetch comes back (objectui#7439). + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + vi.clearAllMocks(); +}); + +describe('the view-config panel\'s edit Save says a refused save, and stays dirty (objectui#11583, handleViewConfigSave)', () => { + it.each([ + { refusal: '403 permission', answer: PERMISSION_DENIED, says: ['Saving views requires the Manage Metadata permission'] }, + { refusal: '422 INVALID_METADATA', answer: INVALID_METADATA, says: ['columns', 'Column "ghost_field" is not a field of crm_deal'] }, + ])('a $refusal refusal shows the door\'s message; Save stays enabled and no draft is announced', async ({ answer, says }) => { + putAnswer = answer; + await mountOnSavedView(makeDataSource()); + await editAndSave(); + + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + for (const text of says) expect(errorDescription()).toContain(text); + expect(toast.success).not.toHaveBeenCalled(); + // Still dirty: the refused edit can be retried with one click. + await waitFor(() => expect(saveButton().disabled).toBe(false)); + expect(screen.queryByTestId('runtime-draft-indicator')).toBeNull(); + }); + + it('a saved edit clears the panel and announces the draft (control)', async () => { + putAnswer = () => json({ success: true }, 200); + await mountOnSavedView(makeDataSource()); + await editAndSave(); + + await screen.findByTestId('runtime-draft-indicator'); + expect(saveButton().disabled).toBe(true); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); + +describe('pin says a refused write (objectui#11583, handlePinView)', () => { + it('a refused pin raises the door\'s message', async () => { + const dataSource = makeDataSource({ + updateView: vi.fn(async () => { throw refusal('Pinning views requires the Manage Metadata permission'); }), + }); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-pin')); + }); + + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect(errorDescription()).toContain('Pinning views requires the Manage Metadata permission'); + expect((dataSource as any).updateView).toHaveBeenCalledWith(OBJECT_NAME, SAVED_ID, { isPinned: true }); + }); + + it('a pin that lands raises nothing (control)', async () => { + const dataSource = makeDataSource(); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-pin')); + }); + await waitFor(() => expect((dataSource as any).updateView).toHaveBeenCalledTimes(1)); + await settle(); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); + +describe('reorder says a refused write (objectui#11583, handleReorderViews)', () => { + it('a refused sortOrder write raises the door\'s message, once', async () => { + const dataSource = makeDataSource({ + updateView: vi.fn(async () => { throw refusal('Reordering views requires the Manage Metadata permission'); }), + }); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-reorder')); + }); + + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect(errorDescription()).toContain('Reordering views requires the Manage Metadata permission'); + expect((dataSource as any).updateView).toHaveBeenCalledWith(OBJECT_NAME, SAVED_ID, { sortOrder: 0 }); + }); + + it('a reorder that lands raises nothing (control)', async () => { + const dataSource = makeDataSource(); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-reorder')); + }); + await waitFor(() => expect((dataSource as any).updateView).toHaveBeenCalledTimes(1)); + await settle(); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); + +describe('a toolbar toggle says a refused write (objectui#11583, persistViewPatch)', () => { + /** Toggle the density, as the list toolbar does, and wait past the 300ms debounce. */ + async function toggleDensity() { + await waitFor(() => expect(typeof listSchema?.onDensityChange).toBe('function')); + act(() => { + listSchema.onDensityChange('compact'); + }); + await act(() => new Promise((resolve) => setTimeout(resolve, 600))); + } + + it('a server refusal raises the door\'s message', async () => { + const dataSource = makeDataSource({ + updateViewConfig: vi.fn(async () => { throw refusal('View settings are refused for this session'); }), + }); + await mountOnSavedView(dataSource); + await toggleDensity(); + + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect(errorDescription()).toContain('View settings are refused for this session'); + expect((dataSource as any).updateViewConfig).toHaveBeenCalledTimes(1); + }); + + it('the client-side permission gate keeps its own message (control)', async () => { + const dataSource = makeDataSource({ + updateViewConfig: vi.fn(async () => { throw new ViewConfigPermissionDeniedError(OBJECT_NAME, SAVED_ID); }), + }); + await mountOnSavedView(dataSource); + await toggleDensity(); + + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + const [, options] = vi.mocked(toast.error).mock.calls[0] as [unknown, unknown]; + expect(options).toBeUndefined(); + }); + + it('a toggle that lands raises nothing (control)', async () => { + const dataSource = makeDataSource(); + await mountOnSavedView(dataSource); + await toggleDensity(); + await waitFor(() => expect((dataSource as any).updateViewConfig).toHaveBeenCalledTimes(1)); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); + +describe('rename, set-as-default and delete already said a refusal (controls, objectui#11583: handleRenameView, handleSetDefaultView, handleDeleteView)', () => { + it('a refused rename raises a toast', async () => { + const dataSource = makeDataSource({ + updateView: vi.fn(async () => { throw refusal('Renaming refused'); }), + }); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-rename')); + }); + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect((dataSource as any).updateView).toHaveBeenCalledWith(OBJECT_NAME, SAVED_ID, { label: 'Renamed' }); + }); + + it('a refused set-as-default raises a toast', async () => { + const dataSource = makeDataSource({ + updateView: vi.fn(async () => { throw refusal('Default refused'); }), + }); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-set-default')); + }); + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect((dataSource as any).updateView).toHaveBeenCalledWith(OBJECT_NAME, SAVED_ID, { isDefault: true }); + }); + + it('a refused delete raises a toast', async () => { + const dataSource = makeDataSource({ + deleteView: vi.fn(async () => { throw refusal('Deleting refused'); }), + }); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-delete')); + }); + // The page asks first; the user confirms. + const dialog = await screen.findByRole('alertdialog'); + const buttons = within(dialog).getAllByRole('button'); + await act(async () => { + fireEvent.click(buttons[buttons.length - 1]); + }); + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect((dataSource as any).deleteView).toHaveBeenCalledWith(OBJECT_NAME, SAVED_ID); + }); +}); diff --git a/packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx b/packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx new file mode 100644 index 0000000000..3319c15fb4 --- /dev/null +++ b/packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx @@ -0,0 +1,161 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11583: the report editor's Save says a refused save. + * + * ## The defect this pins + * + * `ReportView.saveSchema` caught a refused `persistRuntimeMetadata('report', …)` + * with `console.warn('[ReportView] Auto-save failed:', err)` only, so a refused + * report edit looked saved. + * + * ## The cadence, measured here rather than assumed + * + * The card asked whether this save fires per edit. It does not: an inspector + * edit reaches `handleReportFieldChange`, which updates the live preview only, + * and the write fires once per press of the panel's Save. So one refusal is + * one toast, and nothing here needs a burst guard. The first case counts the + * writes after three edits (none) and after Save (one), so a change that made + * the editor save per edit would fail here, before it could stack toasts. + * + * ## What runs + * + * The real `ReportView`, the real `ReportConfigPanel` and the real persistence + * seam, over a metadata client double whose `save` rejects the way + * `MetadataClient` does. The panel's spec inspector is reduced to one edit + * button, and the draft bar (its own reads) to nothing. + * + * Direction, written before the run: on the unmodified tree the refused case + * goes RED (no `toast.error`), and the control stays GREEN. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent, act, cleanup } from '@testing-library/react'; +import { toast } from 'sonner'; + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +vi.mock('@object-ui/plugin-report', async (importOriginal) => ({ + ...(await importOriginal>()), + ReportRenderer: () => null, +})); +vi.mock('@object-ui/plugin-dashboard', async (importOriginal) => ({ + ...(await importOriginal>()), + DrillDownDrawer: () => null, +})); +// The panel's spec inspector, reduced to one edit: a new title. +vi.mock('./metadata-admin/inspectors/ReportDefaultInspector', () => ({ + ReportDefaultInspector: ({ onPatch }: any) => ( + + ), +})); +vi.mock('./RuntimeDraftBar', () => ({ RuntimeDraftBar: () => null })); + +const meta = vi.hoisted(() => ({ value: null as any })); +vi.mock('../providers/MetadataProvider', () => ({ useMetadata: () => meta.value })); + +vi.mock('react-router-dom', () => ({ + useParams: () => ({ reportName: 'revenue_by_month' }), + useNavigate: () => vi.fn(), + useLocation: () => ({ pathname: '/reports/revenue_by_month', search: '' }), +})); + +vi.mock('./useOpenRecordList', () => ({ useOpenRecordList: () => vi.fn() })); +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false }), +})); +const client = vi.hoisted(() => ({ value: null as any })); +vi.mock('./metadata-admin/useMetadata', () => ({ useMetadataClient: () => client.value })); +vi.mock('../providers/AdapterProvider', () => ({ useAdapter: () => ({}) })); +vi.mock('../providers/ExpressionProvider', () => ({ useExpressionContext: () => ({ app: undefined }) })); +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + // The report editor is admin-only. + useWorkspaceAdminStatus: () => ({ isAdmin: true, isResolved: true }), +})); + +import { ReportView } from './ReportView'; + +const refresh = vi.fn(async () => {}); + +function mountReport() { + meta.value = { + apps: [], + objects: [{ name: 'acct', label: 'Account', fields: { industry: { label: 'Industry', type: 'text' } } }], + dashboards: [], + reports: [{ name: 'revenue_by_month', label: 'Revenue by Month', dataSource: { object: 'acct' } }], + pages: [], + loading: false, + error: null, + refresh, + invalidate: () => {}, + ensureType: async () => [], + getItem: vi.fn(async () => null), + getItemsByType: () => [], + getTypeStatus: () => 'ready', + }; + render( ({ data: [] })) } as any} />); +} + +/** Open the editor, make three edits, and press Save. */ +async function editThriceAndSave() { + fireEvent.click(await screen.findByTestId('report-edit-button')); + await screen.findByTestId('report-config-panel'); + for (let i = 0; i < 3; i++) fireEvent.click(screen.getByTestId('stub-report-edit')); + // Edits drive the live preview only: nothing is written yet. + expect(client.value.save).not.toHaveBeenCalled(); + await act(async () => { + fireEvent.click(screen.getByTestId('report-config-save')); + }); + await waitFor(() => expect(client.value.save).toHaveBeenCalledTimes(1)); +} + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.clearAllMocks(); +}); + +describe('the report editor\'s Save says a refused save (objectui#11583, ReportView.saveSchema)', () => { + it('a refused save raises the refusal, once per Save', async () => { + client.value = { + get: vi.fn(async () => null), + save: vi.fn(async () => { + throw Object.assign(new Error('Saving reports requires the Manage Metadata permission'), { status: 403 }); + }), + }; + mountReport(); + await editThriceAndSave(); + + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + const [, options] = vi.mocked(toast.error).mock.calls[0] as [unknown, { description?: unknown } | undefined]; + expect(String(options?.description)).toContain('Saving reports requires the Manage Metadata permission'); + const [type, name, body, opts] = client.value.save.mock.calls[0]; + expect([type, name, opts]).toEqual(['report', 'revenue_by_month', { mode: 'draft' }]); + expect(body).toMatchObject({ label: 'Revenue EDITED' }); + expect(refresh).not.toHaveBeenCalled(); + }); + + it('a save that lands raises nothing and refreshes the read (control)', async () => { + client.value = { get: vi.fn(async () => null), save: vi.fn(async () => ({})) }; + mountReport(); + await editThriceAndSave(); + + await waitFor(() => expect(refresh).toHaveBeenCalled()); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app-shell/src/views/ReportView.tsx b/packages/app-shell/src/views/ReportView.tsx index 8fd014fb3d..6dfd45d632 100644 --- a/packages/app-shell/src/views/ReportView.tsx +++ b/packages/app-shell/src/views/ReportView.tsx @@ -16,6 +16,8 @@ import { preferLocal } from '../utils/preferLocal.js'; import { useAdapter } from '../providers/AdapterProvider.js'; import { useMetadataClient } from './metadata-admin/useMetadata.js'; import { persistRuntimeMetadata } from './runtime-metadata-persistence.js'; +import { formatMetadataError } from '@object-ui/data-objectstack'; +import { toast } from 'sonner'; import { useWorkspaceAdminStatus } from '@object-ui/auth'; import type { DataSource } from '@object-ui/types'; import type { DatasetDrillArgs } from '@object-ui/plugin-report'; @@ -186,9 +188,16 @@ export function ReportView({ dataSource }: { dataSource?: DataSource }) { } } catch (err) { console.warn('[ReportView] Auto-save failed:', err); + // objectui#11583: a refused save is said, with the door's message. + // It fires once per press of the editor's Save (an edit only drives + // the live preview), so one refusal is one toast. + toast.error(t('form.saveError'), { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); } }, - [metadataClient, reportName, refresh], + [metadataClient, reportName, refresh, t], ); // ---- Open / close config panel ------------------------------------------ diff --git a/packages/app-shell/src/views/RuntimeDraftBar.refusal-11583.test.tsx b/packages/app-shell/src/views/RuntimeDraftBar.refusal-11583.test.tsx new file mode 100644 index 0000000000..13711605ff --- /dev/null +++ b/packages/app-shell/src/views/RuntimeDraftBar.refusal-11583.test.tsx @@ -0,0 +1,137 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11583: the draft bar's Publish and Discard say a refused write. + * + * ## The defect this pins + * + * `RuntimeDraftBar.handlePublish` and `handleDiscard` caught a refused + * `publishRuntimeMetadata` / `discardRuntimeDraft` with `console.error` only. + * The spinner stopped, the "unpublished changes" indicator stayed, and nothing + * told the user the draft was neither published nor discarded. + * + * ## What runs + * + * The real bar and the real persistence seam, over a metadata client double + * whose `publish` / `reset` reject the way `MetadataClient` does: an error + * whose message is the refusal's own text. The bar has a draft pending, so + * both buttons render. + * + * Direction, written before the run: on the unmodified tree both refused cases + * go RED (no `toast.error`), and both controls stay GREEN. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent, act, cleanup } from '@testing-library/react'; +import { toast } from 'sonner'; + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +import { RuntimeDraftBar } from './RuntimeDraftBar'; + +const refusal = (message: string) => Object.assign(new Error(message), { status: 403 }); + +function makeMetadataClient(writes: Record = {}) { + return { + get: vi.fn().mockResolvedValue({ type: 'view', name: 'crm_deal.pipeline', item: { label: 'Pipeline' } }), + publish: vi.fn().mockResolvedValue(undefined), + reset: vi.fn().mockResolvedValue(undefined), + ...writes, + }; +} + +async function mountWithDraft(metadataClient: ReturnType, onAfterChange = vi.fn()) { + render( + , + ); + await screen.findByTestId('runtime-draft-bar'); + return onAfterChange; +} + +const errorDescription = () => { + const [, options] = vi.mocked(toast.error).mock.calls[0] as [unknown, { description?: unknown } | undefined]; + return String(options?.description); +}; + +beforeEach(() => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + // Discard asks first; the user says yes. + vi.stubGlobal('confirm', vi.fn(() => true)); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + vi.clearAllMocks(); +}); + +describe('RuntimeDraftBar says a refused Publish (objectui#11583, handlePublish)', () => { + it('a refused publish raises the refusal, and the draft is still announced', async () => { + const client = makeMetadataClient({ + publish: vi.fn().mockRejectedValue(refusal('Publishing views requires the Manage Metadata permission')), + }); + const onAfterChange = await mountWithDraft(client); + await act(async () => { + fireEvent.click(screen.getByTestId('runtime-draft-publish')); + }); + + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect(errorDescription()).toContain('Publishing views requires the Manage Metadata permission'); + expect(screen.getByTestId('runtime-draft-indicator')).toBeTruthy(); + expect(onAfterChange).not.toHaveBeenCalled(); + }); + + it('a publish that lands raises nothing and clears the bar (control)', async () => { + const client = makeMetadataClient(); + const onAfterChange = await mountWithDraft(client); + await act(async () => { + fireEvent.click(screen.getByTestId('runtime-draft-publish')); + }); + + await waitFor(() => expect(screen.queryByTestId('runtime-draft-bar')).toBeNull()); + expect(client.publish).toHaveBeenCalledWith('view', 'crm_deal.pipeline'); + expect(onAfterChange).toHaveBeenCalledTimes(1); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); + +describe('RuntimeDraftBar says a refused Discard (objectui#11583, handleDiscard)', () => { + it('a refused discard raises the refusal, and the draft is still announced', async () => { + const client = makeMetadataClient({ + reset: vi.fn().mockRejectedValue(refusal('Discarding drafts requires the Manage Metadata permission')), + }); + const onAfterChange = await mountWithDraft(client); + await act(async () => { + fireEvent.click(screen.getByTestId('runtime-draft-discard')); + }); + + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect(errorDescription()).toContain('Discarding drafts requires the Manage Metadata permission'); + expect(screen.getByTestId('runtime-draft-indicator')).toBeTruthy(); + expect(onAfterChange).not.toHaveBeenCalled(); + }); + + it('a discard that lands raises nothing and clears the bar (control)', async () => { + const client = makeMetadataClient(); + const onAfterChange = await mountWithDraft(client); + await act(async () => { + fireEvent.click(screen.getByTestId('runtime-draft-discard')); + }); + + await waitFor(() => expect(screen.queryByTestId('runtime-draft-bar')).toBeNull()); + expect(client.reset).toHaveBeenCalledWith('view', 'crm_deal.pipeline', { state: 'draft' }); + expect(onAfterChange).toHaveBeenCalledTimes(1); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app-shell/src/views/RuntimeDraftBar.tsx b/packages/app-shell/src/views/RuntimeDraftBar.tsx index fb587973d0..3b02240c3c 100644 --- a/packages/app-shell/src/views/RuntimeDraftBar.tsx +++ b/packages/app-shell/src/views/RuntimeDraftBar.tsx @@ -22,6 +22,9 @@ import { type RuntimeArtifactType, } from './runtime-metadata-persistence.js'; import { useMetadataLocale, t, tFormat } from './metadata-admin/i18n.js'; +import { useObjectTranslation } from '@object-ui/i18n'; +import { formatMetadataError } from '@object-ui/data-objectstack'; +import { toast } from 'sonner'; export interface RuntimeDraftBarProps { /** Artifact type — the `:type` in `/meta/:type/:name`. */ @@ -75,6 +78,9 @@ export function RuntimeDraftBar({ savedSignal, }: RuntimeDraftBarProps) { const locale = useMetadataLocale(); + // The console packs, for the refusal toasts: the engine table above has no + // failure keys, and its two languages are not the console's ten. + const { t: tConsole } = useObjectTranslation(); const [hasDraft, setHasDraft] = useState(false); const [busy, setBusy] = useState(false); // Track the `name` we've already resumed so reopening the same item doesn't @@ -133,10 +139,16 @@ export function RuntimeDraftBar({ onAfterChange?.(); } catch (err) { console.error('[RuntimeDraftBar] Publish failed:', err); + // objectui#11583: a refused publish is said, with the door's message. + // The draft is still pending, so the indicator stays. + toast.error(tConsole('console.runtimeDraft.publishFailed'), { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); } finally { setBusy(false); } - }, [type, name, metadataClient, dataSource, objectName, onAfterChange]); + }, [type, name, metadataClient, dataSource, objectName, onAfterChange, tConsole]); const handleDiscard = useCallback(async () => { if (!name) return; @@ -153,10 +165,16 @@ export function RuntimeDraftBar({ onAfterChange?.(); } catch (err) { console.error('[RuntimeDraftBar] Discard draft failed:', err); + // objectui#11583: a refused discard is said, with the door's message. + // The draft is still pending, so the indicator stays. + toast.error(tConsole('console.runtimeDraft.discardFailed'), { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); } finally { setBusy(false); } - }, [type, name, metadataClient, dataSource, objectName, onAfterChange, locale]); + }, [type, name, metadataClient, dataSource, objectName, onAfterChange, locale, tConsole]); // flag OFF, or nothing pending → render nothing (zero DOM, zero layout shift). // Nothing pending → render nothing (no indicator, no buttons). diff --git a/packages/app-shell/src/views/ViewConfigPanel.saveOutcome-11583.test.tsx b/packages/app-shell/src/views/ViewConfigPanel.saveOutcome-11583.test.tsx new file mode 100644 index 0000000000..1b420154aa --- /dev/null +++ b/packages/app-shell/src/views/ViewConfigPanel.saveOutcome-11583.test.tsx @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11583: `ViewConfigPanel.handleSave` reads the save's outcome before + * it reports the panel as saved. + * + * ## The defect this pins + * + * In edit mode `handleSave` called `onSave` without waiting, then cleared + * `isDirty` and bumped `savedSignal`. Save is disabled while the panel is not + * dirty, and `savedSignal` makes the draft bar announce "unpublished changes". + * So a refused save looked saved, announced a draft that did not exist, and + * could not be retried without making another edit. + * + * ## The contract this pins + * + * `onSave` may return a promise of the outcome (`ObjectView`'s + * `handleViewConfigSave` does). The panel clears `isDirty` and bumps + * `savedSignal` only when that promise resolves to anything but `false`; a + * `false` or a rejection leaves the panel dirty. Save is disabled while the + * save is in flight, and an edit made during it stays dirty after it lands. + * A host that returns nothing is read as saved, as before. + * + * The end-to-end pin, through the real `ObjectView` and a refusing metadata + * client, is `ObjectView.viewWriteRefusal-11583.test.tsx`. + * + * Direction, written before the run: on the unmodified tree the `false`, + * rejection, in-flight and edit-during-save cases go RED (the panel clears + * before the save settles); the `true` and legacy cases stay GREEN. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent, act, waitFor } from '@testing-library/react'; + +vi.mock('./metadata-admin/inspectors/ViewVariantInspector', () => ({ + ViewVariantInspector: ({ draft, onPatch }: any) => ( + + ), +})); + +// The draft bar, reduced to the two props the panel drives. +vi.mock('./RuntimeDraftBar', () => ({ + RuntimeDraftBar: ({ savedSignal, dirty }: any) => ( + + ), +})); + +import { ViewConfigPanel } from './ViewConfigPanel'; + +const objectDef = { + name: 'obj', + fields: { a: { label: 'A', type: 'text' }, b: { label: 'B', type: 'text' }, c: { label: 'C', type: 'text' } }, +}; + +function mount(onSave: (draft: Record) => unknown) { + render( + , + ); +} + +const save = () => screen.getByTestId('view-config-save') as HTMLButtonElement; +const savedSignal = () => screen.getByTestId('stub-draft-bar').getAttribute('data-saved-signal'); +const edit = () => fireEvent.click(screen.getByTestId('mock-inspector-edit')); + +/** A promise the test settles by hand. */ +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { resolve = r; }); + return { promise, resolve }; +} + +describe('ViewConfigPanel — edit Save reports saved only after the save lands (objectui#11583, handleSave)', () => { + it.each([ + { outcome: 'resolves false', onSave: () => Promise.resolve(false) }, + { outcome: 'rejects', onSave: () => Promise.reject(new Error('refused')) }, + ])('a save that $outcome leaves the panel dirty, and announces no draft', async ({ onSave }) => { + mount(onSave); + edit(); + expect(save().disabled).toBe(false); + + await act(async () => { + fireEvent.click(save()); + }); + + await waitFor(() => expect(save().disabled).toBe(false)); + expect(savedSignal()).toBe('0'); + expect(screen.getByTestId('stub-draft-bar').getAttribute('data-dirty')).toBe('true'); + }); + + it('a save that resolves true clears the panel and announces the draft (control)', async () => { + mount(() => Promise.resolve(true)); + edit(); + await act(async () => { + fireEvent.click(save()); + }); + await waitFor(() => expect(save().disabled).toBe(true)); + expect(savedSignal()).toBe('1'); + }); + + it('a host that returns nothing is read as saved, as before (control)', async () => { + const onSave = vi.fn(); + mount(onSave); + edit(); + await act(async () => { + fireEvent.click(save()); + }); + expect(onSave).toHaveBeenCalledTimes(1); + await waitFor(() => expect(save().disabled).toBe(true)); + expect(savedSignal()).toBe('1'); + }); + + it('Save is disabled while the save is in flight, and an edit made then stays dirty', async () => { + const pending = deferred(); + const onSave = vi.fn(() => pending.promise); + mount(onSave); + edit(); + await act(async () => { + fireEvent.click(save()); + }); + // In flight: a second press cannot send a second save. + expect(save().disabled).toBe(true); + expect(savedSignal()).toBe('0'); + + // The user edits again before the first save lands. + edit(); + await act(async () => { + pending.resolve(true); + await pending.promise; + }); + + // The first save landed, so the draft is announced; the second edit was + // not in it, so the panel is still dirty and Save is live again. + await waitFor(() => expect(savedSignal()).toBe('1')); + expect(save().disabled).toBe(false); + expect(onSave).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/app-shell/src/views/ViewConfigPanel.tsx b/packages/app-shell/src/views/ViewConfigPanel.tsx index e1d3e2e552..3ad733b393 100644 --- a/packages/app-shell/src/views/ViewConfigPanel.tsx +++ b/packages/app-shell/src/views/ViewConfigPanel.tsx @@ -68,8 +68,15 @@ export interface ViewConfigPanelProps { recordCount?: number; /** Called when any view config field changes (local draft update) */ onViewUpdate?: (field: string, value: any) => void; - /** Called to persist all draft changes */ - onSave?: (draft: Record) => void; + /** + * Called to persist all draft changes. Return a promise of the outcome to + * have the panel wait for it (objectui#11583): it reports the edit as + * saved (clears the dirty state and raises the draft indicator) only when + * the promise resolves to anything but `false`. `false` or a rejection + * leaves the panel dirty, so the edit can be saved again. A host that + * returns nothing is read as saved at once, as before. + */ + onSave?: (draft: Record) => void | Promise; /** Called when create-mode view is created */ onCreate?: (config: Record) => void; /** @@ -141,9 +148,12 @@ export function ViewConfigPanel({ open, onClose, mode = 'edit', activeView, obje ); const [draft, setDraft] = useState(initialDraft); const [isDirty, setIsDirty] = useState(false); - // Bumped on each edit-mode save so the draft/publish chrome surfaces the - // "unpublished changes" indicator immediately (the save writes a draft). + // Bumped on each edit-mode save that lands, so the draft/publish chrome + // surfaces the "unpublished changes" indicator immediately (the save + // wrote a draft). Never on a refused one (objectui#11583). const [savedSignal, setSavedSignal] = useState(0); + // An edit-mode save is in flight: Save is disabled until it settles. + const [saving, setSaving] = useState(false); // Mirror the committed draft into a ref so `handlePatch` can compute the // next draft synchronously without a side-effecting state updater. const draftRef = useRef(draft); @@ -189,15 +199,30 @@ export function ViewConfigPanel({ open, onClose, mode = 'edit', activeView, obje } }, [onViewUpdate]); - const handleSave = useCallback(() => { + const handleSave = useCallback(async () => { const flat = inspectorDraftToRuntimeView(draft); if (mode === 'create') { onCreate?.(flat); - } else { - onSave?.(flat); - setSavedSignal((s) => s + 1); + setIsDirty(false); + return; } - setIsDirty(false); + // objectui#11583: the edit is reported as saved only once the host + // says it landed. A refused save keeps the panel dirty, so Save stays + // live and no draft is announced. An edit made while the save is in + // flight is not in it, so it keeps the panel dirty either way. + const savedDraft = draftRef.current; + setSaving(true); + let landed: boolean; + try { + landed = (await onSave?.(flat)) !== false; + } catch { + landed = false; + } finally { + setSaving(false); + } + if (!landed) return; + setSavedSignal((s) => s + 1); + if (draftRef.current === savedDraft) setIsDirty(false); }, [draft, onSave, onCreate, mode]); // Discard = revert any unsaved edits AND close the panel, in both modes @@ -298,7 +323,7 @@ export function ViewConfigPanel({ open, onClose, mode = 'edit', activeView, obje + ), +})); + +// The draft bar, reduced to the one prop the panel drives. +vi.mock('./RuntimeDraftBar', () => ({ + RuntimeDraftBar: ({ dirty }: any) => , +})); + +import { ReportConfigPanel } from './ReportConfigPanel'; + +function mount(onSave: (config: Record) => unknown) { + const onClose = vi.fn(); + render( + , + ); + return onClose; +} + +const save = () => screen.getByTestId('report-config-save') as HTMLButtonElement; +const inspector = () => screen.getByTestId('stub-report-edit'); +const dirty = () => screen.getByTestId('stub-draft-bar').getAttribute('data-dirty'); + +/** A promise the test settles by hand. */ +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { resolve = r; }); + return { promise, resolve }; +} + +describe('ReportConfigPanel — Save closes only after the save lands (objectui#11583, handleSave)', () => { + it.each([ + { outcome: 'resolves false', onSave: () => Promise.resolve(false) }, + { outcome: 'rejects', onSave: () => Promise.reject(new Error('refused')) }, + ])('a save that $outcome keeps the panel open and dirty, with the edit and Save live', async ({ onSave }) => { + const onClose = mount(onSave); + fireEvent.click(inspector()); + expect(dirty()).toBe('true'); + + await act(async () => { + fireEvent.click(save()); + }); + + await waitFor(() => expect(save().disabled).toBe(false)); + expect(onClose).not.toHaveBeenCalled(); + expect(dirty()).toBe('true'); + expect(inspector().getAttribute('data-label')).toBe('Pipeline EDITED'); + }); + + it('a save that resolves true clears the panel and closes it (control)', async () => { + const onSave = vi.fn(() => Promise.resolve(true)); + const onClose = mount(onSave); + fireEvent.click(inspector()); + await act(async () => { + fireEvent.click(save()); + }); + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + expect(onSave).toHaveBeenCalledWith(expect.objectContaining({ label: 'Pipeline EDITED' })); + expect(dirty()).toBe('false'); + }); + + it('a host that returns nothing is read as saved, as before (control)', async () => { + const onSave = vi.fn(); + const onClose = mount(onSave); + fireEvent.click(inspector()); + await act(async () => { + fireEvent.click(save()); + }); + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + expect(onSave).toHaveBeenCalledTimes(1); + }); + + it('while the save is in flight, Save is disabled and the inspector is read-only', async () => { + const pending = deferred(); + const onSave = vi.fn(() => pending.promise); + const onClose = mount(onSave); + fireEvent.click(inspector()); + await act(async () => { + fireEvent.click(save()); + }); + + expect(save().disabled).toBe(true); + expect(inspector().getAttribute('data-readonly')).toBe('true'); + expect(onClose).not.toHaveBeenCalled(); + + await act(async () => { + pending.resolve(true); + await pending.promise; + }); + await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)); + expect(onSave).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/app-shell/src/views/ReportConfigPanel.tsx b/packages/app-shell/src/views/ReportConfigPanel.tsx index 58b9348aaf..2500984e79 100644 --- a/packages/app-shell/src/views/ReportConfigPanel.tsx +++ b/packages/app-shell/src/views/ReportConfigPanel.tsx @@ -47,8 +47,14 @@ export interface ReportConfigPanelProps { onClose: () => void; /** The current report definition (flat spec Report document). */ config: Record | null; - /** Persist all draft changes. */ - onSave: (config: Record) => void; + /** + * Persist all draft changes. Return a promise of the outcome to have the + * panel wait for it (objectui#11583): it clears its dirty state and closes + * only when the promise resolves to anything but `false`. `false` or a + * rejection leaves it open and dirty, with the edit in place, so it can be + * saved again. A host that returns nothing is read as saved at once. + */ + onSave: (config: Record) => void | Promise; /** Called on every field change so the host can drive a live preview. */ onFieldChange?: (key: string, value: any, draft?: Record) => void; /** @@ -84,6 +90,9 @@ export function ReportConfigPanel({ const locale = useMetadataLocale(); // Unsaved-edits flag — gates Publish (mirrors studio's "save first"). const [dirty, setDirty] = useState(false); + // A save is in flight: Save is disabled and the inspector read-only until it + // settles, so no edit lands in a window the save does not carry. + const [saving, setSaving] = useState(false); // Draft state seeded from `config`. Rebuilt only when the source identity // changes (the host stabilizes `config` and bumps it on open / save) — never @@ -113,8 +122,20 @@ export function ReportConfigPanel({ } }, [onFieldChange]); - const handleSave = useCallback(() => { - onSave(draftRef.current); + const handleSave = useCallback(async () => { + // objectui#11583: the edit is reported as saved, and the panel closed, + // only once the host says the save landed. A refused save keeps the panel + // open and dirty with the edit in place, so Save can be pressed again. + setSaving(true); + let landed: boolean; + try { + landed = (await onSave(draftRef.current)) !== false; + } catch { + landed = false; + } finally { + setSaving(false); + } + if (!landed) return; setDirty(false); onClose(); }, [onSave, onClose]); @@ -173,7 +194,7 @@ export function ReportConfigPanel({ type="report" name={typeof draft.name === 'string' ? draft.name : ''} draft={draft} - readOnly={false} + readOnly={saving} locale={locale} onPatch={handlePatch} /> @@ -194,7 +215,7 @@ export function ReportConfigPanel({ - diff --git a/packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx b/packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx index 3319c15fb4..8be6719354 100644 --- a/packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx +++ b/packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx @@ -25,8 +25,19 @@ * `MetadataClient` does. The panel's spec inspector is reduced to one edit * button, and the draft bar (its own reads) to nothing. * + * ## The patch round: the editor waits for the save (objectui#11583) + * + * `ReportConfigPanel.handleSave` called `onSave`, cleared its dirty flag and + * closed before the save settled, so after a refused save the panel was gone + * and a reopen showed the stored report: the edit was lost. Now + * `ReportView.saveSchema` resolves whether it saved, and the panel closes + * only when the save lands. A refused save leaves it open, with the edit in + * the inspector and Save live. + * * Direction, written before the run: on the unmodified tree the refused case - * goes RED (no `toast.error`), and the control stays GREEN. + * goes RED (no `toast.error`), and the control stays GREEN. The patch round's + * assertions went RED on the first round's head for their own reason (the + * panel closed), and its control stays GREEN. */ import * as React from 'react'; @@ -51,8 +62,13 @@ vi.mock('@object-ui/plugin-dashboard', async (importOriginal) => ({ })); // The panel's spec inspector, reduced to one edit: a new title. vi.mock('./metadata-admin/inspectors/ReportDefaultInspector', () => ({ - ReportDefaultInspector: ({ onPatch }: any) => ( - ), @@ -148,6 +164,12 @@ describe('the report editor\'s Save says a refused save (objectui#11583, ReportV expect([type, name, opts]).toEqual(['report', 'revenue_by_month', { mode: 'draft' }]); expect(body).toMatchObject({ label: 'Revenue EDITED' }); expect(refresh).not.toHaveBeenCalled(); + // The patch round: the editor stays open with the edit intact, and Save + // is live for a retry (it used to close before the save settled, and a + // reopen showed the stored report). + expect(screen.getByTestId('report-config-panel')).toBeTruthy(); + expect(screen.getByTestId('stub-report-edit').getAttribute('data-label')).toBe('Revenue EDITED'); + await waitFor(() => expect((screen.getByTestId('report-config-save') as HTMLButtonElement).disabled).toBe(false)); }); it('a save that lands raises nothing and refreshes the read (control)', async () => { @@ -157,5 +179,7 @@ describe('the report editor\'s Save says a refused save (objectui#11583, ReportV await waitFor(() => expect(refresh).toHaveBeenCalled()); expect(toast.error).not.toHaveBeenCalled(); + // A landed save closes the editor, as it always did. + await waitFor(() => expect(screen.queryByTestId('report-config-panel')).toBeNull()); }); }); diff --git a/packages/app-shell/src/views/ReportView.tsx b/packages/app-shell/src/views/ReportView.tsx index 6dfd45d632..3e4244e9e2 100644 --- a/packages/app-shell/src/views/ReportView.tsx +++ b/packages/app-shell/src/views/ReportView.tsx @@ -175,17 +175,19 @@ export function ReportView({ dataSource }: { dataSource?: DataSource }) { }, [editSchema, reportData, getFieldsForObject]); // ---- Save helper -------------------------------------------------------- + // Resolves whether the draft was staged, so the editor closes only on a + // save that landed (objectui#11583). const saveSchema = useCallback( - async (schema: any) => { + async (schema: any): Promise => { + if (!metadataClient) return false; try { - if (metadataClient) { - // ADR-0034: save stages a per-item draft; an explicit Publish - // promotes it (RuntimeDraftBar). `sys_report` is retired. - await persistRuntimeMetadata('report', reportName!, schema, { - metadataClient, - }); - refresh().catch(() => {}); - } + // ADR-0034: save stages a per-item draft; an explicit Publish + // promotes it (RuntimeDraftBar). `sys_report` is retired. + await persistRuntimeMetadata('report', reportName!, schema, { + metadataClient, + }); + refresh().catch(() => {}); + return true; } catch (err) { console.warn('[ReportView] Auto-save failed:', err); // objectui#11583: a refused save is said, with the door's message. @@ -195,6 +197,7 @@ export function ReportView({ dataSource }: { dataSource?: DataSource }) { description: formatMetadataError(err), classNames: { description: 'whitespace-pre-line' }, }); + return false; } }, [metadataClient, reportName, refresh, t], @@ -223,10 +226,14 @@ export function ReportView({ dataSource }: { dataSource?: DataSource }) { ); const handleReportConfigSave = useCallback( - (config: Record) => { + async (config: Record): Promise => { setEditSchema(config); - saveSchema(config); - setConfigVersion((v) => v + 1); + const saved = await saveSchema(config); + // Re-seat the panel's config only on a save that landed: a refused one + // keeps the open panel's draft, edit included, for a retry + // (objectui#11583). + if (saved) setConfigVersion((v) => v + 1); + return saved; }, [saveSchema], ); diff --git a/packages/app-shell/src/views/writeRefusalCensus-11583.test.ts b/packages/app-shell/src/views/writeRefusalCensus-11583.test.ts index 45debb869e..b5817f7dcc 100644 --- a/packages/app-shell/src/views/writeRefusalCensus-11583.test.ts +++ b/packages/app-shell/src/views/writeRefusalCensus-11583.test.ts @@ -118,7 +118,7 @@ const LEDGER: Row[] = [ { file: 'ObjectView.tsx', symbol: 'handleSetDefaultView', callee: 'dispatchViewPatches', calls: 1, kind: 'surfaces', pin: 'ObjectView.viewWriteRefusal-11583.test.tsx', - reason: 'Set as default: says a refusal with a toast; predates this census.', + reason: 'Set as default: a toast with the door\'s message; it was a bare untranslated literal with no description until the objectui#11583 patch round.', }, { file: 'ObjectView.tsx', symbol: 'handleReorderViews', callee: 'dispatchViewPatches', calls: 1, @@ -128,7 +128,7 @@ const LEDGER: Row[] = [ { file: 'ReportView.tsx', symbol: 'saveSchema', callee: 'persistRuntimeMetadata', calls: 1, kind: 'surfaces', pin: 'ReportView.saveRefusal-11583.test.tsx', - reason: 'The report editor\'s Save (once per press, not per edit): a toast with the door\'s message (objectui#11583).', + reason: 'The report editor\'s Save (once per press, not per edit): a toast with the door\'s message, and the editor is told the save failed, so it stays open with the edit (objectui#11583).', }, { file: 'RuntimeDraftBar.tsx', symbol: 'handlePublish', callee: 'publishRuntimeMetadata', calls: 1, From e075976a934dfe70011e4e001145f743d397acba Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 04:44:35 +0000 Subject: [PATCH 6/7] test(app-shell): the #4155 ratchet's explicit-save control reads handleViewConfigSave's own body, not a 600-character window (objectui#11583) The control asserted `handleViewConfigSave[\s\S]{0,600}persistRuntimeMetadata('view'`. objectui#11583 added a docblock and an early return to the handler, moving the call 708 characters after the name with the call still in place, so the window went red (CI shard 4/8 at e3c7e63). The assertion now finds the `const handleViewConfigSave` declaration with the TypeScript parser and requires a `persistRuntimeMetadata('view', ...)` call inside it: same meaning, no byte distance, and a call in another function cannot satisfy it. Claude-Session: https://claude.ai/code/session_01FjqrwXPfSMkSfkKYDSRkN2 Co-authored-by: Claude --- .../views/view-filter-fold.ratchet.test.ts | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/packages/app-shell/src/views/view-filter-fold.ratchet.test.ts b/packages/app-shell/src/views/view-filter-fold.ratchet.test.ts index 7bce97c262..f02cf094fe 100644 --- a/packages/app-shell/src/views/view-filter-fold.ratchet.test.ts +++ b/packages/app-shell/src/views/view-filter-fold.ratchet.test.ts @@ -37,6 +37,7 @@ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; const here = path.dirname(fileURLToPath(import.meta.url)); const objectViewPath = path.join(here, 'ObjectView.tsx'); @@ -53,6 +54,47 @@ const widgetsSrc = readFileSync(widgetsPath, 'utf8'); */ const FILTER_PERSIST_CALLS = /persistViewPatch\s*\([^;]*?\{\s*filter\s*(?::\s*([A-Za-z0-9_.]+))?/gs; +/** + * The initializer of `const NAME = …` in `ObjectView.tsx` — for a handler, its + * `useCallback(…)` call, so the function body and its dependency list — found + * by the TypeScript parser. An assertion about what a handler does is bound to + * the handler this way, not to a byte distance from its name: a fixed-width + * window after the name went red when a docblock and an early return were + * added to `handleViewConfigSave` (objectui#11583), with the call still there. + */ +function declarationInitializer(src: string, name: string): ts.Expression | undefined { + const sf = ts.createSourceFile('ObjectView.tsx', src, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); + let found: ts.Expression | undefined; + const visit = (node: ts.Node) => { + if (found) return; + if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && node.name.text === name) { + found = node.initializer; + return; + } + ts.forEachChild(node, visit); + }; + visit(sf); + return found; +} + +/** The calls to `callee` under `node` whose first argument is the string literal `first`. */ +function callsWithFirstArg(node: ts.Node, callee: string, first: string): ts.CallExpression[] { + const out: ts.CallExpression[] = []; + const visit = (n: ts.Node) => { + if ( + ts.isCallExpression(n) + && ts.isIdentifier(n.expression) + && n.expression.text === callee + && n.arguments.length > 0 + && ts.isStringLiteralLike(n.arguments[0]) + && n.arguments[0].text === first + ) out.push(n); + ts.forEachChild(n, visit); + }; + visit(node); + return out; +} + describe('objectui#4155 ratchet — the filter panel persists nothing', () => { it('the ratchet is reading real source, not an empty string', () => { expect(objectViewSrc.length).toBeGreaterThan(10_000); @@ -102,8 +144,15 @@ describe('objectui#4155 ratchet — the filter panel persists nothing', () => { it('the EXPLICIT save path is untouched — this is the control', () => { // "Save view" through the view config panel still writes the whole view // body through the metadata seam. #4155 removed the automatic write, - // not the deliberate one. - expect(objectViewSrc).toMatch(/handleViewConfigSave[\s\S]{0,600}persistRuntimeMetadata\('view'/); + // not the deliberate one. Read inside the handler's own declaration + // (see `declarationInitializer`), so the assertion cannot be satisfied + // by a call in some other function, nor broken by a comment. + const handler = declarationInitializer(objectViewSrc, 'handleViewConfigSave'); + expect(handler, '`const handleViewConfigSave` is gone from ObjectView.tsx').toBeDefined(); + expect( + callsWithFirstArg(handler!, 'persistRuntimeMetadata', 'view'), + `handleViewConfigSave no longer persists through the metadata seam:\n${handler!.getText().slice(0, 600)}`, + ).not.toHaveLength(0); }); }); From ba2a810bf04a2ead89cfc50e11fe1e94dc9f30aa Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 06:02:28 +0000 Subject: [PATCH 7/7] fix(app-shell): ViewConfigPanel's onSave returns unknown, so it admits every host the void prop admitted (objectui#11583) Contract review 5977112283 (item 1.1), remedy (a) per the seat's answer 5977119047: the widened `void | Promise` return refused, with TS2322, an async host returning nothing, a host resolving to a record and a sync non-void host, all of which the published `void` prop admitted. The prop now returns `unknown`; the runtime keeps reading one signal, a `false` or a rejection, and reads anything else as saved. ReportConfigPanel's unpublished onSave is aligned in the same stroke. A compile-time pin in the app-shell test project assigns those hosts to the prop. The emitted JS of both panels is unchanged. The changeset says what the type admits. Claude-Session: https://claude.ai/code/session_01FjqrwXPfSMkSfkKYDSRkN2 Co-authored-by: Claude --- .changeset/11583-refused-writes-said.md | 5 +- .../app-shell/src/views/ReportConfigPanel.tsx | 13 +-- .../ViewConfigPanel.onSaveHosts-11583.test.ts | 85 +++++++++++++++++++ .../app-shell/src/views/ViewConfigPanel.tsx | 21 +++-- 4 files changed, 109 insertions(+), 15 deletions(-) create mode 100644 packages/app-shell/src/views/ViewConfigPanel.onSaveHosts-11583.test.ts diff --git a/.changeset/11583-refused-writes-said.md b/.changeset/11583-refused-writes-said.md index 69aad898de..642bd69e61 100644 --- a/.changeset/11583-refused-writes-said.md +++ b/.changeset/11583-refused-writes-said.md @@ -31,5 +31,6 @@ save is in flight. The view-config panel waits for the save before it reports the edit as saved. A refused save leaves the panel dirty, so Save stays enabled for a retry, and the "unpublished changes" indicator is not raised for a draft that was never written. Save is disabled while the save is -in flight. `ViewConfigPanel`'s `onSave` may now return a promise of the outcome (`true` saved, -`false` not); a host that returns nothing is read as saved, as before. +in flight. `ViewConfigPanel`'s `onSave` accepts any return, as it did when it was typed `void`: +the panel awaits it, and `false` (returned, or resolved by a promise) or a rejection means the +save was refused; anything else, nothing included, is read as saved. diff --git a/packages/app-shell/src/views/ReportConfigPanel.tsx b/packages/app-shell/src/views/ReportConfigPanel.tsx index 2500984e79..21e7439503 100644 --- a/packages/app-shell/src/views/ReportConfigPanel.tsx +++ b/packages/app-shell/src/views/ReportConfigPanel.tsx @@ -48,13 +48,14 @@ export interface ReportConfigPanelProps { /** The current report definition (flat spec Report document). */ config: Record | null; /** - * Persist all draft changes. Return a promise of the outcome to have the - * panel wait for it (objectui#11583): it clears its dirty state and closes - * only when the promise resolves to anything but `false`. `false` or a - * rejection leaves it open and dirty, with the edit in place, so it can be - * saved again. A host that returns nothing is read as saved at once. + * Persist all draft changes. Any return is accepted; the panel awaits it and + * reads one refusal signal (objectui#11583): `false` (returned, or resolved + * by a promise) or a rejection leaves it open and dirty, with the edit in + * place, so it can be saved again. Anything else, nothing included, is read + * as saved: the dirty state clears and the panel closes. Typed `unknown` for + * the reason `ViewConfigPanelProps.onSave` gives. */ - onSave: (config: Record) => void | Promise; + onSave: (config: Record) => unknown; /** Called on every field change so the host can drive a live preview. */ onFieldChange?: (key: string, value: any, draft?: Record) => void; /** diff --git a/packages/app-shell/src/views/ViewConfigPanel.onSaveHosts-11583.test.ts b/packages/app-shell/src/views/ViewConfigPanel.onSaveHosts-11583.test.ts new file mode 100644 index 0000000000..ac1a686662 --- /dev/null +++ b/packages/app-shell/src/views/ViewConfigPanel.onSaveHosts-11583.test.ts @@ -0,0 +1,85 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Type-level pin for objectui#11583's contract review (record `5977112283`, + * item ①.1): `ViewConfigPanelProps.onSave` admits every host the `void`-typed + * prop admitted. + * + * ## What the defect was + * + * `ViewConfigPanel` is exported from the package entry. objectui#11583 made + * the panel wait for `onSave`'s outcome and widened the prop's return type + * from `void` to `void | Promise`. TypeScript ignores a function's + * return type only when the target's return type is exactly `void`; against + * the union it checks it, and refused (TS2322) hosts the published prop had + * admitted: an `async` host that returns nothing, a host resolving to a + * record, a sync host returning a value. Every in-repo host returns a boolean, + * so the package's own type-check stayed green; the review's out-of-repo probe + * found it. Remedy (a), the seat's call (`5977119047`): the prop returns + * `unknown`, and the runtime keeps reading one signal, + * `(await onSave?.(flat)) !== false`. + * + * ## Why the instrument is `tsc` + * + * The assignments below are checked by `packages/app-shell/tsconfig.test.json`, + * which compiles every `src/**\/*.test.ts` in the package and is chained off + * the package's `type-check` script (CI's `Type Check`). With the prop typed + * back to `void | Promise`, the three host rows each fail with TS2322 + * and the return-type equation fails too. The runtime half (what the panel + * does with each outcome) is pinned in `ViewConfigPanel.saveOutcome-11583.test.tsx`. + * + * `ReportConfigPanelProps.onSave` is unpublished, and is aligned to the same + * shape in the same stroke, so it is held to the same rows. + */ + +import { describe, it, expect } from 'vitest'; +import type { ViewConfigPanelProps } from './ViewConfigPanel'; +import type { ReportConfigPanelProps } from './ReportConfigPanel'; + +type Assert = T; +type IsAny = 0 extends 1 & T ? true : false; +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 ? true : false; + +type ViewOnSave = NonNullable; +type ReportOnSave = ReportConfigPanelProps['onSave']; + +// Guard against the probe lying: were the prop `any`, every row below would +// pass for the wrong reason. +type _ViewOnSaveNotAny = Assert, false>>; +type _ReportOnSaveNotAny = Assert, false>>; +/** The ruled shape: the return is `unknown`, so no host's return is checked. */ +type _ViewOnSaveReturnsUnknown = Assert, unknown>>; +type _ReportOnSaveReturnsUnknown = Assert, unknown>>; + +/** A host's own persistence call; never reaches a network. */ +const persist = async (_draft: Record): Promise => {}; + +// The hosts the `void` prop admitted. Each row is a TS2322 under a union return. +const asyncVoidHost: ViewConfigPanelProps['onSave'] = async (draft) => { + await persist(draft); +}; +const recordHost: ViewConfigPanelProps['onSave'] = async (draft) => ({ ...draft, savedAt: 'now' }); +const syncValueHost: ViewConfigPanelProps['onSave'] = (draft) => Object.keys(draft).length; +// And the two the union admitted, which `unknown` keeps. +const voidHost: ViewConfigPanelProps['onSave'] = () => {}; +const signalHost: ViewConfigPanelProps['onSave'] = async () => false; + +const reportHosts: ReportOnSave[] = [ + async (config) => { + await persist(config); + }, + async (config) => ({ ...config, savedAt: 'now' }), + (config) => Object.keys(config).length, + () => {}, + async () => false, +]; + +describe('ViewConfigPanelProps.onSave admits every host the void prop admitted (objectui#11583, contract review ①.1)', () => { + it('each host is a callable the panel can await', async () => { + const hosts = [asyncVoidHost, recordHost, syncValueHost, voidHost, signalHost, ...reportHosts]; + for (const host of hosts) { + expect(typeof host).toBe('function'); + await expect(Promise.resolve(host!({ label: 'x' }))).resolves.not.toBeInstanceOf(Error); + } + }); +}); diff --git a/packages/app-shell/src/views/ViewConfigPanel.tsx b/packages/app-shell/src/views/ViewConfigPanel.tsx index 3ad733b393..0497429573 100644 --- a/packages/app-shell/src/views/ViewConfigPanel.tsx +++ b/packages/app-shell/src/views/ViewConfigPanel.tsx @@ -69,14 +69,21 @@ export interface ViewConfigPanelProps { /** Called when any view config field changes (local draft update) */ onViewUpdate?: (field: string, value: any) => void; /** - * Called to persist all draft changes. Return a promise of the outcome to - * have the panel wait for it (objectui#11583): it reports the edit as - * saved (clears the dirty state and raises the draft indicator) only when - * the promise resolves to anything but `false`. `false` or a rejection - * leaves the panel dirty, so the edit can be saved again. A host that - * returns nothing is read as saved at once, as before. + * Called to persist all draft changes. Any return is accepted, as with the + * `void` this prop was typed with before objectui#11583; the panel awaits + * it and reads ONE refusal signal: `false` (returned, or resolved by a + * promise) or a rejection leaves the panel dirty, with no draft + * announced, so the edit can be saved again. Anything else, nothing + * included, is read as saved: the dirty state clears and the draft + * indicator is raised. + * + * Typed `unknown`, not a union naming the signal: against a union + * TypeScript checks the host's return type, and refuses hosts the `void` + * prop admitted (an `async` host that returns nothing, one resolving to a + * record, a sync non-void return). The contract review of objectui#11583 + * measured that; `ViewConfigPanel.onSaveHosts-11583.test.ts` pins it. */ - onSave?: (draft: Record) => void | Promise; + onSave?: (draft: Record) => unknown; /** Called when create-mode view is created */ onCreate?: (config: Record) => void; /**