[Discover] Fix navigation to a new from saved search and saved query, fix discover:searchOnPageLoad (#112262)

* [Discover] fix saved search become active

* [Discover] add another fix to be consistent with data fetching code

* [Discover] simplify solution

* [Discover] add functionals

* [Discover] fix saved query bug, add functionals

* [Discover] fix functionals

* [Discover] fix functional test

* [Discover] split saved query tests

* [Discover] preselect logstash index pattern

* [Discover] remove saved query after test complete

* [Discover] change query fill order

* [Discover] try to fix unrelated functional test

* [Discover] one more fix

* [Discover] try to fix one more problem

* [Discover] fix commonly used time range test

* [Discover] revert uisettings init in before statement, do small adjustments

* [Discover] fix unit test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

src/plugins/discover/public/application/apps/main/components/uninitialized/uninitialized.tsx

@@ -32,7 +32,7 @@ export const DiscoverUninitialized = ({ onRefresh }: Props) => {
         </p>
       }
       actions={
-        <EuiButton color="primary" fill onClick={onRefresh}>
+        <EuiButton color="primary" fill onClick={onRefresh} data-test-subj="refreshDataButton">
           <FormattedMessage
             id="discover.uninitializedRefreshButtonText"
             defaultMessage="Refresh data"

src/plugins/discover/public/application/apps/main/discover_main_app.tsx

@@ -38,7 +38,7 @@ export interface DiscoverMainProps {
 }
 
 export function DiscoverMainApp(props: DiscoverMainProps) {
-  const { services, history, indexPatternList } = props;
+  const { savedSearch, services, history, indexPatternList } = props;
   const { chrome, docLinks, uiSettings: config, data } = services;
   const navigateTo = useCallback(
     (path: string) => {
@@ -46,7 +46,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) {
     },
     [history]
   );
-  const savedSearch = props.savedSearch;
 
   /**
    * State related logic

src/plugins/discover/public/application/apps/main/discover_main_route.tsx

@@ -75,8 +75,6 @@ export function DiscoverMainRoute({ services, history }: DiscoverMainProps) {
 
     async function loadSavedSearch() {
       try {
-        // force a refresh if a given saved search without id was saved
-        setSavedSearch(undefined);
         const loadedSavedSearch = await services.getSavedSearchById(savedSearchId);
         const loadedIndexPattern = await loadDefaultOrCurrentIndexPattern(loadedSavedSearch);
         if (loadedSavedSearch && !loadedSavedSearch?.searchSource.getField('index')) {

src/plugins/discover/public/application/apps/main/services/use_discover_state.ts

@@ -96,6 +96,7 @@ export function useDiscoverState({
 
   useEffect(() => {
     const stopSync = stateContainer.initializeAndSync(indexPattern, filterManager, data);
+
     return () => stopSync();
   }, [stateContainer, filterManager, data, indexPattern]);
 
@@ -209,16 +210,13 @@ export function useDiscoverState({
   }, [config, data, savedSearch, reset, stateContainer]);
 
   /**
-   * Initial data fetching, also triggered when index pattern changes
+   * Trigger data fetching on indexPattern or savedSearch changes
    */
   useEffect(() => {
-    if (!indexPattern) {
-      return;
-    }
-    if (initialFetchStatus === FetchStatus.LOADING) {
+    if (indexPattern) {
       refetch$.next();
     }
-  }, [initialFetchStatus, refetch$, indexPattern]);
+  }, [initialFetchStatus, refetch$, indexPattern, savedSearch.id]);
 
   return {
     data$,

src/plugins/discover/public/application/apps/main/services/use_saved_search.ts

@@ -156,6 +156,7 @@ export const useSavedSearch = ({
       refetch$,
       searchSessionManager,
       searchSource,
+      initialFetchStatus,
     });
 
     const subscription = fetch$.subscribe((val) => {

src/plugins/discover/public/application/apps/main/utils/get_fetch_observable.ts

@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 import { merge } from 'rxjs';
-import { debounceTime, filter, tap } from 'rxjs/operators';
+import { debounceTime, filter, skip, tap } from 'rxjs/operators';
 
 import { FetchStatus } from '../../../types';
 import type {
@@ -26,6 +26,7 @@ export function getFetch$({
   main$,
   refetch$,
   searchSessionManager,
+  initialFetchStatus,
 }: {
   setAutoRefreshDone: (val: AutoRefreshDoneFn | undefined) => void;
   data: DataPublicPluginStart;
@@ -33,10 +34,11 @@ export function getFetch$({
   refetch$: DataRefetch$;
   searchSessionManager: DiscoverSearchSessionManager;
   searchSource: SearchSource;
+  initialFetchStatus: FetchStatus;
 }) {
   const { timefilter } = data.query.timefilter;
   const { filterManager } = data.query;
-  return merge(
+  let fetch$ = merge(
     refetch$,
     filterManager.getFetches$(),
     timefilter.getFetch$(),
@@ -58,4 +60,13 @@ export function getFetch$({
     data.query.queryString.getUpdates$(),
     searchSessionManager.newSearchSessionIdFromURL$.pipe(filter((sessionId) => !!sessionId))
   ).pipe(debounceTime(100));
+
+  /**
+   * Skip initial fetch when discover:searchOnPageLoad is disabled.
+   */
+  if (initialFetchStatus === FetchStatus.UNINITIALIZED) {
+    fetch$ = fetch$.pipe(skip(1));
+  }
+
+  return fetch$;
 }

src/plugins/discover/public/application/apps/main/utils/get_fetch_observeable.test.ts

@@ -58,6 +58,7 @@ describe('getFetchObservable', () => {
       data: createDataMock(new Subject(), new Subject(), new Subject(), new Subject()),
       searchSessionManager: searchSessionManagerMock.searchSessionManager,
       searchSource: savedSearchMock.searchSource,
+      initialFetchStatus: FetchStatus.LOADING,
     });
 
     fetch$.subscribe(() => {
@@ -81,6 +82,7 @@ describe('getFetchObservable', () => {
       data: dataMock,
       searchSessionManager: searchSessionManagerMock.searchSessionManager,
       searchSource: savedSearchMockWithTimeField.searchSource,
+      initialFetchStatus: FetchStatus.LOADING,
     });
 
     const fetchfnMock = jest.fn();

src/plugins/discover/public/application/apps/main/utils/get_state_defaults.test.ts

@@ -31,6 +31,7 @@ describe('getStateDefaults', () => {
         "index": "index-pattern-with-timefield-id",
         "interval": "auto",
         "query": undefined,
+        "savedQuery": undefined,
         "sort": Array [
           Array [
             "timestamp",
@@ -59,6 +60,7 @@ describe('getStateDefaults', () => {
         "index": "the-index-pattern-id",
         "interval": "auto",
         "query": undefined,
+        "savedQuery": undefined,
         "sort": Array [],
       }
     `);

src/plugins/discover/public/application/apps/main/utils/get_state_defaults.ts

@@ -47,6 +47,7 @@ export function getStateDefaults({
     interval: 'auto',
     filters: cloneDeep(searchSource.getOwnField('filter')),
     hideChart: undefined,
+    savedQuery: undefined,
   } as AppState;
   if (savedSearch.grid) {
     defaultState.grid = savedSearch.grid;

test/functional/apps/discover/_discover.ts

@@ -233,11 +233,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
 
     describe('time zone switch', () => {
       it('should show bars in the correct time zone after switching', async function () {
-        await kibanaServer.uiSettings.replace({ 'dateFormat:tz': 'America/Phoenix' });
+        await kibanaServer.uiSettings.update({ 'dateFormat:tz': 'America/Phoenix' });
         await PageObjects.common.navigateToApp('discover');
         await PageObjects.header.awaitKibanaChrome();
-        await queryBar.clearQuery();
         await PageObjects.timePicker.setDefaultAbsoluteRange();
+        await queryBar.clearQuery();
 
         log.debug(
           'check that the newest doc timestamp is now -7 hours from the UTC time in the first test'
@@ -246,36 +246,16 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         expect(rowData.startsWith('Sep 22, 2015 @ 16:50:13.253')).to.be.ok();
       });
     });
-    describe('usage of discover:searchOnPageLoad', () => {
-      it('should not fetch data from ES initially when discover:searchOnPageLoad is false', async function () {
-        await kibanaServer.uiSettings.replace({ 'discover:searchOnPageLoad': false });
-        await PageObjects.common.navigateToApp('discover');
-        await PageObjects.header.awaitKibanaChrome();
-
-        expect(await PageObjects.discover.getNrOfFetches()).to.be(0);
-      });
-
-      it('should fetch data from ES initially when discover:searchOnPageLoad is true', async function () {
-        await kibanaServer.uiSettings.replace({ 'discover:searchOnPageLoad': true });
-        await PageObjects.common.navigateToApp('discover');
-        await PageObjects.header.awaitKibanaChrome();
-        await retry.waitFor('number of fetches to be 1', async () => {
-          const nrOfFetches = await PageObjects.discover.getNrOfFetches();
-          return nrOfFetches === 1;
-        });
-      });
-    });
 
     describe('invalid time range in URL', function () {
       it('should get the default timerange', async function () {
-        const prevTime = await PageObjects.timePicker.getTimeConfig();
         await PageObjects.common.navigateToUrl('discover', '#/?_g=(time:(from:now-15m,to:null))', {
           useActualUrl: true,
         });
         await PageObjects.header.awaitKibanaChrome();
         const time = await PageObjects.timePicker.getTimeConfig();
-        expect(time.start).to.be(prevTime.start);
-        expect(time.end).to.be(prevTime.end);
+        expect(time.start).to.be('~ 15 minutes ago');
+        expect(time.end).to.be('now');
       });
     });
 

test/functional/apps/discover/_saved_queries.ts

@@ -17,39 +17,83 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
   const kibanaServer = getService('kibanaServer');
   const PageObjects = getPageObjects(['common', 'discover', 'timePicker']);
   const browser = getService('browser');
-
-  const defaultSettings = {
-    defaultIndex: 'logstash-*',
-  };
   const filterBar = getService('filterBar');
   const queryBar = getService('queryBar');
   const savedQueryManagementComponent = getService('savedQueryManagementComponent');
   const testSubjects = getService('testSubjects');
+  const defaultSettings = {
+    defaultIndex: 'logstash-*',
+  };
+
+  const setUpQueriesWithFilters = async () => {
+    // set up a query with filters and a time filter
+    log.debug('set up a query with filters to save');
+    const fromTime = 'Sep 20, 2015 @ 08:00:00.000';
+    const toTime = 'Sep 21, 2015 @ 08:00:00.000';
+    await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
+    await filterBar.addFilter('extension.raw', 'is one of', 'jpg');
+    await queryBar.setQuery('response:200');
+  };
 
   describe('saved queries saved objects', function describeIndexTests() {
     before(async function () {
       log.debug('load kibana index with default index pattern');
       await kibanaServer.savedObjects.clean({ types: ['search', 'index-pattern'] });
-      await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover.json');
 
-      // and load a set of makelogs data
-      await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
+      await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover.json');
+      await esArchiver.load('test/functional/fixtures/es_archiver/date_nested');
+      await esArchiver.load('test/functional/fixtures/es_archiver/logstash_functional');
+
       await kibanaServer.uiSettings.replace(defaultSettings);
       log.debug('discover');
       await PageObjects.common.navigateToApp('discover');
       await PageObjects.timePicker.setDefaultAbsoluteRange();
     });
 
-    describe('saved query management component functionality', function () {
-      before(async function () {
-        // set up a query with filters and a time filter
-        log.debug('set up a query with filters to save');
-        await queryBar.setQuery('response:200');
-        await filterBar.addFilter('extension.raw', 'is one of', 'jpg');
-        const fromTime = 'Sep 20, 2015 @ 08:00:00.000';
-        const toTime = 'Sep 21, 2015 @ 08:00:00.000';
-        await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
+    after(async () => {
+      await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover');
+      await esArchiver.unload('test/functional/fixtures/es_archiver/date_nested');
+      await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional');
+    });
+
+    describe('saved query selection', () => {
+      before(async () => await setUpQueriesWithFilters());
+
+      it(`should unselect saved query when navigating to a 'new'`, async function () {
+        await savedQueryManagementComponent.saveNewQuery(
+          'test-unselect-saved-query',
+          'mock',
+          true,
+          true
+        );
+
+        await queryBar.submitQuery();
+
+        expect(await filterBar.hasFilter('extension.raw', 'jpg')).to.be(true);
+        expect(await queryBar.getQueryString()).to.eql('response:200');
+
+        await PageObjects.discover.clickNewSearchButton();
+
+        expect(await filterBar.hasFilter('extension.raw', 'jpg')).to.be(false);
+        expect(await queryBar.getQueryString()).to.eql('');
+
+        await PageObjects.discover.selectIndexPattern('date-nested');
+
+        expect(await filterBar.hasFilter('extension.raw', 'jpg')).to.be(false);
+        expect(await queryBar.getQueryString()).to.eql('');
+
+        await PageObjects.discover.selectIndexPattern('logstash-*');
+
+        expect(await filterBar.hasFilter('extension.raw', 'jpg')).to.be(false);
+        expect(await queryBar.getQueryString()).to.eql('');
+
+        // reset state
+        await savedQueryManagementComponent.deleteSavedQuery('test-unselect-saved-query');
       });
+    });
+
+    describe('saved query management component functionality', function () {
+      before(async () => await setUpQueriesWithFilters());
 
       it('should show the saved query management component when there are no saved queries', async () => {
         await savedQueryManagementComponent.openSavedQueryManagementComponent();

test/functional/apps/discover/_search_on_page_load.ts

@@ -0,0 +1,112 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import expect from '@kbn/expect';
+
+import { FtrProviderContext } from '../../ftr_provider_context';
+
+export default function ({ getService, getPageObjects }: FtrProviderContext) {
+  const log = getService('log');
+  const retry = getService('retry');
+  const esArchiver = getService('esArchiver');
+  const queryBar = getService('queryBar');
+  const kibanaServer = getService('kibanaServer');
+  const PageObjects = getPageObjects(['common', 'discover', 'header', 'timePicker']);
+  const testSubjects = getService('testSubjects');
+
+  const defaultSettings = {
+    defaultIndex: 'logstash-*',
+  };
+
+  const initSearchOnPageLoad = async (searchOnPageLoad: boolean) => {
+    await kibanaServer.uiSettings.replace({ 'discover:searchOnPageLoad': searchOnPageLoad });
+    await PageObjects.common.navigateToApp('discover');
+    await PageObjects.header.awaitKibanaChrome();
+  };
+
+  const waitForFetches = (fetchesNumber: number) => async () => {
+    const nrOfFetches = await PageObjects.discover.getNrOfFetches();
+    return nrOfFetches === fetchesNumber;
+  };
+
+  describe('usage of discover:searchOnPageLoad', () => {
+    before(async function () {
+      log.debug('load kibana index with default index pattern');
+
+      await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover.json');
+
+      // and load a set of data
+      await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
+      await esArchiver.load('test/functional/fixtures/es_archiver/date_nested');
+
+      await kibanaServer.uiSettings.replace(defaultSettings);
+      await PageObjects.common.navigateToApp('discover');
+    });
+
+    after(async () => {
+      await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover');
+      await esArchiver.load('test/functional/fixtures/es_archiver/date_nested');
+      await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional');
+    });
+
+    describe(`when it's false`, () => {
+      beforeEach(async () => await initSearchOnPageLoad(false));
+
+      it('should not fetch data from ES initially', async function () {
+        expect(await testSubjects.exists('refreshDataButton')).to.be(true);
+        await retry.waitFor('number of fetches to be 0', waitForFetches(0));
+      });
+
+      it('should not fetch on indexPattern change', async function () {
+        expect(await testSubjects.exists('refreshDataButton')).to.be(true);
+        await retry.waitFor('number of fetches to be 0', waitForFetches(0));
+
+        await PageObjects.discover.selectIndexPattern('date-nested');
+
+        expect(await testSubjects.exists('refreshDataButton')).to.be(true);
+        await retry.waitFor('number of fetches to be 0', waitForFetches(0));
+      });
+
+      it('should fetch data from ES after refreshDataButton click', async function () {
+        expect(await testSubjects.exists('refreshDataButton')).to.be(true);
+        await retry.waitFor('number of fetches to be 0', waitForFetches(0));
+
+        await testSubjects.click('refreshDataButton');
+
+        await retry.waitFor('number of fetches to be 1', waitForFetches(1));
+        expect(await testSubjects.exists('refreshDataButton')).to.be(false);
+      });
+
+      it('should fetch data from ES after submit query', async function () {
+        expect(await testSubjects.exists('refreshDataButton')).to.be(true);
+        await retry.waitFor('number of fetches to be 0', waitForFetches(0));
+
+        await queryBar.submitQuery();
+
+        await retry.waitFor('number of fetches to be 1', waitForFetches(1));
+        expect(await testSubjects.exists('refreshDataButton')).to.be(false);
+      });
+
+      it('should fetch data from ES after choosing commonly used time range', async function () {
+        await PageObjects.discover.selectIndexPattern('logstash-*');
+        expect(await testSubjects.exists('refreshDataButton')).to.be(true);
+        await retry.waitFor('number of fetches to be 0', waitForFetches(0));
+
+        await PageObjects.timePicker.setCommonlyUsedTime('This_week');
+
+        await retry.waitFor('number of fetches to be 1', waitForFetches(1));
+        expect(await testSubjects.exists('refreshDataButton')).to.be(false);
+      });
+    });
+
+    it(`when it's false should fetch data from ES initially`, async function () {
+      await initSearchOnPageLoad(true);
+      await retry.waitFor('number of fetches to be 1', waitForFetches(1));
+    });
+  });
+}

test/functional/apps/discover/index.ts

@@ -51,5 +51,6 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) {
     loadTestFile(require.resolve('./_runtime_fields_editor'));
     loadTestFile(require.resolve('./_huge_fields'));
     loadTestFile(require.resolve('./_date_nested'));
+    loadTestFile(require.resolve('./_search_on_page_load'));
   });
 }

x-pack/test/functional/apps/discover/saved_searches.ts

@@ -16,16 +16,31 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
   const dataGrid = getService('dataGrid');
   const panelActions = getService('dashboardPanelActions');
   const panelActionsTimeRange = getService('dashboardPanelTimeRange');
+  const queryBar = getService('queryBar');
+  const filterBar = getService('filterBar');
   const ecommerceSOPath = 'x-pack/test/functional/fixtures/kbn_archiver/reporting/ecommerce.json';
+  const defaultSettings = {
+    defaultIndex: 'logstash-*',
+    'doc_table:legacy': false,
+  };
+
+  const setTimeRange = async () => {
+    const fromTime = 'Apr 27, 2019 @ 23:56:51.374';
+    const toTime = 'Aug 23, 2019 @ 16:18:51.821';
+    await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
+  };
 
   describe('Discover Saved Searches', () => {
     before('initialize tests', async () => {
       await esArchiver.load('x-pack/test/functional/es_archives/reporting/ecommerce');
+      await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover');
       await kibanaServer.importExport.load(ecommerceSOPath);
-      await kibanaServer.uiSettings.update({ 'doc_table:legacy': false });
+      await kibanaServer.uiSettings.update(defaultSettings);
     });
+
     after('clean up archives', async () => {
       await esArchiver.unload('x-pack/test/functional/es_archives/reporting/ecommerce');
+      await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover');
       await kibanaServer.importExport.unload(ecommerceSOPath);
       await kibanaServer.uiSettings.unset('doc_table:legacy');
     });
@@ -34,9 +49,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       it('should be possible to customize time range for saved searches on dashboards', async () => {
         await PageObjects.common.navigateToApp('dashboard');
         await PageObjects.dashboard.clickNewDashboard();
-        const fromTime = 'Apr 27, 2019 @ 23:56:51.374';
-        const toTime = 'Aug 23, 2019 @ 16:18:51.821';
-        await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
+        await setTimeRange();
         await dashboardAddPanel.clickOpenAddPanel();
         await dashboardAddPanel.addSavedSearch('Ecommerce Data');
         expect(await dataGrid.getDocCount()).to.be(500);
@@ -49,5 +62,35 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         expect(await dataGrid.hasNoResults()).to.be(true);
       });
     });
+
+    it(`should unselect saved search when navigating to a 'new'`, async function () {
+      await PageObjects.common.navigateToApp('discover');
+      await PageObjects.discover.selectIndexPattern('ecommerce');
+      await setTimeRange();
+      await filterBar.addFilter('category', 'is', `Men's Shoes`);
+      await queryBar.setQuery('customer_gender:MALE');
+
+      await PageObjects.discover.saveSearch('test-unselect-saved-search');
+
+      await queryBar.submitQuery();
+
+      expect(await filterBar.hasFilter('category', `Men's Shoes`)).to.be(true);
+      expect(await queryBar.getQueryString()).to.eql('customer_gender:MALE');
+
+      await PageObjects.discover.clickNewSearchButton();
+
+      expect(await filterBar.hasFilter('category', `Men's Shoes`)).to.be(false);
+      expect(await queryBar.getQueryString()).to.eql('');
+
+      await PageObjects.discover.selectIndexPattern('logstash-*');
+
+      expect(await filterBar.hasFilter('category', `Men's Shoes`)).to.be(false);
+      expect(await queryBar.getQueryString()).to.eql('');
+
+      await PageObjects.discover.selectIndexPattern('ecommerce');
+
+      expect(await filterBar.hasFilter('category', `Men's Shoes`)).to.be(false);
+      expect(await queryBar.getQueryString()).to.eql('');
+    });
   });
 }