Skip to content

Commit f13adfa

Browse files
authored
[ML] Single Metric Viewer: Fix time bounds with custom strings. (#55045)
Makes sure to set bounds via timefilter.getBounds() again and not infer directly from globalState to correctly consider custom strings like now-15m.
1 parent 3e46060 commit f13adfa

File tree

6 files changed

+85
-24
lines changed

6 files changed

+85
-24
lines changed

x-pack/legacy/plugins/ml/public/application/components/navigation_menu/top_nav/top_nav.tsx

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ interface Duration {
2323

2424
function getRecentlyUsedRangesFactory(timeHistory: TimeHistory) {
2525
return function(): Duration[] {
26-
return timeHistory.get().map(({ from, to }: TimeRange) => {
27-
return {
28-
start: from,
29-
end: to,
30-
};
31-
});
26+
return (
27+
timeHistory.get()?.map(({ from, to }: TimeRange) => {
28+
return {
29+
start: from,
30+
end: to,
31+
};
32+
}) ?? []
33+
);
3234
};
3335
}
3436

@@ -54,9 +56,18 @@ export const TopNav: FC = () => {
5456

5557
useEffect(() => {
5658
const subscriptions = new Subscription();
57-
subscriptions.add(timefilter.getRefreshIntervalUpdate$().subscribe(timefilterUpdateListener));
58-
subscriptions.add(timefilter.getTimeUpdate$().subscribe(timefilterUpdateListener));
59-
subscriptions.add(timefilter.getEnabledUpdated$().subscribe(timefilterUpdateListener));
59+
const refreshIntervalUpdate$ = timefilter.getRefreshIntervalUpdate$();
60+
if (refreshIntervalUpdate$ !== undefined) {
61+
subscriptions.add(refreshIntervalUpdate$.subscribe(timefilterUpdateListener));
62+
}
63+
const timeUpdate$ = timefilter.getTimeUpdate$();
64+
if (timeUpdate$ !== undefined) {
65+
subscriptions.add(timeUpdate$.subscribe(timefilterUpdateListener));
66+
}
67+
const enabledUpdated$ = timefilter.getEnabledUpdated$();
68+
if (enabledUpdated$ !== undefined) {
69+
subscriptions.add(enabledUpdated$.subscribe(timefilterUpdateListener));
70+
}
6071

6172
return function cleanup() {
6273
subscriptions.unsubscribe();

x-pack/legacy/plugins/ml/public/application/routing/routes/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ export * from './new_job';
1010
export * from './datavisualizer';
1111
export * from './settings';
1212
export * from './data_frame_analytics';
13-
export * from './timeseriesexplorer';
13+
export { timeSeriesExplorerRoute } from './timeseriesexplorer';
1414
export * from './explorer';
1515
export * from './access_denied';
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import React from 'react';
8+
import { MemoryRouter } from 'react-router-dom';
9+
import { render } from '@testing-library/react';
10+
11+
import { I18nProvider } from '@kbn/i18n/react';
12+
13+
import { TimeSeriesExplorerUrlStateManager } from './timeseriesexplorer';
14+
15+
jest.mock('ui/new_platform');
16+
17+
describe('TimeSeriesExplorerUrlStateManager', () => {
18+
test('Initial render shows "No single metric jobs found"', () => {
19+
const props = {
20+
config: { get: () => 'Browser' },
21+
jobsWithTimeRange: [],
22+
};
23+
24+
const { container } = render(
25+
<I18nProvider>
26+
<MemoryRouter>
27+
<TimeSeriesExplorerUrlStateManager {...props} />
28+
</MemoryRouter>
29+
</I18nProvider>
30+
);
31+
32+
expect(container.textContent).toContain('No single metric jobs found');
33+
});
34+
});

x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ interface TimeSeriesExplorerUrlStateManager {
7474
jobsWithTimeRange: MlJobWithTimeRange[];
7575
}
7676

77-
const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateManager> = ({
77+
export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateManager> = ({
7878
config,
7979
jobsWithTimeRange,
8080
}) => {
@@ -102,23 +102,27 @@ const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateManager> =
102102
timefilter.enableAutoRefreshSelector();
103103
}, []);
104104

105+
// We cannot simply infer bounds from the globalState's `time` attribute
106+
// with `moment` since it can contain custom strings such as `now-15m`.
107+
// So when globalState's `time` changes, we update the timefilter and use
108+
// `timefilter.getBounds()` to update `bounds` in this component's state.
109+
const [bounds, setBounds] = useState<TimeRangeBounds | undefined>(undefined);
105110
useEffect(() => {
106111
if (globalState?.time !== undefined) {
107112
timefilter.setTime({
108113
from: globalState.time.from,
109114
to: globalState.time.to,
110115
});
116+
117+
const timefilterBounds = timefilter.getBounds();
118+
// Only if both min/max bounds are valid moment times set the bounds.
119+
// An invalid string restored from globalState might return `undefined`.
120+
if (timefilterBounds?.min !== undefined && timefilterBounds?.max !== undefined) {
121+
setBounds(timefilter.getBounds());
122+
}
111123
}
112124
}, [globalState?.time?.from, globalState?.time?.to]);
113125

114-
let bounds: TimeRangeBounds | undefined;
115-
if (globalState?.time !== undefined) {
116-
bounds = {
117-
min: moment(globalState.time.from),
118-
max: moment(globalState.time.to),
119-
};
120-
}
121-
122126
const selectedJobIds = globalState?.ml?.jobIds;
123127
// Sort selectedJobIds so we can be sure comparison works when stringifying.
124128
if (Array.isArray(selectedJobIds)) {
@@ -140,14 +144,17 @@ const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateManager> =
140144
}, [JSON.stringify(selectedJobIds)]);
141145

142146
// Next we get globalState and appState information to pass it on as props later.
143-
// If a job change is going on, we fall back to defaults (as if appState was already cleard),
147+
// If a job change is going on, we fall back to defaults (as if appState was already cleared),
144148
// otherwise the page could break.
145149
const selectedDetectorIndex = isJobChange
146150
? 0
147151
: +appState?.mlTimeSeriesExplorer?.detectorIndex || 0;
148152
const selectedEntities = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.entities;
149153
const selectedForecastId = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.forecastId;
150-
const zoom = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.zoom;
154+
const zoom: {
155+
from: string;
156+
to: string;
157+
} = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.zoom;
151158

152159
const selectedJob = selectedJobIds && mlJobService.getJob(selectedJobIds[0]);
153160

x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,25 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { Timefilter } from 'ui/timefilter';
87
import { FC } from 'react';
98

9+
import { Timefilter } from 'ui/timefilter';
10+
11+
import { getDateFormatTz, TimeRangeBounds } from '../explorer/explorer_utils';
12+
1013
declare const TimeSeriesExplorer: FC<{
1114
appStateHandler: (action: string, payload: any) => void;
15+
autoZoomDuration?: number;
16+
bounds?: TimeRangeBounds;
1217
dateFormatTz: string;
18+
jobsWithTimeRange: any[];
19+
lastRefresh: number;
1320
selectedJobIds: string[];
1421
selectedDetectorIndex: number;
1522
selectedEntities: any[];
1623
selectedForecastId: string;
1724
setGlobalState: (arg: any) => void;
1825
tableInterval: string;
1926
tableSeverity: number;
20-
timefilter: Timefilter;
27+
zoom?: { from: string; to: string };
2128
}>;

x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,10 @@ export class TimeSeriesExplorer extends React.Component {
195195
selectedDetectorIndex: PropTypes.number,
196196
selectedEntities: PropTypes.object,
197197
selectedForecastId: PropTypes.string,
198+
setGlobalState: PropTypes.func.isRequired,
198199
tableInterval: PropTypes.string,
199200
tableSeverity: PropTypes.number,
201+
zoom: PropTypes.object,
200202
};
201203

202204
state = getTimeseriesexplorerDefaultState();
@@ -481,7 +483,7 @@ export class TimeSeriesExplorer extends React.Component {
481483
zoom,
482484
} = this.props;
483485

484-
if (selectedJobIds === undefined) {
486+
if (selectedJobIds === undefined || bounds === undefined) {
485487
return;
486488
}
487489

0 commit comments

Comments
 (0)