Skip to content

Commit

Permalink
[ML] Single metric viewer: Fix top nav refresh behaviour. (#44860) (#…
Browse files Browse the repository at this point in the history
…44989)

This PR fixes a regression which didn't stop the date picker refresh when switching from the anomaly detection jobs list to single metric viewer. Includes unit tests which would fail without the fix.
  • Loading branch information
walterra committed Sep 7, 2019
1 parent c3e24ac commit 2d837fa
Show file tree
Hide file tree
Showing 5 changed files with 242 additions and 14 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* 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 { mount, shallow } from 'enzyme';
import React from 'react';

import { uiTimefilterMock } from '../../../contexts/ui/__mocks__/mocks';
import { mlTimefilterRefresh$ } from '../../../services/timefilter_refresh_service';

import { MlSuperDatePickerWithUpdate, TopNav } from './top_nav';

uiTimefilterMock.enableAutoRefreshSelector();
uiTimefilterMock.enableTimeRangeSelector();

jest.mock('../../../contexts/ui/use_ui_context');

const noop = () => {};

describe('Navigation Menu: <TopNav />', () => {
beforeEach(() => {
jest.useFakeTimers();
});

afterEach(() => {
jest.useRealTimers();
});

test('Minimal initialization.', () => {
const refreshListener = jest.fn();
const refreshSubscription = mlTimefilterRefresh$.subscribe(refreshListener);

const wrapper = shallow(<TopNav />);
expect(wrapper).toMatchSnapshot();
expect(refreshListener).toBeCalledTimes(0);

refreshSubscription.unsubscribe();
});

// The following tests are written against MlSuperDatePickerWithUpdate
// instead of TopNav. TopNav uses hooks and we cannot writing tests
// with async hook updates yet until React 16.9 is available.

// MlSuperDatePickerWithUpdate fixes an issue with EuiSuperDatePicker
// which didn't make it into Kibana 7.4. We should be able to just
// use EuiSuperDatePicker again once the following PR is in EUI:
// https://github.com/elastic/eui/pull/2298

test('Listen for consecutive super date picker refreshs.', async () => {
const onRefresh = jest.fn();

const componentRefresh = mount(
<MlSuperDatePickerWithUpdate
onTimeChange={noop}
isPaused={false}
onRefresh={onRefresh}
refreshInterval={10}
/>
);

const instanceRefresh = componentRefresh.instance();

jest.advanceTimersByTime(10);
// @ts-ignore
await instanceRefresh.asyncInterval.__pendingFn;
jest.advanceTimersByTime(10);
// @ts-ignore
await instanceRefresh.asyncInterval.__pendingFn;

expect(onRefresh).toBeCalledTimes(2);
});

test('Switching refresh interval to pause should stop onRefresh being called.', async () => {
const onRefresh = jest.fn();

const componentRefresh = mount(
<MlSuperDatePickerWithUpdate
onTimeChange={noop}
isPaused={false}
onRefresh={onRefresh}
refreshInterval={10}
/>
);

const instanceRefresh = componentRefresh.instance();

jest.advanceTimersByTime(10);
// @ts-ignore
await instanceRefresh.asyncInterval.__pendingFn;
componentRefresh.setProps({ isPaused: true, refreshInterval: 0 });
jest.advanceTimersByTime(10);
// @ts-ignore
await instanceRefresh.asyncInterval.__pendingFn;

expect(onRefresh).toBeCalledTimes(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,37 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, Fragment, useState, useEffect } from 'react';
import React, { Component, FC, Fragment, useState, useEffect } from 'react';
import { Subscription } from 'rxjs';
import { EuiSuperDatePicker } from '@elastic/eui';
import { EuiSuperDatePicker, EuiSuperDatePickerProps } from '@elastic/eui';
import { TimeHistory, TimeRange } from 'ui/timefilter';

import { mlTimefilterRefresh$ } from '../../../services/timefilter_refresh_service';
import { useUiContext } from '../../../contexts/ui/use_ui_context';

interface ComponentWithConstructor<T> extends Component {
new (): Component<T>;
}

const MlSuperDatePicker = (EuiSuperDatePicker as any) as ComponentWithConstructor<
EuiSuperDatePickerProps
>;

// This part fixes a problem with EuiSuperDater picker where it would not reflect
// a prop change of isPaused on the internal interval. This fix will be released
// with EUI 13.7.0 but only 13.6.1 without the fix made it into Kibana 7.4 so
// it's copied here.
export class MlSuperDatePickerWithUpdate extends MlSuperDatePicker {
componentDidUpdate = () => {
// @ts-ignore
this.stopInterval();
if (!this.props.isPaused) {
// @ts-ignore
this.startInterval(this.props.refreshInterval);
}
};
}

interface Duration {
start: string;
end: string;
Expand Down Expand Up @@ -96,20 +119,14 @@ export const TopNav: FC = () => {
<Fragment>
{(isAutoRefreshSelectorEnabled || isTimeRangeSelectorEnabled) && (
<div className="mlNavigationMenu__topNav">
<EuiSuperDatePicker
<MlSuperDatePickerWithUpdate
start={time.from}
end={time.to}
isPaused={refreshInterval.pause}
isAutoRefreshOnly={!isTimeRangeSelectorEnabled}
refreshInterval={refreshInterval.value}
onTimeChange={updateFilter}
onRefresh={() => {
// This check is a workaround to catch a bug in EuiSuperDatePicker which
// might not have disabled the refresh interval after a props change.
if (!refreshInterval.pause) {
mlTimefilterRefresh$.next();
}
}}
onRefresh={() => mlTimefilterRefresh$.next()}
onRefreshChange={updateInterval}
recentlyUsedRanges={recentlyUsedRanges}
dateFormat={dateFormat}
Expand Down
41 changes: 37 additions & 4 deletions x-pack/legacy/plugins/ml/public/contexts/ui/__mocks__/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const uiChromeMock = {
get: (key: string) => {
switch (key) {
case 'dateFormat':
return {};
return 'MMM D, YYYY @ HH:mm:ss.SSS';
case 'theme:darkMode':
return false;
case 'timepicker:timeDefaults':
Expand All @@ -26,12 +26,45 @@ export const uiChromeMock = {
},
};

interface RefreshInterval {
value: number;
pause: boolean;
}

const time = {
from: 'Thu Aug 29 2019 02:04:19 GMT+0200',
to: 'Sun Sep 29 2019 01:45:36 GMT+0200',
};

export const uiTimefilterMock = {
getRefreshInterval: () => '30s',
getTime: () => ({ from: 0, to: 0 }),
enableAutoRefreshSelector() {
this.isAutoRefreshSelectorEnabled = true;
},
enableTimeRangeSelector() {
this.isTimeRangeSelectorEnabled = true;
},
getEnabledUpdated$() {
return { subscribe: jest.fn() };
},
getRefreshInterval() {
return this.refreshInterval;
},
getRefreshIntervalUpdate$() {
return { subscribe: jest.fn() };
},
getTime: () => time,
getTimeUpdate$() {
return { subscribe: jest.fn() };
},
isAutoRefreshSelectorEnabled: false,
isTimeRangeSelectorEnabled: false,
refreshInterval: { value: 0, pause: true },
on: (event: string, reload: () => void) => {},
setRefreshInterval(refreshInterval: RefreshInterval) {
this.refreshInterval = refreshInterval;
},
};

export const uiTimeHistoryMock = {
get: () => [{ from: 0, to: 0 }],
get: () => [time],
};
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,10 @@ export class TimeSeriesExplorer extends React.Component {
}

refresh = () => {
if (this.state.loading) {
return;
}

const { appStateHandler, timefilter } = this.props;
const {
detectorId: currentDetectorId,
Expand Down

0 comments on commit 2d837fa

Please sign in to comment.