From e02ce55c23a8103cf4d6cbc1f6daec739929e7da Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 4 Sep 2019 17:57:22 +0200 Subject: [PATCH 1/4] [ML] Fix top nav refresh behavior. --- .../ml/public/components/navigation_menu/top_nav/top_nav.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 6e38b37c33a24..bcf7607ccfa7f 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 @@ -106,7 +106,8 @@ export const TopNav: FC = () => { 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) { + const newRefreshInterval = timefilter.getRefreshInterval(); + if (!newRefreshInterval.pause) { mlTimefilterRefresh$.next(); } }} From e632c2e4254dae4aa45376e74ee375fd01fdec32 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Thu, 5 Sep 2019 10:56:12 +0200 Subject: [PATCH 2/4] [ML] Adds unit tests. --- .../__snapshots__/top_nav.test.tsx.snap | 75 ++++++++++++++++++ .../navigation_menu/top_nav/top_nav.test.tsx | 77 +++++++++++++++++++ .../ml/public/contexts/ui/__mocks__/mocks.ts | 41 +++++++++- 3 files changed, 189 insertions(+), 4 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..b898558b20738 --- /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..2c0a8ea9f10b3 --- /dev/null +++ b/x-pack/legacy/plugins/ml/public/components/navigation_menu/top_nav/top_nav.test.tsx @@ -0,0 +1,77 @@ +/* + * 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 { TopNav } from './top_nav'; + +uiTimefilterMock.enableAutoRefreshSelector(); +uiTimefilterMock.enableTimeRangeSelector(); + +jest.mock('../../../contexts/ui/use_ui_context'); + +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(); + }); + + test('Listen for super date picker refresh.', done => { + const refreshListener = jest.fn(); + const refreshSubscription = mlTimefilterRefresh$.subscribe(refreshListener); + + uiTimefilterMock.setRefreshInterval({ value: 5, pause: false }); + + mount(); + + setTimeout(() => { + expect(refreshListener).toBeCalledTimes(1); + refreshSubscription.unsubscribe(); + done(); + }, 10); + + jest.runOnlyPendingTimers(); + }); + + test('Switching refresh interval to pause should stop listener being called.', done => { + const refreshListener = jest.fn(); + const refreshSubscription = mlTimefilterRefresh$.subscribe(refreshListener); + + uiTimefilterMock.setRefreshInterval({ value: 5, pause: false }); + + mount(); + + setTimeout(() => { + uiTimefilterMock.setRefreshInterval({ value: 0, pause: true }); + }, 10); + + setTimeout(() => { + expect(refreshListener).toBeCalledTimes(2); + refreshSubscription.unsubscribe(); + done(); + }, 20); + + jest.runOnlyPendingTimers(); + }); +}); 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], }; From c6fd39114a08390de0f608eddc35b2b2dc0b1855 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 6 Sep 2019 09:13:45 +0200 Subject: [PATCH 3/4] [ML] Fixes internal EuiSuperDatePicker interval update after props change. --- .../__snapshots__/top_nav.test.tsx.snap | 2 +- .../navigation_menu/top_nav/top_nav.tsx | 38 +++++++++++++------ .../timeseriesexplorer/timeseriesexplorer.js | 4 ++ 3 files changed, 32 insertions(+), 12 deletions(-) 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 index b898558b20738..61bba452f49ca 100644 --- 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 @@ -5,7 +5,7 @@ exports[`Navigation Menu: Minimal initialization. 1`] = `
- 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. +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; @@ -96,21 +119,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. - const newRefreshInterval = timefilter.getRefreshInterval(); - if (!newRefreshInterval.pause) { - mlTimefilterRefresh$.next(); - } - }} + onRefresh={() => mlTimefilterRefresh$.next()} onRefreshChange={updateInterval} recentlyUsedRanges={recentlyUsedRanges} dateFormat={dateFormat} 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, From 924c728d43e21b7e96b4e3cb6809737821361202 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 6 Sep 2019 11:37:54 +0200 Subject: [PATCH 4/4] [ML] Fix tests. --- .../navigation_menu/top_nav/top_nav.test.tsx | 92 ++++++++++++------- .../navigation_menu/top_nav/top_nav.tsx | 2 +- 2 files changed, 58 insertions(+), 36 deletions(-) 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 index 2c0a8ea9f10b3..d98077da230c8 100644 --- 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 @@ -10,13 +10,15 @@ import React from 'react'; import { uiTimefilterMock } from '../../../contexts/ui/__mocks__/mocks'; import { mlTimefilterRefresh$ } from '../../../services/timefilter_refresh_service'; -import { TopNav } from './top_nav'; +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(); @@ -37,41 +39,61 @@ describe('Navigation Menu: ', () => { refreshSubscription.unsubscribe(); }); - test('Listen for super date picker refresh.', done => { - const refreshListener = jest.fn(); - const refreshSubscription = mlTimefilterRefresh$.subscribe(refreshListener); - - uiTimefilterMock.setRefreshInterval({ value: 5, pause: false }); - - mount(); - - setTimeout(() => { - expect(refreshListener).toBeCalledTimes(1); - refreshSubscription.unsubscribe(); - done(); - }, 10); - - jest.runOnlyPendingTimers(); + // 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 listener being called.', done => { - const refreshListener = jest.fn(); - const refreshSubscription = mlTimefilterRefresh$.subscribe(refreshListener); - - uiTimefilterMock.setRefreshInterval({ value: 5, pause: false }); - - mount(); - - setTimeout(() => { - uiTimefilterMock.setRefreshInterval({ value: 0, pause: true }); - }, 10); - - setTimeout(() => { - expect(refreshListener).toBeCalledTimes(2); - refreshSubscription.unsubscribe(); - done(); - }, 20); - - jest.runOnlyPendingTimers(); + 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 70d908bdc405e..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 @@ -25,7 +25,7 @@ const MlSuperDatePicker = (EuiSuperDatePicker as any) as ComponentWithConstructo // 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. -class MlSuperDatePickerWithUpdate extends MlSuperDatePicker { +export class MlSuperDatePickerWithUpdate extends MlSuperDatePicker { componentDidUpdate = () => { // @ts-ignore this.stopInterval();