fix(reports): open existing subscription instead of failing on duplicate

superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx

@@ -894,6 +894,165 @@ test('submit failure dispatches danger toast and keeps modal open', async () =>
   fetchMock.removeRoute('post-fail');
 });
 
+const subscriberBootstrapData = (): BootstrapData => ({
+  ...bootstrapData(),
+  user: {
+    firstName: 'Dashboard',
+    lastName: 'Subscriber',
+    username: 'subscriber',
+    isActive: true,
+    isAnonymous: false,
+    userId: 7,
+    roles: {},
+    permissions: {},
+    groups: [],
+  },
+});
+
+test('duplicate subscription opens the existing report for editing', async () => {
+  mockedGetBootstrapData.mockReturnValue(subscriberBootstrapData());
+  fetchMock.post(
+    'glob:*/api/v1/report/subscribe',
+    {
+      status: 409,
+      body: {
+        errors: [{ message: 'Resource already has an attached report.' }],
+      },
+    },
+    { name: 'post-duplicate-subscribe' },
+  );
+  fetchMock.modifyRoute('report-list', {
+    response: {
+      result: [
+        {
+          id: 12,
+          dashboard_id: 1,
+          creation_method: 'dashboards',
+          name: 'My existing report',
+          crontab: '0 9 * * *',
+          report_format: 'PNG',
+        },
+      ],
+    },
+  });
+  const onHide = jest.fn();
+  const store = createStore({}, reducerIndex);
+  render(<ReportModal {...defaultProps} chart={undefined} onHide={onHide} />, {
+    useRedux: true,
+    store,
+  });
+
+  // A width typed into the abandoned create form must not carry over onto
+  // the existing report, since the list endpoint does not return custom_width.
+  await userEvent.type(
+    screen.getByPlaceholderText('Input custom width in pixels'),
+    '1200',
+  );
+  await userEvent.click(screen.getByRole('button', { name: 'Add' }));
+
+  expect(await screen.findByText('Edit email report')).toBeInTheDocument();
+  expect(screen.getByTestId('report-name-test')).toHaveDisplayValue(
+    'My existing report',
+  );
+  expect(
+    screen.getByPlaceholderText('Input custom width in pixels'),
+  ).toHaveDisplayValue('');
+  const toasts = (store.getState() as any).messageToasts;
+  expect(
+    toasts.some((toast: { text: string }) =>
+      toast.text.includes('already subscribed to this dashboard'),
+    ),
+  ).toBe(true);
+  expect(
+    toasts.some((toast: { text: string }) =>
+      toast.text.includes('Failed to create report'),
+    ),
+  ).toBe(false);
+  expect(screen.queryByText('Failed to create report')).not.toBeInTheDocument();
+  expect(onHide).not.toHaveBeenCalled();
+
+  fetchMock.modifyRoute('report-list', { response: {} });
+  fetchMock.removeRoute('post-duplicate-subscribe');
+});
+
+test('duplicate chart subscription opens the existing chart report for editing', async () => {
+  mockedGetBootstrapData.mockReturnValue(subscriberBootstrapData());
+  fetchMock.post(
+    'glob:*/api/v1/report/subscribe',
+    {
+      status: 409,
+      body: {
+        errors: [{ message: 'Resource already has an attached report.' }],
+      },
+    },
+    { name: 'post-duplicate-chart-subscribe' },
+  );
+  fetchMock.modifyRoute('report-list', {
+    response: {
+      result: [
+        {
+          id: 13,
+          chart_id: 1,
+          creation_method: 'charts',
+          name: 'My existing chart report',
+          crontab: '0 9 * * *',
+          report_format: 'PNG',
+        },
+      ],
+    },
+  });
+  const chartProps = {
+    ...defaultProps,
+    dashboardId: undefined,
+    chart: { id: 1, sliceFormData: { viz_type: VizType.Line } },
+    chartName: 'My Line Chart',
+    creationMethod: 'charts' as const,
+  };
+  const store = createStore({}, reducerIndex);
+  render(<ReportModal {...chartProps} />, { useRedux: true, store });
+
+  await userEvent.click(screen.getByRole('button', { name: 'Add' }));
+
+  expect(await screen.findByText('Edit email report')).toBeInTheDocument();
+  expect(screen.getByTestId('report-name-test')).toHaveDisplayValue(
+    'My existing chart report',
+  );
+  const toasts = (store.getState() as any).messageToasts;
+  expect(
+    toasts.some((toast: { text: string }) =>
+      toast.text.includes('already subscribed to this chart'),
+    ),
+  ).toBe(true);
+
+  fetchMock.modifyRoute('report-list', { response: {} });
+  fetchMock.removeRoute('post-duplicate-chart-subscribe');
+});
+
+test('duplicate subscription shows the error when the existing report cannot be found', async () => {
+  mockedGetBootstrapData.mockReturnValue(subscriberBootstrapData());
+  fetchMock.post(
+    'glob:*/api/v1/report/subscribe',
+    {
+      status: 409,
+      body: {
+        errors: [{ message: 'Resource already has an attached report.' }],
+      },
+    },
+    { name: 'post-duplicate-subscribe-missing' },
+  );
+  const store = createStore({}, reducerIndex);
+  render(<ReportModal {...defaultProps} />, { useRedux: true, store });
+
+  await userEvent.click(screen.getByRole('button', { name: 'Add' }));
+
+  expect(
+    await screen.findByText('Failed to create report'),
+  ).toBeInTheDocument();
+  expect(screen.getByText('Schedule a new email report')).toBeInTheDocument();
+
+  fetchMock.removeRoute('post-duplicate-subscribe-missing');
+});
+
 // ---------------------------------------------------------------------------
 // Error Handling section tests
 // ---------------------------------------------------------------------------

