diff --git a/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx b/superset/assets/spec/javascripts/components/AlteredSliceTag_spec.jsx index 00ee84f8baa..2b403f5fe61 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 cf22883d342..b4a9ffc1ca5 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