From a7e4818ab878b99a961890b8e223a88e4140cf4a Mon Sep 17 00:00:00 2001 From: Mike Bridge Date: Fri, 30 Oct 2026 10:21:09 -0600 Subject: [PATCH] fix(semantic-layer): explain a refused semantic view batch in the add modal The bulk creation endpoint refuses an oversized batch with HTTP 400 before any provider lookup, naming the configured limit. The Add Semantic View modal only parsed HTTP 422 bodies, so that refusal surfaced as the generic failure toast. Show the host's refusal inline and as a toast, keep the selection so the user can trim it and retry, and fall back to the generic message for anything other than a plain string message. --- .../AddSemanticViewModal.test.tsx | 109 ++++++++++++++++++ .../semanticViews/AddSemanticViewModal.tsx | 19 ++- .../features/semanticViews/creationResult.ts | 12 ++ 3 files changed, 135 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/features/semanticViews/AddSemanticViewModal.test.tsx b/superset-frontend/src/features/semanticViews/AddSemanticViewModal.test.tsx index eac84aaa3d0d..02deef69db50 100644 --- a/superset-frontend/src/features/semanticViews/AddSemanticViewModal.test.tsx +++ b/superset-frontend/src/features/semanticViews/AddSemanticViewModal.test.tsx @@ -527,6 +527,115 @@ test('shows toast when add semantic views fails', async () => { expect(props.onHide).not.toHaveBeenCalled(); }); +test.each([ + [ + 'the default cap', + { + message: + 'Too many views in one request: 101 provided, at most 100 allowed.', + }, + 'Too many views in one request: 101 provided, at most 100 allowed.', + ], + [ + 'a non-default cap', + { + message: + 'Too many views in one request: 26 provided, at most 25 allowed.', + }, + 'Too many views in one request: 26 provided, at most 25 allowed.', + ], + [ + 'a refusal without a message', + { message: { detail: 'private provider details' } }, + 'An error occurred while adding semantic views', + ], + [ + 'a refusal that is not an object', + 'private provider details', + 'An error occurred while adding semantic views', + ], +])( + 'a batch refused before any lookup explains %s and keeps the selection', + async (_label, body, expected) => { + mockedGet.mockResolvedValue({ + json: { result: [{ uuid: 'layer-1', name: 'Test layer' }] }, + }); + mockedPost.mockImplementation(({ endpoint }: { endpoint: string }) => { + if (endpoint.endsWith('/schema/runtime')) + return Promise.resolve({ json: { result: { properties: {} } } }); + if (endpoint.endsWith('/views')) + return Promise.resolve({ + json: { + result: [ + { name: 'orders', already_added: false }, + { name: 'customers', already_added: false }, + ], + }, + }); + return Promise.reject({ status: 400, json: async () => body }); + }); + const props = createProps(); + render(); + await selectOption('Semantic layer', 'Test layer'); + await selectOption('Semantic views', 'orders'); + await selectOption('Semantic views', 'customers'); + await userEvent.click( + screen.getByRole('button', { name: /add 2 view\(s\)/i }), + ); + const alert = await screen.findByRole('alert'); + expect(alert).toHaveTextContent(expected); + expect(alert).not.toHaveTextContent('private provider details'); + expect(props.addDangerToast).toHaveBeenCalledTimes(1); + expect(props.addDangerToast).toHaveBeenCalledWith(expected); + expect(props.addSuccessToast).not.toHaveBeenCalled(); + expect(props.onSuccess).not.toHaveBeenCalled(); + expect(props.onHide).not.toHaveBeenCalled(); + // The refused batch stays selected so the user can trim it and retry. + expect( + screen.getByRole('button', { name: /add 2 view\(s\)/i }), + ).toBeEnabled(); + }, +); + +test('a batch refusal cannot update a closed and reopened modal', async () => { + let refuseSave: (reason: unknown) => void = () => {}; + const pendingSave = new Promise((_resolve, reject) => { + refuseSave = reject; + }); + mockedGet.mockResolvedValue({ + json: { result: [{ uuid: 'layer-1', name: 'Test layer' }] }, + }); + mockedPost.mockImplementation(({ endpoint }: { endpoint: string }) => { + if (endpoint.endsWith('/schema/runtime')) + return Promise.resolve({ json: { result: { properties: {} } } }); + if (endpoint.endsWith('/views')) + return Promise.resolve({ + json: { result: [{ name: 'orders', already_added: false }] }, + }); + return pendingSave; + }); + const props = createProps(); + const { rerender } = render(); + await selectOption('Semantic layer', 'Test layer'); + await selectOption('Semantic views', 'orders'); + await userEvent.click( + screen.getByRole('button', { name: /add 1 view\(s\)/i }), + ); + rerender(); + rerender(); + await act(async () => { + refuseSave({ + status: 400, + json: async () => ({ + message: + 'Too many views in one request: 2 provided, at most 1 allowed.', + }), + }); + }); + expect(props.addDangerToast).not.toHaveBeenCalled(); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); +}); + // --------------------------------------------------------------------------- // Large-catalog / refresh-stability regression tests (sc-107832) // --------------------------------------------------------------------------- diff --git a/superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx b/superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx index 7db276d8f272..d58edfd56206 100644 --- a/superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx +++ b/superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx @@ -45,6 +45,7 @@ import { SCHEMA_REFRESH_DEBOUNCE_MS, } from 'src/features/semanticLayers/jsonFormsHelpers'; import { + batchRefusalMessage, creationErrorMessage, isRecord, parseCreationResult, @@ -641,13 +642,21 @@ export default function AddSemanticViewModal({ body = json; } catch (error) { // SupersetClient rejects non-2xx responses before parsing their body. - if ( - !isRecord(error) || - error.status !== 422 || - typeof error.json !== 'function' - ) { + if (!isRecord(error) || typeof error.json !== 'function') { throw error; } + if (error.status === 400) { + // The whole batch was refused before any lookup (for example, more + // views than the configured limit): explain why and keep the + // selection so the user can trim it and retry. + const refusal = batchRefusalMessage(await error.json()); + if (!refusal) throw error; + if (gen !== fetchGenRef.current) return; + setCreationErrors([{ name: '', message: refusal }]); + addDangerToast(refusal); + return; + } + if (error.status !== 422) throw error; body = await error.json(); } const { created, errors, failedCount } = parseCreationResult(body); diff --git a/superset-frontend/src/features/semanticViews/creationResult.ts b/superset-frontend/src/features/semanticViews/creationResult.ts index 1348e909ceec..730178f269ca 100644 --- a/superset-frontend/src/features/semanticViews/creationResult.ts +++ b/superset-frontend/src/features/semanticViews/creationResult.ts @@ -53,6 +53,18 @@ function validationText(value: unknown, depth = 0): string { export const creationErrorMessage = (): string => t('An error occurred while adding semantic views'); +/** + * The host refuses an oversized or malformed batch with HTTP 400 before any + * provider lookup runs, so its ``message`` is host text that names the + * configured limit. Anything but a plain string falls back to the generic + * message rather than echoing an unexpected body. + */ +export function batchRefusalMessage(body: unknown): string | null { + if (!isRecord(body) || typeof body.message !== 'string') return null; + const message = body.message.trim(); + return message || null; +} + /** Only consume the documented batch result, not arbitrary transport errors. */ export function parseCreationResult(body: unknown): CreationResult { const result = isRecord(body) && isRecord(body.result) ? body.result : {};