Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Dec 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(),
Expand All @@ -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(() => {
Expand All @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -93,6 +93,7 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
config,
jobsWithTimeRange,
}) => {
const { toasts } = useNotifications();
const toastNotificationService = useToastNotificationService();
const [
timeSeriesExplorerUrlState,
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
Original file line number Diff line number Diff line change
@@ -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()];
});
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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> {}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);

Expand Down
Loading