superset-frontend/src/features/reports/ReportModal/actions.ts

@@ -192,7 +192,11 @@ export const subscribeReport =
         dispatch(addSuccessToast(t('The report has been created')));
       })
       .catch(err => {
-        dispatch(addDangerToast(t('Failed to create report')));
+        // A 409 means the user is already subscribed to this chart or
+        // dashboard; the caller opens that subscription instead of failing.
+        if (err?.status !== 409) {
+          dispatch(addDangerToast(t('Failed to create report')));
+        }
         throw err;
       });
 

superset-frontend/src/features/reports/ReportModal/index.tsx

@@ -34,7 +34,7 @@ import {
 } from '@superset-ui/core';
 import { Alert } from '@apache-superset/core/components';
 import { SupersetTheme } from '@apache-superset/core/theme';
-import { useSelector } from 'react-redux';
+import { useSelector, useStore } from 'react-redux';
 import {
   editReport,
   fetchUISpecificReport,
@@ -88,6 +88,7 @@ import {
 interface ReportProps {
   onHide: () => {};
   addDangerToast: (msg: string) => void;
+  addInfoToast: (msg: string) => void;
   show: boolean;
   userEmail: string;
   ccEmail: string;
@@ -123,6 +124,7 @@ const EMPTY_OBJECT = {};
 
 function ReportModal({
   onHide,
+  addInfoToast,
   show = false,
   dashboardId,
   chart,
@@ -178,6 +180,7 @@ function ReportModal({
   const [executeAsSelf, setExecuteAsSelf] = useState(false);
 
   const dispatch = useAppDispatch();
+  const store = useStore();
   // Report fetch logic
   const report = useSelector<any, ReportObject>(state => {
     const isChartReport = creationMethod === CreationMethod.Charts;
@@ -239,6 +242,11 @@ function ReportModal({
       }),
     };
 
+    const resourceId =
+      creationMethod === CreationMethod.Charts ? chart?.id : dashboardId;
+    const filterField =
+      creationMethod === CreationMethod.Charts ? 'chart_id' : 'dashboard_id';
+
     setCurrentReport({ isSubmitting: true, error: undefined });
     try {
       if (isEditMode && currentReport.id) {
@@ -269,26 +277,52 @@ function ReportModal({
         // Subscribe path: creation_method, editors, and recipients are set server-side.
         await dispatch(subscribeReport(commonFields as ReportObject));
       }
-      const resourceId =
-        creationMethod === CreationMethod.Charts ? chart?.id : dashboardId;
       if (
         isFeatureEnabled(FeatureFlag.AlertReportDynamicExecutor) &&
         resourceId != null
       ) {
         await dispatch(
           fetchUISpecificReport({
             userId: currentUserId,
-            filterField:
-              creationMethod === CreationMethod.Charts
-                ? 'chart_id'
-                : 'dashboard_id',
+            filterField,
             creationMethod,
             resourceId,
           }),
         );
       }
       onHide();
     } catch (e) {
+      if (
+        !isEditMode &&
+        (e as Response)?.status === 409 &&
+        resourceId != null &&
+        currentUserId !== undefined
+      ) {
+        // The user already subscribed to this chart or dashboard, e.g. from
+        // another tab. Load that subscription so the modal switches to editing
+        // it instead of reporting a failure.
+        await dispatch(
+          fetchUISpecificReport({
+            userId: currentUserId,
+            filterField,
+            creationMethod,
+            resourceId,
+          }),
+        );
+        if (reportSelector(store.getState(), creationMethod, resourceId)) {
+          addInfoToast(
+            creationMethod === CreationMethod.Charts
+              ? t(
+                  'You are already subscribed to this chart. Your existing report has been opened for editing.',
+                )
+              : t(
+                  'You are already subscribed to this dashboard. Your existing report has been opened for editing.',
+                ),
+          );
+          setCurrentReport({ isSubmitting: false, custom_width: undefined });
+          return;
+        }
+      }
       const { error } = await getClientErrorObject(e);
       setCurrentReport({ error });
     }