[ML] Fix the browser's history navigation (#80902)

* [ML] fix data picker and refresh state * [ML] fix default pause value * [ML] fix jest test * [ML] global state as a source of truth for refreshInterval
elastic · Oct 19, 2020 · 3d4f748 · 3d4f748
1 parent 5b1eb63
commit 3d4f748
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 45 deletions.
diff --git a/...ugins/ml/public/application/components/annotations/annotations_table/annotations_table.js b/...ugins/ml/public/application/components/annotations/annotations_table/annotations_table.js
@@ -273,7 +273,7 @@ class AnnotationsTableUI extends Component {
         timeRange,
         refreshInterval: {
           display: 'Off',
-          pause: false,
+          pause: true,
           value: 0,
         },
         jobIds: [job.job_id],

diff --git a/x-pack/plugins/ml/public/application/components/anomalies_table/links_menu.js b/x-pack/plugins/ml/public/application/components/anomalies_table/links_menu.js
@@ -191,7 +191,7 @@ class LinksMenuUI extends Component {
         jobIds: [record.job_id],
         refreshInterval: {
           display: 'Off',
-          pause: false,
+          pause: true,
           value: 0,
         },
         timeRange: {
@@ -307,7 +307,7 @@ class LinksMenuUI extends Component {
           const _g = rison.encode({
             refreshInterval: {
               display: 'Off',
-              pause: false,
+              pause: true,
               value: 0,
             },
             time: {

diff --git a/...public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx b/...public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { FC, Fragment, useState, useEffect } from 'react';
+import React, { FC, Fragment, useState, useEffect, useCallback } from 'react';
 import { Subscription } from 'rxjs';
 import { EuiSuperDatePicker, OnRefreshProps } from '@elastic/eui';
 import { TimeRange, TimeHistoryContract } from 'src/plugins/data/public';
@@ -48,13 +48,15 @@ export const DatePickerWrapper: FC = () => {
   const [globalState, setGlobalState] = useUrlState('_g');
   const getRecentlyUsedRanges = getRecentlyUsedRangesFactory(history);
 
-  const [refreshInterval, setRefreshInterval] = useState<RefreshInterval>(
-    globalState?.refreshInterval ?? timefilter.getRefreshInterval()
+  const refreshInterval: RefreshInterval =
+    globalState?.refreshInterval ?? timefilter.getRefreshInterval();
+
+  const setRefreshInterval = useCallback(
+    (refreshIntervalUpdate: RefreshInterval) => {
+      setGlobalState('refreshInterval', refreshIntervalUpdate);
+    },
+    [setGlobalState]
   );
-  useEffect(() => {
-    setGlobalState({ refreshInterval });
-    timefilter.setRefreshInterval(refreshInterval);
-  }, [refreshInterval?.pause, refreshInterval?.value, setGlobalState]);
 
   const [time, setTime] = useState(timefilter.getTime());
   const [recentlyUsedRanges, setRecentlyUsedRanges] = useState(getRecentlyUsedRanges());
@@ -71,34 +73,35 @@ export const DatePickerWrapper: FC = () => {
     const subscriptions = new Subscription();
     const refreshIntervalUpdate$ = timefilter.getRefreshIntervalUpdate$();
     if (refreshIntervalUpdate$ !== undefined) {
-      subscriptions.add(refreshIntervalUpdate$.subscribe(timefilterUpdateListener));
+      subscriptions.add(
+        refreshIntervalUpdate$.subscribe((r) => {
+          setRefreshInterval(timefilter.getRefreshInterval());
+        })
+      );
     }
     const timeUpdate$ = timefilter.getTimeUpdate$();
     if (timeUpdate$ !== undefined) {
-      subscriptions.add(timeUpdate$.subscribe(timefilterUpdateListener));
+      subscriptions.add(
+        timeUpdate$.subscribe((v) => {
+          setTime(timefilter.getTime());
+        })
+      );
     }
     const enabledUpdated$ = timefilter.getEnabledUpdated$();
     if (enabledUpdated$ !== undefined) {
-      subscriptions.add(enabledUpdated$.subscribe(timefilterUpdateListener));
+      subscriptions.add(
+        enabledUpdated$.subscribe((w) => {
+          setIsAutoRefreshSelectorEnabled(timefilter.isAutoRefreshSelectorEnabled());
+          setIsTimeRangeSelectorEnabled(timefilter.isTimeRangeSelectorEnabled());
+        })
+      );
     }
 
     return function cleanup() {
       subscriptions.unsubscribe();
     };
   }, []);
 
-  useEffect(() => {
-    // Force re-render with up-to-date values when isTimeRangeSelectorEnabled/isAutoRefreshSelectorEnabled are changed.
-    timefilterUpdateListener();
-  }, [isTimeRangeSelectorEnabled, isAutoRefreshSelectorEnabled]);
-
-  function timefilterUpdateListener() {
-    setTime(timefilter.getTime());
-    setRefreshInterval(timefilter.getRefreshInterval());
-    setIsAutoRefreshSelectorEnabled(timefilter.isAutoRefreshSelectorEnabled());
-    setIsTimeRangeSelectorEnabled(timefilter.isTimeRangeSelectorEnabled());
-  }
-
   function updateFilter({ start, end }: Duration) {
     const newTime = { from: start, to: end };
     // Update timefilter for controllers listening for changes

diff --git a/...blic/application/jobs/jobs_list/components/job_details/forecasts_table/forecasts_table.js b/...blic/application/jobs/jobs_list/components/job_details/forecasts_table/forecasts_table.js
@@ -136,7 +136,7 @@ export class ForecastsTableUI extends Component {
         },
         refreshInterval: {
           display: 'Off',
-          pause: false,
+          pause: true,
           value: 0,
         },
         jobIds: [this.props.job.job_id],

diff --git a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx
@@ -98,7 +98,7 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
 
   const refresh = useRefresh();
   useEffect(() => {
-    if (refresh !== undefined) {
+    if (refresh !== undefined && refresh.lastRefresh !== lastRefresh) {
       setLastRefresh(refresh?.lastRefresh);
 
       if (refresh.timeRange !== undefined) {
@@ -109,7 +109,7 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
         });
       }
     }
-  }, [refresh?.lastRefresh]);
+  }, [refresh?.lastRefresh, lastRefresh, setGlobalState]);
 
   // We cannot simply infer bounds from the globalState's `time` attribute
   // with `moment` since it can contain custom strings such as `now-15m`.
@@ -147,18 +147,6 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
   const previousSelectedJobIds = usePrevious(selectedJobIds);
   const isJobChange = !isEqual(previousSelectedJobIds, selectedJobIds);
 
-  // Use a side effect to clear appState when changing jobs.
-  useEffect(() => {
-    if (selectedJobIds !== undefined && previousSelectedJobIds !== undefined) {
-      setLastRefresh(Date.now());
-      appStateHandler(APP_STATE_ACTION.CLEAR);
-    }
-    const validatedJobId = validateJobSelection(jobsWithTimeRange, selectedJobIds, setGlobalState);
-    if (typeof validatedJobId === 'string') {
-      setSelectedJobId(validatedJobId);
-    }
-  }, [JSON.stringify(selectedJobIds)]);
-
   // Next we get globalState and appState information to pass it on as props later.
   // If a job change is going on, we fall back to defaults (as if appState was already cleared),
   // otherwise the page could break.
@@ -216,9 +204,21 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
 
       setAppState('mlTimeSeriesExplorer', mlTimeSeriesExplorer);
     },
-    [JSON.stringify([appState, globalState])]
+    [JSON.stringify(appState?.mlTimeSeriesExplorer), setAppState]
   );
 
+  // Use a side effect to clear appState when changing jobs.
+  useEffect(() => {
+    if (selectedJobIds !== undefined && previousSelectedJobIds !== undefined) {
+      setLastRefresh(Date.now());
+      appStateHandler(APP_STATE_ACTION.CLEAR);
+    }
+    const validatedJobId = validateJobSelection(jobsWithTimeRange, selectedJobIds, setGlobalState);
+    if (typeof validatedJobId === 'string') {
+      setSelectedJobId(validatedJobId);
+    }
+  }, [JSON.stringify(selectedJobIds)]);
+
   const boundsMinMs = bounds?.min?.valueOf();
   const boundsMaxMs = bounds?.max?.valueOf();
 

diff --git a/x-pack/plugins/ml/public/application/services/job_service.js b/x-pack/plugins/ml/public/application/services/job_service.js
@@ -805,7 +805,7 @@ function createResultsUrl(jobIds, start, end, resultsPage, mode = 'absolute') {
   }
 
   path += `?_g=(ml:(jobIds:!(${idString}))`;
-  path += `,refreshInterval:(display:Off,pause:!f,value:0),time:(from:'${from}'`;
+  path += `,refreshInterval:(display:Off,pause:!t,value:0),time:(from:'${from}'`;
   path += `,to:'${to}'`;
   if (mode === 'invalid') {
     path += `,mode:invalid`;

diff --git a/x-pack/plugins/ml/public/application/util/chart_utils.js b/x-pack/plugins/ml/public/application/util/chart_utils.js
@@ -239,7 +239,7 @@ export async function getExploreSeriesLink(mlUrlGenerator, series) {
       jobIds: [series.jobId],
       refreshInterval: {
         display: 'Off',
-        pause: false,
+        pause: true,
         value: 0,
       },
       timeRange: {

diff --git a/x-pack/plugins/ml/public/application/util/url_state.test.tsx b/x-pack/plugins/ml/public/application/util/url_state.test.tsx
@@ -24,7 +24,7 @@ describe('getUrlState', () => {
   test('properly decode url with _g and _a', () => {
     expect(
       parseUrlState(
-        "?_a=(mlExplorerFilter:(),mlExplorerSwimlane:(viewByFieldName:action),query:(query_string:(analyze_wildcard:!t,query:'*')))&_g=(ml:(jobIds:!(dec-2)),refreshInterval:(display:Off,pause:!f,value:0),time:(from:'2019-01-01T00:03:40.000Z',mode:absolute,to:'2019-08-30T11:55:07.000Z'))&savedSearchId=571aaf70-4c88-11e8-b3d7-01146121b73d"
+        "?_a=(mlExplorerFilter:(),mlExplorerSwimlane:(viewByFieldName:action),query:(query_string:(analyze_wildcard:!t,query:'*')))&_g=(ml:(jobIds:!(dec-2)),refreshInterval:(display:Off,pause:!t,value:0),time:(from:'2019-01-01T00:03:40.000Z',mode:absolute,to:'2019-08-30T11:55:07.000Z'))&savedSearchId=571aaf70-4c88-11e8-b3d7-01146121b73d"
       )
     ).toEqual({
       _a: {
@@ -45,7 +45,7 @@ describe('getUrlState', () => {
         },
         refreshInterval: {
           display: 'Off',
-          pause: false,
+          pause: true,
           value: 0,
         },
         time: {