From 7e12bf129d77301a9509bfbd04dd9c5e487ae721 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Fri, 16 Oct 2020 20:47:39 +0200 Subject: [PATCH 1/4] [ML] fix data picker and refresh state --- .../date_picker_wrapper.tsx | 46 ++++++++++++------- .../routing/routes/timeseriesexplorer.tsx | 30 ++++++------ .../application/services/job_service.js | 2 +- 3 files changed, 46 insertions(+), 32 deletions(-) diff --git a/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx b/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx index 409bd11e0bde3..66d8eec4a1ef6 100644 --- a/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx +++ b/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx @@ -51,10 +51,23 @@ export const DatePickerWrapper: FC = () => { const [refreshInterval, setRefreshInterval] = useState( globalState?.refreshInterval ?? timefilter.getRefreshInterval() ); + useEffect(() => { + if ( + globalState?.refreshInterval?.value === refreshInterval?.value && + globalState?.refreshInterval?.pause === refreshInterval?.pause + ) { + return; + } + setGlobalState({ refreshInterval }); timefilter.setRefreshInterval(refreshInterval); - }, [refreshInterval?.pause, refreshInterval?.value, setGlobalState]); + }, [ + globalState?.refreshInterval, + refreshInterval?.pause, + refreshInterval?.value, + setGlobalState, + ]); const [time, setTime] = useState(timefilter.getTime()); const [recentlyUsedRanges, setRecentlyUsedRanges] = useState(getRecentlyUsedRanges()); @@ -71,15 +84,28 @@ 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() { @@ -87,18 +113,6 @@ export const DatePickerWrapper: FC = () => { }; }, []); - 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/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx index 03588872d6be0..e4cf43ac91727 100644 --- 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 { - if (refresh !== undefined) { + if (refresh !== undefined && refresh.lastRefresh !== lastRefresh) { setLastRefresh(refresh?.lastRefresh); if (refresh.timeRange !== undefined) { @@ -109,7 +109,7 @@ export const TimeSeriesExplorerUrlStateManager: FC { - 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 { + 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 index 4aa1f7ef81d59..cdcd4a7ab7327 100644 --- 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`; From e0ddc89103b0621ea23ef3fbee5a19e5eee14732 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Fri, 16 Oct 2020 20:59:19 +0200 Subject: [PATCH 2/4] [ML] fix default pause value --- .../annotations/annotations_table/annotations_table.js | 2 +- .../application/components/anomalies_table/links_menu.js | 4 ++-- .../components/job_details/forecasts_table/forecasts_table.js | 2 +- x-pack/plugins/ml/public/application/util/chart_utils.js | 2 +- x-pack/plugins/ml/public/application/util/url_state.test.tsx | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/ml/public/application/components/annotations/annotations_table/annotations_table.js b/x-pack/plugins/ml/public/application/components/annotations/annotations_table/annotations_table.js index 7eb280c6247c2..100b2afcc97ce 100644 --- a/x-pack/plugins/ml/public/application/components/annotations/annotations_table/annotations_table.js +++ b/x-pack/plugins/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 index 079d56da60e5e..3d968d133ea45 100644 --- 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/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/forecasts_table/forecasts_table.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/forecasts_table/forecasts_table.js index 44ebde634714c..933c7150f44d5 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/forecasts_table/forecasts_table.js +++ b/x-pack/plugins/ml/public/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/util/chart_utils.js b/x-pack/plugins/ml/public/application/util/chart_utils.js index ca55bb10b13d5..f8355b26bdab0 100644 --- 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 index 9c03369648554..949f51373c524 100644 --- a/x-pack/plugins/ml/public/application/util/url_state.test.tsx +++ b/x-pack/plugins/ml/public/application/util/url_state.test.tsx @@ -45,7 +45,7 @@ describe('getUrlState', () => { }, refreshInterval: { display: 'Off', - pause: false, + pause: true, value: 0, }, time: { From fc0b8f0b25d35257b014bdbc0c0d1adcca88270a Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 19 Oct 2020 10:00:51 +0200 Subject: [PATCH 3/4] [ML] fix jest test --- x-pack/plugins/ml/public/application/util/url_state.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 949f51373c524..2921674621360 100644 --- 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: { From 0f407105441f70f98ba3c3e663c48b9d55f79d5a Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 19 Oct 2020 10:30:58 +0200 Subject: [PATCH 4/4] [ML] global state as a source of truth for refreshInterval --- .../date_picker_wrapper.tsx | 29 ++++++------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx b/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx index 66d8eec4a1ef6..7d1a616d57114 100644 --- a/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx +++ b/x-pack/plugins/ml/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,26 +48,15 @@ export const DatePickerWrapper: FC = () => { const [globalState, setGlobalState] = useUrlState('_g'); const getRecentlyUsedRanges = getRecentlyUsedRangesFactory(history); - const [refreshInterval, setRefreshInterval] = useState( - globalState?.refreshInterval ?? timefilter.getRefreshInterval() - ); + const refreshInterval: RefreshInterval = + globalState?.refreshInterval ?? timefilter.getRefreshInterval(); - useEffect(() => { - if ( - globalState?.refreshInterval?.value === refreshInterval?.value && - globalState?.refreshInterval?.pause === refreshInterval?.pause - ) { - return; - } - - setGlobalState({ refreshInterval }); - timefilter.setRefreshInterval(refreshInterval); - }, [ - globalState?.refreshInterval, - refreshInterval?.pause, - refreshInterval?.value, - setGlobalState, - ]); + const setRefreshInterval = useCallback( + (refreshIntervalUpdate: RefreshInterval) => { + setGlobalState('refreshInterval', refreshIntervalUpdate); + }, + [setGlobalState] + ); const [time, setTime] = useState(timefilter.getTime()); const [recentlyUsedRanges, setRecentlyUsedRanges] = useState(getRecentlyUsedRanges());