refactor: Arash/new state report (#16987)

* code dry (#16358) * pexdax refactor (#16333) * refactor progress (#16339) * fix: Header Actions test refactor (#16336) * fixed tests * Update index.tsx Co-authored-by: Elizabeth Thompson <[email protected]> * Fetch bug fixed (#16376) * continued refactoring (#16377) * 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 <[email protected]> * 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 <[email protected]> Co-authored-by: Elizabeth Thompson <[email protected]> * refactor(reports): Arash/again refactor reports (#16872) * pexdax refactor (#16333) * refactor progress (#16339) * fix: Header Actions test refactor (#16336) * fixed tests * Update index.tsx Co-authored-by: Elizabeth Thompson <[email protected]> * code dry (#16358) * Fetch bug fixed (#16376) * continued refactoring (#16377) * refactor: Reports - ReportModal (#16622) * refactoring progress * removed consoles * Working, but with 2 fetches * it is still not working Co-authored-by: Lyndsi Kay Williams <[email protected]> Co-authored-by: Elizabeth Thompson <[email protected]> * next changes Co-authored-by: Lyndsi Kay Williams <[email protected]> Co-authored-by: Elizabeth Thompson <[email protected]>
apache · Feb 9, 2022 · 82004da · 82004da
1 parent 4387935
commit 82004da
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 34 deletions.
diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { useState, useEffect, useRef } from 'react';
+import React, { useState, useEffect } from 'react';
 import { useSelector, useDispatch } from 'react-redux';
 import { t, SupersetTheme, css, useTheme } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
@@ -49,19 +49,23 @@ export default function HeaderReportActionsDropDown({
   const reports: Record<number, AlertObject> = useSelector<any, AlertObject>(
     state => state.reports,
   );
+  const report: AlertObject = Object.values(reports).filter(report => {
+    if (dashboardId) {
+      return report.dashboard_id === dashboardId;
+    }
+    return report.chart_id === chart?.id;
+  })[0];
+
   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 dashboardIdRef = useRef(dashboardId);
+  const [showModal, setShowModal] = useState<boolean>(false);
   const toggleActiveKey = async (data: AlertObject, checked: boolean) => {
     if (data?.id) {
       toggleActive(data, checked);
@@ -104,17 +108,13 @@ export default function HeaderReportActionsDropDown({
   }, []);
 
   useEffect(() => {
-    if (
-      canAddReports() &&
-      dashboardId &&
-      dashboardId !== dashboardIdRef.current
-    ) {
+    if (canAddReports()) {
       dispatch(
         fetchUISpecificReport({
           userId: user.userId,
-          filterField: 'dashboard_id',
-          creationMethod: 'dashboards',
-          resourceId: dashboardId,
+          filterField: dashboardId ? 'dashboard_id' : 'chart_id',
+          creationMethod: dashboardId ? 'dashboards' : 'charts',
+          resourceId: dashboardId || chart?.id,
         }),
       );
     }
@@ -148,14 +148,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,9 +25,8 @@ import React, {
   FunctionComponent,
 } from 'react';
 import { t, SupersetTheme } from '@superset-ui/core';
+import { useDispatch, useSelector } from 'react-redux';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
-import { bindActionCreators } from 'redux';
-import { connect, useDispatch, useSelector } from 'react-redux';
 import { addReport, editReport } from 'src/reports/actions/reports';
 import { AlertObject } from 'src/views/CRUD/alert/types';
 
@@ -78,14 +77,14 @@ export interface ReportObject {
 }
 interface ReportProps {
   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 {
@@ -167,7 +166,7 @@ const reportReducer = (
 const ReportModal: FunctionComponent<ReportProps> = ({
   onReportAdd,
   onHide,
-  show = false,
+  showModal = false,
   dashboardId,
   chart,
   userId,
@@ -306,7 +305,7 @@ const ReportModal: FunctionComponent<ReportProps> = ({
 
   return (
     <StyledModal
-      show={show}
+      show={showModal}
       onHide={onHide}
       title={wrappedTitle}
       footer={renderModalFooter}
@@ -394,7 +393,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
@@ -167,13 +167,6 @@ class Header extends React.PureComponent {
     this.startPeriodicRender(refreshFrequency * 1000);
   }
 
-  componentDidUpdate(prevProps) {
-    if (this.props.refreshFrequency !== prevProps.refreshFrequency) {
-      const { refreshFrequency } = this.props;
-      this.startPeriodicRender(refreshFrequency * 1000);
-    }
-  }
-
   UNSAFE_componentWillReceiveProps(nextProps) {
     if (
       UNDO_LIMIT - nextProps.undoLength <= 0 &&
@@ -539,6 +532,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-frontend/src/reports/reducers/reports.js b/superset-frontend/src/reports/reducers/reports.js
@@ -19,10 +19,13 @@
 /* eslint-disable camelcase */
 import { SET_REPORT, ADD_REPORT, EDIT_REPORT } from '../actions/reports';
 
+// Talk about the delete
+
 export default function reportsReducer(state = {}, action) {
   const actionHandlers = {
     [SET_REPORT]() {
       return {
+        ...state,
         ...action.report.result.reduce(
           (obj, report) => ({ ...obj, [report.id]: report }),
           {},

diff --git a/superset-frontend/src/views/CRUD/alert/types.ts b/superset-frontend/src/views/CRUD/alert/types.ts
@@ -62,10 +62,12 @@ export type AlertObject = {
   chart?: MetaObject;
   changed_by?: user;
   changed_on_delta_humanized?: string;
+  chart_id: number;
   created_by?: user;
   created_on?: string;
   crontab?: string;
   dashboard?: MetaObject;
+  dashboard_id?: number;
   database?: MetaObject;
   description?: string;
   force_screenshot: boolean;

diff --git a/superset/reports/api.py b/superset/reports/api.py
@@ -125,12 +125,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",