From 43985e7a0be1a7c1a2a8153cc7b2e2c77807f68a Mon Sep 17 00:00:00 2001 From: BrandanBurgess Date: Mon, 26 Oct 2026 19:18:25 -0400 Subject: [PATCH 1/3] fix(reports): open existing subscription instead of failing on duplicate Subscribing to a chart or dashboard the user already has a report for returns a 409 from /api/v1/report/subscribe. The report modal showed a generic "Failed to create report" error. On a 409 it now loads the existing subscription into the modal in edit mode and explains why. Co-Authored-By: Claude Opus 5.5 --- .../reports/ReportModal/ReportModal.test.tsx | 97 +++++++++++++++++++ .../features/reports/ReportModal/actions.ts | 6 +- .../features/reports/ReportModal/index.tsx | 48 +++++++-- 3 files changed, 143 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx index 41385d6b8f6f..5483315ea1bd 100644 --- a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx +++ b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx @@ -894,6 +894,103 @@ 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, + }); + + 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', + ); + 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 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..83a0dad9a59d 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 }); + return; + } + } const { error } = await getClientErrorObject(e); setCurrentReport({ error }); } From 7dd83d1f80a77a0cb4763b7bd3d257b20015ddaa Mon Sep 17 00:00:00 2001 From: BrandanBurgess Date: Thu, 29 Oct 2026 16:57:39 -0400 Subject: [PATCH 2/3] fix(reports): clear custom width when opening the existing subscription The report list endpoint does not return custom_width, so after recovering from a duplicate subscription the Screenshot width field kept the value typed into the abandoned create form, and saving would PUT it onto the existing report. Clear it, matching what opening via "Manage email report" does. Co-Authored-By: Claude Opus 5.5 --- .../features/reports/ReportModal/ReportModal.test.tsx | 11 ++++++++++- .../src/features/reports/ReportModal/index.tsx | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx index 5483315ea1bd..b997f1229e5a 100644 --- a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx +++ b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx @@ -937,17 +937,26 @@ test('duplicate subscription opens the existing report for editing', async () => }); const onHide = jest.fn(); const store = createStore({}, reducerIndex); - render(, { + 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 }) => diff --git a/superset-frontend/src/features/reports/ReportModal/index.tsx b/superset-frontend/src/features/reports/ReportModal/index.tsx index 83a0dad9a59d..7829d4864a67 100644 --- a/superset-frontend/src/features/reports/ReportModal/index.tsx +++ b/superset-frontend/src/features/reports/ReportModal/index.tsx @@ -319,7 +319,7 @@ function ReportModal({ 'You are already subscribed to this dashboard. Your existing report has been opened for editing.', ), ); - setCurrentReport({ isSubmitting: false }); + setCurrentReport({ isSubmitting: false, custom_width: undefined }); return; } } From 3468cd60f46db4b4ee2d1e094bf18968366657b2 Mon Sep 17 00:00:00 2001 From: BrandanBurgess Date: Thu, 29 Oct 2026 19:30:57 -0400 Subject: [PATCH 3/3] test(reports): cover duplicate subscription recovery for charts Co-Authored-By: Claude Opus 5.5 --- .../reports/ReportModal/ReportModal.test.tsx | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx index b997f1229e5a..0adda8ce5ca9 100644 --- a/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx +++ b/superset-frontend/src/features/reports/ReportModal/ReportModal.test.tsx @@ -975,6 +975,59 @@ test('duplicate subscription opens the existing report for editing', async () => 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(