diff --git a/packages/app-shell/src/views/metadata-admin/MetadataTypeActions.test.tsx b/packages/app-shell/src/views/metadata-admin/MetadataTypeActions.test.tsx index a18578e3a8..ebaf9581c9 100644 --- a/packages/app-shell/src/views/metadata-admin/MetadataTypeActions.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/MetadataTypeActions.test.tsx @@ -13,6 +13,11 @@ const mockFetch = vi.fn(async () => ({ })); vi.mock('@object-ui/auth', () => ({ createAuthenticatedFetch: () => mockFetch })); +// Toasts are the only observable output of the error path. +const toastError = vi.fn(); +const toastSuccess = vi.fn(); +vi.mock('sonner', () => ({ toast: { error: (...a: unknown[]) => toastError(...a), success: (...a: unknown[]) => toastSuccess(...a) } })); + // Stub the param dialog: when open, expose a button that resolves with values — // lets us drive the collect-params promise without the heavy field renderers. vi.mock('../ActionParamDialog', () => ({ @@ -31,7 +36,11 @@ vi.mock('../ActionResultDialog', () => ({ import { MetadataTypeActions } from './MetadataTypeActions'; -beforeEach(() => mockFetch.mockClear()); +beforeEach(() => { + mockFetch.mockClear(); + toastError.mockClear(); + toastSuccess.mockClear(); +}); describe('MetadataTypeActions', () => { it('runs an api action without params directly (no dialog)', async () => { @@ -79,4 +88,72 @@ describe('MetadataTypeActions', () => { await waitFor(() => expect(screen.getByTestId('result-dialog')).toBeTruthy()); expect(screen.getByTestId('result-dialog').textContent).toContain('done'); }); + + // ── framework#3843: the declared `{ success, data }` envelope ────────────── + // Service route modules (`/api/v1/datasources`, `/api/v1/packages`, + // `/api/settings`, …) answer `BaseResponseSchema`, so an action's payload + // arrives under `data`. Both shapes are accepted so this repo is not coupled + // to the framework's module-by-module merge order. + + it('unwraps the { success, data } envelope before binding the result dialog', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + statusText: 'OK', + json: async () => ({ success: true, data: { message: 'connection ok', latencyMs: 7 } }), + } as never); + render( + , + ); + fireEvent.click(screen.getByTitle('Probe')); + await waitFor(() => expect(screen.getByTestId('result-dialog')).toBeTruthy()); + const shown = screen.getByTestId('result-dialog').textContent ?? ''; + expect(shown).toContain('connection ok'); + // The wrapper itself must not reach the dialog's `path` bindings. + expect(JSON.parse(shown)).toEqual({ message: 'connection ok', latencyMs: 7 }); + }); + + it('reads the failure message out of the nested error object, not "[object Object]"', async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 400, + statusText: 'Bad Request', + json: async () => ({ success: false, error: { code: 'datasource_admin_error', message: 'duplicate name' } }), + } as never); + render( + , + ); + fireEvent.click(screen.getByTitle('Probe')); + await waitFor(() => expect(toastError).toHaveBeenCalled()); + const msg = String(toastError.mock.calls[0][0]); + expect(msg).toContain('duplicate name'); + expect(msg).not.toContain('[object Object]'); + }); + + it('still reads a pre-envelope bare-string error — merge order is not a coupling', async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 503, + statusText: 'Service Unavailable', + json: async () => ({ error: 'datasource_admin_unavailable' }), + } as never); + render( + , + ); + fireEvent.click(screen.getByTitle('Probe')); + await waitFor(() => expect(toastError).toHaveBeenCalled()); + expect(String(toastError.mock.calls[0][0])).toContain('datasource_admin_unavailable'); + }); }); diff --git a/packages/app-shell/src/views/metadata-admin/MetadataTypeActions.tsx b/packages/app-shell/src/views/metadata-admin/MetadataTypeActions.tsx index b85b5be409..5bc8db2369 100644 --- a/packages/app-shell/src/views/metadata-admin/MetadataTypeActions.tsx +++ b/packages/app-shell/src/views/metadata-admin/MetadataTypeActions.tsx @@ -155,16 +155,39 @@ export function MetadataTypeActions({ entry, location, recordId, onAfter }: Meta if (method !== 'GET' && method !== 'DELETE') init.body = JSON.stringify(params); const res = await authFetch(url, init); - let data: Record | null = null; + let body: Record | null = null; try { - data = (await res.json()) as Record; + body = (await res.json()) as Record; } catch { /* non-JSON / empty body — fall back to status text */ } - if (!res.ok || (data && data.success === false)) { + // framework#3843: the service route modules now answer the declared + // `{ success, data }` envelope (`BaseResponseSchema`), so the payload a + // `resultDialog` binds to — and the `message` the success toast reads — + // lives under `data` rather than at the top level. Unwrap it here, in the + // one place every `type: 'api'` action passes through. + // + // Deliberately tolerant of an already-unwrapped body: endpoints an action + // may target are converted module by module (framework#3675 → #3689 → + // #3843), so this repo must not be coupled to the merge order of that + // sequence. Same reason the two attachment openers read + // `body?.url ?? body?.data?.url` for framework#3689. + const data = + body && typeof body.success === 'boolean' && 'data' in body + ? (body.data as Record | null) + : body; + + if (!res.ok || (body && body.success === false)) { + // `error` is `{ code, message }` in the envelope, and was a bare string + // before it — read both so a partially-converted backend still explains + // itself instead of toasting "[object Object]". + const err = body?.error as { message?: string } | string | undefined; const detail = - (data?.error as string) || (data?.message as string) || `HTTP ${res.status} ${res.statusText}`.trim(); + (typeof err === 'object' && err !== null ? err.message : undefined) || + (typeof err === 'string' ? err : undefined) || + (body?.message as string) || + `HTTP ${res.status} ${res.statusText}`.trim(); toast.error(`${action.errorMessage ? `${action.errorMessage}: ` : ''}${title}: ${detail}`); return; } diff --git a/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx b/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx index 1d9f2da712..260221e804 100644 --- a/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx +++ b/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx @@ -172,14 +172,40 @@ export function DatasourceResourcePage(_props: { type?: string }): React.ReactEl const [loading, setLoading] = React.useState(true); const [busy, setBusy] = React.useState(null); + /** + * Fetch + unwrap the declared response envelope. + * + * framework#3843 moved `/api/v1/datasources/*` onto `BaseResponseSchema`, so + * every body is now `{ success, data }` / `{ success: false, error: { code, + * message } }`. Unwrapping here keeps all nine call sites below reading their + * payload key unchanged (`data.datasources`, `data.drivers`, `data.tables`, + * `{ draft }`, …) — the keys did not change, only their depth. + * + * Both shapes are accepted on purpose, error and success alike. This is the + * same tolerance the console already carries for framework#3689 (the + * attachment openers' `body?.url ?? body?.data?.url`), and it exists so the + * two repos are not coupled by merge order: a console on either side of the + * framework change talks to a backend on either side of it. It is a migration + * device, not a permanent second contract — the producer is the authority + * (framework Prime Directive #12). + */ const api = React.useCallback( async (path: string, init?: RequestInit) => { const res = await authFetch(`${SERVER}${path}`, { credentials: 'include', headers: { 'Content-Type': 'application/json' }, ...init }); const text = await res.text(); let body: any = text; try { body = JSON.parse(text); } catch { /* keep text */ } - if (!res.ok) throw new Error((body && (body.message || body.error)) || `HTTP ${res.status}`); - return body; + if (!res.ok) { + // `error` is `{ code, message }` post-#3843 and was a bare string + // before it; without the first read this threw "[object Object]". + const detail = + (body && typeof body.error === 'object' && body.error?.message) || + (body && typeof body.error === 'string' ? body.error : undefined) || + (body && body.message) || + `HTTP ${res.status}`; + throw new Error(detail); + } + return body && typeof body.success === 'boolean' && 'data' in body ? body.data : body; }, [authFetch], );