You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2021/11/24 18:46:47 UTC

[superset] 13/14: refactor: Arash/new state report (#16987)

This is an automated email from the ASF dual-hosted git repository.

elizabeth pushed a commit to branch refactorReports
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 6151b5532d0df6e2d03c2b3fadc3b93ba2463ba2
Author: AAfghahi <48...@users.noreply.github.com>
AuthorDate: Thu Oct 28 16:39:14 2021 -0400

    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 <es...@gmail.com>
    
    * 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 <es...@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 <55...@users.noreply.github.com>
    Co-authored-by: Elizabeth Thompson <es...@gmail.com>
    
    * 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 <es...@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
    
    * it is still not working
    
    Co-authored-by: Lyndsi Kay Williams <55...@users.noreply.github.com>
    Co-authored-by: Elizabeth Thompson <es...@gmail.com>
    
    * next changes
    
    Co-authored-by: Lyndsi Kay Williams <55...@users.noreply.github.com>
    Co-authored-by: Elizabeth Thompson <es...@gmail.com>
---
 .../HeaderReportActionsDropdown/index.tsx          | 32 +++++++++++-----------
 .../src/components/ReportModal/index.tsx           | 18 +++++-------
 .../src/dashboard/components/Header/index.jsx      |  8 +-----
 superset-frontend/src/reports/reducers/reports.js  |  3 ++
 superset-frontend/src/views/CRUD/alert/types.ts    |  2 ++
 superset/reports/api.py                            |  2 ++
 6 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
index 40a7c35..7beb61a 100644
--- 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
index 20fecdc..6c39293 100644
--- 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
index 308b147..1ddf4a1 100644
--- a/superset-frontend/src/dashboard/components/Header/index.jsx
+++ b/superset-frontend/src/dashboard/components/Header/index.jsx
@@ -183,13 +183,6 @@ class Header extends React.PureComponent {
 >>>>>>> code dry (#16358)
   }
 
-  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 &&
@@ -593,6 +586,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
index 8b582d0..54cf493 100644
--- 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
index 1c40cdb..cbf9da0 100644
--- 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;
   grace_period?: number;
diff --git a/superset/reports/api.py b/superset/reports/api.py
index 1a5907f..410f663 100644
--- a/superset/reports/api.py
+++ b/superset/reports/api.py
@@ -123,12 +123,14 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
         "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",