[Discover] Address reverting unsaved changes issue to unpin filters (#…

…172063)

- Closes #172025

## Summary

This PR changes filters diffing logic for the "Unsaved changes" badge
and fixes "Revert changes" action of it. What to expect when testing:

- If user pins a filter and the order of filters does not change, then
the badge should not appear.
- If user has 2 filters for example and pins the second one, then this
action would change filters order and the badge would appear.
- If user presses "Revert changes" action, then newly pinned filters
would become unpinned.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

src/plugins/discover/public/application/main/services/discover_app_state_container.ts

@@ -16,6 +16,7 @@ import {
   COMPARE_ALL_OPTIONS,
   compareFilters,
   Filter,
+  FilterCompareOptions,
   FilterStateStore,
   Query,
 } from '@kbn/es-query';
@@ -328,13 +329,17 @@ export function setState(
 /**
  * Helper function to compare 2 different filter states
  */
-export function isEqualFilters(filtersA?: Filter[] | Filter, filtersB?: Filter[] | Filter) {
+export function isEqualFilters(
+  filtersA?: Filter[] | Filter,
+  filtersB?: Filter[] | Filter,
+  comparatorOptions: FilterCompareOptions = COMPARE_ALL_OPTIONS
+) {
   if (!filtersA && !filtersB) {
     return true;
   } else if (!filtersA || !filtersB) {
     return false;
   }
-  return compareFilters(filtersA, filtersB, COMPARE_ALL_OPTIONS);
+  return compareFilters(filtersA, filtersB, comparatorOptions);
 }
 
 /**

src/plugins/discover/public/application/main/services/discover_saved_search_container.ts

@@ -8,19 +8,25 @@
 
 import { SavedSearch } from '@kbn/saved-search-plugin/public';
 import { BehaviorSubject } from 'rxjs';
+import { COMPARE_ALL_OPTIONS, FilterCompareOptions } from '@kbn/es-query';
 import type { SearchSourceFields } from '@kbn/data-plugin/common';
 import type { DataView } from '@kbn/data-views-plugin/common';
 import { SavedObjectSaveOpts } from '@kbn/saved-objects-plugin/public';
-import { isEqual } from 'lodash';
+import { isEqual, isFunction } from 'lodash';
 import { restoreStateFromSavedSearch } from '../../../services/saved_searches/restore_from_saved_search';
 import { updateSavedSearch } from '../utils/update_saved_search';
 import { addLog } from '../../../utils/add_log';
 import { handleSourceColumnState } from '../../../utils/state_helpers';
-import { DiscoverAppState } from './discover_app_state_container';
+import { DiscoverAppState, isEqualFilters } from './discover_app_state_container';
 import { DiscoverServices } from '../../../build_services';
 import { getStateDefaults } from '../utils/get_state_defaults';
 import type { DiscoverGlobalStateContainer } from './discover_global_state_container';
 
+const FILTERS_COMPARE_OPTIONS: FilterCompareOptions = {
+  ...COMPARE_ALL_OPTIONS,
+  state: false, // We don't compare filter types (global vs appState).
+};
+
 export interface UpdateParams {
   /**
    * The next data view to be used
@@ -279,11 +285,13 @@ export function isEqualSavedSearch(savedSearchPrev: SavedSearch, savedSearchNext
   const hasChangesInSearchSource = (
     ['filter', 'query', 'index'] as Array<keyof SearchSourceFields>
   ).some((key) => {
-    const prevValue =
-      key === 'index' ? prevSearchSource.getField(key)?.id : prevSearchSource.getField(key);
-    const nextValue =
-      key === 'index' ? nextSearchSource.getField(key)?.id : nextSearchSource.getField(key);
-    const isSame = isEqual(prevValue, nextValue);
+    const prevValue = getSearchSourceFieldValueForComparison(prevSearchSource, key);
+    const nextValue = getSearchSourceFieldValueForComparison(nextSearchSource, key);
+
+    const isSame =
+      key === 'filter'
+        ? isEqualFilters(prevValue, nextValue, FILTERS_COMPARE_OPTIONS) // if a filter gets pinned and the order of filters does not change, we don't show the unsaved changes badge
+        : isEqual(prevValue, nextValue);
 
     if (!isSame) {
       addLog('[savedSearch] difference between initial and changed version', {
@@ -304,3 +312,19 @@ export function isEqualSavedSearch(savedSearchPrev: SavedSearch, savedSearchNext
 
   return true;
 }
+
+function getSearchSourceFieldValueForComparison(
+  searchSource: SavedSearch['searchSource'],
+  searchSourceFieldName: keyof SearchSourceFields
+) {
+  if (searchSourceFieldName === 'index') {
+    return searchSource.getField('index')?.id;
+  }
+
+  if (searchSourceFieldName === 'filter') {
+    const filterField = searchSource.getField('filter');
+    return isFunction(filterField) ? filterField() : filterField;
+  }
+
+  return searchSource.getField(searchSourceFieldName);
+}

src/plugins/discover/public/application/main/services/discover_state.ts

@@ -459,6 +459,16 @@ export function getDiscoverStateContainer({
       timefilter: services.timefilter,
     });
     const newAppState = getDefaultAppState(nextSavedSearch, services);
+
+    // a saved search can't have global (pinned) filters so we can reset global filters state
+    const globalFilters = globalStateContainer.get()?.filters;
+    if (globalFilters) {
+      await globalStateContainer.set({
+        ...globalStateContainer.get(),
+        filters: [],
+      });
+    }
+
     await appStateContainer.replaceUrlState(newAppState);
     return nextSavedSearch;
   };

src/plugins/discover/public/application/main/utils/update_saved_search.test.ts

@@ -70,7 +70,7 @@ describe('updateSavedSearch', () => {
       },
     });
     expect(savedSearch.searchSource.getField('query')).toEqual(query);
-    expect(savedSearch.searchSource.getField('filter')).toEqual([appFilter, globalFilter]);
+    expect(savedSearch.searchSource.getField('filter')).toEqual([globalFilter, appFilter]);
   });
 
   it('should set query and filters from services', async () => {

src/plugins/discover/public/application/main/utils/update_saved_search.ts

@@ -52,7 +52,7 @@ export function updateSavedSearch({
 
     savedSearch.searchSource
       .setField('query', state.query ?? undefined)
-      .setField('filter', [...appFilters, ...globalFilters]);
+      .setField('filter', [...globalFilters, ...appFilters]);
   }
   if (state) {
     savedSearch.columns = state.columns || [];

test/functional/apps/discover/group3/_unsaved_changes_badge.ts

@@ -10,12 +10,14 @@ import expect from '@kbn/expect';
 import { FtrProviderContext } from '../ftr_provider_context';
 
 const SAVED_SEARCH_NAME = 'test saved search';
+const SAVED_SEARCH_WITH_FILTERS_NAME = 'test saved search with filters';
 
 export default function ({ getService, getPageObjects }: FtrProviderContext) {
   const esArchiver = getService('esArchiver');
   const kibanaServer = getService('kibanaServer');
   const testSubjects = getService('testSubjects');
   const dataGrid = getService('dataGrid');
+  const filterBar = getService('filterBar');
   const PageObjects = getPageObjects([
     'settings',
     'common',
@@ -163,5 +165,34 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       await PageObjects.discover.waitUntilSearchingHasFinished();
       await testSubjects.missingOrFail('unsavedChangesBadge');
     });
+
+    it('should not show the badge after pinning the first filter but after disabling a filter', async () => {
+      await filterBar.addFilter({ field: 'extension', operation: 'is', value: 'png' });
+      await filterBar.addFilter({ field: 'bytes', operation: 'exists' });
+      await PageObjects.discover.saveSearch(SAVED_SEARCH_WITH_FILTERS_NAME);
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+
+      await testSubjects.missingOrFail('unsavedChangesBadge');
+
+      await filterBar.toggleFilterPinned('extension');
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await filterBar.isFilterPinned('extension')).to.be(true);
+
+      await testSubjects.missingOrFail('unsavedChangesBadge');
+
+      await filterBar.toggleFilterNegated('bytes');
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await filterBar.isFilterNegated('bytes')).to.be(true);
+
+      await testSubjects.existOrFail('unsavedChangesBadge');
+
+      await PageObjects.discover.revertUnsavedChanges();
+      await testSubjects.missingOrFail('unsavedChangesBadge');
+
+      expect(await filterBar.getFilterCount()).to.be(2);
+      expect(await filterBar.isFilterPinned('extension')).to.be(false);
+      expect(await filterBar.isFilterNegated('bytes')).to.be(false);
+      expect(await PageObjects.discover.getHitCount()).to.be('1,373');
+    });
   });
 }

test/functional/services/filter_bar.ts

@@ -178,6 +178,11 @@ export class FilterBarService extends FtrService {
     return (await filter.getAttribute('data-test-subj')).includes('filter-pinned');
   }
 
+  public async isFilterNegated(key: string): Promise<boolean> {
+    const filter = await this.testSubjects.find(`~filter & ~filter-key-${key}`);
+    return (await filter.getAttribute('data-test-subj')).includes('filter-negated');
+  }
+
   public async getFilterCount(): Promise<number> {
     const filters = await this.testSubjects.findAll('~filter');
     return filters.length;

x-pack/test_serverless/functional/test_suites/common/discover/group3/_unsaved_changes_badge.ts

@@ -9,12 +9,14 @@ import expect from '@kbn/expect';
 import { FtrProviderContext } from '../../../../ftr_provider_context';
 
 const SAVED_SEARCH_NAME = 'test saved search';
+const SAVED_SEARCH_WITH_FILTERS_NAME = 'test saved search with filters';
 
 export default function ({ getService, getPageObjects }: FtrProviderContext) {
   const esArchiver = getService('esArchiver');
   const kibanaServer = getService('kibanaServer');
   const testSubjects = getService('testSubjects');
   const dataGrid = getService('dataGrid');
+  const filterBar = getService('filterBar');
   const PageObjects = getPageObjects([
     'settings',
     'common',
@@ -162,5 +164,34 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       await PageObjects.discover.waitUntilSearchingHasFinished();
       await testSubjects.missingOrFail('unsavedChangesBadge');
     });
+
+    it('should not show the badge after pinning the first filter but after disabling a filter', async () => {
+      await filterBar.addFilter({ field: 'extension', operation: 'is', value: 'png' });
+      await filterBar.addFilter({ field: 'bytes', operation: 'exists' });
+      await PageObjects.discover.saveSearch(SAVED_SEARCH_WITH_FILTERS_NAME);
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+
+      await testSubjects.missingOrFail('unsavedChangesBadge');
+
+      await filterBar.toggleFilterPinned('extension');
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await filterBar.isFilterPinned('extension')).to.be(true);
+
+      await testSubjects.missingOrFail('unsavedChangesBadge');
+
+      await filterBar.toggleFilterNegated('bytes');
+      await PageObjects.discover.waitUntilSearchingHasFinished();
+      expect(await filterBar.isFilterNegated('bytes')).to.be(true);
+
+      await testSubjects.existOrFail('unsavedChangesBadge');
+
+      await PageObjects.discover.revertUnsavedChanges();
+      await testSubjects.missingOrFail('unsavedChangesBadge');
+
+      expect(await filterBar.getFilterCount()).to.be(2);
+      expect(await filterBar.isFilterPinned('extension')).to.be(false);
+      expect(await filterBar.isFilterNegated('bytes')).to.be(false);
+      expect(await PageObjects.discover.getHitCount()).to.be('1,373');
+    });
   });
 }