superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From maximebeauche...@apache.org
Subject [incubator-superset] branch master updated: Improve false negative on AlteredSliceTag (#6578)
Date Tue, 08 Jan 2019 20:23:22 GMT
This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin 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 accc754  Improve false negative on AlteredSliceTag (#6578)
accc754 is described below

commit accc754a87955de1e788205d8fe8562726353e35
Author: Maxime Beauchemin <maximebeauchemin@gmail.com>
AuthorDate: Tue Jan 8 12:23:12 2019 -0800

    Improve false negative on AlteredSliceTag (#6578)
    
    The "altered" tag in the explore view shows up more often than it
    should. By treating null, [] {}, undefined as identical will help reporting
    only the differences that matter.
---
 .../components/AlteredSliceTag_spec.jsx            | 24 ++++++++++++++++++++++
 superset/assets/src/components/AlteredSliceTag.jsx | 24 +++++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx b/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx
index 00ee84f..2b403f5 100644
--- a/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx
+++ b/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx
@@ -284,4 +284,28 @@ describe('AlteredSliceTag', () => {
       expect(wrapper.instance().formatValue(filters, 'adhoc_filters')).toBe(expected);
     });
   });
+  describe('isEqualish', () => {
+    it('considers null, undefined, {} and [] as equal', () => {
+      const inst = wrapper.instance();
+      expect(inst.isEqualish(null, undefined)).toBe(true);
+      expect(inst.isEqualish(null, [])).toBe(true);
+      expect(inst.isEqualish(null, {})).toBe(true);
+      expect(inst.isEqualish(undefined, {})).toBe(true);
+    });
+    it('considers empty strings are the same as null', () => {
+      const inst = wrapper.instance();
+      expect(inst.isEqualish(undefined, '')).toBe(true);
+      expect(inst.isEqualish(null, '')).toBe(true);
+    });
+    it('considers deeply equal objects as equal', () => {
+      const inst = wrapper.instance();
+      expect(inst.isEqualish('', '')).toBe(true);
+      expect(inst.isEqualish({ a: 1, b: 2, c: 3 }, { a: 1, b: 2, c: 3 })).toBe(true);
+      // Out of order
+      expect(inst.isEqualish({ a: 1, b: 2, c: 3 }, { b: 2, a: 1, c: 3 })).toBe(true);
+
+      // Actually  not equal
+      expect(inst.isEqualish({ a: 1, b: 2, z: 9 }, { a: 1, b: 2, c: 3 })).toBe(false);
+    });
+  });
 });
diff --git a/superset/assets/src/components/AlteredSliceTag.jsx b/superset/assets/src/components/AlteredSliceTag.jsx
index cf22883..b4a9ffc 100644
--- a/superset/assets/src/components/AlteredSliceTag.jsx
+++ b/superset/assets/src/components/AlteredSliceTag.jsx
@@ -12,6 +12,24 @@ const propTypes = {
   currentFormData: PropTypes.object.isRequired,
 };
 
+function alterForComparison(value) {
+  // Considering `[]`, `{}`, `null` and `undefined` as identical
+  // for this purpose
+  if (value === undefined || value === null || value === '') {
+    return null;
+  } else if (typeof value === 'object') {
+    if (Array.isArray(value) && value.length === 0) {
+      return null;
+    }
+    const keys = Object.keys(value);
+    if (keys && keys.length === 0) {
+      return null;
+    }
+  }
+  return value;
+}
+
+
 export default class AlteredSliceTag extends React.Component {
 
   constructor(props) {
@@ -45,13 +63,17 @@ export default class AlteredSliceTag extends React.Component {
       if (['filters', 'having', 'having_filters', 'where'].includes(fdKey)) {
         continue;
       }
-      if (!isEqual(ofd[fdKey], cfd[fdKey])) {
+      if (!this.isEqualish(ofd[fdKey], cfd[fdKey])) {
         diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
       }
     }
     return diffs;
   }
 
+  isEqualish(val1, val2) {
+    return isEqual(alterForComparison(val1), alterForComparison(val2));
+  }
+
   formatValue(value, key) {
     // Format display value based on the control type
     // or the value type


Mime
View raw message