Skip to content

Commit

Permalink
Fix disco filters #2 (elastic#50061)
Browse files Browse the repository at this point in the history
* closes elastic#44349, as well as SDH issue elastic#64

* added tests to cover fix

* eslint

* Fixed test import
  • Loading branch information
Liza Katz authored Nov 10, 2019
1 parent 78e9d93 commit 29aeec8
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,15 @@ describe('filter_manager', () => {
expect(updateListener.callCount).toBe(1);
});

test('app state should accept array', async () => {
test('app state should accept array and preserve order', async () => {
const f1 = getFilter(esFilters.FilterStateStore.APP_STATE, false, false, 'age', 34);
const f2 = getFilter(esFilters.FilterStateStore.APP_STATE, false, false, 'gender', 'female');

filterManager.addFilters([f1]);
filterManager.addFilters([f2]);
const appFilters = filterManager.getAppFilters();
expect(appFilters).toHaveLength(2);
expect(appFilters).toEqual([f2, f1]);
expect(appFilters).toEqual([f1, f2]);
expect(filterManager.getGlobalFilters()).toHaveLength(0);
});

Expand All @@ -206,7 +207,7 @@ describe('filter_manager', () => {
expect(updateListener.callCount).toBe(1);
});

test('global state should be accept array', async () => {
test('global state should be accept array and preserve order', async () => {
const f1 = getFilter(esFilters.FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
const f2 = getFilter(
esFilters.FilterStateStore.GLOBAL_STATE,
Expand All @@ -215,11 +216,36 @@ describe('filter_manager', () => {
'gender',
'female'
);

filterManager.addFilters([f1, f2]);
expect(filterManager.getAppFilters()).toHaveLength(0);
const globalFilters = filterManager.getGlobalFilters();
expect(globalFilters).toHaveLength(2);
expect(globalFilters).toEqual([f2, f1]);
expect(globalFilters).toEqual([f1, f2]);
});

test('mixed filters: global filters should stay in the beginning', async () => {
const f1 = getFilter(esFilters.FilterStateStore.GLOBAL_STATE, false, false, 'age', 34);
const f2 = getFilter(esFilters.FilterStateStore.APP_STATE, false, false, 'gender', 'female');
filterManager.addFilters([f1, f2]);
const filters = filterManager.getFilters();
expect(filters).toHaveLength(2);
expect(filters).toEqual([f1, f2]);
});

test('mixed filters: global filters should move to the beginning', async () => {
const f1 = getFilter(esFilters.FilterStateStore.APP_STATE, false, false, 'age', 34);
const f2 = getFilter(
esFilters.FilterStateStore.GLOBAL_STATE,
false,
false,
'gender',
'female'
);
filterManager.addFilters([f1, f2]);
const filters = filterManager.getFilters();
expect(filters).toHaveLength(2);
expect(filters).toEqual([f2, f1]);
});

test('add multiple filters at once', async () => {
Expand Down
13 changes: 9 additions & 4 deletions src/plugins/data/public/query/filter_manager/filter_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,16 @@ export class FilterManager {

private handleStateUpdate(newFilters: esFilters.Filter[]) {
// global filters should always be first

newFilters.sort(({ $state: a }: esFilters.Filter, { $state: b }: esFilters.Filter): number => {
return a!.store === esFilters.FilterStateStore.GLOBAL_STATE &&
b!.store !== esFilters.FilterStateStore.GLOBAL_STATE
? -1
: 1;
if (a!.store === b!.store) {
return 0;
} else {
return a!.store === esFilters.FilterStateStore.GLOBAL_STATE &&
b!.store !== esFilters.FilterStateStore.GLOBAL_STATE
? -1
: 1;
}
});

const filtersUpdated = !_.isEqual(this.filters, newFilters);
Expand Down

0 comments on commit 29aeec8

Please sign in to comment.