Bugfix dashboard unpins filters (elastic#62301)

Fixes following cases: Saving dashboard with pinned filter unpins it. Do not save pinned filters with dashboard see elastic#62301 (comment) When navigating with global filter to dashboard with same saved filter, filter becomes unpinned When navigating from listing to dashboard with saved filter, back button didn't work Co-authored-by: Elastic Machine <[email protected]>
Dosant · Apr 12, 2020 · 4297147 · 4297147
1 parent fcd2718
commit 4297147
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 24 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
@@ -148,6 +148,7 @@ export class DashboardAppController {
     });
 
     // sync initial app filters from state to filterManager
+    // if there is an existing similar global filter, then leave it as global
     filterManager.setAppFilters(_.cloneDeep(dashboardStateManager.appState.filters));
     // setup syncing of app filters between appState and filterManager
     const stopSyncingAppFilters = connectToQueryState(
@@ -175,13 +176,16 @@ export class DashboardAppController {
     }
 
     // starts syncing `_g` portion of url with query services
-    // note: dashboard_state_manager.ts syncs `_a` portion of url
     // it is important to start this syncing after `dashboardStateManager.syncTimefilterWithDashboard(timefilter);` above is run,
-    // otherwise it will case redundant browser history record
+    // otherwise it will case redundant browser history records
     const { stop: stopSyncingQueryServiceStateWithUrl } = syncQueryStateWithUrl(
       queryService,
       kbnUrlStateStorage
     );
+
+    // starts syncing `_a` portion of url
+    dashboardStateManager.startStateSyncing();
+
     $scope.showSaveQuery = dashboardCapabilities.saveQuery as boolean;
 
     const getShouldShowEditHelp = () =>

diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts
@@ -200,8 +200,10 @@ export class DashboardStateManager {
       },
       stateStorage: this.kbnUrlStateStorage,
     });
+  }
 
-    // actually start syncing state with container
+  public startStateSyncing() {
+    this.saveState({ replace: true });
     this.stateSyncRef.start();
   }
 

diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/update_saved_dashboard.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/update_saved_dashboard.ts
@@ -22,6 +22,7 @@ import { RefreshInterval, TimefilterContract } from 'src/plugins/data/public';
 import { FilterUtils } from './filter_utils';
 import { SavedObjectDashboard } from '../../../../../../../plugins/dashboard/public';
 import { DashboardAppState } from '../types';
+import { esFilters } from '../../../../../../../plugins/data/public';
 
 export function updateSavedDashboard(
   savedDashboard: SavedObjectDashboard,
@@ -48,4 +49,9 @@ export function updateSavedDashboard(
     'value',
   ]);
   savedDashboard.refreshInterval = savedDashboard.timeRestore ? timeRestoreObj : undefined;
+  // save only unpinned filters
+  const unpinnedFilters = savedDashboard
+    .getFilters()
+    .filter(filter => !esFilters.isFilterPinned(filter));
+  savedDashboard.searchSource.setField('filter', unpinnedFilters);
 }
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
@@ -204,18 +204,19 @@ describe('filter_manager', () => {
       ).toBe(3);
     });
 
