Fix disco filters #2 (elastic#50061) (elastic#50121)

* closes elastic#44349, as well as SDH issue elastic#64 * added tests to cover fix * eslint * Fixed test import
spalger · Nov 10, 2019 · 6c03dd2 · 6c03dd2
1 parent 719b834
commit 6c03dd2
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 8 deletions.
diff --git a/src/plugins/data/public/query/filter_manager/filter_manager.test.ts b/src/plugins/data/public/query/filter_manager/filter_manager.test.ts
@@ -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);
     });
 
@@ -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,
@@ -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 () => {

diff --git a/src/plugins/data/public/query/filter_manager/filter_manager.ts b/src/plugins/data/public/query/filter_manager/filter_manager.ts
@@ -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);