From 61e8445f064463563a307726c1a7e8942b523f2d Mon Sep 17 00:00:00 2001 From: hannahwestra25 Date: Tue, 11 Aug 2026 17:53:17 -0400 Subject: [PATCH] fix: show initializer create errors inside the open dialog When POST /api/initializers/settings rejects a create request, the error was only shown on the page behind the modal. Now the error is surfaced inside the active dialog via an externalError prop on InitializerParametersDialog. - Change onAdd return type from Promise to Promise<{ added: boolean; error?: string }> so the error message propagates back to the dialog layer - Add externalError prop to InitializerParametersDialog to display server errors alongside validation errors - Add addError state in AdditionalInitializers to track and clear server errors - Add tests for both the dialog prop and the integration flow Fixes #2346 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../AdditionalInitializers.test.tsx | 31 +++++++++++++++++++ .../Initializers/AdditionalInitializers.tsx | 21 ++++++++++--- .../InitializerParametersDialog.test.tsx | 14 +++++++++ .../InitializerParametersDialog.tsx | 6 ++-- .../components/Initializers/Initializers.tsx | 5 +-- 5 files changed, 69 insertions(+), 8 deletions(-) diff --git a/frontend/src/components/Initializers/AdditionalInitializers.test.tsx b/frontend/src/components/Initializers/AdditionalInitializers.test.tsx index 47644d0a16..dc667a4a7a 100644 --- a/frontend/src/components/Initializers/AdditionalInitializers.test.tsx +++ b/frontend/src/components/Initializers/AdditionalInitializers.test.tsx @@ -342,4 +342,35 @@ describe('AdditionalInitializers', () => { expect(defaultProps.onAdd).toHaveBeenCalledWith('load_default_datasets', null) }) + + it('should show a server error inside the add dialog when onAdd fails', async () => { + const user = userEvent.setup() + + const props = { + ...defaultProps, + registeredInitializers: [refreshInitializer], + onAdd: jest.fn().mockRejectedValue(new Error('Invalid days value.')), + } + + render( + + + , + ) + + fireEvent.change(screen.getByRole('combobox', { name: 'Initializer to add' }), { + target: { value: 'refresh_datasets' }, + }) + await user.click(screen.getByRole('button', { name: 'Add initializer' })) + + const dialog = await screen.findByRole('dialog', {}, { timeout: 3000 }) + await within(dialog).findByText('Add refresh_datasets initializer') + fireEvent.change(within(dialog).getByTestId('param-days'), { target: { value: '12' } }) + await user.click(await within(dialog).findByRole('button', { name: 'Add', hidden: true })) + + expect(await within(dialog).findByRole('alert', { hidden: true })).toHaveTextContent( + 'Invalid days value.', + ) + expect(dialog).toBeInTheDocument() + }) }) diff --git a/frontend/src/components/Initializers/AdditionalInitializers.tsx b/frontend/src/components/Initializers/AdditionalInitializers.tsx index d9a69d781b..2778040895 100644 --- a/frontend/src/components/Initializers/AdditionalInitializers.tsx +++ b/frontend/src/components/Initializers/AdditionalInitializers.tsx @@ -9,6 +9,7 @@ import type { UpdateAdditionalInitializerRequest, } from '@/types' +import { toApiError } from '@/services/errors' import { useAdditionalInitializersStyles } from './AdditionalInitializers.styles' import { formatInitializerParameters, formatSupportedParameterSummary } from './initializerFormatting' import { resolveRegisteredInitializer } from './initializerLookup' @@ -133,6 +134,7 @@ export default function AdditionalInitializers({ const listStyles = useAdditionalInitializersStyles() const [selectedInitializerName, setSelectedInitializerName] = useState('') const [addDialogOpen, setAddDialogOpen] = useState(false) + const [addError, setAddError] = useState(null) const initializerName = selectedInitializerName || registeredInitializers[0]?.initializer_name || '' const selectedInitializer = registeredInitializers.find( (initializer) => initializer.initializer_name === initializerName, @@ -142,9 +144,14 @@ export default function AdditionalInitializers({ if (!initializerName) { return } - const added = await onAdd(initializerName, parameters) - if (added) { - setAddDialogOpen(false) + setAddError(null) + try { + const added = await onAdd(initializerName, parameters) + if (added) { + setAddDialogOpen(false) + } + } catch (e) { + setAddError(toApiError(e).detail) } } @@ -211,8 +218,14 @@ export default function AdditionalInitializers({ initializer={selectedInitializer} initialParameters={null} submitting={creating} + externalError={addError} onSubmit={handleAdd} - onOpenChange={setAddDialogOpen} + onOpenChange={(open) => { + setAddDialogOpen(open) + if (!open) { + setAddError(null) + } + }} /> )} diff --git a/frontend/src/components/Initializers/InitializerParametersDialog.test.tsx b/frontend/src/components/Initializers/InitializerParametersDialog.test.tsx index d0fda42461..5fa0d84c73 100644 --- a/frontend/src/components/Initializers/InitializerParametersDialog.test.tsx +++ b/frontend/src/components/Initializers/InitializerParametersDialog.test.tsx @@ -207,4 +207,18 @@ describe('InitializerParametersDialog', () => { expect(screen.getByRole('button', { name: 'Add...' })).toBeDisabled() expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled() }) + + it('displays an external error passed via externalError prop', () => { + render( + + + , + ) + + expect(screen.getByRole('alert')).toHaveTextContent('Server rejected the request.') + }) }) diff --git a/frontend/src/components/Initializers/InitializerParametersDialog.tsx b/frontend/src/components/Initializers/InitializerParametersDialog.tsx index 44e8ba832d..1d2c56dd8f 100644 --- a/frontend/src/components/Initializers/InitializerParametersDialog.tsx +++ b/frontend/src/components/Initializers/InitializerParametersDialog.tsx @@ -31,6 +31,7 @@ interface InitializerParametersDialogProps { initializer: RegisteredInitializer | null initialParameters?: Record | null submitting?: boolean + externalError?: string | null onSubmit: (parameters: Record | null) => void | Promise onOpenChange: (open: boolean) => void } @@ -41,6 +42,7 @@ export default function InitializerParametersDialog({ initializer, initialParameters = null, submitting = false, + externalError = null, onSubmit, onOpenChange, }: InitializerParametersDialogProps) { @@ -112,9 +114,9 @@ export default function InitializerParametersDialog({ This initializer takes no parameters. )} - {error && ( + {(error || externalError) && ( - {error} + {error || externalError} )} diff --git a/frontend/src/components/Initializers/Initializers.tsx b/frontend/src/components/Initializers/Initializers.tsx index bbd655dfa2..4e9860aba3 100644 --- a/frontend/src/components/Initializers/Initializers.tsx +++ b/frontend/src/components/Initializers/Initializers.tsx @@ -88,8 +88,9 @@ export default function Initializers() { await refetchSettingsOnly() return true } catch (error) { - setStatusMessage({ intent: 'error', text: toApiError(error).detail }) - return false + const detail = toApiError(error).detail + setStatusMessage({ intent: 'error', text: detail }) + throw error } finally { setCreating(false) }