-    test('should set app filters and remove any duplicated global filters', async function() {
-      filterManager.addFilters(readyFilters, true);
+    test('should set app filters and merge them with duplicate global filters', async function() {
+      const [filter, ...otherFilters] = readyFilters;
+      filterManager.addFilters(otherFilters, true);
       const appFilter1 = _.cloneDeep(readyFilters[1]);
       const appFilter2 = _.cloneDeep(readyFilters[2]);
 
-      filterManager.setAppFilters([appFilter1, appFilter2]);
+      filterManager.setAppFilters([filter, appFilter1, appFilter2]);
 
       const newGlobalFilters = filterManager.getGlobalFilters();
       const newAppFilters = filterManager.getAppFilters();
 
-      expect(newGlobalFilters).toHaveLength(1);
-      expect(newAppFilters).toHaveLength(2);
+      expect(newGlobalFilters).toHaveLength(2);
+      expect(newAppFilters).toHaveLength(1);
     });
 
     test('should set global filters and remove any duplicated app filters', async function() {

diff --git a/src/plugins/data/public/query/filter_manager/filter_manager.ts b/src/plugins/data/public/query/filter_manager/filter_manager.ts
@@ -177,13 +177,9 @@ export class FilterManager {
   public setGlobalFilters(newGlobalFilters: Filter[]) {
     newGlobalFilters = mapAndFlattenFilters(newGlobalFilters);
     FilterManager.setFiltersStore(newGlobalFilters, FilterStateStore.GLOBAL_STATE, true);
-    const { appFilters: currentAppFilters } = this.getPartitionedFilters();
-    // remove duplicates from current app filters, to make sure global will take precedence
-    const filteredAppFilters = currentAppFilters.filter(
-      appFilter => !newGlobalFilters.find(globalFilter => compareFilters(globalFilter, appFilter))
-    );
+    const { appFilters } = this.getPartitionedFilters();
     const newFilters = this.mergeIncomingFilters({
-      appFilters: filteredAppFilters,
+      appFilters,
       globalFilters: newGlobalFilters,
     });
 
@@ -198,14 +194,9 @@ export class FilterManager {
   public setAppFilters(newAppFilters: Filter[]) {
     newAppFilters = mapAndFlattenFilters(newAppFilters);
     FilterManager.setFiltersStore(newAppFilters, FilterStateStore.APP_STATE, true);
-    const { globalFilters: currentGlobalFilters } = this.getPartitionedFilters();
-    // remove duplicates from current global filters, to make sure app will take precedence
-    const filteredGlobalFilters = currentGlobalFilters.filter(
-      globalFilter => !newAppFilters.find(appFilter => compareFilters(appFilter, globalFilter))
-    );
-
+    const { globalFilters } = this.getPartitionedFilters();
     const newFilters = this.mergeIncomingFilters({
-      globalFilters: filteredGlobalFilters,
+      globalFilters,
       appFilters: newAppFilters,
     });
     this.handleStateUpdate(newFilters);

diff --git a/src/plugins/data/public/ui/filter_bar/filter_item.tsx b/src/plugins/data/public/ui/filter_bar/filter_item.tsx
@@ -88,6 +88,7 @@ class FilterItemUI extends Component<Props, State> {
     const dataTestSubjDisabled = `filter-${
       this.props.filter.meta.disabled ? 'disabled' : 'enabled'
     }`;
+    const dataTestSubjPinned = `filter-${isFilterPinned(filter) ? 'pinned' : 'unpinned'}`;
 
     const classes = classNames(
       'globalFilterItem',
@@ -107,7 +108,7 @@ class FilterItemUI extends Component<Props, State> {
         className={classes}
         iconOnClick={() => this.props.onRemove()}
         onClick={this.handleBadgeClick}
-        data-test-subj={`filter ${dataTestSubjDisabled} ${dataTestSubjKey} ${dataTestSubjValue}`}
+        data-test-subj={`filter ${dataTestSubjDisabled} ${dataTestSubjKey} ${dataTestSubjValue} ${dataTestSubjPinned}`}
       />
     );
 

diff --git a/test/functional/apps/dashboard/dashboard_filter_bar.js b/test/functional/apps/dashboard/dashboard_filter_bar.js
@@ -27,6 +27,7 @@ export default function({ getService, getPageObjects }) {
   const pieChart = getService('pieChart');
   const esArchiver = getService('esArchiver');
   const kibanaServer = getService('kibanaServer');
+  const browser = getService('browser');
   const PageObjects = getPageObjects(['common', 'dashboard', 'header', 'visualize', 'timePicker']);
 
   describe('dashboard filter bar', () => {
@@ -126,9 +127,46 @@ export default function({ getService, getPageObjects }) {
 
         const filterCount = await filterBar.getFilterCount();
         expect(filterCount).to.equal(1);
+        await pieChart.expectPieSliceCount(1);
+      });
 
+      it("restoring filters doesn't break back button", async () => {
+        await browser.goBack();
+        await PageObjects.dashboard.expectExistsDashboardLandingPage();
+        await browser.goForward();
+        await PageObjects.header.waitUntilLoadingHasFinished();
+        await PageObjects.dashboard.waitForRenderComplete();
         await pieChart.expectPieSliceCount(1);
       });
+
+      it("saving with pinned filter doesn't unpin them", async () => {
+        const filterKey = 'bytes';
+        await filterBar.toggleFilterPinned(filterKey);
+        await PageObjects.dashboard.switchToEditMode();
+        await PageObjects.dashboard.saveDashboard('saved with pinned filters', {
+          saveAsNew: true,
+        });
+        expect(await filterBar.isFilterPinned(filterKey)).to.be(true);
+        await pieChart.expectPieSliceCount(1);
+      });
+
+      it("navigating to a dashboard with global filter doesn't unpin it if same filter is saved with dashboard", async () => {
+        await PageObjects.dashboard.preserveCrossAppState();
+        await PageObjects.dashboard.gotoDashboardLandingPage();
+        await PageObjects.dashboard.loadSavedDashboard('with filters');
+        await PageObjects.header.waitUntilLoadingHasFinished();
+        expect(await filterBar.isFilterPinned('bytes')).to.be(true);
+        await pieChart.expectPieSliceCount(1);
+      });
+
+      it("pinned filters aren't saved", async () => {
+        await filterBar.removeFilter('bytes');
+        await PageObjects.dashboard.gotoDashboardLandingPage();
+        await PageObjects.dashboard.loadSavedDashboard('saved with pinned filters');
+        await PageObjects.header.waitUntilLoadingHasFinished();
+        expect(await filterBar.getFilterCount()).to.be(0);
+        await pieChart.expectPieSliceCount(5);
+      });
     });
 
     describe('saved search filtering', function() {

diff --git a/test/functional/services/filter_bar.ts b/test/functional/services/filter_bar.ts
@@ -32,10 +32,16 @@ export function FilterBarProvider({ getService, getPageObjects }: FtrProviderCon
      * @param value filter value
      * @param enabled filter status
      */
-    public async hasFilter(key: string, value: string, enabled: boolean = true): Promise<boolean> {
+    public async hasFilter(
+      key: string,
+      value: string,
+      enabled: boolean = true,
+      pinned: boolean = false
+    ): Promise<boolean> {
       const filterActivationState = enabled ? 'enabled' : 'disabled';
+      const filterPinnedState = pinned ? 'pinned' : 'unpinned';
       return testSubjects.exists(
-        `filter filter-${filterActivationState} filter-key-${key} filter-value-${value}`,
+        `filter filter-${filterActivationState} filter-key-${key} filter-value-${value} filter-${filterPinnedState}`,
         {
           allowHidden: true,
         }
@@ -80,6 +86,11 @@ export function FilterBarProvider({ getService, getPageObjects }: FtrProviderCon
       await PageObjects.header.awaitGlobalLoadingIndicatorHidden();
     }
 
+    public async isFilterPinned(key: string): Promise<boolean> {
+      const filter = await testSubjects.find(`~filter & ~filter-key-${key}`);
+      return (await filter.getAttribute('data-test-subj')).includes('filter-pinned');
+    }
+
     public async getFilterCount(): Promise<number> {
       const filters = await testSubjects.findAll('~filter');
       return filters.length;