[Discover] Disable refresh interval for data views without time fields and rollups (#137134)

* [Discover] Fix refresh interval running for non time series data

* [Discover] Clean up disable auto refresh interval code, and fix issue where going back and forward through history was not updating data view

* [Discover] Clean up discover_state tests

* [Discover] Add tests for disable auto refresh

* [Discover] Add comments for state syncing code

src/plugins/discover/public/application/main/hooks/use_discover_state.ts

@@ -9,6 +9,7 @@ import { useMemo, useEffect, useState, useCallback } from 'react';
 import usePrevious from 'react-use/lib/usePrevious';
 import { isEqual } from 'lodash';
 import { History } from 'history';
+import { DataViewType } from '@kbn/data-views-plugin/public';
 import {
   isOfAggregateQueryType,
   getIndexPatternFromSQLQuery,
@@ -257,6 +258,19 @@ export function useDiscoverState({
     }
   }, [initialFetchStatus, refetch$, indexPattern, savedSearch.id]);
 
+  /**
+   * We need to make sure the auto refresh interval is disabled for
+   * non-time series data or rollups since we don't show the date picker
+   */
+  useEffect(() => {
+    if (
+      indexPattern &&
+      (!indexPattern.isTimeBased() || indexPattern.type === DataViewType.ROLLUP)
+    ) {
+      stateContainer.pauseAutoRefreshInterval();
+    }
+  }, [indexPattern, stateContainer]);
+
   const getResultColumns = useCallback(() => {
     if (documentState.result?.length && documentState.fetchStatus === FetchStatus.COMPLETE) {
       const firstRow = documentState.result[0];

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

@@ -26,6 +26,8 @@ const uiSettingsMock = {
 } as IUiSettingsClient;
 
 describe('Test discover state', () => {
+  let stopSync = () => {};
+
   beforeEach(async () => {
     history = createBrowserHistory();
     history.push('/');
@@ -35,10 +37,11 @@ describe('Test discover state', () => {
       uiSettings: uiSettingsMock,
     });
     await state.replaceUrlAppState({});
-    await state.startSync();
+    stopSync = state.startSync();
   });
   afterEach(() => {
-    state.stopSync();
+    stopSync();
+    stopSync = () => {};
   });
   test('setting app state and syncing to URL', async () => {
     state.setAppState({ index: 'modified' });
@@ -77,6 +80,13 @@ describe('Test discover state', () => {
     state.setAppState({ index: 'second' });
     expect(state.getPreviousAppState()).toEqual(stateA);
   });
+
+  test('pauseAutoRefreshInterval sets refreshInterval.pause to true', async () => {
+    history.push('/#?_g=(refreshInterval:(pause:!f,value:5000))');
+    expect(getCurrentUrl()).toBe('/#?_g=(refreshInterval:(pause:!f,value:5000))');
+    await state.pauseAutoRefreshInterval();
+    expect(getCurrentUrl()).toBe('/#?_g=(refreshInterval:(pause:!t,value:5000))');
+  });
 });
 describe('Test discover initial state sort handling', () => {
   test('Non-empty sort in URL should not fallback to state defaults', async () => {
@@ -89,7 +99,7 @@ describe('Test discover initial state sort handling', () => {
       uiSettings: uiSettingsMock,
     });
     await state.replaceUrlAppState({});
-    await state.startSync();
+    const stopSync = state.startSync();
     expect(state.appStateContainer.getState().sort).toMatchInlineSnapshot(`
       Array [
         Array [
@@ -98,6 +108,7 @@ describe('Test discover initial state sort handling', () => {
         ],
       ]
     `);
+    stopSync();
   });
   test('Empty sort in URL should allow fallback state defaults', async () => {
     history = createBrowserHistory();
@@ -109,7 +120,7 @@ describe('Test discover initial state sort handling', () => {
       uiSettings: uiSettingsMock,
     });
     await state.replaceUrlAppState({});
-    await state.startSync();
+    const stopSync = state.startSync();
     expect(state.appStateContainer.getState().sort).toMatchInlineSnapshot(`
       Array [
         Array [
@@ -118,6 +129,7 @@ describe('Test discover initial state sort handling', () => {
         ],
       ]
     `);
+    stopSync();
   });
 });
 

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

@@ -31,6 +31,7 @@ import {
   connectToQueryState,
   DataPublicPluginStart,
   FilterManager,
+  QueryState,
   SearchSessionInfoProvider,
   syncQueryStateWithUrl,
 } from '@kbn/data-plugin/public';
@@ -149,13 +150,9 @@ export interface GetStateReturn {
     data: DataPublicPluginStart
   ) => () => void;
   /**
-   * Start sync between state and URL
+   * Start sync between state and URL -- only used for testing
    */
-  startSync: () => void;
-  /**
-   * Stop sync between state and URL
-   */
-  stopSync: () => void;
+  startSync: () => () => void;
   /**
    * Set app state to with a partial new app state
    */
@@ -184,8 +181,14 @@ export interface GetStateReturn {
    * Reset AppState to default, discarding all changes
    */
   resetAppState: () => void;
+  /**
+   * Pause the auto refresh interval without pushing an entry to history
+   */
+  pauseAutoRefreshInterval: () => Promise<void>;
 }
+
 const APP_STATE_URL_KEY = '_a';
+const GLOBAL_STATE_URL_KEY = '_g';
 
 /**
  * Builds and returns appState and globalState containers and helper functions
@@ -229,22 +232,43 @@ export function getState({
     },
   };
 
-  const { start, stop } = syncState({
-    storageKey: APP_STATE_URL_KEY,
-    stateContainer: appStateContainerModified,
-    stateStorage,
-  });
+  // Calling syncState from within initializeAndSync causes state syncing issues.
+  // syncState takes a snapshot of the initial state when it's called to compare
+  // against before syncing state updates. When syncState is called from outside
+  // of initializeAndSync, the snapshot doesn't get reset when the data view is
+  // changed. Then when the user presses the back button, the new state appears
+  // to be the same as the initial state, so syncState ignores the update.
+  const syncAppState = () =>
+    syncState({
+      storageKey: APP_STATE_URL_KEY,
+      stateContainer: appStateContainerModified,
+      stateStorage,
+    });
 
   const replaceUrlAppState = async (newPartial: AppState = {}) => {
     const state = { ...appStateContainer.getState(), ...newPartial };
     await stateStorage.set(APP_STATE_URL_KEY, state, { replace: true });
   };
 
+  const pauseAutoRefreshInterval = async () => {
+    const state = stateStorage.get<QueryState>(GLOBAL_STATE_URL_KEY);
+    if (state?.refreshInterval && !state.refreshInterval.pause) {
+      await stateStorage.set(
+        GLOBAL_STATE_URL_KEY,
+        { ...state, refreshInterval: { ...state?.refreshInterval, pause: true } },
+        { replace: true }
+      );
+    }
+  };
+
   return {
     kbnUrlStateStorage: stateStorage,
     appStateContainer: appStateContainerModified,
-    startSync: start,
-    stopSync: stop,
+    startSync: () => {
+      const { start, stop } = syncAppState();
+      start();
+      return stop;
+    },
     setAppState: (newPartial: AppState) => setState(appStateContainerModified, newPartial),
     replaceUrlAppState,
     resetInitialAppState: () => {
@@ -260,6 +284,7 @@ export function getState({
     getPreviousAppState: () => previousAppState,
     flushToUrl: () => stateStorage.kbnUrlControls.flush(),
     isAppStateDirty: () => !isEqualState(initialAppState, appStateContainer.getState()),
+    pauseAutoRefreshInterval,
     initializeAndSync: (
       indexPattern: DataView,
       filterManager: FilterManager,
@@ -294,6 +319,8 @@ export function getState({
         stateStorage
       );
 
+      const { start, stop } = syncAppState();
+
       replaceUrlAppState({}).then(() => {
         start();
       });

test/functional/apps/discover/_indexpattern_without_timefield.ts

@@ -103,5 +103,16 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         async () => !(await PageObjects.timePicker.timePickerExists())
       );
     });
+
+    it('should disable the auto refresh interval when switching to a data view without a time field', async () => {
+      const autoRefreshInterval = 5;
+      await PageObjects.discover.selectIndexPattern('with-timefield');
+      await PageObjects.timePicker.startAutoRefresh(autoRefreshInterval);
+      let url = await browser.getCurrentUrl();
+      expect(url).to.contain(`refreshInterval:(pause:!f,value:${autoRefreshInterval * 1000})`);
+      await PageObjects.discover.selectIndexPattern('without-timefield');
+      url = await browser.getCurrentUrl();
+      expect(url).to.contain(`refreshInterval:(pause:!t,value:${autoRefreshInterval * 1000})`);
+    });
   });
 }