From 9ed423967e387edb4d02769bfc7fb44dcb7cf1ca Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Fri, 18 Dec 2020 15:27:39 +0100 Subject: [PATCH 1/4] [ML] fix swim lane time selection adjustment --- .../contexts/kibana/__mocks__/index.ts | 1 + .../kibana/__mocks__/use_timefilter.ts | 40 ++++++ .../explorer/hooks/use_selected_cells.test.ts | 133 ++++++++++++++++++ .../explorer/hooks/use_selected_cells.ts | 80 +++++------ .../application/routing/routes/explorer.tsx | 2 +- 5 files changed, 215 insertions(+), 41 deletions(-) create mode 100644 x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/use_timefilter.ts create mode 100644 x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts diff --git a/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/index.ts b/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/index.ts index 7051abe6dc34ec..dd405ddcc04fa1 100644 --- a/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/index.ts +++ b/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/index.ts @@ -5,3 +5,4 @@ */ export { useMlKibana } from './kibana_context'; +export { useTimefilter } from './use_timefilter'; diff --git a/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/use_timefilter.ts b/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/use_timefilter.ts new file mode 100644 index 00000000000000..949de51cf2c2de --- /dev/null +++ b/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/use_timefilter.ts @@ -0,0 +1,40 @@ +/* + * 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 { TimefilterContract } from '../../../../../../../../src/plugins/data/public'; +import { Observable } from 'rxjs'; + +/** + * Copy from {@link '../../../../../../../../src/plugins/data/public/query/timefilter/timefilter_service.mock'} + */ +const timefilterMock: jest.Mocked = { + isAutoRefreshSelectorEnabled: jest.fn(), + isTimeRangeSelectorEnabled: jest.fn(), + isTimeTouched: jest.fn(), + getEnabledUpdated$: jest.fn(), + getTimeUpdate$: jest.fn(), + getRefreshIntervalUpdate$: jest.fn(), + getAutoRefreshFetch$: jest.fn(() => new Observable()), + getFetch$: jest.fn(), + getTime: jest.fn(), + setTime: jest.fn(), + setRefreshInterval: jest.fn(), + getRefreshInterval: jest.fn(), + getActiveBounds: jest.fn(), + disableAutoRefreshSelector: jest.fn(), + disableTimeRangeSelector: jest.fn(), + enableAutoRefreshSelector: jest.fn(), + enableTimeRangeSelector: jest.fn(), + getBounds: jest.fn(), + calculateBounds: jest.fn(), + createFilter: jest.fn(), + getRefreshIntervalDefaults: jest.fn(), + getTimeDefaults: jest.fn(), +}; + +export const useTimefilter = jest.fn(() => { + return timefilterMock; +}); diff --git a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts new file mode 100644 index 00000000000000..f2ed8815a635d3 --- /dev/null +++ b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts @@ -0,0 +1,133 @@ +/* + * 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 moment from 'moment'; +import { renderHook } from '@testing-library/react-hooks'; +import { useSelectedCells } from './use_selected_cells'; +import { ExplorerAppState } from '../../../../common/types/ml_url_generator'; +import { TimefilterContract } from '../../../../../../../src/plugins/data/public'; + +import { useTimefilter } from '../../contexts/kibana'; + +jest.mock('../../contexts/kibana'); + +describe('useSelectedCells', () => { + test('should no set state when the cell selection is correct', () => { + (useTimefilter() as jest.Mocked).getBounds.mockReturnValue({ + min: moment(1498824778 * 1000), + max: moment(1502366798 * 1000), + }); + + const urlState = { + mlExplorerSwimlane: { + selectedType: 'overall', + selectedLanes: ['Overall'], + selectedTimes: [1498780800, 1498867200], + showTopFieldValues: true, + viewByFieldName: 'apache2.access.remote_ip', + viewByFromPage: 1, + viewByPerPage: 10, + }, + mlExplorerFilter: {}, + } as ExplorerAppState; + + const setUrlState = jest.fn(); + + const bucketInterval = 86400; + + renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval)); + + expect(setUrlState).not.toHaveBeenCalled(); + }); + + test('should reset cell selection when it is completely out of range', () => { + (useTimefilter() as jest.Mocked).getBounds.mockReturnValue({ + min: moment(1501071178 * 1000), + max: moment(1502366798 * 1000), + }); + + const urlState = { + mlExplorerSwimlane: { + selectedType: 'overall', + selectedLanes: ['Overall'], + selectedTimes: [1498780800, 1498867200], + showTopFieldValues: true, + viewByFieldName: 'apache2.access.remote_ip', + viewByFromPage: 1, + viewByPerPage: 10, + }, + mlExplorerFilter: {}, + } as ExplorerAppState; + + const setUrlState = jest.fn(); + + const bucketInterval = 86400; + + const { result } = renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval)); + + expect(result.current[0]).toEqual({ + lanes: ['Overall'], + showTopFieldValues: true, + times: [1498780800, 1498867200], + type: 'overall', + viewByFieldName: 'apache2.access.remote_ip', + }); + + expect(setUrlState).toHaveBeenCalledWith({ + mlExplorerSwimlane: { + viewByFieldName: 'apache2.access.remote_ip', + viewByFromPage: 1, + viewByPerPage: 10, + }, + }); + }); + + test('should adjust cell selection time boundaries based on the main time range', () => { + (useTimefilter() as jest.Mocked).getBounds.mockReturnValue({ + min: moment(1501071178 * 1000), + max: moment(1502366798 * 1000), + }); + + const urlState = { + mlExplorerSwimlane: { + selectedType: 'overall', + selectedLanes: ['Overall'], + selectedTimes: [1498780800, 1502366798], + showTopFieldValues: true, + viewByFieldName: 'apache2.access.remote_ip', + viewByFromPage: 1, + viewByPerPage: 10, + }, + mlExplorerFilter: {}, + } as ExplorerAppState; + + const setUrlState = jest.fn(); + + const bucketInterval = 86400; + + const { result } = renderHook(() => useSelectedCells(urlState, setUrlState, bucketInterval)); + + expect(result.current[0]).toEqual({ + lanes: ['Overall'], + showTopFieldValues: true, + times: [1498780800, 1502366798], + type: 'overall', + viewByFieldName: 'apache2.access.remote_ip', + }); + + expect(setUrlState).toHaveBeenCalledWith({ + mlExplorerSwimlane: { + selectedLanes: ['Overall'], + selectedTimes: [1498780800, 1502366798], + selectedType: 'overall', + showTopFieldValues: true, + viewByFieldName: 'apache2.access.remote_ip', + viewByFromPage: 1, + viewByPerPage: 10, + }, + }); + }); +}); diff --git a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts index 7a279afc22ae77..f8176db5d42832 100644 --- a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts +++ b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts @@ -5,7 +5,6 @@ */ import { useCallback, useEffect, useMemo } from 'react'; -import { Duration } from 'moment'; import { SWIMLANE_TYPE } from '../explorer_constants'; import { AppStateSelectedCells } from '../explorer_utils'; import { ExplorerAppState } from '../../../../common/types/ml_url_generator'; @@ -14,10 +13,9 @@ import { useTimefilter } from '../../contexts/kibana'; export const useSelectedCells = ( appState: ExplorerAppState, setAppState: (update: Partial) => void, - bucketInterval: Duration | undefined + bucketIntervalInSeconds: number | undefined ): [AppStateSelectedCells | undefined, (swimlaneSelectedCells: AppStateSelectedCells) => void] => { const timeFilter = useTimefilter(); - const timeBounds = timeFilter.getBounds(); // keep swimlane selection, restore selectedCells from AppState @@ -76,43 +74,45 @@ export const useSelectedCells = ( * Adjust cell selection with respect to the time boundaries. * Reset it entirely when it out of range. */ - useEffect(() => { - if (selectedCells?.times === undefined || bucketInterval === undefined) return; - - let [selectedFrom, selectedTo] = selectedCells.times; - - const rangeFrom = timeBounds.min!.unix(); - /** - * Because each cell on the swim lane represent the fixed bucket interval, - * the selection range could be outside of the time boundaries with - * correction within the bucket interval. - */ - const rangeTo = timeBounds.max!.unix() + bucketInterval.asSeconds(); - - selectedFrom = Math.max(selectedFrom, rangeFrom); - - selectedTo = Math.min(selectedTo, rangeTo); - - const isSelectionOutOfRange = rangeFrom > selectedTo || rangeTo < selectedFrom; - - if (isSelectionOutOfRange) { - // reset selection - setSelectedCells(); - return; - } - - if (selectedFrom !== rangeFrom || selectedTo !== rangeTo) { - setSelectedCells({ - ...selectedCells, - times: [selectedFrom, selectedTo], - }); - } - }, [ - timeBounds.min?.valueOf(), - timeBounds.max?.valueOf(), - selectedCells, - bucketInterval?.asMilliseconds(), - ]); + useEffect( + function adjustSwimLaneTimeSelection() { + if (selectedCells?.times === undefined || bucketIntervalInSeconds === undefined) return; + + const [selectedFrom, selectedTo] = selectedCells.times; + + /** + * Because each cell on the swim lane represent the fixed bucket interval, + * the selection range could be outside of the time boundaries with + * correction within the bucket interval. + */ + const rangeFrom = timeBounds.min!.unix() - bucketIntervalInSeconds; + const rangeTo = timeBounds.max!.unix() + bucketIntervalInSeconds; + + const resultFrom = Math.max(selectedFrom, rangeFrom); + const resultTo = Math.min(selectedTo, rangeTo); + + const isSelectionOutOfRange = rangeFrom > resultTo || rangeTo < resultFrom; + + if (isSelectionOutOfRange) { + // reset selection + setSelectedCells(); + return; + } + + if (selectedFrom === resultFrom && selectedTo === resultTo) { + // selection is correct, no need to adjust the range + return; + } + + if (resultFrom !== rangeFrom || resultTo !== rangeTo) { + setSelectedCells({ + ...selectedCells, + times: [selectedFrom, selectedTo], + }); + } + }, + [timeBounds.min?.unix(), timeBounds.max?.unix(), selectedCells, bucketIntervalInSeconds] + ); return [selectedCells, setSelectedCells]; }; diff --git a/x-pack/plugins/ml/public/application/routing/routes/explorer.tsx b/x-pack/plugins/ml/public/application/routing/routes/explorer.tsx index 15ce3847f4d9cc..0075846571cee5 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/explorer.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/explorer.tsx @@ -201,7 +201,7 @@ const ExplorerUrlStateManager: FC = ({ jobsWithTim const [selectedCells, setSelectedCells] = useSelectedCells( explorerUrlState, setExplorerUrlState, - explorerState?.swimlaneBucketInterval + explorerState?.swimlaneBucketInterval?.asSeconds() ); useEffect(() => { From 8f85ae904ff92ce62f26185b246f84daca8053c2 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 21 Dec 2020 10:49:07 +0100 Subject: [PATCH 2/4] [ML] fix adjustment --- .../application/explorer/hooks/use_selected_cells.test.ts | 2 +- .../ml/public/application/explorer/hooks/use_selected_cells.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts index f2ed8815a635d3..3ddea460e79318 100644 --- a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts +++ b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts @@ -121,7 +121,7 @@ describe('useSelectedCells', () => { expect(setUrlState).toHaveBeenCalledWith({ mlExplorerSwimlane: { selectedLanes: ['Overall'], - selectedTimes: [1498780800, 1502366798], + selectedTimes: [1500984778, 1502366798], selectedType: 'overall', showTopFieldValues: true, viewByFieldName: 'apache2.access.remote_ip', diff --git a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts index f8176db5d42832..4ce828b0b76337 100644 --- a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts +++ b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.ts @@ -107,7 +107,7 @@ export const useSelectedCells = ( if (resultFrom !== rangeFrom || resultTo !== rangeTo) { setSelectedCells({ ...selectedCells, - times: [selectedFrom, selectedTo], + times: [resultFrom, resultTo], }); } }, From 3a948fc34567c3000a1617272f0e0cf41f4f76d7 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 21 Dec 2020 13:01:58 +0100 Subject: [PATCH 3/4] [ML] fix tooManyBuckets condition --- .../explorer_charts/explorer_charts_container_service.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container_service.js b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container_service.js index 0f1b9c77afced5..47087e776d6dd3 100644 --- a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container_service.js +++ b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_charts_container_service.js @@ -694,7 +694,10 @@ function calculateChartRange( chartRange.min = chartRange.min + maxBucketSpanMs; } - if (chartRange.min > selectedEarliestMs || chartRange.max < selectedLatestMs) { + if ( + (chartRange.min > selectedEarliestMs || chartRange.max < selectedLatestMs) && + chartRange.max - chartRange.min < selectedLatestMs - selectedEarliestMs + ) { tooManyBuckets = true; } From d42a64ef9131eb19bbf6f3cd22b9879bfc7308ed Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 21 Dec 2020 15:32:37 +0100 Subject: [PATCH 4/4] [ML] fix typo --- .../application/explorer/hooks/use_selected_cells.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts index 3ddea460e79318..08c8d11987f196 100644 --- a/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts +++ b/x-pack/plugins/ml/public/application/explorer/hooks/use_selected_cells.test.ts @@ -15,7 +15,7 @@ import { useTimefilter } from '../../contexts/kibana'; jest.mock('../../contexts/kibana'); describe('useSelectedCells', () => { - test('should no set state when the cell selection is correct', () => { + test('should not set state when the cell selection is correct', () => { (useTimefilter() as jest.Mocked).getBounds.mockReturnValue({ min: moment(1498824778 * 1000), max: moment(1502366798 * 1000),