[incubator-superset] branch master updated: Avoid removing custom sql adhoc metric when columns change (#7877)

[michellet]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 86fdceb  Avoid removing custom sql adhoc metric when columns change 
(#7877)
86fdceb is described below

commit 86fdceb2361ab564646ce477ac02b89633c38c69
Author: michellethomas <michelle.q.tho...@gmail.com>
AuthorDate: Tue Jul 16 16:23:12 2019 -0700

    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
---
 .../explore/components/MetricsControl_spec.jsx     | 35 ++++++++++++++++++++++
 .../explore/components/controls/MetricsControl.jsx |  2 +-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git 
a/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx 
b/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx
index 31bddf8..143bf8f 100644
--- 
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
index 1e49355..b079d21 100644
--- 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));
 }