fix(reports): look up fetched report schedules by the list API id fields

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

@@ -16,6 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import fetchMock from 'fetch-mock';
 import { act, render, screen, userEvent } from 'spec/helpers/testing-library';
 import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core';
 import { Menu, MenuItem } from '@superset-ui/core/components/Menu';
@@ -143,6 +144,36 @@ describe('Header Report Dropdown', () => {
     mockedIsFeatureEnabled.mockRestore();
   });
 
+  afterEach(() => {
+    fetchMock.clearHistory().removeRoutes();
+  });
+
+  test('switches to the manage menu once the existing report is fetched', async () => {
+    const mockedProps = createProps();
+    fetchMock.get('glob:*/api/v1/report/?q=*', {
+      count: 1,
+      result: [
+        {
+          active: true,
+          creation_method: 'dashboards',
+          crontab: '0 12 * * 1',
+          dashboard_id: 1,
+          chart_id: null,
+          id: 5,
+          name: 'Weekly Report',
+          report_format: 'PNG',
+          type: 'Report',
+        },
+      ],
+    });
+    act(() => {
+      setup(mockedProps, stateWithOnlyUser);
+    });
+    expect(await screen.findByText('Email reports active')).toBeInTheDocument();
+    expect(screen.getByText('Edit email report')).toBeInTheDocument();
+    expect(screen.getByText('Delete email report')).toBeInTheDocument();
+  });
+
   test('renders correctly', () => {
     const mockedProps = createProps();
     act(() => {

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

@@ -170,10 +170,45 @@ test('creates a new email report via modal Add button', async () => {
   // creation_method, editors, and recipients are set server-side; not in the client payload
   expect(body.creation_method).toBeUndefined();
   expect(body.recipients).toBeUndefined();
+  expect(body.dashboard).toBe(1);
+  expect(body.chart).toBeUndefined();
 
   fetchMock.removeRoute('post-subscribe');
 });
 
+test('creating a chart report in a dashboard context sends only the chart id', async () => {
+  // Explore opened from a dashboard passes a dashboardId alongside the chart;
+  // a payload carrying both ids is rejected by the API with a 422.
+  fetchMock.post(
+    'glob:*/api/v1/report/subscribe',
+    { id: 1, result: {} },
+    { name: 'post-subscribe-chart' },
+  );
+
+  const chartFromDashboardProps = {
+    ...defaultProps,
+    dashboardId: 7,
+    chart: { id: 119, sliceFormData: { viz_type: VizType.Line } },
+    creationMethod: 'charts' as const,
+  };
+  render(<ReportModal {...chartFromDashboardProps} />, { useRedux: true });
+
+  const addButton = screen.getByRole('button', { name: /add/i });
+  await userEvent.click(addButton);
+
+  await waitFor(() => {
+    const postCalls = fetchMock.callHistory.calls('post-subscribe-chart');
+    expect(postCalls).toHaveLength(1);
+  });
+
+  const postCalls = fetchMock.callHistory.calls('post-subscribe-chart');
+  const body = JSON.parse(postCalls[0].options.body as string);
+  expect(body.chart).toBe(119);
+  expect(body.dashboard).toBeUndefined();
+
+  fetchMock.removeRoute('post-subscribe-chart');
+});
+
 test('text-based chart hides screenshot width and shows message content', () => {
   // Table is text-based: should show message content but hide custom width
   const textChartProps = {
@@ -288,6 +323,76 @@ test('renders edit mode when report exists in store', () => {
   expect(screen.getByRole('button', { name: /save/i })).toBeInTheDocument();
 });
 
+test('edit mode in a dashboard context resolves and saves the chart report', async () => {
+  // With both schedules in the store, the modal must load the chart's own
+  // report, not the dashboard's, and PUT to its id with only the chart set.
+  const dashboardReport = {
+    id: 42,
+    name: 'Existing Dashboard Report',
+    creation_method: 'dashboards',
+    crontab: '0 12 * * 1',
+    dashboard_id: 7,
+    chart_id: null,
+    report_format: 'PNG',
+    active: true,
+    type: 'Report',
+    timezone: 'America/New_York',
+  };
+  const chartReport = {
+    ...dashboardReport,
+    id: 77,
+    name: 'Existing Chart Report',
+    creation_method: 'charts',
+    dashboard_id: null,
+    chart_id: 119,
+  };
+  const store = createStore(
+    {
+      reports: {
+        dashboards: { 7: dashboardReport },
+        charts: { 119: chartReport },
+      },
+    },
+    reducerIndex,
+  );
+
+  fetchMock.put(
+    'glob:*/api/v1/report/77',
+    { id: 77, result: {} },
+    { name: 'put-report-77' },
+  );
+
+  const chartFromDashboardProps = {
+    ...defaultProps,
+    dashboardId: 7,
+    chart: { id: 119, sliceFormData: { viz_type: VizType.Line } },
+    creationMethod: 'charts' as const,
+  };
+  render(<ReportModal {...chartFromDashboardProps} />, {
+    useRedux: true,
+    store,
+  });
+
+  expect(screen.getByTestId('report-name-test')).toHaveDisplayValue(
+    'Existing Chart Report',
+  );
+
+  const saveButton = screen.getByRole('button', { name: /save/i });
+  await userEvent.click(saveButton);
+
+  await waitFor(() => {
+    const calls = fetchMock.callHistory.calls('put-report-77');
+    expect(calls.length).toBeGreaterThan(0);
+  });
+
+  const calls = fetchMock.callHistory.calls('put-report-77');
+  const body = JSON.parse(calls[calls.length - 1].options.body as string);
+  expect(body.chart).toBe(119);
+  expect(body.dashboard).toBeUndefined();
+
+  fetchMock.removeRoute('put-report-77');
+});
+
 test('edit mode dispatches editReport via PUT on save', async () => {
   const existingReport = {
     id: 42,
@@ -352,6 +457,67 @@ test('edit mode dispatches editReport via PUT on save', async () => {
   fetchMock.removeRoute('put-report-42');
 });
 
+test('edit mode preserves the fetched report_format on save', async () => {
+  // XLSX is never the visualization default, so this fails if the stored
+  // format is dropped and the save falls back to defaultNotificationFormat.
+  const fetchedReport = {
+    active: true,
+    chart_id: 96,
+    creation_method: 'charts',
+    crontab: '0 12 * * 1',
+    dashboard_id: null,
+    description: '',
+    id: 44,
+    name: 'Existing XLSX Report',
+    report_format: 'XLSX',
+    timezone: 'America/New_York',
+    type: 'Report',
+  };
+  const store = createStore(
+    {
+      reports: {
+        charts: { 96: fetchedReport },
+      },
+    },
+    reducerIndex,
+  );
+
+  fetchMock.put(
+    'glob:*/api/v1/report/44',
+    { id: 44, result: {} },
+    {
+      name: 'put-report-44',
+    },
+  );
+
+  const chartProps = {
+    ...defaultProps,
+    dashboardId: undefined,
+    chart: { id: 96, sliceFormData: { viz_type: VizType.Line } },
+    creationMethod: 'charts' as const,
+  };
+  render(<ReportModal {...chartProps} />, { useRedux: true, store });
+
+  expect(screen.getByText('Edit email report')).toBeInTheDocument();
+  expect(
+    screen.getByRole('radio', { name: 'Formatted Excel attached in email' }),
+  ).toBeChecked();
+
+  const saveButton = screen.getByRole('button', { name: /save/i });
+  await userEvent.click(saveButton);
+
+  await waitFor(() => {
+    const calls = fetchMock.callHistory.calls('put-report-44');
+    expect(calls.length).toBeGreaterThan(0);
+  });
+
+  const calls = fetchMock.callHistory.calls('put-report-44');
+  const body = JSON.parse(calls[calls.length - 1].options.body as string);
+  expect(body.report_format).toBe('XLSX');
+
+  fetchMock.removeRoute('put-report-44');
+});
+
 test('edit mode does not fall back to user id when subject id is unavailable', async () => {
   mockedGetBootstrapData.mockReturnValue(
     bootstrapData({

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

@@ -252,6 +252,8 @@ export interface DeletableReport {
   creation_method?: string;
   dashboard?: number;
   chart?: number;
+  dashboard_id?: number | null;
+  chart_id?: number | null;
 }
 
 export interface DeleteReportAction {

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

@@ -170,12 +170,13 @@ function ReportModal({
   const dispatch = useDispatch();
   // Report fetch logic
   const report = useSelector<any, ReportObject>(state => {
-    const resourceType = dashboardId
-      ? CreationMethod.Dashboards
-      : CreationMethod.Charts;
+    const isChartReport = creationMethod === CreationMethod.Charts;
     return (
-      reportSelector(state, resourceType, dashboardId || chart?.id) ||
-      EMPTY_OBJECT
+      reportSelector(
+        state,
+        isChartReport ? CreationMethod.Charts : CreationMethod.Dashboards,
+        isChartReport ? chart?.id : dashboardId,
+      ) || EMPTY_OBJECT
     );
   });
   const isEditMode = report && Object.keys(report).length;
@@ -194,8 +195,9 @@ function ReportModal({
       active: true,
       force_screenshot: false,
       custom_width: currentReport.custom_width,
-      dashboard: dashboardId,
-      chart: chart?.id,
+      ...(creationMethod === CreationMethod.Charts
+        ? { chart: chart?.id }
+        : { dashboard: dashboardId }),
       name: currentReport.name,
       description: currentReport.description,
       crontab: currentReport.crontab,

superset-frontend/src/features/reports/ReportModal/reducer.test.ts

@@ -53,7 +53,7 @@ const makeReport = (overrides: Partial<ReportObject> = {}): ReportObject => ({
 });
 
 test('SET_REPORT stores report keyed by resourceId under creationMethod', () => {
-  const report = makeReport({ id: 5, dashboard: 10 });
+  const report = makeReport({ id: 5, dashboard_id: 10, chart_id: null });
   const action: SetReportAction = {
     type: SET_REPORT,
     report: { result: [report] },
@@ -84,10 +84,11 @@ test('SET_REPORT removes entry when API returns empty result', () => {
   expect(result.dashboards?.[10]).toBeUndefined();
 });
 
-test('SET_REPORT uses chart property when filterField is chart_id', () => {
+test('SET_REPORT matches on chart_id when filterField is chart_id', () => {
   const report = makeReport({
     id: 7,
-    chart: 42,
+    chart_id: 42,
+    dashboard_id: null,
     creation_method: 'charts',
   });
   const action: SetReportAction = {
@@ -103,6 +104,23 @@ test('SET_REPORT uses chart property when filterField is chart_id', () => {
   expect(result.charts?.[42]).toEqual(report);
 });
 
+test('SET_REPORT keeps an existing report when the fetched payload matches it', () => {
+  const existing = makeReport({ id: 5, dashboard_id: 10 });
+  const initial: ReportsState = { dashboards: { 10: existing } };
+  const refetched = makeReport({ id: 5, dashboard_id: 10, active: false });
+  const action: SetReportAction = {
+    type: SET_REPORT,
+    report: { result: [refetched] },
+    resourceId: 10,
+    creationMethod: 'dashboards',
+    filterField: 'dashboard_id',
+  };
+
+  const result = reportsReducer(initial, action);
+
+  expect(result.dashboards?.[10]).toEqual(refetched);
+});
+
 test('ADD_REPORT keys dashboard report by dashboard id', () => {
   const action: AddReportAction = {
     type: ADD_REPORT,
@@ -191,6 +209,64 @@ test('DELETE_REPORT removes report from state', () => {
   expect(result.dashboards?.[10]).toBeUndefined();
 });
 
+test('DELETE_REPORT removes a report fetched from the list API', () => {
+  const report = makeReport({
+    id: 5,
+    dashboard_id: 10,
+    creation_method: 'dashboards',
+  });
+  const initial: ReportsState = { dashboards: { 10: report } };
+  const action: DeleteReportAction = {
+    type: DELETE_REPORT,
+    report: {
+      id: 5,
+      dashboard_id: 10,
+      creation_method: 'dashboards',
+    },
+  };
+
+  const result = reportsReducer(initial, action);
+
+  expect(result.dashboards?.[10]).toBeUndefined();
+});
+
+test('DELETE_REPORT keys a list API chart report by chart_id', () => {
+  const report = makeReport({
+    id: 7,
+    dashboard_id: null,
+    chart_id: 42,
+    creation_method: 'charts',
+  });
+  const initial: ReportsState = { charts: { 42: report } };
+  const action: DeleteReportAction = {
+    type: DELETE_REPORT,
+    report: {
+      id: 7,
+      dashboard_id: null,
+      chart_id: 42,
+      creation_method: 'charts',
+    },
+  };
+
+  const result = reportsReducer(initial, action);
+
+  expect(result.charts?.[42]).toBeUndefined();
+});
+
+test('DELETE_REPORT returns unchanged state when no resource id is present', () => {
+  const initial: ReportsState = {
+    dashboards: { 10: makeReport({ id: 5, dashboard_id: 10 }) },
+  };
+  const action: DeleteReportAction = {
+    type: DELETE_REPORT,
+    report: { id: 5, creation_method: 'dashboards' },
+  };
+
+  const result = reportsReducer(initial, action);
+
+  expect(result).toBe(initial);
+});
+
 test('DELETE_REPORT for alerts_reports keys by report id', () => {
   const report = makeReport({ id: 99, creation_method: 'alerts_reports' });
   const initial: ReportsState = { alerts_reports: { 99: report } };