fix(explore): Metric control breaks when saved metric deleted from da…

…taset (#17503)

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,

superset-frontend/src/components/ReportModal/index.tsx

@@ -83,7 +83,11 @@ interface ReportProps {
   userEmail: string;
   dashboardId?: number;
   chart?: ChartState;
+<<<<<<< HEAD
   props?: any;
+=======
+  props: any;
+>>>>>>> be2e1ecf6... code dry (#16358)
 }
 
 interface ReportPayloadType {

superset-frontend/src/dashboard/components/Header/index.jsx

@@ -168,6 +168,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 &&

superset-frontend/src/explore/components/ExploreChartHeader/index.jsx

@@ -20,7 +20,12 @@ import React from 'react';
 import { connect } from 'react-redux';
 import { bindActionCreators } from 'redux';
 import PropTypes from 'prop-types';
-import { styled, t } from '@superset-ui/core';
+import {
+  styled,
+  t,
+  SupersetClient,
+  CategoricalColorNamespace,
+} from '@superset-ui/core';
 import { Tooltip } from 'src/components/Tooltip';
 import { toggleActive, deleteActiveReport } from 'src/reports/actions/reports';
 import HeaderReportActionsDropdown from 'src/components/ReportModal/HeaderReportActionsDropdown';
@@ -105,6 +110,38 @@ export class ExploreChartHeader extends React.PureComponent {
     };
     this.openPropertiesModal = this.openPropertiesModal.bind(this);
     this.closePropertiesModal = this.closePropertiesModal.bind(this);
+    this.fetchChartDashboardData = this.fetchChartDashboardData.bind(this);
+  }
+
+  componentDidMount() {
+    const { dashboardId } = this.props;
+    if (dashboardId) {
+      this.fetchChartDashboardData();
+    }
+  }
+
+  async fetchChartDashboardData() {
+    const { dashboardId, slice } = this.props;
+    const response = await SupersetClient.get({
+      endpoint: `/api/v1/chart/${slice.slice_id}`,
+    });
+    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();
+
+      Object.keys(labelColors).forEach(label => {
+        categoricalNamespace.setColor(label, labelColors[label]);
+      });
+    }
   }
 
   getSliceName() {

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx

@@ -120,6 +120,62 @@ test('remove selected custom metric when metric gets removed from dataset', () =
   expect(screen.getByText('SUM(Column B)')).toBeVisible();
 });
 
+test('remove selected custom metric when metric gets removed from dataset for single-select metric control', () => {
+  let metricValue = 'metric_b';
+
+  const onChange = (val: any) => {
+    metricValue = val;
+  };
+
+  const { rerender } = render(
+    <DndMetricSelect
+      {...defaultProps}
+      value={metricValue}
+      onChange={onChange}
+      multi={false}
+    />,
+    {
+      useDnd: true,
+    },
+  );
+
+  expect(screen.getByText('Metric B')).toBeVisible();
+  expect(
+    screen.queryByText('Drop column or metric here'),
+  ).not.toBeInTheDocument();
+
+  const newPropsWithRemovedMetric = {
+    ...defaultProps,
+    savedMetrics: [
+      {
+        metric_name: 'metric_a',
+        expression: 'expression_a',
+      },
+    ],
+  };
+
+  // rerender twice - first to update columns, second to update value
+  rerender(
+    <DndMetricSelect
+      {...newPropsWithRemovedMetric}
+      value={metricValue}
+      onChange={onChange}
+      multi={false}
+    />,
+  );
+  rerender(
+    <DndMetricSelect
+      {...newPropsWithRemovedMetric}
+      value={metricValue}
+      onChange={onChange}
+      multi={false}
+    />,
+  );
+
+  expect(screen.queryByText('Metric B')).not.toBeInTheDocument();
+  expect(screen.getByText('Drop column or metric here')).toBeVisible();
+});
+
 test('remove selected adhoc metric when column gets removed from dataset', async () => {
   let metricValues = ['metric_a', 'metric_b', adhocMetricA, adhocMetricB];
   const onChange = (val: any[]) => {

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx

@@ -87,7 +87,7 @@ const getMetricsMatchingCurrentDataset = (
   savedMetrics: (savedMetricType | Metric)[],
   prevColumns: ColumnMeta[],
   prevSavedMetrics: (savedMetricType | Metric)[],
-) => {
+): ValueType[] => {
   const areSavedMetricsEqual =
     !prevSavedMetrics || isEqual(prevSavedMetrics, savedMetrics);
   const areColsEqual = !prevColumns || isEqual(prevColumns, columns);
@@ -96,16 +96,17 @@ const getMetricsMatchingCurrentDataset = (
     return values;
   }
   return values.reduce((acc: ValueType[], metric) => {
-    if (
-      (typeof metric === 'string' || (metric as Metric).metric_name) &&
-      (areSavedMetricsEqual ||
+    if (typeof metric === 'string' || (metric as Metric).metric_name) {
+      if (
+        areSavedMetricsEqual ||
         savedMetrics?.some(
           savedMetric =>
             savedMetric.metric_name === metric ||
             savedMetric.metric_name === (metric as Metric).metric_name,
-        ))
-    ) {
-      acc.push(metric);
+        )
+      ) {
+        acc.push(metric);
+      }
       return acc;
     }
 

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,
-        },
       };
     },