[ML] Single Metric Viewer: Fix zoom on forecast selection. (#55685)

Fixes adjusting the focus chart zoom range when a forecast gets selected. The code is cleaned up so appStateHandler to set the zoom range in the url is only called in one place within contextChartSelected().
2020-01-23 17:16:09 +01:00 · 2020-01-23 17:16:09 +01:00 · a4cf4f4b75
parent 0c25cb53b2
commit a4cf4f4b75
4 changed files with 83 additions and 77 deletions
--- a/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx
+++ b/x-pack/legacy/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx
@ -72,6 +72,11 @@ const PageWrapper: FC<PageProps> = ({ config, deps }) => {
  );
 };

+interface AppStateZoom {
+  from: string;
+  to: string;
+}
+
 interface TimeSeriesExplorerUrlStateManager {
  config: any;
  jobsWithTimeRange: MlJobWithTimeRange[];
@ -159,10 +164,9 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
    : +appState?.mlTimeSeriesExplorer?.detectorIndex || 0;
  const selectedEntities = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.entities;
  const selectedForecastId = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.forecastId;
-  const zoom: {
-    from: string;
-    to: string;
-  } = isJobChange ? undefined : appState?.mlTimeSeriesExplorer?.zoom;
+  const zoom: AppStateZoom | undefined = isJobChange
+    ? undefined
+    : appState?.mlTimeSeriesExplorer?.zoom;

  const selectedJob = selectedJobId !== undefined ? mlJobService.getJob(selectedJobId) : undefined;
  const timeSeriesJobs = createTimeSeriesJobData(mlJobService.jobs);
@ -195,6 +199,7 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan

        case APP_STATE_ACTION.SET_FORECAST_ID:
          mlTimeSeriesExplorer.forecastId = payload;
+          delete mlTimeSeriesExplorer.zoom;
          break;

        case APP_STATE_ACTION.SET_ZOOM:
@ -213,6 +218,11 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan

  const boundsMinMs = bounds?.min?.valueOf();
  const boundsMaxMs = bounds?.max?.valueOf();
+
+  const [selectedForecastIdProp, setSelectedForecastIdProp] = useState<string | undefined>(
+    appState?.mlTimeSeriesExplorer?.forecastId
+  );
+
  useEffect(() => {
    if (
      autoZoomDuration !== undefined &&
@ -221,6 +231,9 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
      selectedJob !== undefined &&
      selectedForecastId !== undefined
    ) {
+      if (selectedForecastIdProp !== selectedForecastId) {
+        setSelectedForecastIdProp(undefined);
+      }
      mlForecastService
        .getForecastDateRange(selectedJob, selectedForecastId)
        .then(resp => {
@ -231,20 +244,6 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
          const earliest = moment(resp.earliest || boundsMinMs);
          const latest = moment(resp.latest || boundsMaxMs);

-          // Set the zoom to centre on the start of the forecast range, depending
-          // on the time range of the forecast and data.
-          // const earliestDataDate = first(contextChartData).date;
-          const zoomLatestMs = Math.min(
-            earliest.valueOf() + autoZoomDuration / 2,
-            latest.valueOf()
-          );
-          const zoomEarliestMs = zoomLatestMs - autoZoomDuration;
-          const zoomState = {
-            from: moment(zoomEarliestMs).toISOString(),
-            to: moment(zoomLatestMs).toISOString(),
-          };
-          appStateHandler(APP_STATE_ACTION.SET_ZOOM, zoomState);
-
          if (earliest.isBefore(moment(boundsMinMs)) || latest.isAfter(moment(boundsMaxMs))) {
            const earliestMs = Math.min(earliest.valueOf(), boundsMinMs);
            const latestMs = Math.max(latest.valueOf(), boundsMaxMs);
@ -253,6 +252,7 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
              to: moment(latestMs).toISOString(),
            });
          }
+          setSelectedForecastIdProp(selectedForecastId);
        })
        .catch(resp => {
          // eslint-disable-next-line no-console
@ -282,6 +282,11 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
    return null;
  }

+  const zoomProp: AppStateZoom | undefined =
+    typeof selectedForecastId === 'string' && selectedForecastIdProp === undefined
+      ? undefined
+      : zoom;
+
  return (
    <TimeSeriesExplorer
      {...{
@ -293,12 +298,11 @@ export const TimeSeriesExplorerUrlStateManager: FC<TimeSeriesExplorerUrlStateMan
        selectedJobId,
        selectedDetectorIndex,
        selectedEntities,
-        selectedForecastId,
-        setGlobalState,
+        selectedForecastId: selectedForecastIdProp,
        tableInterval: tableInterval.val,
        tableSeverity: tableSeverity.val,
        timefilter,
-        zoom,
+        zoom: zoomProp,
      }}
    />
  );
--- a/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js
+++ b/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js
@ -385,6 +385,7 @@ const TimeseriesChartIntl = injectI18n(
      drawContextElements(context, this.vizWidth, contextChartHeight, swimlaneHeight);
    }

+    contextChartInitialized = false;
    drawContextChartSelection() {
      const {
        contextChartData,
@ -446,7 +447,10 @@ const TimeseriesChartIntl = injectI18n(
        };
        if (!_.isEqual(newSelectedBounds, this.selectedBounds)) {
          this.selectedBounds = newSelectedBounds;
-          contextChartSelected({ from: contextXScaleDomain[0], to: contextXScaleDomain[1] });
+          if (this.contextChartInitialized === false) {
+            this.contextChartInitialized = true;
+            contextChartSelected({ from: contextXScaleDomain[0], to: contextXScaleDomain[1] });
+          }
        }
      }
    }
--- a/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts
+++ b/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.d.ts
@ -19,8 +19,7 @@ declare const TimeSeriesExplorer: FC<{
  selectedJobId: string;
  selectedDetectorIndex: number;
  selectedEntities: any[];
-  selectedForecastId: string;
-  setGlobalState: (arg: any) => void;
+  selectedForecastId?: string;
  tableInterval: string;
  tableSeverity: number;
  zoom?: { from: string; to: string };
--- a/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js
+++ b/x-pack/legacy/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js
@ -188,6 +188,24 @@ export class TimeSeriesExplorer extends React.Component {
   */
  contextChart$ = new Subject();

+  /**
+   * Returns field names that don't have a selection yet.
+   */
+  getFieldNamesWithEmptyValues = () => {
+    const latestEntityControls = this.getControlsForDetector();
+    return latestEntityControls
+      .filter(({ fieldValue }) => !fieldValue)
+      .map(({ fieldName }) => fieldName);
+  };
+
+  /**
+   * Checks if all entity control dropdowns have a selection.
+   */
+  arePartitioningFieldsProvided = () => {
+    const fieldNamesWithEmptyValues = this.getFieldNamesWithEmptyValues();
+    return fieldNamesWithEmptyValues.length === 0;
+  };
+
  detectorIndexChangeHandler = e => {
    const { appStateHandler } = this.props;
    const id = e.target.value;
@ -296,7 +314,17 @@ export class TimeSeriesExplorer extends React.Component {
  }

  contextChartSelected = selection => {
+    const zoomState = {
+      from: selection.from.toISOString(),
+      to: selection.to.toISOString(),
+    };
+
+    if (isEqual(this.props.zoom, zoomState) && this.state.focusChartData !== undefined) {
+      return;
+    }
+
    this.contextChart$.next(selection);
+    this.props.appStateHandler(APP_STATE_ACTION.SET_ZOOM, zoomState);
  };

  entityFieldValueChanged = (entity, fieldValue) => {
@ -509,27 +537,38 @@ export class TimeSeriesExplorer extends React.Component {
              (Array.isArray(stateUpdate.contextForecastData) &&
                stateUpdate.contextForecastData.length > 0);
            stateUpdate.loading = false;
+
            // Set zoomFrom/zoomTo attributes in scope which will result in the metric chart automatically
            // selecting the specified range in the context chart, and so loading that date range in the focus chart.
-            if (stateUpdate.contextChartData.length) {
+            // Only touch the zoom range if data for the context chart has been loaded and all necessary
+            // partition fields have a selection.
+            if (
+              stateUpdate.contextChartData.length &&
+              this.arePartitioningFieldsProvided() === true
+            ) {
              // Check for a zoom parameter in the appState (URL).
              let focusRange = calculateInitialFocusRange(
                zoom,
                stateUpdate.contextAggregationInterval,
                bounds
              );
-
-              if (focusRange === undefined) {
+              if (
+                focusRange === undefined ||
+                this.previousSelectedForecastId !== this.props.selectedForecastId
+              ) {
                focusRange = calculateDefaultFocusRange(
                  autoZoomDuration,
                  stateUpdate.contextAggregationInterval,
                  stateUpdate.contextChartData,
                  stateUpdate.contextForecastData
                );
+                this.previousSelectedForecastId = this.props.selectedForecastId;
              }

-              stateUpdate.zoomFrom = focusRange[0];
-              stateUpdate.zoomTo = focusRange[1];
+              this.contextChartSelected({
+                from: focusRange[0],
+                to: focusRange[1],
+              });
            }

            this.setState(stateUpdate);
@ -881,11 +920,6 @@ export class TimeSeriesExplorer extends React.Component {
            ...refreshFocusData,
            ...tableData,
          });
-          const zoomState = {
-            from: selection.from.toISOString(),
-            to: selection.to.toISOString(),
-          };
-          this.props.appStateHandler(APP_STATE_ACTION.SET_ZOOM, zoomState);
        })
    );

@ -917,6 +951,11 @@ export class TimeSeriesExplorer extends React.Component {
      if (this.props.selectedForecastId !== undefined) {
        // Ensure the forecast data will be shown if hidden previously.
        this.setState({ showForecast: true });
+        // Not best practice but we need the previous value for another comparison
+        // once all the data was loaded.
+        if (previousProps !== undefined) {
+          this.previousSelectedForecastId = previousProps.selectedForecastId;
+        }
      }
    }

@ -927,8 +966,7 @@ export class TimeSeriesExplorer extends React.Component {
      !isEqual(previousProps.selectedDetectorIndex, this.props.selectedDetectorIndex) ||
      !isEqual(previousProps.selectedEntities, this.props.selectedEntities) ||
      !isEqual(previousProps.selectedForecastId, this.props.selectedForecastId) ||
-      previousProps.selectedJobId !== this.props.selectedJobId ||
-      !isEqual(previousProps.zoom, this.props.zoom)
+      previousProps.selectedJobId !== this.props.selectedJobId
    ) {
      const fullRefresh =
        previousProps === undefined ||
@ -961,41 +999,6 @@ export class TimeSeriesExplorer extends React.Component {
    ) {
      tableControlsListener();
    }
-
-    if (
-      this.props.autoZoomDuration === undefined ||
-      this.props.selectedForecastId !== undefined ||
-      this.state.contextAggregationInterval === undefined ||
-      this.state.contextChartData === undefined ||
-      this.state.contextChartData.length === 0
-    ) {
-      return;
-    }
-
-    const defaultRange = calculateDefaultFocusRange(
-      this.props.autoZoomDuration,
-      this.state.contextAggregationInterval,
-      this.state.contextChartData,
-      this.state.contextForecastData
-    );
-
-    const selection = {
-      from: this.state.zoomFrom,
-      to: this.state.zoomTo,
-    };
-
-    if (
-      (selection.from.getTime() !== defaultRange[0].getTime() ||
-        selection.to.getTime() !== defaultRange[1].getTime()) &&
-      isNaN(Date.parse(selection.from)) === false &&
-      isNaN(Date.parse(selection.to)) === false
-    ) {
-      const zoomState = {
-        from: selection.from.toISOString(),
-        to: selection.to.toISOString(),
-      };
-      this.props.appStateHandler(APP_STATE_ACTION.SET_ZOOM, zoomState);
-    }
  }

  componentWillUnmount() {
@ -1070,12 +1073,8 @@ export class TimeSeriesExplorer extends React.Component {

    const selectedJob = mlJobService.getJob(selectedJobId);
    const entityControls = this.getControlsForDetector();
-
-    const fieldNamesWithEmptyValues = entityControls
-      .filter(({ fieldValue }) => !fieldValue)
-      .map(({ fieldName }) => fieldName);
-
-    const arePartitioningFieldsProvided = fieldNamesWithEmptyValues.length === 0;
+    const fieldNamesWithEmptyValues = this.getFieldNamesWithEmptyValues();
+    const arePartitioningFieldsProvided = this.arePartitioningFieldsProvided();

    const detectorSelectOptions = getViewableDetectors(selectedJob).map(d => ({
      value: d.index,