fix: [dashboard] should not trigger chart refresh when filter not applicable (#9891)

* fix: [dashboard][filter] should not trigger chart refresh when filter is not applicable

* fix comments
This commit is contained in:
Grace Guo 2020-05-26 10:14:12 -07:00 committed by GitHub
parent 7f6dbf838e
commit e6a55d8858
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 102 additions and 44 deletions

View File

@ -62,10 +62,10 @@ describe('Dashboard', () => {
// activeFilters map use id_column) as key
const OVERRIDE_FILTERS = {
'1_region': [],
'2_country_name': ['USA'],
'3_region': [],
'3_country_name': ['USA'],
'1_region': { values: [], scope: [1] },
'2_country_name': { values: ['USA'], scope: [1, 2] },
'3_region': { values: [], scope: [1] },
'3_country_name': { values: ['USA'], scope: [] },
};
it('should render a DashboardBuilder', () => {
@ -143,19 +143,13 @@ describe('Dashboard', () => {
it('should call refresh if a filter is added', () => {
const newFilter = {
gender: ['boy', 'girl'],
gender: { values: ['boy', 'girl'], scope: [1] },
};
wrapper.setProps({
activeFilters: {
...OVERRIDE_FILTERS,
...newFilter,
},
activeFilters: newFilter,
});
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual({
...OVERRIDE_FILTERS,
...newFilter,
});
expect(wrapper.instance().appliedFilters).toEqual(newFilter);
});
it('should call refresh if a filter is removed', () => {
@ -167,17 +161,55 @@ describe('Dashboard', () => {
});
it('should call refresh if a filter is changed', () => {
const newFilters = {
...OVERRIDE_FILTERS,
'1_region': { values: ['Canada'], scope: [1] },
};
wrapper.setProps({
activeFilters: {
...OVERRIDE_FILTERS,
'1_region': ['Canada'],
},
activeFilters: newFilters,
});
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual({
expect(wrapper.instance().appliedFilters).toEqual(newFilters);
expect(refreshSpy.getCall(0).args[0]).toEqual([1]);
});
it('should call refresh with multiple chart ids', () => {
const newFilters = {
...OVERRIDE_FILTERS,
'1_region': ['Canada'],
'2_country_name': { values: ['New Country'], scope: [1, 2] },
};
wrapper.setProps({
activeFilters: newFilters,
});
expect(refreshSpy.callCount).toBe(1);
expect(wrapper.instance().appliedFilters).toEqual(newFilters);
expect(refreshSpy.getCall(0).args[0]).toEqual([1, 2]);
});
it('should call refresh if a filter scope is changed', () => {
const newFilters = {
...OVERRIDE_FILTERS,
'3_country_name': { values: ['USA'], scope: [2] },
};
wrapper.setProps({
activeFilters: newFilters,
});
expect(refreshSpy.callCount).toBe(1);
expect(refreshSpy.getCall(0).args[0]).toEqual([2]);
});
it('should call refresh with empty [] if a filter is changed but scope is not applicable', () => {
const newFilters = {
...OVERRIDE_FILTERS,
'3_country_name': { values: ['CHINA'], scope: [] },
};
wrapper.setProps({
activeFilters: newFilters,
});
expect(refreshSpy.callCount).toBe(1);
expect(refreshSpy.getCall(0).args[0]).toEqual([]);
});
});
});

View File

@ -145,31 +145,7 @@ class Dashboard extends React.PureComponent {
const { activeFilters } = this.props;
// do not apply filter when dashboard in edit mode
if (!editMode && !areObjectsEqual(appliedFilters, activeFilters)) {
// refresh charts if a filter was removed, added, or changed
const currFilterKeys = Object.keys(activeFilters);
const appliedFilterKeys = Object.keys(appliedFilters);
const allKeys = new Set(currFilterKeys.concat(appliedFilterKeys));
const affectedChartIds = [];
[...allKeys].forEach(filterKey => {
if (!currFilterKeys.includes(filterKey)) {
// removed filter?
[].push.apply(affectedChartIds, appliedFilters[filterKey].scope);
} else if (!appliedFilterKeys.includes(filterKey)) {
// added filter?
[].push.apply(affectedChartIds, activeFilters[filterKey].scope);
} else {
// changed filter field value or scope?
const affectedScope = (activeFilters[filterKey].scope || []).concat(
appliedFilters[filterKey].scope || [],
);
[].push.apply(affectedChartIds, affectedScope);
}
});
const idSet = new Set(affectedChartIds);
this.refreshCharts([...idSet]);
this.appliedFilters = activeFilters;
this.applyFilters();
}
if (hasUnsavedChanges) {
@ -205,6 +181,56 @@ class Dashboard extends React.PureComponent {
return Object.values(this.props.charts);
}
applyFilters() {
const appliedFilters = this.appliedFilters;
const { activeFilters } = this.props;
// refresh charts if a filter was removed, added, or changed
const currFilterKeys = Object.keys(activeFilters);
const appliedFilterKeys = Object.keys(appliedFilters);
const allKeys = new Set(currFilterKeys.concat(appliedFilterKeys));
const affectedChartIds = [];
[...allKeys].forEach(filterKey => {
if (!currFilterKeys.includes(filterKey)) {
// filterKey is removed?
affectedChartIds.push(...appliedFilters[filterKey].scope);
} else if (!appliedFilterKeys.includes(filterKey)) {
// filterKey is newly added?
affectedChartIds.push(...activeFilters[filterKey].scope);
} else {
// if filterKey changes value,
// update charts in its scope
if (
!areObjectsEqual(
appliedFilters[filterKey].values,
activeFilters[filterKey].values,
)
) {
affectedChartIds.push(...activeFilters[filterKey].scope);
}
// if filterKey changes scope,
// update all charts in its scope
if (
!areObjectsEqual(
appliedFilters[filterKey].scope,
activeFilters[filterKey].scope,
)
) {
const chartsInScope = (activeFilters[filterKey].scope || []).concat(
appliedFilters[filterKey].scope || [],
);
affectedChartIds.push(...chartsInScope);
}
}
});
// remove dup in affectedChartIds
this.refreshCharts([...new Set(affectedChartIds)]);
this.appliedFilters = activeFilters;
}
refreshCharts(ids) {
ids.forEach(id => {
this.props.actions.triggerQuery(true, id);