You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by aa...@apache.org on 2022/02/09 23:03:25 UTC

[superset] 17/18: fix(explore): Metric control breaks when saved metric deleted from dataset (#17503)

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

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

commit 29081699d61fa358d01f4db5074fe900aea2f9fc
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Wed Nov 24 13:06:11 2021 +0100

    fix(explore): Metric control breaks when saved metric deleted from dataset (#17503)
---
 .../HeaderReportActionsDropdown/index.tsx          | 20 +++++++----
 .../src/components/ReportModal/index.tsx           |  4 +++
 .../src/dashboard/components/Header/index.jsx      |  7 ++++
 .../components/ExploreChartHeader/index.jsx        | 39 ++++++++++------------
 superset-frontend/src/reports/reducers/reports.js  | 20 ++++++-----
 5 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
index f558010..b9de984 100644
--- a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
+++ b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
@@ -46,15 +46,21 @@ export default function HeaderReportActionsDropDown({
   chart?: ChartState;
 }) {
   const dispatch = useDispatch();
-  const reports: Record<number, AlertObject> = useSelector<any, AlertObject>(
-    state => state.reports,
-  );
-  const report: AlertObject = Object.values(reports).filter(report => {
+  const report: AlertObject = useSelector<any, AlertObject>(state => {
     if (dashboardId) {
-      return report.dashboard_id === dashboardId;
+      return state.reports.dashboards?.[dashboardId];
+    }
+    if (chart?.id) {
+      return state.reports.charts?.[chart.id];
     }
-    return report.chart_id === chart?.id;
-  })[0];
+    return {};
+  });
+  // const report: ReportObject = Object.values(reports).filter(report => {
+  //   if (dashboardId) {
+  //     return report.dashboards?.[dashboardId];
+  //   }
+  //   // return report.charts?.[chart?.id]
+  // })[0];
 
   const user: UserWithPermissionsAndRoles = useSelector<
     any,
diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx
index 33feb11..79f471c 100644
--- a/superset-frontend/src/components/ReportModal/index.tsx
+++ b/superset-frontend/src/components/ReportModal/index.tsx
@@ -84,7 +84,11 @@ interface ReportProps {
   userEmail: string;
   dashboardId?: number;
   chart?: ChartState;
+<<<<<<< HEAD
   props?: any;
+=======
+  props: any;
+>>>>>>> be2e1ecf6... code dry (#16358)
 }
 
 interface ReportPayloadType {
diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx
index 57e61d1..a1cdf01 100644
--- a/superset-frontend/src/dashboard/components/Header/index.jsx
+++ b/superset-frontend/src/dashboard/components/Header/index.jsx
@@ -167,6 +167,13 @@ 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 &&
diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
index 1244635..3e5aa4d 100644
--- a/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartHeader/index.jsx
@@ -123,31 +123,26 @@ export class ExploreChartHeader extends React.PureComponent {
 
   async fetchChartDashboardData() {
     const { dashboardId, slice } = this.props;
-    await SupersetClient.get({
+    const response = await SupersetClient.get({
       endpoint: `/api/v1/chart/${slice.slice_id}`,
-    })
-      .then(res => {
-        const response = res?.json?.result;
-        if (response && response.dashboards && response.dashboards.length) {
-          const { dashboards } = response;
-          const dashboard =
-            dashboardId &&
-            dashboards.length &&
-            dashboards.find(d => d.id === dashboardId);
+    });
+    const chart = response.json.result;
+    const dashboards = chart.dashboards || [];
+    const dashboard =
+      dashboardId &&
+      dashboards.length &&
+      dashboards.find(d => d.id === dashboardId);
 
-          if (dashboard && dashboard.json_metadata) {
-            // setting the chart to use the dashboard custom label colors if any
-            const labelColors =
-              JSON.parse(dashboard.json_metadata).label_colors || {};
-            const categoricalNamespace = CategoricalColorNamespace.getNamespace();
+    if (dashboard && dashboard.json_metadata) {
+      // setting the chart to use the dashboard custom label colors if any
+      const labelColors =
+        JSON.parse(dashboard.json_metadata).label_colors || {};
+      const categoricalNamespace = CategoricalColorNamespace.getNamespace();
 
-            Object.keys(labelColors).forEach(label => {
-              categoricalNamespace.setColor(label, labelColors[label]);
-            });
-          }
-        }
-      })
-      .catch(() => {});
+      Object.keys(labelColors).forEach(label => {
+        categoricalNamespace.setColor(label, labelColors[label]);
+      });
+    }
   }
 
   getSliceName() {
diff --git a/superset-frontend/src/reports/reducers/reports.js b/superset-frontend/src/reports/reducers/reports.js
index de23f57..a18d72e 100644
--- a/superset-frontend/src/reports/reducers/reports.js
+++ b/superset-frontend/src/reports/reducers/reports.js
@@ -43,14 +43,13 @@ export default function reportsReducer(state = {}, action) {
     [SET_REPORT]() {
       // Grabs the first report with a dashboard id that
       // matches the parameter report's dashboard_id
-      const reportWithDashboard = action.report.result.find(
+      const reportWithDashboard = action.report.result?.find(
         report => !!report.dashboard_id,
       );
-
       // Grabs the first report with a chart id that
       // matches the parameter report's chart.id
-      const reportWithChart = action.report.result.find(
-        report => !!report.chart.id,
+      const reportWithChart = action.report.result?.find(
+        report => !!report.chart?.id,
       );
 
       // This organizes report by its type, dashboard or chart
@@ -64,12 +63,17 @@ export default function reportsReducer(state = {}, action) {
           },
         };
       }
+      if (reportWithChart) {
+        return {
+          ...state,
+          charts: {
+            ...state.chart,
+            [reportWithChart.chart.id]: reportWithChart,
+          },
+        };
+      }
       return {
         ...state,
-        charts: {
-          ...state.chart,
-          [reportWithChart.chart.id]: reportWithChart,
-        },
       };
     },