diff --git a/.changeset/11583-refused-writes-said.md b/.changeset/11583-refused-writes-said.md new file mode 100644 index 0000000000..642bd69e61 --- /dev/null +++ b/.changeset/11583-refused-writes-said.md @@ -0,0 +1,36 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/i18n': patch +--- + +A refused save, pin, reorder, view setting, report save, publish or discard in the console is now said to the user, and the view-config panel no longer reports a refused save as saved (objectui#11583). + +objectui#11578 made the two Create View doors say a refused save. The console's other metadata +writes on the object page, the report page and the draft bar still caught a refusal with a +console line and nothing else, so a permission refusal or a spec refusal looked like a saved +change: + +- the view-config panel's Save on an existing view; +- pinning or unpinning a view, and reordering views in "Manage views"; +- a toolbar setting on a list view (density, sort, columns, hidden fields), when the server + refuses it (the console's own permission check already said its refusal, and still does); +- the report editor's Save; +- Publish and Discard draft on the draft bar of those two editors. + +Each now raises the refusal through the console's error toast, with the save door's own +message: the field-anchored issues of a validation refusal, one per line, or the refusal's text. +The draft bar's toasts lead with "Publish failed" or "Discard failed", two new strings in all +ten language packs; the others lead with "Failed to save". Set as default, which already raised +an untranslated "Failed to set default view" with no reason, now does the same. + +The report editor waits for its save. It closes once the report is saved; a refused save leaves +it open with the edit in place, so Save can be pressed again (it used to close at once, and +reopening it showed the stored report). Save is disabled, and the editor read-only, while the +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` 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/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 2f4764a69a..ec24e597bf 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]); @@ -2304,7 +2332,11 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co setRefreshKey(k => k + 1); } catch (err) { console.error('[ViewTabBar] Failed to set default view:', err); - toast.error('Failed to set default view'); + // objectui#11583: said like its siblings, with the door's message. + toast.error(t('form.saveError'), { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); } }, [dataSource, objectName, savedViews, isSavedView, t]); @@ -2331,10 +2363,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..84a7a1d5ac --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.viewWriteRefusal-11583.test.tsx @@ -0,0 +1,477 @@ +// 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. + * + * - `handleSetDefaultView` raised the untranslated literal + * 'Failed to set default view', with no description, so the refusal's own + * reason never reached the user (the objectui#11583 patch round). + * + * Rename 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. The + * set-as-default case was added in the patch round and went RED on the first + * round's head for its own reason: a toast, but no description. + */ + +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('set-as-default says a refused write with the door\'s message (objectui#11583, handleSetDefaultView)', () => { + it('a refused set-as-default raises the door\'s message, not a bare literal', async () => { + const dataSource = makeDataSource({ + updateView: vi.fn(async () => { throw refusal('Setting a default view requires the Manage Metadata permission'); }), + }); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-set-default')); + }); + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + expect(errorDescription()).toContain('Setting a default view requires the Manage Metadata permission'); + expect(vi.mocked(toast.error).mock.calls[0][0]).not.toBe('Failed to set default view'); + expect((dataSource as any).updateView).toHaveBeenCalledWith(OBJECT_NAME, SAVED_ID, { isDefault: true }); + }); + + it('a set-as-default that lands raises nothing (control)', async () => { + const dataSource = makeDataSource(); + await mountOnSavedView(dataSource); + await act(async () => { + fireEvent.click(screen.getByTestId('stub-set-default')); + }); + await waitFor(() => expect((dataSource as any).updateView).toHaveBeenCalledWith(OBJECT_NAME, SAVED_ID, { isDefault: true })); + await settle(); + expect(toast.error).not.toHaveBeenCalled(); + }); +}); + +describe('rename and delete already said a refusal (controls, objectui#11583: handleRenameView, 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 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/ReportConfigPanel.saveOutcome-11583.test.tsx b/packages/app-shell/src/views/ReportConfigPanel.saveOutcome-11583.test.tsx new file mode 100644 index 0000000000..277a8c08b2 --- /dev/null +++ b/packages/app-shell/src/views/ReportConfigPanel.saveOutcome-11583.test.tsx @@ -0,0 +1,143 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11583 (patch round): `ReportConfigPanel.handleSave` reads the + * save's outcome before it reports the edit as saved. + * + * ## The defect this pins + * + * `handleSave` called `onSave(draftRef.current)`, cleared `dirty` and called + * `onClose()` before the save settled. Measured through the real `ReportView` + * after the first round's toast: a refused save closed the panel, and a reopen + * showed the stored report, so the edit was lost. The same shape as + * `ViewConfigPanel.handleSave` (row 2 of the card), and the same direction. + * + * ## The contract this pins + * + * `onSave` may return a promise of the outcome (`ReportView`'s + * `handleReportConfigSave` does). The panel clears `dirty` and closes only + * when that promise resolves to anything but `false`; `false` or a rejection + * leaves it open, dirty, with the edit in the inspector and Save live. While + * the save is in flight Save is disabled and the inspector is read-only, so + * no edit can land in a window the save does not carry. A host that returns + * nothing is read as saved, as before. + * + * The end-to-end pin, through the real `ReportView`, is + * `ReportView.saveRefusal-11583.test.tsx`. + * + * Direction, written before the run: on the first round's head the `false`, + * rejection and in-flight cases go RED (the panel closes 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/ReportDefaultInspector', () => ({ + ReportDefaultInspector: ({ draft, readOnly, onPatch }: any) => ( + + ), +})); + +// 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..21e7439503 100644 --- a/packages/app-shell/src/views/ReportConfigPanel.tsx +++ b/packages/app-shell/src/views/ReportConfigPanel.tsx @@ -47,8 +47,15 @@ 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. 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) => unknown; /** Called on every field change so the host can drive a live preview. */ onFieldChange?: (key: string, value: any, draft?: Record) => void; /** @@ -84,6 +91,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 +123,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 +195,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 +216,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 new file mode 100644 index 0000000000..8be6719354 --- /dev/null +++ b/packages/app-shell/src/views/ReportView.saveRefusal-11583.test.tsx @@ -0,0 +1,185 @@ +// 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. + * + * ## 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. 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'; +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, draft }: 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(); + // 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 () => { + client.value = { get: vi.fn(async () => null), save: vi.fn(async () => ({})) }; + mountReport(); + await editThriceAndSave(); + + 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 8fd014fb3d..3e4244e9e2 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'; @@ -173,22 +175,32 @@ 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. + // 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' }, + }); + return false; } }, - [metadataClient, reportName, refresh], + [metadataClient, reportName, refresh, t], ); // ---- Open / close config panel ------------------------------------------ @@ -214,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/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..405b57c2b7 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`. */ @@ -63,6 +66,20 @@ export interface RuntimeDraftBarProps { savedSignal?: number; } +/** + * The leads of the bar's refusal toasts (objectui#11583), from the console's + * language packs. The engine table this file reads its labels from has no + * failure keys, and it covers two languages, not the console's ten. A hook of + * its own so its `t` is the packs' and the bar's `t` stays the table's. + */ +function useRefusalTitles(): { publishFailed: string; discardFailed: string } { + const { t } = useObjectTranslation(); + return { + publishFailed: t('console.runtimeDraft.publishFailed'), + discardFailed: t('console.runtimeDraft.discardFailed'), + }; +} + export function RuntimeDraftBar({ type, name, @@ -75,6 +92,7 @@ export function RuntimeDraftBar({ savedSignal, }: RuntimeDraftBarProps) { const locale = useMetadataLocale(); + const refusalTitles = useRefusalTitles(); 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 +151,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(refusalTitles.publishFailed, { + description: formatMetadataError(err), + classNames: { description: 'whitespace-pre-line' }, + }); } finally { setBusy(false); } - }, [type, name, metadataClient, dataSource, objectName, onAfterChange]); + }, [type, name, metadataClient, dataSource, objectName, onAfterChange, refusalTitles.publishFailed]); const handleDiscard = useCallback(async () => { if (!name) return; @@ -153,10 +177,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(refusalTitles.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, refusalTitles.discardFailed]); // 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.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.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..0497429573 100644 --- a/packages/app-shell/src/views/ViewConfigPanel.tsx +++ b/packages/app-shell/src/views/ViewConfigPanel.tsx @@ -68,8 +68,22 @@ 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. 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) => unknown; /** Called when create-mode view is created */ onCreate?: (config: Record) => void; /** @@ -141,9 +155,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 +206,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 +330,7 @@ export function ViewConfigPanel({ open, onClose, mode = 'edit', activeView, obje