[superset] branch master updated: fix(explore): Metric control breaks when saved metric deleted from dataset (#17503)

[kg...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 7353a2b  fix(explore): Metric control breaks when saved metric deleted from dataset (#17503)
7353a2b is described below

commit 7353a2bd75ae5c76458615443f530a688e78db1c
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)
---
 .../DndMetricSelect.test.tsx                       | 56 ++++++++++++++++++++++
 .../DndColumnSelectControl/DndMetricSelect.tsx     | 15 +++---
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
index b859aa0..ae6a8b3 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
+++ b/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[]) => {
diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
index 1936eb8..2e7dc57 100644
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
+++ b/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;
     }