Retain pinned filters when loading and clearing saved queries (#54307)

When we originally implemented Saved Queries we had them overwrite pinned filters on load and on clear. This caused the issue in #53258. If you have a saved query loaded in Discover for example and you navigate to a different app and then back to Discover, that saved query will get get reloaded since app state is retained when navigating back and forth between apps. If you created a pinned filter in between visits to Discover, it will get removed when the saved query is reloaded. This issue made me reconsider our previous decision. I think pinned filters should not be affected by loading or clearing a saved query, since they are pinned they should only be removed if the user explicitly asks for it. This solves the reported issue and I also think it makes the UI more intuitive.
elastic · Jan 21, 2020 · 8ca25c4 · 8ca25c4
1 parent fca83a6
commit 8ca25c4
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 21 deletions.
diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx
@@ -470,18 +470,21 @@ export class DashboardAppController {
           language:
             localStorage.get('kibana.userQueryLanguage') || config.get('search:queryLanguage'),
         },
-        []
+        queryFilter.getGlobalFilters()
       );
       // Making this method sync broke the updates.
       // Temporary fix, until we fix the complex state in this file.
-      setTimeout(queryFilter.removeAll, 0);
+      setTimeout(() => {
+        queryFilter.setFilters(queryFilter.getGlobalFilters());
+      }, 0);
     };
 
     const updateStateFromSavedQuery = (savedQuery: SavedQuery) => {
-      dashboardStateManager.applyFilters(
-        savedQuery.attributes.query,
-        savedQuery.attributes.filters || []
-      );
+      const savedQueryFilters = savedQuery.attributes.filters || [];
+      const globalFilters = queryFilter.getGlobalFilters();
+      const allFilters = [...globalFilters, ...savedQueryFilters];
+
+      dashboardStateManager.applyFilters(savedQuery.attributes.query, allFilters);
       if (savedQuery.attributes.timefilter) {
         timefilter.setTime({
           from: savedQuery.attributes.timefilter.from,
@@ -494,7 +497,7 @@ export class DashboardAppController {
       // Making this method sync broke the updates.
       // Temporary fix, until we fix the complex state in this file.
       setTimeout(() => {
-        queryFilter.setFilters(savedQuery.attributes.filters || []);
+        queryFilter.setFilters(allFilters);
       }, 0);
     };
 

diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js
@@ -1000,15 +1000,17 @@ function discoverController(
       query: '',
       language: localStorage.get('kibana.userQueryLanguage') || config.get('search:queryLanguage'),
     };
-    filterManager.removeAll();
+    filterManager.setFilters(filterManager.getGlobalFilters());
     $state.save();
     $scope.fetch();
   };
 
   const updateStateFromSavedQuery = savedQuery => {
     $state.query = savedQuery.attributes.query;
     $state.save();
-    filterManager.setFilters(savedQuery.attributes.filters || []);
+    const savedQueryFilters = savedQuery.attributes.filters || [];
+    const globalFilters = filterManager.getGlobalFilters();
+    filterManager.setFilters([...globalFilters, ...savedQueryFilters]);
 
     if (savedQuery.attributes.timefilter) {
       timefilter.setTime({

diff --git a/src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/editor.js b/src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/editor.js
@@ -501,7 +501,7 @@ function VisualizeAppController(
       language:
         localStorage.get('kibana.userQueryLanguage') || uiSettings.get('search:queryLanguage'),
     };
-    queryFilter.removeAll();
+    queryFilter.setFilters(queryFilter.getGlobalFilters());
     $state.save();
     $scope.fetch();
   };
@@ -510,7 +510,9 @@ function VisualizeAppController(
     $state.query = savedQuery.attributes.query;
     $state.save();
 
-    queryFilter.setFilters(savedQuery.attributes.filters || []);
+    const savedQueryFilters = savedQuery.attributes.filters || [];
+    const globalFilters = queryFilter.getGlobalFilters();
+    queryFilter.setFilters([...globalFilters, ...savedQueryFilters]);
 
     if (savedQuery.attributes.timefilter) {
       timefilter.setTime({

diff --git a/x-pack/legacy/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/legacy/plugins/lens/public/app_plugin/app.test.tsx
@@ -12,7 +12,12 @@ import { EditorFrameInstance } from '../types';
 import { Storage } from '../../../../../../src/plugins/kibana_utils/public';
 import { Document, SavedObjectStore } from '../persistence';
 import { mount } from 'enzyme';
-import { esFilters, IFieldType, IIndexPattern } from '../../../../../../src/plugins/data/public';
+import {
+  esFilters,
+  FilterManager,
+  IFieldType,
+  IIndexPattern,
+} from '../../../../../../src/plugins/data/public';
 import { dataPluginMock } from '../../../../../../src/plugins/data/public/mocks';
 const dataStartMock = dataPluginMock.createStartContract();
 
@@ -60,6 +65,10 @@ function createMockFilterManager() {
       subscriber();
     },
     getFilters: () => filters,
+    getGlobalFilters: () => {
+      // @ts-ignore
+      return filters.filter(esFilters.isFilterPinned);
+    },
     removeAll: () => {
       filters = [];
       subscriber();
@@ -821,7 +830,7 @@ describe('Lens App', () => {
       );
     });
 
-    it('clears all existing filters when the active saved query is cleared', () => {
+    it('clears all existing unpinned filters when the active saved query is cleared', () => {
       const args = makeDefaultArgs();
       args.editorFrame = frame;
 
@@ -834,8 +843,13 @@ describe('Lens App', () => {
 
       const indexPattern = ({ id: 'index1' } as unknown) as IIndexPattern;
       const field = ({ name: 'myfield' } as unknown) as IFieldType;
+      const pinnedField = ({ name: 'pinnedField' } as unknown) as IFieldType;
 
-      args.data.query.filterManager.setFilters([esFilters.buildExistsFilter(field, indexPattern)]);
+      const unpinned = esFilters.buildExistsFilter(field, indexPattern);
+      const pinned = esFilters.buildExistsFilter(pinnedField, indexPattern);
+      FilterManager.setFiltersStore([pinned], esFilters.FilterStateStore.GLOBAL_STATE);
+
+      args.data.query.filterManager.setFilters([pinned, unpinned]);
       instance.update();
 
       instance.find(TopNavMenu).prop('onClearSavedQuery')!();
@@ -844,7 +858,7 @@ describe('Lens App', () => {
       expect(frame.mount).toHaveBeenLastCalledWith(
         expect.any(Element),
         expect.objectContaining({
-          filters: [],
+          filters: [pinned],
         })
       );
     });

diff --git a/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx b/x-pack/legacy/plugins/lens/public/app_plugin/app.tsx
@@ -239,7 +239,9 @@ export function App({
                 setState(s => ({ ...s, savedQuery }));
               }}
               onSavedQueryUpdated={savedQuery => {
-                data.query.filterManager.setFilters(savedQuery.attributes.filters || state.filters);
+                const savedQueryFilters = savedQuery.attributes.filters || [];
+                const globalFilters = data.query.filterManager.getGlobalFilters();
+                data.query.filterManager.setFilters([...globalFilters, ...savedQueryFilters]);
                 setState(s => ({
                   ...s,
                   savedQuery: { ...savedQuery }, // Shallow query for reference issues
@@ -252,11 +254,11 @@ export function App({
                 }));
               }}
               onClearSavedQuery={() => {
-                data.query.filterManager.removeAll();
+                data.query.filterManager.setFilters(data.query.filterManager.getGlobalFilters());
                 setState(s => ({
                   ...s,
                   savedQuery: undefined,
-                  filters: [],
+                  filters: data.query.filterManager.getGlobalFilters(),
                   query: {
                     query: '',
                     language:

diff --git a/x-pack/legacy/plugins/maps/public/angular/map_controller.js b/x-pack/legacy/plugins/maps/public/angular/map_controller.js
@@ -153,7 +153,7 @@ app.controller(
       delete $scope.savedQuery;
       delete $state.savedQuery;
       onQueryChange({
-        filters: [],
+        filters: filterManager.getGlobalFilters(),
         query: {
           query: '',
           language: localStorage.get('kibana.userQueryLanguage'),
@@ -162,6 +162,10 @@ app.controller(
     };
 
     function updateStateFromSavedQuery(savedQuery) {
+      const savedQueryFilters = savedQuery.attributes.filters || [];
+      const globalFilters = filterManager.getGlobalFilters();
+      const allFilters = [...savedQueryFilters, ...globalFilters];
+
       if (savedQuery.attributes.timefilter) {
         if (savedQuery.attributes.timefilter.refreshInterval) {
           $scope.onRefreshChange({
@@ -170,13 +174,13 @@ app.controller(
           });
         }
         onQueryChange({
-          filters: savedQuery.attributes.filters || [],
+          filters: allFilters,
           query: savedQuery.attributes.query,
           time: savedQuery.attributes.timefilter,
         });
       } else {
         onQueryChange({
-          filters: savedQuery.attributes.filters || [],
+          filters: allFilters,
           query: savedQuery.attributes.query,
         });
       }