From dd7608a64c42fe5df20033cd2e519c532a349449 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 17 Jan 2020 14:45:06 +0100 Subject: [PATCH] [ML] Single Metric Viewer: Fix time bounds with custom strings. (#55045) (#55163) Makes sure to set bounds via timefilter.getBounds() again and not infer directly from globalState to correctly consider custom strings like now-15m. --- .../navigation_menu/top_nav/top_nav.tsx | 29 +++++++++++----- .../application/routing/routes/index.ts | 2 +- .../routes/timeseriesexplorer.test.tsx | 34 +++++++++++++++++++ .../routing/routes/timeseriesexplorer.tsx | 29 ++++++++++------ .../timeseriesexplorer.d.ts | 11 ++++-- .../timeseriesexplorer/timeseriesexplorer.js | 4 ++- 6 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.test.tsx diff --git a/x-pack/legacy/plugins/ml/public/application/components/navigation_menu/top_nav/top_nav.tsx b/x-pack/legacy/plugins/ml/public/application/components/navigation_menu/top_nav/top_nav.tsx index ca6146f3e23b5..eb068f40716bc 100644 --- a/x-pack/legacy/plugins/ml/public/application/components/navigation_menu/top_nav/top_nav.tsx +++ b/x-pack/legacy/plugins/ml/public/application/components/navigation_menu/top_nav/top_nav.tsx @@ -23,12 +23,14 @@ interface Duration { function getRecentlyUsedRangesFactory(timeHistory: TimeHistory) { return function(): Duration[] { - return timeHistory.get().map(({ from, to }: TimeRange) => { - return { - start: from, - end: to, - }; - }); + return ( + timeHistory.get()?.map(({ from, to }: TimeRange) => { + return { + start: from, + end: to, + }; + }) ?? [] + ); }; } @@ -54,9 +56,18 @@ export const TopNav: FC = () => { useEffect(() => { const subscriptions = new Subscription(); - subscriptions.add(timefilter.getRefreshIntervalUpdate$().subscribe(timefilterUpdateListener)); - subscriptions.add(timefilter.getTimeUpdate$().subscribe(timefilterUpdateListener)); - subscriptions.add(timefilter.getEnabledUpdated$().subscribe(timefilterUpdateListener)); + const refreshIntervalUpdate$ = timefilter.getRefreshIntervalUpdate$(); + if (refreshIntervalUpdate$ !== undefined) { + subscriptions.add(refreshIntervalUpdate$.subscribe(timefilterUpdateListener)); + } + const timeUpdate$ = timefilter.getTimeUpdate$(); + if (timeUpdate$ !== undefined) { + subscriptions.add(timeUpdate$.subscribe(timefilterUpdateListener)); + } + const enabledUpdated$ = timefilter.getEnabledUpdated$(); + if (enabledUpdated$ !== undefined) { + subscriptions.add(enabledUpdated$.subscribe(timefilterUpdateListener)); + } return function cleanup() { subscriptions.unsubscribe(); diff --git a/x-pack/legacy/plugins/ml/public/application/routing/routes/index.ts b/x-pack/legacy/plugins/ml/public/application/routing/routes/index.ts index 89ed35d5588f2..44111ae32cd30 100644 --- a/x-pack/legacy/plugins/ml/public/application/routing/routes/index.ts +++ b/x-pack/legacy/plugins/ml/public/application/routing/routes/index.ts @@ -10,6 +10,6 @@ export * from './new_job'; export * from './datavisualizer'; export * from './settings'; export * from './data_frame_analytics'; -export * from './timeseriesexplorer'; +export { timeSeriesExplorerRoute } from './timeseriesexplorer'; export * from './explorer'; export * from './access_denied'; diff --git a/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.test.tsx b/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.test.tsx new file mode 100644 index 0000000000000..6917ec718d3a8 --- /dev/null +++ b/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.test.tsx @@ -0,0 +1,34 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import { render } from '@testing-library/react'; + +import { I18nProvider } from '@kbn/i18n/react'; + +import { TimeSeriesExplorerUrlStateManager } from './timeseriesexplorer'; + +jest.mock('ui/new_platform'); + +describe('TimeSeriesExplorerUrlStateManager', () => { + test('Initial render shows "No single metric jobs found"', () => { + const props = { + config: { get: () => 'Browser' }, + jobsWithTimeRange: [], + }; + + const { container } = render( + + + + + + ); + + expect(container.textContent).toContain('No single metric jobs found'); + }); +}); diff --git a/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx b/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx index cbf54a70ea74f..c3c644d43fa59 100644 --- a/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx +++ b/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx @@ -74,7 +74,7 @@ interface TimeSeriesExplorerUrlStateManager { jobsWithTimeRange: MlJobWithTimeRange[]; } -const TimeSeriesExplorerUrlStateManager: FC = ({ +export const TimeSeriesExplorerUrlStateManager: FC = ({ config, jobsWithTimeRange, }) => { @@ -102,23 +102,27 @@ const TimeSeriesExplorerUrlStateManager: FC = timefilter.enableAutoRefreshSelector(); }, []); + // We cannot simply infer bounds from the globalState's `time` attribute + // with `moment` since it can contain custom strings such as `now-15m`. + // So when globalState's `time` changes, we update the timefilter and use + // `timefilter.getBounds()` to update `bounds` in this component's state. + const [bounds, setBounds] = useState(undefined); useEffect(() => { if (globalState?.time !== undefined) { timefilter.setTime({ from: globalState.time.from, to: globalState.time.to, }); + + const timefilterBounds = timefilter.getBounds(); + // Only if both min/max bounds are valid moment times set the bounds. + // An invalid string restored from globalState might return `undefined`. + if (timefilterBounds?.min !== undefined && timefilterBounds?.max !== undefined) { + setBounds(timefilter.getBounds()); + } } }, [globalState?.time?.from, globalState?.time?.to]); - let bounds: TimeRangeBounds | undefined; - if (globalState?.time !== undefined) { - bounds = { - min: moment(globalState.time.from), - max: moment(globalState.time.to), - }; - } - const selectedJobIds = globalState?.ml?.jobIds; // Sort selectedJobIds so we can be sure comparison works when stringifying. if (Array.isArray(selectedJobIds)) { @@ -140,14 +144,17 @@ const TimeSeriesExplorerUrlStateManager: FC = }, [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 cleard), + // If a job change is going on, we fall back to defaults (as if appState was already cleared), // otherwise the page could break. const selectedDetectorIndex = isJobChange ? 0 : +appState?.mlTimeSeriesExplorer?.detectorIndex || 0; const selectedEntities = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.entities; const selectedForecastId = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.forecastId; - const zoom = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.zoom; + const zoom: { + from: string; + to: string; + } = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.zoom; const selectedJob = selectedJobIds && mlJobService.getJob(selectedJobIds[0]); diff --git a/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts b/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts index 3edbbc1af2323..651c609004236 100644 --- a/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts +++ b/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts @@ -4,12 +4,19 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Timefilter } from 'ui/timefilter'; import { FC } from 'react'; +import { Timefilter } from 'ui/timefilter'; + +import { getDateFormatTz, TimeRangeBounds } from '../explorer/explorer_utils'; + declare const TimeSeriesExplorer: FC<{ appStateHandler: (action: string, payload: any) => void; + autoZoomDuration?: number; + bounds?: TimeRangeBounds; dateFormatTz: string; + jobsWithTimeRange: any[]; + lastRefresh: number; selectedJobIds: string[]; selectedDetectorIndex: number; selectedEntities: any[]; @@ -17,5 +24,5 @@ declare const TimeSeriesExplorer: FC<{ setGlobalState: (arg: any) => void; tableInterval: string; tableSeverity: number; - timefilter: Timefilter; + zoom?: { from: string; to: string }; }>; diff --git a/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js b/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js index 016f054430fa3..1862ead045743 100644 --- a/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js +++ b/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js @@ -195,8 +195,10 @@ export class TimeSeriesExplorer extends React.Component { selectedDetectorIndex: PropTypes.number, selectedEntities: PropTypes.object, selectedForecastId: PropTypes.string, + setGlobalState: PropTypes.func.isRequired, tableInterval: PropTypes.string, tableSeverity: PropTypes.number, + zoom: PropTypes.object, }; state = getTimeseriesexplorerDefaultState(); @@ -481,7 +483,7 @@ export class TimeSeriesExplorer extends React.Component { zoom, } = this.props; - if (selectedJobIds === undefined) { + if (selectedJobIds === undefined || bounds === undefined) { return; }