refactor(reports): Arash/refactor reports (#16855)

* pexdax refactor (#16333) * refactor progress (#16339) * fix: Header Actions test refactor (#16336) * fixed tests * Update index.tsx Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> * code dry (#16358) * Fetch bug fixed (#16376) * continued refactoring (#16377) * refactor: Reports - ReportModal (#16622) * refactoring progress * removed consoles * Working, but with 2 fetches * report pickup Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com> Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>
apache · Oct 28, 2021 · 8827a04 · 8827a04
1 parent 5e8bd9d
commit 8827a04
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 35 deletions.
diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
@@ -46,21 +46,23 @@ export default function HeaderReportActionsDropDown({
   chart?: ChartState;
 }) {
   const dispatch = useDispatch();
-  const reports: Record<number, AlertObject> = useSelector<any, AlertObject>(
-    state => state.reports,
+  const reports: any = useSelector<any>(state =>
+    Object.values(state.reports).filter((report: any) =>
+      dashboardId
+        ? report.dashboard_id === dashboardId
+        : report.chart_id === chart?.id,
+    ),
   );
   const user: UserWithPermissionsAndRoles = useSelector<
     any,
     UserWithPermissionsAndRoles
   >(state => state.user || state.explore?.user);
-  const reportsIds = Object.keys(reports || []);
-  const report: AlertObject = reports?.[reportsIds[0]];
   const [
     currentReportDeleting,
     setCurrentReportDeleting,
   ] = useState<AlertObject | null>(null);
   const theme = useTheme();
-  const [showModal, setShowModal] = useState(false);
+  const [showModal, setShowModal] = useState<boolean>(false);
   const toggleActiveKey = async (data: AlertObject, checked: boolean) => {
     if (data?.id) {
       toggleActive(data, checked);
@@ -102,27 +104,14 @@ export default function HeaderReportActionsDropDown({
     }
   }, []);
 
-  useEffect(() => {
-    if (canAddReports()) {
-      dispatch(
-        fetchUISpecificReport({
-          userId: user.userId,
-          filterField: 'dashboard_id',
-          creationMethod: 'dashboards',
-          resourceId: dashboardId,
-        }),
-      );
-    }
-  }, [dashboardId]);
-
   const menu = () => (
     <Menu selectable={false} css={{ width: '200px' }}>
       <Menu.Item>
         {t('Email reports active')}
         <Switch
           data-test="toggle-active"
-          checked={report?.active}
-          onClick={(checked: boolean) => toggleActiveKey(report, checked)}
+          checked={reports?.active}
+          onClick={(checked: boolean) => toggleActiveKey(reports, checked)}
           size="small"
           css={{ marginLeft: theme.gridUnit * 2 }}
         />
@@ -131,7 +120,7 @@ export default function HeaderReportActionsDropDown({
         {t('Edit email report')}
       </Menu.Item>
       <Menu.Item
-        onClick={() => setCurrentReportDeleting(report)}
+        onClick={() => setCurrentReportDeleting(reports)}
         css={deleteColor}
       >
         {t('Delete email report')}
@@ -143,14 +132,14 @@ export default function HeaderReportActionsDropDown({
     canAddReports() && (
       <>
         <ReportModal
-          show={showModal}
-          onHide={() => setShowModal(false)}
           userId={user.userId}
+          showModal={showModal}
+          onHide={() => setShowModal(false)}
           userEmail={user.email}
           dashboardId={dashboardId}
           chart={chart}
         />
-        {report ? (
+        {reports ? (
           <>
             <NoAnimationDropdown
               // ref={ref}

diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx
@@ -25,8 +25,7 @@ import React, {
   FunctionComponent,
 } from 'react';
 import { t, SupersetTheme } from '@superset-ui/core';
-import { bindActionCreators } from 'redux';
-import { connect, useDispatch, useSelector } from 'react-redux';
+import { useDispatch, useSelector } from 'react-redux';
 import { addReport, editReport } from 'src/reports/actions/reports';
 import { AlertObject } from 'src/views/CRUD/alert/types';
 
@@ -77,14 +76,14 @@ interface ReportProps {
   addDangerToast: (msg: string) => void;
   addSuccessToast: (msg: string) => void;
   addReport: (report?: ReportObject) => {};
-  onHide: () => {};
+  onHide: () => void;
   onReportAdd: (report?: ReportObject) => {};
-  show: boolean;
+  showModal: boolean;
   userId: number;
   userEmail: string;
   dashboardId?: number;
   chart?: ChartState;
-  props: any;
+  props?: any;
 }
 
 interface ReportPayloadType {
@@ -154,7 +153,7 @@ const reportReducer = (
 const ReportModal: FunctionComponent<ReportProps> = ({
   onReportAdd,
   onHide,
-  show = false,
+  showModal = false,
   dashboardId,
   chart,
   userId,
@@ -291,7 +290,7 @@ const ReportModal: FunctionComponent<ReportProps> = ({
 
   return (
     <StyledModal
-      show={show}
+      show={showModal}
       onHide={onClose}
       title={wrappedTitle}
       footer={renderModalFooter}
@@ -381,7 +380,4 @@ const ReportModal: FunctionComponent<ReportProps> = ({
   );
 };
 
-const mapDispatchToProps = (dispatch: any) =>
-  bindActionCreators({ addReport, editReport }, dispatch);
-
-export default connect(null, mapDispatchToProps)(withToasts(ReportModal));
+export default withToasts(ReportModal);
diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx
@@ -517,6 +517,7 @@ class Header extends React.PureComponent {
                 </span>
               )}
               <HeaderReportActionsDropdown
+                key={dashboardInfo.id}
                 toggleActive={this.props.toggleActive}
                 deleteActiveReport={this.props.deleteActiveReport}
                 dashboardId={dashboardInfo.id}

diff --git a/superset/reports/api.py b/superset/reports/api.py
@@ -123,12 +123,14 @@ def ensure_alert_reports_enabled(self) -> Optional[Response]:
         "changed_by.last_name",
         "changed_on",
         "changed_on_delta_humanized",
+        "chart_id",
         "created_by.first_name",
         "created_by.last_name",
         "created_on",
         "creation_method",
         "crontab",
         "crontab_humanized",
+        "dashboard_id",
         "description",
         "id",
         "last_eval_dttm",