From 8a2d6e61174f5414e1acf4f6d17e8de75b11bb27 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Wed, 23 Dec 2020 18:17:53 +0100 Subject: [PATCH] [ML] Enforce pause when it's set to false with 0 refresh interval (#86805) (#86889) * [ML] Enforce pause when it's set to false with 0 refresh interval * [ML] add mocks, fix unit tests --- .../select_interval/select_interval.tsx | 2 +- .../date_picker_wrapper.test.tsx | 93 +++++++------------ .../date_picker_wrapper.tsx | 43 ++++----- .../routes/timeseriesexplorer.test.tsx | 57 +++++++++--- .../routing/routes/timeseriesexplorer.tsx | 10 +- .../__mocks__/index.ts | 7 ++ .../use_timeseriesexplorer_url_state.ts | 9 ++ .../timeseriesexplorer.d.ts | 9 +- .../validate_job_selection.ts | 7 +- .../application/util/__mocks__/url_state.tsx | 27 ++++++ .../ml/public/application/util/url_state.tsx | 4 +- 11 files changed, 162 insertions(+), 106 deletions(-) create mode 100644 x-pack/plugins/ml/public/application/services/toast_notification_service/__mocks__/index.ts create mode 100644 x-pack/plugins/ml/public/application/timeseriesexplorer/hooks/__mocks__/use_timeseriesexplorer_url_state.ts create mode 100644 x-pack/plugins/ml/public/application/util/__mocks__/url_state.tsx diff --git a/x-pack/plugins/ml/public/application/components/controls/select_interval/select_interval.tsx b/x-pack/plugins/ml/public/application/components/controls/select_interval/select_interval.tsx index 059ab48daa27e..0fc1d458399dd 100644 --- a/x-pack/plugins/ml/public/application/components/controls/select_interval/select_interval.tsx +++ b/x-pack/plugins/ml/public/application/components/controls/select_interval/select_interval.tsx @@ -51,7 +51,7 @@ function optionValueToInterval(value: string) { return interval; } -const TABLE_INTERVAL_DEFAULT = optionValueToInterval('auto'); +export const TABLE_INTERVAL_DEFAULT = optionValueToInterval('auto'); export const useTableInterval = (): [TableInterval, (v: TableInterval) => void] => { return usePageUrlState('mlSelectInterval', TABLE_INTERVAL_DEFAULT); diff --git a/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.test.tsx b/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.test.tsx index e2aa13f6019ed..b882531eb75b0 100644 --- a/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.test.tsx +++ b/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.test.tsx @@ -5,15 +5,31 @@ */ import { mount } from 'enzyme'; +import { render } from '@testing-library/react'; import React from 'react'; -import { MemoryRouter } from 'react-router-dom'; import { EuiSuperDatePicker } from '@elastic/eui'; +import { useUrlState } from '../../../util/url_state'; import { mlTimefilterRefresh$ } from '../../../services/timefilter_refresh_service'; import { DatePickerWrapper } from './date_picker_wrapper'; +jest.mock('@elastic/eui', () => { + const EuiSuperDatePickerMock = jest.fn(() => { + return null; + }); + return { EuiSuperDatePicker: EuiSuperDatePickerMock }; +}); + +jest.mock('../../../util/url_state', () => { + return { + useUrlState: jest.fn(() => { + return [{ refreshInterval: { value: 0, pause: true } }, jest.fn()]; + }), + }; +}); + jest.mock('../../../contexts/kibana', () => ({ useMlKibana: () => { return { @@ -25,9 +41,11 @@ jest.mock('../../../contexts/kibana', () => ({ timefilter: { getRefreshInterval: jest.fn(), setRefreshInterval: jest.fn(), - getTime: jest.fn(), - isAutoRefreshSelectorEnabled: jest.fn(), - isTimeRangeSelectorEnabled: jest.fn(), + getTime: jest.fn(() => { + return { from: '', to: '' }; + }), + isAutoRefreshSelectorEnabled: jest.fn(() => true), + isTimeRangeSelectorEnabled: jest.fn(() => true), getRefreshIntervalUpdate$: jest.fn(), getTimeUpdate$: jest.fn(), getEnabledUpdated$: jest.fn(), @@ -41,11 +59,12 @@ jest.mock('../../../contexts/kibana', () => ({ }, })); -const noop = () => {}; +const MockedEuiSuperDatePicker = EuiSuperDatePicker as jest.MockedClass; describe('Navigation Menu: ', () => { beforeEach(() => { jest.useFakeTimers(); + MockedEuiSuperDatePicker.mockClear(); }); afterEach(() => { @@ -56,66 +75,22 @@ describe('Navigation Menu: ', () => { const refreshListener = jest.fn(); const refreshSubscription = mlTimefilterRefresh$.subscribe(refreshListener); - const wrapper = mount( - - - - ); + const wrapper = mount(); expect(wrapper.find(DatePickerWrapper)).toHaveLength(1); expect(refreshListener).toBeCalledTimes(0); refreshSubscription.unsubscribe(); }); - // The following tests are written against EuiSuperDatePicker - // instead of DatePickerWrapper. DatePickerWrapper uses hooks and we cannot write tests - // with async hook updates yet until React 16.9 is available. - 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('should not allow disabled pause with 0 refresh interval', () => { + // arrange + (useUrlState as jest.Mock).mockReturnValue([{ refreshInterval: { pause: false, value: 0 } }]); + + // act + render(); - 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); + // assert + const calledWith = MockedEuiSuperDatePicker.mock.calls[0][0]; + expect(calledWith.isPaused).toBe(true); }); }); diff --git a/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx b/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx index a4dc78ea53a77..dc046241f82b9 100644 --- a/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx +++ b/x-pack/plugins/ml/public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { FC, Fragment, useCallback, useEffect, useState } from 'react'; +import React, { FC, useCallback, useEffect, useState } from 'react'; import { Subscription } from 'rxjs'; import { debounce } from 'lodash'; @@ -122,24 +122,25 @@ export const DatePickerWrapper: FC = () => { setRefreshInterval({ pause, value }); } - return ( - - {(isAutoRefreshSelectorEnabled || isTimeRangeSelectorEnabled) && ( -
- -
- )} - - ); + /** + * Enforce pause when it's set to false with 0 refresh interval. + */ + const isPaused = refreshInterval.pause || (!refreshInterval.pause && !refreshInterval.value); + + return isAutoRefreshSelectorEnabled || isTimeRangeSelectorEnabled ? ( +
+ +
+ ) : null; }; diff --git a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.test.tsx b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.test.tsx index b60a265560455..97ea27c5fe40a 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.test.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.test.tsx @@ -5,12 +5,44 @@ */ 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'; +import { TimeSeriesExplorer } from '../../timeseriesexplorer'; +import { TimeSeriesExplorerPage } from '../../timeseriesexplorer/timeseriesexplorer_page'; +import { TimeseriesexplorerNoJobsFound } from '../../timeseriesexplorer/components/timeseriesexplorer_no_jobs_found'; + +jest.mock('../../services/toast_notification_service'); + +jest.mock('../../timeseriesexplorer', () => ({ + TimeSeriesExplorer: jest.fn(() => { + return null; + }), +})); + +jest.mock('../../timeseriesexplorer/timeseriesexplorer_page', () => ({ + TimeSeriesExplorerPage: jest.fn(({ children }) => { + return <>{children}; + }), +})); + +jest.mock('../../timeseriesexplorer/components/timeseriesexplorer_no_jobs_found', () => ({ + TimeseriesexplorerNoJobsFound: jest.fn(() => { + return null; + }), +})); + +const MockedTimeSeriesExplorer = TimeSeriesExplorer as jest.MockedClass; +const MockedTimeSeriesExplorerPage = TimeSeriesExplorerPage as jest.MockedFunction< + typeof TimeSeriesExplorerPage +>; +const MockedTimeseriesexplorerNoJobsFound = TimeseriesexplorerNoJobsFound as jest.MockedFunction< + typeof TimeseriesexplorerNoJobsFound +>; + +jest.mock('../../util/url_state'); + +jest.mock('../../timeseriesexplorer/hooks/use_timeseriesexplorer_url_state'); jest.mock('../../contexts/kibana/kibana_context', () => { // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -59,27 +91,22 @@ jest.mock('../../contexts/kibana/kibana_context', () => { }; }); -jest.mock('../../util/dependency_cache', () => ({ - getToastNotifications: () => ({ addSuccess: jest.fn(), addDanger: jest.fn() }), -})); - -jest.mock('../../../../shared_imports'); - describe('TimeSeriesExplorerUrlStateManager', () => { - test('Initial render shows "No single metric jobs found"', () => { + test('should render TimeseriesexplorerNoJobsFound when no jobs provided', () => { const props = { config: { get: () => 'Browser' }, jobsWithTimeRange: [], }; - const { container } = render( + render( - - - + ); - expect(container.textContent).toContain('No single metric jobs found'); + // assert + expect(MockedTimeSeriesExplorer).not.toHaveBeenCalled(); + expect(MockedTimeSeriesExplorerPage).toHaveBeenCalled(); + expect(MockedTimeseriesexplorerNoJobsFound).toHaveBeenCalled(); }); }); diff --git a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx index 7de59cba495af..857e894d404ae 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx @@ -11,7 +11,7 @@ import moment from 'moment'; import { i18n } from '@kbn/i18n'; -import { NavigateToPath } from '../../contexts/kibana'; +import { NavigateToPath, useNotifications } from '../../contexts/kibana'; import { MlJobWithTimeRange } from '../../../../common/types/anomaly_detection_jobs'; @@ -93,6 +93,7 @@ export const TimeSeriesExplorerUrlStateManager: FC { + const { toasts } = useNotifications(); const toastNotificationService = useToastNotificationService(); const [ timeSeriesExplorerUrlState, @@ -249,7 +250,12 @@ export const TimeSeriesExplorerUrlStateManager: FC { + return [{}, jest.fn()]; +}); diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts index 8159dbb8ade06..26525505420de 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts @@ -4,11 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ -import { FC } from 'react'; +import React from 'react'; import { TimeRangeBounds } from '../explorer/explorer_utils'; -declare const TimeSeriesExplorer: FC<{ +interface Props { appStateHandler: (action: string, payload: any) => void; autoZoomDuration: number; bounds: TimeRangeBounds; @@ -21,4 +21,7 @@ declare const TimeSeriesExplorer: FC<{ tableInterval: string; tableSeverity: number; zoom?: { from?: string; to?: string }; -}>; +} + +// eslint-disable-next-line react/prefer-stateless-function +declare class TimeSeriesExplorer extends React.Component {} diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/validate_job_selection.ts b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/validate_job_selection.ts index cd8a10a9e1f99..1781d0ee6369b 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/validate_job_selection.ts +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer_utils/validate_job_selection.ts @@ -8,8 +8,7 @@ import { difference, without } from 'lodash'; import { i18n } from '@kbn/i18n'; -import { getToastNotifications } from '../../util/dependency_cache'; - +import { ToastsStart } from 'kibana/public'; import { MlJobWithTimeRange } from '../../../../common/types/anomaly_detection_jobs'; import { getTimeRangeFromSelection } from '../../components/job_selector/job_select_service_utils'; @@ -24,9 +23,9 @@ import { createTimeSeriesJobData } from './timeseriesexplorer_utils'; export function validateJobSelection( jobsWithTimeRange: MlJobWithTimeRange[], selectedJobIds: string[], - setGlobalState: (...args: any) => void + setGlobalState: (...args: any) => void, + toastNotifications: ToastsStart ) { - const toastNotifications = getToastNotifications(); const jobs = createTimeSeriesJobData(mlJobService.jobs); const timeSeriesJobIds: string[] = jobs.map((j: any) => j.id); diff --git a/x-pack/plugins/ml/public/application/util/__mocks__/url_state.tsx b/x-pack/plugins/ml/public/application/util/__mocks__/url_state.tsx new file mode 100644 index 0000000000000..cb237b951d8dd --- /dev/null +++ b/x-pack/plugins/ml/public/application/util/__mocks__/url_state.tsx @@ -0,0 +1,27 @@ +/* + * 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 { AppStateKey } from '../url_state'; +import { TABLE_INTERVAL_DEFAULT } from '../../components/controls/select_interval/select_interval'; + +export const useUrlState = jest.fn((accessor: '_a' | '_g') => { + if (accessor === '_g') { + return [{ refreshInterval: { value: 0, pause: true } }, jest.fn()]; + } +}); + +export const usePageUrlState = jest.fn((pageKey: AppStateKey) => { + let state: unknown; + switch (pageKey) { + case 'timeseriesexplorer': + state = {}; + break; + case 'mlSelectInterval': + state = TABLE_INTERVAL_DEFAULT; + break; + } + return [state, jest.fn()]; +}); diff --git a/x-pack/plugins/ml/public/application/util/url_state.tsx b/x-pack/plugins/ml/public/application/util/url_state.tsx index 569e7bcc7b7e1..b565a0f7b7a73 100644 --- a/x-pack/plugins/ml/public/application/util/url_state.tsx +++ b/x-pack/plugins/ml/public/application/util/url_state.tsx @@ -73,7 +73,9 @@ export const urlStateStore = createContext({ searchString: '', setUrlState: () => {}, }); + const { Provider } = urlStateStore; + export const UrlStateProvider: FC = ({ children }) => { const history = useHistory(); const { search: searchString } = useLocation(); @@ -164,7 +166,7 @@ export const useUrlState = (accessor: Accessor) => { type LegacyUrlKeys = 'mlExplorerSwimlane'; -type AppStateKey = +export type AppStateKey = | 'mlSelectSeverity' | 'mlSelectInterval' | 'mlAnomaliesTable'