fix: Disables email reports for unsaved charts (apache#23588)

(cherry picked from commit 290920c)
airbnb · Apr 7, 2023 · 0c34f29 · 0c34f29
1 parent 27773db
commit 0c34f29
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 31 deletions.
diff --git a/superset-frontend/src/components/ReportModal/HeaderReportDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportDropdown/index.tsx
@@ -19,6 +19,7 @@
 import React, { useState, useEffect } from 'react';
 import { usePrevious } from 'src/hooks/usePrevious';
 import { useSelector, useDispatch } from 'react-redux';
+import { isEmpty } from 'lodash';
 import {
   t,
   SupersetTheme,
@@ -103,6 +104,9 @@ export interface HeaderReportProps {
   showReportSubMenu?: boolean;
 }
 
+// Same instance to be used in useEffects
+const EMPTY_OBJECT = {};
+
 export default function HeaderReportDropDown({
   dashboardId,
   chart,
@@ -116,7 +120,10 @@ export default function HeaderReportDropDown({
     const resourceType = dashboardId
       ? CreationMethod.DASHBOARDS
       : CreationMethod.CHARTS;
-    return reportSelector(state, resourceType, dashboardId || chart?.id);
+    return (
+      reportSelector(state, resourceType, dashboardId || chart?.id) ||
+      EMPTY_OBJECT
+    );
   });
 
   const isReportActive: boolean = report?.active || false;
@@ -133,6 +140,12 @@ export default function HeaderReportDropDown({
       // this is in the case that there is an anonymous user.
       return false;
     }
+
+    // Cannot add reports if the resource is not saved
+    if (!(dashboardId || chart?.id)) {
+      return false;
+    }
+
     const roles = Object.keys(user.roles || []);
     const permissions = roles.map(key =>
       user.roles[key].filter(
@@ -200,7 +213,21 @@ export default function HeaderReportDropDown({
   };
 
   const textMenu = () =>
-    report ? (
+    isEmpty(report) ? (
+      <Menu selectable={false} css={onMenuHover}>
+        <Menu.Item onClick={handleShowMenu}>
+          {DropdownItemExtension ? (
+            <StyledDropdownItemWithIcon>
+              <div>{t('Set up an email report')}</div>
+              <DropdownItemExtension />
+            </StyledDropdownItemWithIcon>
+          ) : (
+            t('Set up an email report')
+          )}
+        </Menu.Item>
+        <Menu.Divider />
+      </Menu>
+    ) : (
       isDropdownVisible && (
         <Menu selectable={false} css={{ border: 'none' }}>
           <Menu.Item
@@ -220,20 +247,6 @@ export default function HeaderReportDropDown({
           </Menu.Item>
         </Menu>
       )
-    ) : (
-      <Menu selectable={false} css={onMenuHover}>
-        <Menu.Item onClick={handleShowMenu}>
-          {DropdownItemExtension ? (
-            <StyledDropdownItemWithIcon>
-              <div>{t('Set up an email report')}</div>
-              <DropdownItemExtension />
-            </StyledDropdownItemWithIcon>
-          ) : (
-            t('Set up an email report')
-          )}
-        </Menu.Item>
-        <Menu.Divider />
-      </Menu>
     );
   const menu = () => (
     <Menu selectable={false} css={{ width: '200px' }}>
@@ -260,7 +273,17 @@ export default function HeaderReportDropDown({
   );
 
   const iconMenu = () =>
-    report ? (
+    isEmpty(report) ? (
+      <span
+        role="button"
+        title={t('Schedule email report')}
+        tabIndex={0}
+        className="action-button action-schedule-report"
+        onClick={() => setShowModal(true)}
+      >
+        <Icons.Calendar />
+      </span>
+    ) : (
       <>
         <NoAnimationDropdown
           overlay={menu()}
@@ -278,16 +301,6 @@ export default function HeaderReportDropDown({
           </span>
         </NoAnimationDropdown>
       </>
-    ) : (
-      <span
-        role="button"
-        title={t('Schedule email report')}
-        tabIndex={0}
-        className="action-button action-schedule-report"
-        onClick={() => setShowModal(true)}
-      >
-        <Icons.Calendar />
-      </span>
     );
 
   return (

diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx
@@ -93,6 +93,9 @@ type ReportObjectState = Partial<ReportObject> & {
   isSubmitting?: boolean;
 };
 
+// Same instance to be used in useEffects
+const EMPTY_OBJECT = {};
+
 function ReportModal({
   onHide,
   show = false,
@@ -147,7 +150,10 @@ function ReportModal({
     const resourceType = dashboardId
       ? CreationMethod.DASHBOARDS
       : CreationMethod.CHARTS;
-    return reportSelector(state, resourceType, dashboardId || chart?.id);
+    return (
+      reportSelector(state, resourceType, dashboardId || chart?.id) ||
+      EMPTY_OBJECT
+    );
   });
   const isEditMode = report && Object.keys(report).length;
 

diff --git a/superset-frontend/src/reports/actions/reports.js b/superset-frontend/src/reports/actions/reports.js
@@ -143,6 +143,8 @@ export function toggleActive(report, isActive) {
   };
 }
 
+export const DELETE_REPORT = 'DELETE_REPORT';
+
 export function deleteActiveReport(report) {
   return function deleteActiveReportThunk(dispatch) {
     return SupersetClient.delete({
@@ -152,7 +154,7 @@ export function deleteActiveReport(report) {
         dispatch(addDangerToast(t('Your report could not be deleted')));
       })
       .finally(() => {
-        dispatch(structureFetchAction);
+        dispatch({ type: DELETE_REPORT, report });
         dispatch(addSuccessToast(t('Deleted: %s', report.name)));
       });
   };

diff --git a/superset-frontend/src/reports/reducers/reports.js b/superset-frontend/src/reports/reducers/reports.js
@@ -18,7 +18,13 @@
  */
 /* eslint-disable camelcase */
 // eslint-disable-next-line import/no-extraneous-dependencies
-import { SET_REPORT, ADD_REPORT, EDIT_REPORT } from '../actions/reports';
+import { omit } from 'lodash';
+import {
+  SET_REPORT,
+  ADD_REPORT,
+  EDIT_REPORT,
+  DELETE_REPORT,
+} from '../actions/reports';
 
 export default function reportsReducer(state = {}, action) {
   const actionHandlers = {
@@ -78,6 +84,17 @@ export default function reportsReducer(state = {}, action) {
         },
       };
     },
+
+    [DELETE_REPORT]() {
+      const { report } = action;
+      const reportTypeId = report.dashboard || report.chart;
+      return {
+        ...state,
+        [report.creation_method]: {
+          ...omit(state[report.creation_method], reportTypeId),
+        },
+      };
+    },
   };
 
   if (action.type in actionHandlers) {

diff --git a/superset-frontend/src/views/CRUD/hooks.ts b/superset-frontend/src/views/CRUD/hooks.ts
@@ -870,5 +870,5 @@ export const reportSelector = (
   if (resourceId) {
     return state.reports[resourceType]?.[resourceId];
   }
-  return {};
+  return null;
 };