From 704667feb8bfa0c1b90c3a98d171a76665d18c34 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 14:14:30 +0200 Subject: [PATCH] [ML] Fixes Anomaly Explorer Swimlane race condition, adds tests. (#22814) (#22923) This PR addresses parts of #22642: - It gets rid of the use of var that = this;. - dragSelect's action strings are moved to a constants file. - Adds jest tests for the ExplorerSwimlane component. This also fixes the following bugs: - The way we subscribe listeners to the events of the dragSelect library could result in the same event being triggered multiple times. This in turn could cause race conditions when on each event new data gets fetched but in between angular's scope gets updated and could end up in a non-intended way. The result of this were view-by swimlanes not updating correctly or anomaly charts showing non-related charts. This PR fixes it by filtering out consecutive swimlane click events. - When the angular based chart container wrapper directive gets destroyed/re-esetup when using the job pick, it missed unmounting the react component, it didn't trigger componentWillUnmount()and didn't unsubscribe from dragSelectListener. --- .../__mocks__/mock_overall_swimlane.json | 35 ++ .../explorer_swimlane.test.js.snap | 3 + .../explorer_charts_container_service.js | 4 +- .../ml/public/explorer/explorer_constants.js | 15 + .../ml/public/explorer/explorer_controller.js | 110 +++++- .../ml/public/explorer/explorer_swimlane.js | 364 ++++++++++-------- .../public/explorer/explorer_swimlane.test.js | 118 ++++++ .../explorer/explorer_swimlane_directive.js | 23 +- 8 files changed, 486 insertions(+), 186 deletions(-) create mode 100644 x-pack/plugins/ml/public/explorer/__mocks__/mock_overall_swimlane.json create mode 100644 x-pack/plugins/ml/public/explorer/__snapshots__/explorer_swimlane.test.js.snap create mode 100644 x-pack/plugins/ml/public/explorer/explorer_constants.js create mode 100644 x-pack/plugins/ml/public/explorer/explorer_swimlane.test.js diff --git a/x-pack/plugins/ml/public/explorer/__mocks__/mock_overall_swimlane.json b/x-pack/plugins/ml/public/explorer/__mocks__/mock_overall_swimlane.json new file mode 100644 index 0000000000000..16af0fdd36e07 --- /dev/null +++ b/x-pack/plugins/ml/public/explorer/__mocks__/mock_overall_swimlane.json @@ -0,0 +1,35 @@ +{ + "laneLabels": [ + "Overall" + ], + "points": [ + { + "laneLabel": "Overall", + "time": 1486425600, + "value": 0 + }, + { + "laneLabel": "Overall", + "time": 1486440000, + "value": 0.01107053 + }, + { + "laneLabel": "Overall", + "time": 1486454400, + "value": 0.1870243 + }, + { + "laneLabel": "Overall", + "time": 1486468800, + "value": 0.5947769 + }, + { + "laneLabel": "Overall", + "time": 1486483200, + "value": 0 + } + ], + "interval": 14400, + "earliest": 1486425600, + "latest": 1486483200 +} diff --git a/x-pack/plugins/ml/public/explorer/__snapshots__/explorer_swimlane.test.js.snap b/x-pack/plugins/ml/public/explorer/__snapshots__/explorer_swimlane.test.js.snap new file mode 100644 index 0000000000000..d601b30ca757a --- /dev/null +++ b/x-pack/plugins/ml/public/explorer/__snapshots__/explorer_swimlane.test.js.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`ExplorerSwimlane Overall swimlane 1`] = `"
Overall
2017-02-07T00:00:00Z2017-02-07T00:30:00Z2017-02-07T01:00:00Z2017-02-07T01:30:00Z2017-02-07T02:00:00Z2017-02-07T02:30:00Z2017-02-07T03:00:00Z2017-02-07T03:30:00Z2017-02-07T04:00:00Z2017-02-07T04:30:00Z2017-02-07T05:00:00Z2017-02-07T05:30:00Z2017-02-07T06:00:00Z2017-02-07T06:30:00Z2017-02-07T07:00:00Z2017-02-07T07:30:00Z2017-02-07T08:00:00Z2017-02-07T08:30:00Z2017-02-07T09:00:00Z2017-02-07T09:30:00Z2017-02-07T10:00:00Z2017-02-07T10:30:00Z2017-02-07T11:00:00Z2017-02-07T11:30:00Z2017-02-07T12:00:00Z2017-02-07T12:30:00Z2017-02-07T13:00:00Z2017-02-07T13:30:00Z2017-02-07T14:00:00Z2017-02-07T14:30:00Z2017-02-07T15:00:00Z2017-02-07T15:30:00Z2017-02-07T16:00:00Z
"`; diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container_service.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container_service.js index b114cd93fffb3..26aa4abc302bb 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container_service.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container_service.js @@ -7,9 +7,9 @@ /* - * Angular controller for the container for the anomaly charts in the + * Service for the container for the anomaly charts in the * Machine Learning Explorer dashboard. - * The controller processes the data required to draw each of the charts + * The service processes the data required to draw each of the charts * and manages the layout of the charts in the containing div. */ diff --git a/x-pack/plugins/ml/public/explorer/explorer_constants.js b/x-pack/plugins/ml/public/explorer/explorer_constants.js new file mode 100644 index 0000000000000..cd5dee537a0f6 --- /dev/null +++ b/x-pack/plugins/ml/public/explorer/explorer_constants.js @@ -0,0 +1,15 @@ +/* + * 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. + */ + +/* + * Contains values for ML anomaly explorer. + */ + +export const DRAG_SELECT_ACTION = { + NEW_SELECTION: 'newSelection', + ELEMENT_SELECT: 'elementSelect', + DRAG_START: 'dragStart' +}; diff --git a/x-pack/plugins/ml/public/explorer/explorer_controller.js b/x-pack/plugins/ml/public/explorer/explorer_controller.js index 0f5c59d70620e..c4815658e730d 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_controller.js +++ b/x-pack/plugins/ml/public/explorer/explorer_controller.js @@ -40,6 +40,7 @@ import { mlFieldFormatService } from 'plugins/ml/services/field_format_service'; import { JobSelectServiceProvider } from 'plugins/ml/components/job_select_list/job_select_service'; import { isTimeSeriesViewDetector } from 'plugins/ml/../common/util/job_utils'; import { timefilter } from 'ui/timefilter'; +import { DRAG_SELECT_ACTION } from './explorer_constants'; uiRoutes .when('/explorer/?', { @@ -55,6 +56,15 @@ uiRoutes import { uiModules } from 'ui/modules'; const module = uiModules.get('apps/ml'); +function getDefaultViewBySwimlaneData() { + return { + fieldName: '', + laneLabels: [], + points: [], + interval: 3600 + }; +} + module.controller('MlExplorerController', function ( $scope, $timeout, @@ -83,7 +93,10 @@ module.controller('MlExplorerController', function ( const VIEW_BY_JOB_LABEL = 'job ID'; const ALLOW_CELL_RANGE_SELECTION = mlExplorerDashboardService.allowCellRangeSelection; + // make sure dragSelect is only available if the mouse point is actually over a swimlane let disableDragSelectOnMouseLeave = true; + // skip listening to clicks on swimlanes while they are loading to avoid race conditions + let skipCellClicks = true; $scope.queryFilters = []; const dragSelect = new DragSelect({ @@ -95,7 +108,7 @@ module.controller('MlExplorerController', function ( if (elements.length > 0) { mlExplorerDashboardService.dragSelect.changed({ - action: 'newSelection', + action: DRAG_SELECT_ACTION.NEW_SELECTION, elements }); } @@ -105,7 +118,7 @@ module.controller('MlExplorerController', function ( onDragStart() { if (ALLOW_CELL_RANGE_SELECTION) { mlExplorerDashboardService.dragSelect.changed({ - action: 'dragStart' + action: DRAG_SELECT_ACTION.DRAG_START }); disableDragSelectOnMouseLeave = false; } @@ -113,7 +126,7 @@ module.controller('MlExplorerController', function ( onElementSelect() { if (ALLOW_CELL_RANGE_SELECTION) { mlExplorerDashboardService.dragSelect.changed({ - action: 'elementSelect' + action: DRAG_SELECT_ACTION.ELEMENT_SELECT }); } } @@ -127,8 +140,7 @@ module.controller('MlExplorerController', function ( }; $scope.viewBySwimlaneOptions = []; - $scope.viewBySwimlaneData = { 'fieldName': '', 'laneLabels': [], - 'points': [], 'interval': 3600 }; + $scope.viewBySwimlaneData = getDefaultViewBySwimlaneData(); $scope.initializeVis = function () { // Initialize the AppState in which to store filters. @@ -342,9 +354,32 @@ module.controller('MlExplorerController', function ( return influencers; } + // This queue tracks click events while the swimlanes are loading. + // To avoid race conditions we keep the click events cellData in this queue + // and trigger another event only after the current loading is done. + // The queue is necessary since a click in the overall swimlane triggers + // an update of the viewby swimlanes. If we'd just ignored click events + // during the loading, we could miss programmatically triggered events like + // those coming via AppState when a selection is part of the URL. + const swimlaneCellClickListenerQueue = []; + + // swimlaneCellClickListener could trigger multiple times with the same data. + // we track the previous click data here to be able to compare it and filter + // consecutive calls with the same data. + let previousListenerData = null; + // Listener for click events in the swimlane and load corresponding anomaly data. // Empty cellData is passed on clicking outside a cell with score > 0. - const swimlaneCellClickListener = function (cellData) { + // The reset argument is useful when we intentionally want to reset state comparison + // of click events and want to pass through. + // For example, toggling showCharts isn't considered in the comparison + // and would therefor fail to update properly. + const swimlaneCellClickListener = function (cellData, skipComparison = false) { + if (skipCellClicks === true) { + swimlaneCellClickListenerQueue.push(cellData); + return; + } + if (_.keys(cellData).length === 0) { // Swimlane deselection - clear anomalies section. if ($scope.viewByLoadedForTimeFormatted) { @@ -352,11 +387,28 @@ module.controller('MlExplorerController', function ( loadViewBySwimlane([]); } clearSelectedAnomalies(); + previousListenerData = null; } else { const timerange = getSelectionTimeRange(cellData); $scope.cellData = cellData; if (cellData.score > 0) { + const jobIds = (cellData.fieldName === VIEW_BY_JOB_LABEL) ? + cellData.laneLabels : $scope.getSelectedJobIds(); + const influencers = getSelectionInfluencers(cellData); + + const listenerData = { + jobIds, + influencers, + start: timerange.earliestMs, + end: timerange.latestMs, + cellData + }; + if (_.isEqual(listenerData, previousListenerData) && skipComparison === false) { + return; + } + previousListenerData = listenerData; + if (cellData.fieldName === undefined) { // Click is in one of the cells in the Overall swimlane - reload the 'view by' swimlane // to show the top 'view by' values for the selected time. @@ -364,12 +416,8 @@ module.controller('MlExplorerController', function ( $scope.viewByLoadedForTimeFormatted = moment(timerange.earliestMs).format('MMMM Do YYYY, HH:mm'); } - const jobIds = (cellData.fieldName === VIEW_BY_JOB_LABEL) ? - cellData.laneLabels : $scope.getSelectedJobIds(); - const influencers = getSelectionInfluencers(cellData); - - loadAnomaliesTableData(); loadDataForCharts(jobIds, influencers, timerange.earliestMs, timerange.latestMs); + loadAnomaliesTableData(); } else { // Multiple cells are selected, all with a score of 0 - clear all anomalies. $scope.$evalAsync(() => { @@ -397,7 +445,8 @@ module.controller('MlExplorerController', function ( const checkboxShowChartsListener = function () { const showCharts = mlCheckboxShowChartsService.state.get('showCharts'); if (showCharts && $scope.cellData !== undefined) { - swimlaneCellClickListener($scope.cellData); + // passing true as the second argument skips click event filtering + swimlaneCellClickListener($scope.cellData, true); } else { const timerange = getSelectionTimeRange($scope.cellData); mlExplorerDashboardService.anomalyDataChange.changed( @@ -451,7 +500,19 @@ module.controller('MlExplorerController', function ( navListener(); }); + // track the request to be able to ignore out of date requests + // and avoid race conditions ending up with the wrong charts. + let requestCount = 0; function loadDataForCharts(jobIds, influencers, earliestMs, latestMs) { + // Just skip doing the request when this function is called without + // the minimum required data. + if ($scope.cellData === undefined && influencers.length === 0) { + return; + } + + const newRequestCount = ++requestCount; + requestCount = newRequestCount; + // Loads the data used to populate the anomaly charts and the Top Influencers List. if (influencers.length === 0) { getTopInfluencers(jobIds, earliestMs, latestMs); @@ -462,6 +523,11 @@ module.controller('MlExplorerController', function ( jobIds, influencers, 0, earliestMs, latestMs, 500 ) .then((resp) => { + // Ignore this response if it's returned by an out of date promise + if (newRequestCount < requestCount) { + return; + } + if ($scope.cellData !== undefined && _.keys($scope.cellData).length > 0) { $scope.anomalyChartRecords = resp.records; console.log('Explorer anomaly charts data set:', $scope.anomalyChartRecords); @@ -686,6 +752,7 @@ module.controller('MlExplorerController', function ( $timeout(() => { $scope.$broadcast('render'); mlExplorerDashboardService.swimlaneDataChange.changed('overall'); + skipCellClicks = false; }, 0); }); @@ -710,20 +777,33 @@ module.controller('MlExplorerController', function ( } function loadViewBySwimlane(fieldValues) { + // reset the swimlane data to avoid flickering where the old dataset would briefly show up. + $scope.viewBySwimlaneData = getDefaultViewBySwimlaneData(); + + skipCellClicks = true; // finish() function, called after each data set has been loaded and processed. // The last one to call it will trigger the page render. function finish() { console.log('Explorer view by swimlane data set:', $scope.viewBySwimlaneData); + if (swimlaneCellClickListenerQueue.length > 0) { + const cellData = swimlaneCellClickListenerQueue.pop(); + swimlaneCellClickListenerQueue.length = 0; + swimlaneCellClickListener(cellData); + return; + } // Fire event to indicate swimlane data has changed. // Need to use $timeout to ensure this happens after the child scope is updated with the new data. $timeout(() => { + skipCellClicks = false; mlExplorerDashboardService.swimlaneDataChange.changed('viewBy'); }, 0); } - if ($scope.selectedJobs === undefined || - $scope.swimlaneViewByFieldName === undefined || $scope.swimlaneViewByFieldName === null) { - $scope.viewBySwimlaneData = { 'fieldName': '', 'laneLabels': [], 'points': [], 'interval': 3600 }; + if ( + $scope.selectedJobs === undefined || + $scope.swimlaneViewByFieldName === undefined || + $scope.swimlaneViewByFieldName === null + ) { finish(); } else { // Ensure the search bounds align to the bucketing interval used in the swimlane so diff --git a/x-pack/plugins/ml/public/explorer/explorer_swimlane.js b/x-pack/plugins/ml/public/explorer/explorer_swimlane.js index a04e4d0b9276e..bb20a73afdd9c 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_swimlane.js +++ b/x-pack/plugins/ml/public/explorer/explorer_swimlane.js @@ -10,6 +10,7 @@ * React component for rendering Explorer dashboard swimlanes. */ +import PropTypes from 'prop-types'; import React from 'react'; import _ from 'lodash'; @@ -23,9 +24,13 @@ import { numTicksForDateFormat } from '../util/chart_utils'; import { getSeverityColor } from '../../common/util/anomaly_utils'; import { mlEscape } from '../util/string_utils'; import { mlChartTooltipService } from '../components/chart_tooltip/chart_tooltip_service'; +import { DRAG_SELECT_ACTION } from './explorer_constants'; export class ExplorerSwimlane extends React.Component { static propTypes = { + appState: PropTypes.object.isRequired, + lanes: PropTypes.array.isRequired, + mlExplorerDashboardService: PropTypes.object.isRequired } constructor(props) { @@ -37,203 +42,232 @@ export class ExplorerSwimlane extends React.Component { componentWillUnmount() { const { mlExplorerDashboardService } = this.props; - mlExplorerDashboardService.dragSelect.unwatch(this.dragSelectListener); - + mlExplorerDashboardService.dragSelect.unwatch(this.boundDragSelectListener); + const element = $(this.rootNode); + element.empty(); } componentDidMount() { const element = $(this.rootNode).parent(); - - const { - appState, - mlExplorerDashboardService, - swimlaneData, - swimlaneType - } = this.props; + const { mlExplorerDashboardService } = this.props; // Consider the setting to support to select a range of cells if (!mlExplorerDashboardService.allowCellRangeSelection) { element.addClass('ml-hide-range-selection'); } - // Listen for dragSelect events - const that = this; - this.dragSelectListener = function ({ action, elements = [] }) { - if (action === 'newSelection' && elements.length > 0) { - const firstCellData = $(elements[0]).data('click'); - if (typeof firstCellData !== 'undefined' && swimlaneType === firstCellData.swimlaneType) { - const selectedData = elements.reduce((d, e) => { - const cellData = $(e).data('click'); - d.bucketScore = Math.max(d.bucketScore, cellData.bucketScore); - d.laneLabels.push(cellData.laneLabel); - d.times.push(cellData.time); - return d; - }, { - bucketScore: 0, - laneLabels: [], - times: [] - }); + // save the bound dragSelectListener to this property so it can be accessed again + // in componentWillUnmount(), otherwise mlExplorerDashboardService.dragSelect.unwatch + // is not able to check properly if it's still the same listener + this.boundDragSelectListener = this.dragSelectListener.bind(this); + mlExplorerDashboardService.dragSelect.watch(this.boundDragSelectListener); - selectedData.laneLabels = _.uniq(selectedData.laneLabels); - selectedData.times = _.uniq(selectedData.times); - cellClick(elements, selectedData); - } + this.renderSwimlane(); + } - that.setState({ cellMouseoverActive: true }); - } else if (action === 'elementSelect') { - element.addClass('ml-dragselect-dragging'); - return; - } else if (action === 'dragStart') { - that.setState({ cellMouseoverActive: false }); - return; - } - element.removeClass('ml-dragselect-dragging'); - elements.map(e => $(e).removeClass('ds-selected')); - }; + componentDidUpdate() { + this.renderSwimlane(); + } - mlExplorerDashboardService.dragSelect.watch(this.dragSelectListener); + // property to remember the bound dragSelectListener + boundDragSelectListener = null; - function cellClick(cellsToSelect, { laneLabels, bucketScore, times }) { - if (cellsToSelect.length > 1 || bucketScore > 0) { - selectCell(cellsToSelect, laneLabels, times, bucketScore, true); - } else { - that.clearSelection(); + // property for cellClick data comparison to be able to filter + // consecutive click events with the same data. + previousSelectedData = null; + + // Listen for dragSelect events + dragSelectListener({ action, elements = [] }) { + const element = $(this.rootNode).parent(); + const { swimlaneType } = this.props; + + if (action === DRAG_SELECT_ACTION.NEW_SELECTION && elements.length > 0) { + const firstCellData = $(elements[0]).data('click'); + if (typeof firstCellData !== 'undefined' && swimlaneType === firstCellData.swimlaneType) { + const selectedData = elements.reduce((d, e) => { + const cellData = $(e).data('click'); + d.bucketScore = Math.max(d.bucketScore, cellData.bucketScore); + d.laneLabels.push(cellData.laneLabel); + d.times.push(cellData.time); + return d; + }, { + bucketScore: 0, + laneLabels: [], + times: [] + }); + + selectedData.laneLabels = _.uniq(selectedData.laneLabels); + selectedData.times = _.uniq(selectedData.times); + if (_.isEqual(selectedData, this.previousSelectedData) === false) { + this.cellClick(elements, selectedData); + this.previousSelectedData = selectedData; + } } + + this.setState({ cellMouseoverActive: true }); + } else if (action === DRAG_SELECT_ACTION.ELEMENT_SELECT) { + element.addClass('ml-dragselect-dragging'); + return; + } else if (action === DRAG_SELECT_ACTION.DRAG_START) { + this.setState({ cellMouseoverActive: false }); + return; } - this.checkForSelection = function () { - // Check for selection in the AppState and reselect the corresponding swimlane cell - // if the time range and lane label are still in view. - const selectionState = appState.mlExplorerSwimlane; - const selectedType = _.get(selectionState, 'selectedType', undefined); - const viewBy = _.get(selectionState, 'viewBy', ''); - if (swimlaneType !== selectedType && selectedType !== undefined) { - $('.lane-label', element).addClass('lane-label-masked'); - $('.sl-cell-inner', element).addClass('sl-cell-inner-masked'); - } + this.previousSelectedData = null; + element.removeClass('ml-dragselect-dragging'); + elements.map(e => $(e).removeClass('ds-selected')); + } - if ((swimlaneType !== selectedType) || - (swimlaneData.fieldName !== undefined && swimlaneData.fieldName !== viewBy)) { - // Not this swimlane which was selected. - return; - } + cellClick(cellsToSelect, { laneLabels, bucketScore, times }) { + if (cellsToSelect.length > 1 || bucketScore > 0) { + this.selectCell(cellsToSelect, laneLabels, times, bucketScore, true); + } else { + this.clearSelection(); + } + } - const cellsToSelect = []; - const selectedLanes = _.get(selectionState, 'selectedLanes', []); - const selectedTimes = _.get(selectionState, 'selectedTimes', []); - const selectedTimeExtent = d3.extent(selectedTimes); - - const lanes = swimlaneData.laneLabels; - const startTime = swimlaneData.earliest; - const endTime = swimlaneData.latest; - - selectedLanes.forEach((selectedLane) => { - if (lanes.indexOf(selectedLane) > -1 && selectedTimeExtent[0] >= startTime && selectedTimeExtent[1] <= endTime) { - // Locate matching cell - look for exact time, otherwise closest before. - const $swimlanes = element.find('.ml-swimlanes').first(); - const laneCells = $('div[data-lane-label="' + mlEscape(selectedLane) + '"]', $swimlanes); - if (laneCells.length === 0) { - return; - } + checkForSelection() { + const element = $(this.rootNode).parent(); - for (let i = 0; i < laneCells.length; i++) { - const cell = laneCells[i]; - const cellTime = $(cell).attr('data-time'); - if (cellTime >= selectedTimeExtent[0] && cellTime <= selectedTimeExtent[1]) { - cellsToSelect.push(cell); - } - } - } - }); - const selectedMaxBucketScore = cellsToSelect.reduce((maxBucketScore, cell) => { - return Math.max(maxBucketScore, +$(cell).attr('data-score') || 0); - }, 0); - if (cellsToSelect.length > 1 || selectedMaxBucketScore > 0) { - selectCell(cellsToSelect, selectedLanes, selectedTimes, selectedMaxBucketScore); - } else { - // Clear selection from state as previous selection is no longer applicable. - that.clearSelection(); - } - }; + const { + appState, + swimlaneData, + swimlaneType + } = this.props; - function selectCell(cellsToSelect, laneLabels, times, bucketScore, checkEqualSelection = false) { - $('.lane-label', '.ml-explorer-swimlane').addClass('lane-label-masked'); - $('.sl-cell-inner,.sl-cell-inner-dragselect', '.ml-explorer-swimlane').addClass('sl-cell-inner-masked'); - $('.sl-cell-inner.sl-cell-inner-selected,.sl-cell-inner-dragselect.sl-cell-inner-selected', - '.ml-explorer-swimlane').removeClass('sl-cell-inner-selected'); - - $(cellsToSelect).find('.sl-cell-inner,.sl-cell-inner-dragselect') - .removeClass('sl-cell-inner-masked') - .addClass('sl-cell-inner-selected'); - - $('.lane-label').filter(function () { - return laneLabels.indexOf($(this).text()) > -1; - }).removeClass('lane-label-masked'); - - if (swimlaneType === 'viewBy') { - // If selecting a cell in the 'view by' swimlane, indicate the corresponding time in the Overall swimlane. - const overallSwimlane = $('ml-explorer-swimlane[swimlane-type="overall"]'); - times.forEach(time => { - const overallCell = $('div[data-time="' + time + '"]', overallSwimlane).find('.sl-cell-inner,.sl-cell-inner-dragselect'); - overallCell.addClass('sl-cell-inner-selected'); - }); - } + // Check for selection in the AppState and reselect the corresponding swimlane cell + // if the time range and lane label are still in view. + const selectionState = appState.mlExplorerSwimlane; + const selectedType = _.get(selectionState, 'selectedType', undefined); + const viewBy = _.get(selectionState, 'viewBy', ''); + if (swimlaneType !== selectedType && selectedType !== undefined) { + $('.lane-label', element).addClass('lane-label-masked'); + $('.sl-cell-inner', element).addClass('sl-cell-inner-masked'); + } - // Check if the same cells were selected again, if so clear the selection, - // otherwise activate the new selection. The two objects are built for - // comparison because we cannot simply compare to "scope.appState.mlExplorerSwimlane" - // since it also includes the "viewBy" attribute which might differ depending - // on whether the overall or viewby swimlane was selected. - if (checkEqualSelection && _.isEqual( - { - selectedType: appState.mlExplorerSwimlane.selectedType, - selectedLanes: appState.mlExplorerSwimlane.selectedLanes, - selectedTimes: appState.mlExplorerSwimlane.selectedTimes - }, - { - selectedType: swimlaneType, - selectedLanes: laneLabels, - selectedTimes: times + if ((swimlaneType !== selectedType) || + (swimlaneData.fieldName !== undefined && swimlaneData.fieldName !== viewBy)) { + // Not this swimlane which was selected. + return; + } + + const cellsToSelect = []; + const selectedLanes = _.get(selectionState, 'selectedLanes', []); + const selectedTimes = _.get(selectionState, 'selectedTimes', []); + const selectedTimeExtent = d3.extent(selectedTimes); + + const lanes = swimlaneData.laneLabels; + const startTime = swimlaneData.earliest; + const endTime = swimlaneData.latest; + + selectedLanes.forEach((selectedLane) => { + if (lanes.indexOf(selectedLane) > -1 && selectedTimeExtent[0] >= startTime && selectedTimeExtent[1] <= endTime) { + // Locate matching cell - look for exact time, otherwise closest before. + const $swimlanes = element.find('.ml-swimlanes').first(); + const laneCells = $('div[data-lane-label="' + mlEscape(selectedLane) + '"]', $swimlanes); + if (laneCells.length === 0) { + return; + } + + for (let i = 0; i < laneCells.length; i++) { + const cell = laneCells[i]; + const cellTime = $(cell).attr('data-time'); + if (cellTime >= selectedTimeExtent[0] && cellTime <= selectedTimeExtent[1]) { + cellsToSelect.push(cell); + } } - )) { - that.clearSelection(); - } else { - appState.mlExplorerSwimlane.selectedType = swimlaneType; - appState.mlExplorerSwimlane.selectedLanes = laneLabels; - appState.mlExplorerSwimlane.selectedTimes = times; - appState.save(); - - mlExplorerDashboardService.swimlaneCellClick.changed({ - fieldName: swimlaneData.fieldName, - laneLabels, - time: d3.extent(times), - interval: swimlaneData.interval, - score: bucketScore - }); } + }); + const selectedMaxBucketScore = cellsToSelect.reduce((maxBucketScore, cell) => { + return Math.max(maxBucketScore, +$(cell).attr('data-score') || 0); + }, 0); + if (cellsToSelect.length > 1 || selectedMaxBucketScore > 0) { + this.selectCell(cellsToSelect, selectedLanes, selectedTimes, selectedMaxBucketScore); + } else { + // Clear selection from state as previous selection is no longer applicable. + this.clearSelection(); } + } - this.clearSelection = function () { - $('.lane-label', '.ml-explorer-swimlane').removeClass('lane-label-masked'); - $('.sl-cell-inner', '.ml-explorer-swimlane').removeClass('sl-cell-inner-masked'); - $('.sl-cell-inner.sl-cell-inner-selected', '.ml-explorer-swimlane').removeClass('sl-cell-inner-selected'); - $('.sl-cell-inner-dragselect.sl-cell-inner-selected', '.ml-explorer-swimlane').removeClass('sl-cell-inner-selected'); - $('.ds-selected', '.ml-explorer-swimlane').removeClass('ds-selected'); + selectCell(cellsToSelect, laneLabels, times, bucketScore, checkEqualSelection = false) { + const { + appState, + mlExplorerDashboardService, + swimlaneData, + swimlaneType + } = this.props; - delete appState.mlExplorerSwimlane.selectedType; - delete appState.mlExplorerSwimlane.selectedLanes; - delete appState.mlExplorerSwimlane.selectedTimes; - appState.save(); + $('.lane-label', '.ml-explorer-swimlane').addClass('lane-label-masked'); + $('.sl-cell-inner,.sl-cell-inner-dragselect', '.ml-explorer-swimlane').addClass('sl-cell-inner-masked'); + $('.sl-cell-inner.sl-cell-inner-selected,.sl-cell-inner-dragselect.sl-cell-inner-selected', + '.ml-explorer-swimlane').removeClass('sl-cell-inner-selected'); + + $(cellsToSelect).find('.sl-cell-inner,.sl-cell-inner-dragselect') + .removeClass('sl-cell-inner-masked') + .addClass('sl-cell-inner-selected'); + + $('.lane-label').filter(function () { + return laneLabels.indexOf($(this).text()) > -1; + }).removeClass('lane-label-masked'); + + if (swimlaneType === 'viewBy') { + // If selecting a cell in the 'view by' swimlane, indicate the corresponding time in the Overall swimlane. + const overallSwimlane = $('ml-explorer-swimlane[swimlane-type="overall"]'); + times.forEach(time => { + const overallCell = $('div[data-time="' + time + '"]', overallSwimlane).find('.sl-cell-inner,.sl-cell-inner-dragselect'); + overallCell.addClass('sl-cell-inner-selected'); + }); + } - mlExplorerDashboardService.swimlaneCellClick.changed({}); - }; + // Check if the same cells were selected again, if so clear the selection, + // otherwise activate the new selection. The two objects are built for + // comparison because we cannot simply compare to "appState.mlExplorerSwimlane" + // since it also includes the "viewBy" attribute which might differ depending + // on whether the overall or viewby swimlane was selected. + if (checkEqualSelection && _.isEqual( + { + selectedType: appState.mlExplorerSwimlane.selectedType, + selectedLanes: appState.mlExplorerSwimlane.selectedLanes, + selectedTimes: appState.mlExplorerSwimlane.selectedTimes + }, + { + selectedType: swimlaneType, + selectedLanes: laneLabels, + selectedTimes: times + } + )) { + this.clearSelection(); + } else { + appState.mlExplorerSwimlane.selectedType = swimlaneType; + appState.mlExplorerSwimlane.selectedLanes = laneLabels; + appState.mlExplorerSwimlane.selectedTimes = times; + appState.save(); - this.renderSwimlane(); + mlExplorerDashboardService.swimlaneCellClick.changed({ + fieldName: swimlaneData.fieldName, + laneLabels, + time: d3.extent(times), + interval: swimlaneData.interval, + score: bucketScore + }); + } } - componentDidUpdate() { - this.renderSwimlane(); + clearSelection() { + const { appState, mlExplorerDashboardService } = this.props; + $('.lane-label', '.ml-explorer-swimlane').removeClass('lane-label-masked'); + $('.sl-cell-inner', '.ml-explorer-swimlane').removeClass('sl-cell-inner-masked'); + $('.sl-cell-inner.sl-cell-inner-selected', '.ml-explorer-swimlane').removeClass('sl-cell-inner-selected'); + $('.sl-cell-inner-dragselect.sl-cell-inner-selected', '.ml-explorer-swimlane').removeClass('sl-cell-inner-selected'); + $('.ds-selected', '.ml-explorer-swimlane').removeClass('ds-selected'); + + delete appState.mlExplorerSwimlane.selectedType; + delete appState.mlExplorerSwimlane.selectedLanes; + delete appState.mlExplorerSwimlane.selectedTimes; + appState.save(); + + mlExplorerDashboardService.swimlaneCellClick.changed({}); } renderSwimlane() { diff --git a/x-pack/plugins/ml/public/explorer/explorer_swimlane.test.js b/x-pack/plugins/ml/public/explorer/explorer_swimlane.test.js new file mode 100644 index 0000000000000..46ecaf020badb --- /dev/null +++ b/x-pack/plugins/ml/public/explorer/explorer_swimlane.test.js @@ -0,0 +1,118 @@ +/* + * 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 mockOverallSwimlaneData from './__mocks__/mock_overall_swimlane.json'; + +import moment from 'moment-timezone'; +import { mount } from 'enzyme'; +import React from 'react'; + +import { ExplorerSwimlane } from './explorer_swimlane'; + +function getExplorerSwimlaneMocks() { + const appState = { + mlExplorerSwimlane: {}, + save: jest.fn() + }; + + const mlExplorerDashboardService = { + allowCellRangeSelection: false, + dragSelect: { + watch: jest.fn(), + unwatch: jest.fn() + }, + swimlaneCellClick: { + changed: jest.fn() + }, + swimlaneRenderDone: { + changed: jest.fn() + } + }; + + const MlTimeBucketsMethods = { + setInterval: jest.fn(), + getScaledDateFormat: jest.fn() + }; + const MlTimeBuckets = jest.fn(() => MlTimeBucketsMethods); + MlTimeBuckets.mockMethods = MlTimeBucketsMethods; + + const swimlaneData = {}; + + return { + appState, + mlExplorerDashboardService, + MlTimeBuckets, + swimlaneData + }; +} + +describe('ExplorerSwimlane', () => { + const mockedGetBBox = { x: 0, y: -11.5, width: 12.1875, height: 14.5 }; + const originalGetBBox = SVGElement.prototype.getBBox; + beforeEach(() => { + moment.tz.setDefault('UTC'); + SVGElement.prototype.getBBox = () => mockedGetBBox; + }); + afterEach(() => { + moment.tz.setDefault('Browser'); + SVGElement.prototype.getBBox = originalGetBBox; + }); + + test('Minimal initialization', () => { + const mocks = getExplorerSwimlaneMocks(); + + const wrapper = mount(); + + expect(wrapper.html()).toBe( + `
` + + `
` + ); + + // test calls to mock functions + expect(mocks.appState.save.mock.calls).toHaveLength(1); + expect(mocks.mlExplorerDashboardService.swimlaneRenderDone.changed.mock.calls).toHaveLength(1); + expect(mocks.mlExplorerDashboardService.dragSelect.watch.mock.calls).toHaveLength(1); + expect(mocks.mlExplorerDashboardService.dragSelect.unwatch.mock.calls).toHaveLength(0); + expect(mocks.mlExplorerDashboardService.swimlaneCellClick.changed.mock.calls).toHaveLength(1); + expect(mocks.MlTimeBuckets.mockMethods.setInterval.mock.calls).toHaveLength(1); + expect(mocks.MlTimeBuckets.mockMethods.getScaledDateFormat.mock.calls).toHaveLength(1); + }); + + test('Overall swimlane', () => { + const mocks = getExplorerSwimlaneMocks(); + + const wrapper = mount(); + + expect(wrapper.html()).toMatchSnapshot(); + + // test calls to mock functions + expect(mocks.appState.save.mock.calls).toHaveLength(0); + expect(mocks.mlExplorerDashboardService.swimlaneRenderDone.changed.mock.calls).toHaveLength(1); + expect(mocks.mlExplorerDashboardService.dragSelect.watch.mock.calls).toHaveLength(1); + expect(mocks.mlExplorerDashboardService.dragSelect.unwatch.mock.calls).toHaveLength(0); + expect(mocks.mlExplorerDashboardService.swimlaneCellClick.changed.mock.calls).toHaveLength(0); + expect(mocks.MlTimeBuckets.mockMethods.setInterval.mock.calls).toHaveLength(1); + expect(mocks.MlTimeBuckets.mockMethods.getScaledDateFormat.mock.calls).toHaveLength(1); + }); +}); diff --git a/x-pack/plugins/ml/public/explorer/explorer_swimlane_directive.js b/x-pack/plugins/ml/public/explorer/explorer_swimlane_directive.js index 406f1bd82eb84..fc7adcce51b62 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_swimlane_directive.js +++ b/x-pack/plugins/ml/public/explorer/explorer_swimlane_directive.js @@ -10,6 +10,7 @@ * AngularJS directive for rendering Explorer dashboard swimlanes. */ +import _ from 'lodash'; import React from 'react'; import ReactDOM from 'react-dom'; @@ -33,11 +34,21 @@ module.directive('mlExplorerSwimlane', function ($compile, Private, mlExplorerDa element.on('$destroy', () => { mlExplorerDashboardService.swimlaneDataChange.unwatch(swimlaneDataChangeListener); + // unmountComponentAtNode() needs to be called so dragSelectListener within + // the ExplorerSwimlane component gets unwatched properly. + ReactDOM.unmountComponentAtNode(element[0]); scope.$destroy(); }); const MlTimeBuckets = Private(IntervalHelperProvider); + // This triggers the render function quite aggressively, but we want to make sure we don't miss + // any updates to related scopes of directives and/or controllers. However, we do a deep comparison + // of current and future props to filter redundant render triggers. + scope.$watch(function () { + render(); + }); + let previousProps = null; function render() { if (scope.swimlaneData === undefined) { return; @@ -57,10 +68,14 @@ module.directive('mlExplorerSwimlane', function ($compile, Private, mlExplorerDa appState: scope.appState }; - ReactDOM.render( - React.createElement(ExplorerSwimlane, props), - element[0] - ); + if (_.isEqual(props, previousProps) === false) { + ReactDOM.render( + React.createElement(ExplorerSwimlane, props), + element[0] + ); + previousProps = props; + } + } }