[ML] Enforce pause when it's set to false with 0 refresh interval (#8…

…6805) * [ML] Enforce pause when it's set to false with 0 refresh interval * [ML] add mocks, fix unit tests
elastic · Dec 23, 2020 · d4d70f2 · d4d70f2
1 parent cfec38e
commit d4d70f2
Show file tree

Hide file tree

Showing 11 changed files with 162 additions and 106 deletions.
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
@@ -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<TableInterval>('mlSelectInterval', TABLE_INTERVAL_DEFAULT);

diff --git a/...c/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.test.tsx b/...c/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<typeof EuiSuperDatePicker>;
 
 describe('Navigation Menu: <DatePickerWrapper />', () => {
   beforeEach(() => {
     jest.useFakeTimers();
+    MockedEuiSuperDatePicker.mockClear();
   });
 
   afterEach(() => {
@@ -56,66 +75,22 @@ describe('Navigation Menu: <DatePickerWrapper />', () => {
     const refreshListener = jest.fn();
     const refreshSubscription = mlTimefilterRefresh$.subscribe(refreshListener);
 
-    const wrapper = mount(
-      <MemoryRouter>
-        <DatePickerWrapper />
-      </MemoryRouter>
-    );
+    const wrapper = mount(<DatePickerWrapper />);
     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(
-      <EuiSuperDatePicker
-        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('should not allow disabled pause with 0 refresh interval', () => {
+    // arrange
+    (useUrlState as jest.Mock).mockReturnValue([{ refreshInterval: { pause: false, value: 0 } }]);
+
+    // act
+    render(<DatePickerWrapper />);
 
-  test('Switching refresh interval to pause should stop onRefresh being called.', async () => {
-    const onRefresh = jest.fn();
-
-    const componentRefresh = mount(
-      <EuiSuperDatePicker
-        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);
+    // assert
+    const calledWith = MockedEuiSuperDatePicker.mock.calls[0][0];
+    expect(calledWith.isPaused).toBe(true);
   });
 });
diff --git a/...public/application/components/navigation_menu/date_picker_wrapper/date_picker_wrapper.tsx b/...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 (
-    <Fragment>
-      {(isAutoRefreshSelectorEnabled || isTimeRangeSelectorEnabled) && (
-        <div className="mlNavigationMenu__datePickerWrapper">
-          <EuiSuperDatePicker
-            start={time.from}
-            end={time.to}
-            isPaused={refreshInterval.pause}
-            isAutoRefreshOnly={!isTimeRangeSelectorEnabled}
-            refreshInterval={refreshInterval.value}
-            onTimeChange={updateFilter}
-            onRefresh={updateLastRefresh}
-            onRefreshChange={updateInterval}
-            recentlyUsedRanges={recentlyUsedRanges}
-            dateFormat={dateFormat}
-          />
-        </div>
-      )}
-    </Fragment>
-  );
+  /**
+   * Enforce pause when it's set to false with 0 refresh interval.
+   */
+  const isPaused = refreshInterval.pause || (!refreshInterval.pause && !refreshInterval.value);
+
+  return isAutoRefreshSelectorEnabled || isTimeRangeSelectorEnabled ? (
+    <div className="mlNavigationMenu__datePickerWrapper">
+      <EuiSuperDatePicker
+        start={time.from}
+        end={time.to}
+        isPaused={isPaused}
+        isAutoRefreshOnly={!isTimeRangeSelectorEnabled}
+        refreshInterval={refreshInterval.value}
+        onTimeChange={updateFilter}
+        onRefresh={updateLastRefresh}
+        onRefreshChange={updateInterval}
+        recentlyUsedRanges={recentlyUsedRanges}
+        dateFormat={dateFormat}
+      />
+    </div>
+  ) : 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
@@ -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<typeof TimeSeriesExplorer>;
+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(
       <I18nProvider>
-        <MemoryRouter>
-          <TimeSeriesExplorerUrlStateManager {...props} />
-        </MemoryRouter>
+        <TimeSeriesExplorerUrlStateManager {...props} />
       </I18nProvider>
     );
 
-    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
@@ -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<TimeSeriesExplorerUrlStateMan
   config,
   jobsWithTimeRange,
 }) => {
+  const { toasts } = useNotifications();
   const toastNotificationService = useToastNotificationService();
   const [
     timeSeriesExplorerUrlState,
@@ -249,7 +250,12 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
       setLastRefresh(Date.now());
       appStateHandler(APP_STATE_ACTION.CLEAR);
     }
-    const validatedJobId = validateJobSelection(jobsWithTimeRange, selectedJobIds, setGlobalState);
+    const validatedJobId = validateJobSelection(
+      jobsWithTimeRange,
+      selectedJobIds,
+      setGlobalState,
+      toasts
+    );
     if (typeof validatedJobId === 'string') {
       setSelectedJobId(validatedJobId);
     }

diff --git a/x-pack/plugins/ml/public/application/services/toast_notification_service/__mocks__/index.ts b/x-pack/plugins/ml/public/application/services/toast_notification_service/__mocks__/index.ts
@@ -0,0 +1,7 @@
+/*
+ * 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.
+ */
+
+export const useToastNotificationService = jest.fn();
diff --git a/...public/application/timeseriesexplorer/hooks/__mocks__/use_timeseriesexplorer_url_state.ts b/...public/application/timeseriesexplorer/hooks/__mocks__/use_timeseriesexplorer_url_state.ts
@@ -0,0 +1,9 @@
+/*
+ * 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.
+ */
+
+export const useTimeSeriesExplorerUrlState = jest.fn(() => {
+  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
@@ -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<Props, any> {}
diff --git a/.../public/application/timeseriesexplorer/timeseriesexplorer_utils/validate_job_selection.ts b/.../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);