diff --git a/static/app/views/detectors/components/forms/common/getDetectorResponseErrorMessage.spec.tsx b/static/app/views/detectors/components/forms/common/getDetectorResponseErrorMessage.spec.tsx new file mode 100644 index 00000000000000..ead47a916e345f --- /dev/null +++ b/static/app/views/detectors/components/forms/common/getDetectorResponseErrorMessage.spec.tsx @@ -0,0 +1,37 @@ +import {getDetectorResponseErrorMessage} from 'sentry/views/detectors/components/forms/common/getDetectorResponseErrorMessage'; + +describe('getDetectorResponseErrorMessage', () => { + it('returns undefined for undefined', () => { + expect(getDetectorResponseErrorMessage(undefined)).toBeUndefined(); + }); + + it('returns undefined for empty object', () => { + expect(getDetectorResponseErrorMessage({})).toBeUndefined(); + }); + + it('handles {detail: "message"} shape', () => { + expect(getDetectorResponseErrorMessage({detail: 'Something went wrong'})).toBe( + 'Something went wrong' + ); + }); + + it('handles {field: ["message"]} shape', () => { + expect(getDetectorResponseErrorMessage({name: ['Name is required']})).toBe( + 'Name is required' + ); + }); + + it('handles {dataSources: {field: ["message"]}} shape', () => { + expect( + getDetectorResponseErrorMessage({dataSources: {query: ['Invalid query']}}) + ).toBe('Invalid query'); + }); + + it('returns the first message when multiple fields have errors', () => { + const result = getDetectorResponseErrorMessage({ + name: ['Name error'], + query: ['Query error'], + }); + expect(result).toBe('Name error'); + }); +}); diff --git a/static/app/views/detectors/components/forms/common/getDetectorResponseErrorMessage.tsx b/static/app/views/detectors/components/forms/common/getDetectorResponseErrorMessage.tsx new file mode 100644 index 00000000000000..7a530b512f6cd1 --- /dev/null +++ b/static/app/views/detectors/components/forms/common/getDetectorResponseErrorMessage.tsx @@ -0,0 +1,35 @@ +/** + * Extracts the first human-readable error message from the detector API error response + * so that we can surface it to the user in a toast. + * + * TODO: When migrating to the new form components, we should consider adding this + * functionality generically + */ +export function getDetectorResponseErrorMessage( + responseJSON: Record | undefined +): string | undefined { + if (!responseJSON) { + return undefined; + } + return findFirstMessage(responseJSON); +} + +function findFirstMessage(obj: Record): string | undefined { + for (const value of Object.values(obj)) { + if (typeof value === 'string') { + return value; + } + if (Array.isArray(value)) { + if (typeof value[0] === 'string') { + return value[0]; + } + } + if (typeof value === 'object' && value !== null && !Array.isArray(value)) { + const nested = findFirstMessage(value as Record); + if (nested) { + return nested; + } + } + } + return undefined; +} diff --git a/static/app/views/detectors/components/forms/cron/index.tsx b/static/app/views/detectors/components/forms/cron/index.tsx index 7487e05dc8d5d8..52351572406af4 100644 --- a/static/app/views/detectors/components/forms/cron/index.tsx +++ b/static/app/views/detectors/components/forms/cron/index.tsx @@ -36,6 +36,28 @@ const FORM_SECTIONS = [ CronIssuePreview, AutomateSection, ]; +/** + * Maps API errors in the `dataSources` property to the correct form fields. + * For Crons, `slug` may return error messages, and we want those to show + * up under the `name` field. + * + * TODO: When converting to the new form components, find a less fragile way to do this. + */ +const mapCronDetectorFormErrors = (error: unknown) => { + if (typeof error !== 'object' || error === null) { + return error; + } + + if ('dataSources' in error) { + if (typeof error.dataSources === 'object' && error.dataSources !== null) { + if ('slug' in error.dataSources) { + return {...error, name: error.dataSources.slug}; + } + return {...error, ...error.dataSources}; + } + } + return error; +}; function CronDetectorForm({detector}: {detector?: CronDetector}) { const dataSource = detector?.dataSources[0]; @@ -76,6 +98,7 @@ export function NewCronDetectorForm() { initialFormData={{ scheduleType: CRON_DEFAULT_SCHEDULE_TYPE, }} + mapFormErrors={mapCronDetectorFormErrors} disabledCreate={ showingPlatformGuide ? t( @@ -95,6 +118,7 @@ export function EditExistingCronDetectorForm({detector}: {detector: CronDetector detector={detector} formDataToEndpointPayload={cronFormDataToEndpointPayload} savedDetectorToFormData={cronSavedDetectorToFormData} + mapFormErrors={mapCronDetectorFormErrors} > diff --git a/static/app/views/detectors/hooks/useCreateDetectorFormSubmit.tsx b/static/app/views/detectors/hooks/useCreateDetectorFormSubmit.tsx index 3893384dd43b83..900e41f3b7798b 100644 --- a/static/app/views/detectors/hooks/useCreateDetectorFormSubmit.tsx +++ b/static/app/views/detectors/hooks/useCreateDetectorFormSubmit.tsx @@ -9,9 +9,11 @@ import type { DetectorType, } from 'sentry/types/workflowEngine/detectors'; import {trackAnalytics} from 'sentry/utils/analytics'; +import {RequestError} from 'sentry/utils/requestError/requestError'; import {useNavigate} from 'sentry/utils/useNavigate'; import {useOrganization} from 'sentry/utils/useOrganization'; import {getDetectorAnalyticsPayload} from 'sentry/views/detectors/components/forms/common/getDetectorAnalyticsPayload'; +import {getDetectorResponseErrorMessage} from 'sentry/views/detectors/components/forms/common/getDetectorResponseErrorMessage'; import {useCreateDetector} from 'sentry/views/detectors/hooks'; import {makeMonitorDetailsPathname} from 'sentry/views/detectors/pathnames'; @@ -79,14 +81,18 @@ export function useCreateDetectorFormSubmit< } onSubmitSuccess?.(resultDetector); - } catch (error) { + } catch (error: unknown) { trackAnalytics('monitor.created', { organization, detector_type: payload.type, success: false, }); - addErrorMessage(t('Unable to create monitor')); + addErrorMessage( + (error instanceof RequestError + ? getDetectorResponseErrorMessage(error.responseJSON) + : null) ?? t('Unable to create monitor') + ); if (onError) { onError(error); diff --git a/static/app/views/detectors/new-setting.spec.tsx b/static/app/views/detectors/new-setting.spec.tsx index adf9ab44d7a961..7199722c9d8f87 100644 --- a/static/app/views/detectors/new-setting.spec.tsx +++ b/static/app/views/detectors/new-setting.spec.tsx @@ -16,6 +16,7 @@ import { } from 'sentry-test/reactTestingLibrary'; import {selectEvent} from 'sentry-test/selectEvent'; +import * as indicators from 'sentry/actionCreators/indicator'; import {OrganizationStore} from 'sentry/stores/organizationStore'; import {ProjectsStore} from 'sentry/stores/projectsStore'; import DetectorNewSettings from 'sentry/views/detectors/new-settings'; @@ -1241,5 +1242,39 @@ describe('DetectorEdit', () => { }) ); }); + + it('displays slug errors on the name field and in a toast', async () => { + const mockAddErrorMessage = jest.spyOn(indicators, 'addErrorMessage'); + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/projects/${project.id}/detectors/`, + method: 'POST', + statusCode: 400, + body: { + dataSources: {slug: ['The slug "new-test-cron-job" is already in use.']}, + }, + }); + + render(, { + organization, + initialRouterConfig: cronRouterConfig, + }); + + const title = await screen.findByText('New Monitor'); + await userEvent.click(title); + await userEvent.keyboard('new-test-cron-job{enter}'); + + await userEvent.click(screen.getByRole('button', {name: 'Create Monitor'})); + + await waitFor(() => { + expect(mockAddErrorMessage).toHaveBeenCalledWith( + 'The slug "new-test-cron-job" is already in use.' + ); + }); + + // The slug error is mapped to the name field and shown inline + expect( + await screen.findByText('The slug "new-test-cron-job" is already in use.') + ).toBeInTheDocument(); + }); }); });