From 3b2e0948d629624419b621f0f7c2b511c49424b2 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 6 Sep 2019 04:41:36 -0700 Subject: [PATCH] [ML] Single metric viewer: Fix top nav refresh behaviour. (#44860) 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. --- .../__snapshots__/top_nav.test.tsx.snap | 75 ++++++++++++++ .../navigation_menu/top_nav/top_nav.test.tsx | 99 +++++++++++++++++++ .../navigation_menu/top_nav/top_nav.tsx | 37 +++++-- .../ml/public/contexts/ui/__mocks__/mocks.ts | 41 +++++++- .../timeseriesexplorer/timeseriesexplorer.js | 4 + 5 files changed, 242 insertions(+), 14 deletions(-) create mode 100644 x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/__snapshots__/top_nav.test.tsx.snap create mode 100644 x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.test.tsx diff --git a/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/__snapshots__/top_nav.test.tsx.snap b/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/__snapshots__/top_nav.test.tsx.snap new file mode 100644 index 0000000000000..61bba452f49ca --- /dev/null +++ b/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/__snapshots__/top_nav.test.tsx.snap @@ -0,0 +1,75 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Navigation Menu: Minimal initialization. 1`] = ` + +
+ +
+ +`; diff --git a/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.test.tsx b/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.test.tsx new file mode 100644 index 0000000000000..d98077da230c8 --- /dev/null +++ b/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.test.tsx @@ -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: ', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + test('Minimal initialization.', () => { + const refreshListener = jest.fn(); + const refreshSubscription = mlTimefilterRefresh$.subscribe(refreshListener); + + const wrapper = shallow(); + 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( + + ); + + 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( + + ); + + 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); + }); +}); diff --git a/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.tsx b/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.tsx index 0596df80fa449..f32632cd321ae 100644 --- a/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.tsx +++ b/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.tsx @@ -4,15 +4,38 @@ * 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 } from 'ui/timefilter'; import { TimeRange } from 'src/plugins/data/public'; import { mlTimefilterRefresh$ } from '../../../services/timefilter_refresh_service'; import { useUiContext } from '../../../contexts/ui/use_ui_context'; +interface ComponentWithConstructor extends Component { + new (): Component; +} + +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; @@ -97,20 +120,14 @@ export const TopNav: FC = () => { {(isAutoRefreshSelectorEnabled || isTimeRangeSelectorEnabled) && (
- { - // 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} diff --git a/x-pack/legacy/plugins/ml/public/contexts/ui/__mocks__/mocks.ts b/x-pack/legacy/plugins/ml/public/contexts/ui/__mocks__/mocks.ts index 90bc028a7cd37..e6e51f0bdecf4 100644 --- a/x-pack/legacy/plugins/ml/public/contexts/ui/__mocks__/mocks.ts +++ b/x-pack/legacy/plugins/ml/public/contexts/ui/__mocks__/mocks.ts @@ -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': @@ -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], }; diff --git a/x-pack/legacy/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.js b/x-pack/legacy/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.js index db316f67688fe..ab29e0ba303a9 100644 --- a/x-pack/legacy/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.js +++ b/x-pack/legacy/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.js @@ -467,6 +467,10 @@ export class TimeSeriesExplorer extends React.Component { } refresh = () => { + if (this.state.loading) { + return; + } + const { appStateHandler, timefilter } = this.props; const { detectorId: currentDetectorId,