Add better handling for cron slug errors

Author: malwilley

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');
+  });
+});

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<string, unknown> | undefined
+): string | undefined {
+  if (!responseJSON) {
+    return undefined;
+  }
+  return findFirstMessage(responseJSON);
+}
+
+function findFirstMessage(obj: Record<string, unknown>): 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<string, unknown>);
+      if (nested) {
+        return nested;
+      }
+    }
+  }
+  return undefined;
+}

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}
     >
       <CronDetectorForm detector={detector} />
     </EditDetectorLayout>

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);

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(<DetectorNewSettings />, {
+        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();
+    });
   });
 });