mirror of https://github.com/apache/superset.git
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.
This commit is contained in:
parent
49e3638eee
commit
accc754a87
|
@ -284,4 +284,28 @@ describe('AlteredSliceTag', () => {
|
||||||
expect(wrapper.instance().formatValue(filters, 'adhoc_filters')).toBe(expected);
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -12,6 +12,24 @@ const propTypes = {
|
||||||
currentFormData: PropTypes.object.isRequired,
|
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 {
|
export default class AlteredSliceTag extends React.Component {
|
||||||
|
|
||||||
constructor(props) {
|
constructor(props) {
|
||||||
|
@ -45,13 +63,17 @@ export default class AlteredSliceTag extends React.Component {
|
||||||
if (['filters', 'having', 'having_filters', 'where'].includes(fdKey)) {
|
if (['filters', 'having', 'having_filters', 'where'].includes(fdKey)) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (!isEqual(ofd[fdKey], cfd[fdKey])) {
|
if (!this.isEqualish(ofd[fdKey], cfd[fdKey])) {
|
||||||
diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
|
diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] };
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return diffs;
|
return diffs;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
isEqualish(val1, val2) {
|
||||||
|
return isEqual(alterForComparison(val1), alterForComparison(val2));
|
||||||
|
}
|
||||||
|
|
||||||
formatValue(value, key) {
|
formatValue(value, key) {
|
||||||
// Format display value based on the control type
|
// Format display value based on the control type
|
||||||
// or the value type
|
// or the value type
|
||||||
|
|
Loading…
Reference in New Issue