Avoid removing custom sql adhoc metric when columns change (#7877)

* Avoid removing custom sql adhoc metric when columns change * Add tests to confirm sql metric does not get removed
apache · Jul 16, 2019 · 86fdceb · 86fdceb
1 parent a36c136
commit 86fdceb
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 1 deletion.
diff --git a/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx b/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx
@@ -333,5 +333,40 @@ describe('MetricsControl', () => {
         'SUM(',
       )).toBe(true);
     });
+
+    it('Removes metrics if savedMetrics changes', () => {
+      const { props, wrapper, onChange } = setup({
+        value: [
+          {
+            expressionType: EXPRESSION_TYPES.SIMPLE,
+            column: { type: 'double', column_name: 'value' },
+            aggregate: AGGREGATES.SUM,
+            label: 'SUM(value)',
+            optionName: 'blahblahblah',
+          },
+        ],
+      });
+      expect(wrapper.state('value')).toHaveLength(1);
+
+      wrapper.setProps({ ...props, columns: [] });
+      expect(onChange.lastCall.args).toEqual([[]]);
+    });
+
+    it('Does not remove custom sql metric if savedMetrics changes', () => {
+      const { props, wrapper, onChange } = setup({
+        value: [
+          {
+            expressionType: EXPRESSION_TYPES.SQL,
+            sqlExpression: 'COUNT(*)',
+            label: 'old label',
+            hasCustomLabel: true,
+          },
+        ],
+      });
+      expect(wrapper.state('value')).toHaveLength(1);
+
+      wrapper.setProps({ ...props, columns: [] });
+      expect(onChange.calledOnce).toEqual(false);
+    });
   });
 });
diff --git a/superset/assets/src/explore/components/controls/MetricsControl.jsx b/superset/assets/src/explore/components/controls/MetricsControl.jsx
@@ -71,7 +71,7 @@ function columnsContainAllMetrics(value, nextProps) {
     .filter(metric => metric)
     // find column names
     .map(metric => metric.column ? metric.column.column_name : metric.column_name || metric)
-    .filter(name => name)
+    .filter(name => name && typeof name === 'string')
     .every(name => columnNames.has(name));
 }