diff --git a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx index 41385d6b8f6f..0adda8ce5ca9 100644 --- a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx +++ b/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(, { + 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(, { 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(, { 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 // --------------------------------------------------------------------------- diff --git a/superset-frontend/src/features/reports/ReportModal/actions.ts b/superset-frontend/src/features/reports/ReportModal/actions.ts index 607df3a1aab2..1b8ac6794378 100644 --- a/superset-frontend/src/features/reports/ReportModal/actions.ts +++ b/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; }); diff --git a/superset-frontend/src/features/reports/ReportModal/index.tsx b/superset-frontend/src/features/reports/ReportModal/index.tsx index 9069c9386c47..7829d4864a67 100644 --- a/superset-frontend/src/features/reports/ReportModal/index.tsx +++ b/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(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,8 +277,6 @@ 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 @@ -278,10 +284,7 @@ function ReportModal({ await dispatch( fetchUISpecificReport({ userId: currentUserId, - filterField: - creationMethod === CreationMethod.Charts - ? 'chart_id' - : 'dashboard_id', + filterField, creationMethod, resourceId, }), @@ -289,6 +292,37 @@ function ReportModal({ } 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 }); }