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 <[email protected]>
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
