Fix visualization individual timeranges (#17123) (#17383)

* Fix visualization individual timeranges * Make Vega request handler use passed in timeRange * Remove unneeded private variable * Use timeRange for courier caching check * Fix developer documentation * Fix date_histogram * Fix issue * Fix broken tests * Fix issue in discover visualization * Fix vega tests * Fix issue with saved search visualizations * Update timeRange correctly in editor
elastic · Mar 26, 2018 · 5b22bbb · 5b22bbb
1 parent e71a9cf
commit 5b22bbb
Show file tree

Hide file tree

Showing 13 changed files with 100 additions and 60 deletions.
diff --git a/docs/development/visualize/development-create-visualization.asciidoc b/docs/development/visualize/development-create-visualization.asciidoc
@@ -294,8 +294,12 @@ Using 'none' as your request handles means your visualization does not require a
 
 [[development-custom-request-handler]]
 ==== custom request handler
-You can define your custom request handler by providing a function with the following definition:
-`function (vis, appState, uiState, searchSource) { ... }`
+You can define your custom request handler by providing a function with the following signature:
+`function (vis, { uiState, appState, timeRange }) { ... }`
+
+The `timeRange` will be an object with a `from` and `to` key, that can contain
+datemath expressions, like `now-7d`. You can use the `datemath` library to parse
+them.
 
 This function must return a promise, which should get resolved with new data that will be passed to responseHandler.
 
@@ -306,7 +310,7 @@ It's up to function to decide when it wants to issue a new request or return pre
 -----------
 import { VisFactoryProvider } from 'ui/vis/vis_factory';
 
-const myRequestHandler = async (vis, appState, uiState, searchSource) => {
+const myRequestHandler = async (vis, { appState, uiState, timeRange }) => {
   const data = ... parse ...
   return data;
 };

diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js
@@ -710,6 +710,10 @@ function discoverController(
         return $scope.vis.getAggConfig().toDsl();
       });
     }
+
+    $scope.vis.filters = {
+      timeRange: timefilter.time
+    };
   }
 
   function resolveIndexPatternLoading() {

diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.html b/src/core_plugins/kibana/public/visualize/editor/editor.html
@@ -72,6 +72,7 @@
     app-state="state"
     editor-mode="chrome.getVisible()"
     show-spy-panel="chrome.getVisible()"
+    time-range="timeRange"
     >
 
   </visualize>

diff --git a/src/core_plugins/kibana/public/visualize/editor/editor.js b/src/core_plugins/kibana/public/visualize/editor/editor.js
@@ -189,6 +189,7 @@ function VisEditor($scope, $route, timefilter, AppState, $window, kbnUrl, courie
     $scope.isAddToDashMode = () => addToDashMode;
 
     $scope.timefilter = timefilter;
+    $scope.timeRange = timefilter.time;
     $scope.opts = _.pick($scope, 'doSave', 'savedVis', 'shareData', 'timefilter', 'isAddToDashMode');
 
     stateMonitor = stateMonitorFactory.create($state, stateDefaults);
@@ -219,6 +220,12 @@ function VisEditor($scope, $route, timefilter, AppState, $window, kbnUrl, courie
       }
     });
 
+    const updateTimeRange = () => {
+      $scope.timeRange = timefilter.time;
+    };
+
+    timefilter.on('update', updateTimeRange);
+
     // update the searchSource when filters update
     $scope.$listen(queryFilter, 'update', function () {
       $state.save();
@@ -233,6 +240,7 @@ function VisEditor($scope, $route, timefilter, AppState, $window, kbnUrl, courie
     $scope.$on('$destroy', function () {
       savedVis.destroy();
       stateMonitor.destroy();
+      timefilter.off('update', updateTimeRange);
     });
   }
 

diff --git a/src/core_plugins/metrics/public/kbn_vis_types/request_handler.js b/src/core_plugins/metrics/public/kbn_vis_types/request_handler.js
@@ -8,25 +8,25 @@ const MetricsRequestHandlerProvider = function (Private, Notifier, config, timef
 
   return {
     name: 'metrics',
-    handler: function (vis, appState, uiState) {
+    handler: function (vis, { uiState, timeRange }) {
       const timezone = Private(timezoneProvider)();
       return new Promise((resolve) => {
         const panel = vis.params;
         const uiStateObj = uiState.get(panel.type, {});
-        const timeRange = vis.params.timeRange || timefilter.getBounds();
+        const parsedTimeRange = timefilter.calculateBounds(timeRange);
         const scaledDataFormat = config.get('dateFormat:scaled');
         const dateFormat = config.get('dateFormat');
         if (panel && panel.id) {
           const params = {
-            timerange: { timezone, ...timeRange },
+            timerange: { timezone, ...parsedTimeRange },
             filters: [dashboardContext()],
             panels: [panel],
             state: uiStateObj
           };
 
           try {
             const maxBuckets = config.get('metrics:max_buckets');
-            validateInterval(timeRange, panel, maxBuckets);
+            validateInterval(parsedTimeRange, panel, maxBuckets);
             const httpResult = $http.post('../api/metrics/vis/data', params)
               .then(resp => ({ dateFormat, scaledDataFormat, timezone, ...resp.data }))
               .catch(resp => { throw resp.data; });

diff --git a/src/core_plugins/timelion/public/vis/timelion_request_handler.js b/src/core_plugins/timelion/public/vis/timelion_request_handler.js
@@ -2,7 +2,7 @@ import _ from 'lodash';
 import { dashboardContextProvider } from 'plugins/kibana/dashboard/dashboard_context';
 
 import { timezoneProvider } from 'ui/vis/lib/timezone';
-const TimelionRequestHandlerProvider = function (Private, Notifier, $http, $rootScope, timefilter) {
+const TimelionRequestHandlerProvider = function (Private, Notifier, $http) {
   const timezone = Private(timezoneProvider)();
   const dashboardContext = Private(dashboardContextProvider);
 
@@ -12,28 +12,20 @@ const TimelionRequestHandlerProvider = function (Private, Notifier, $http, $root
 
   return {
     name: 'timelion',
-    handler: function (vis /*, appState, uiState, queryFilter*/) {
+    handler: function (vis, { timeRange }) {
 
       return new Promise((resolve, reject) => {
         const expression = vis.params.expression;
         if (!expression) return;
 
-        let timeFilter = timefilter.time;
-        if (vis.params.timeRange) {
-          timeFilter = {
-            mode: 'absolute',
-            from: vis.params.timeRange.min.toJSON(),
-            to: vis.params.timeRange.max.toJSON()
-          };
-        }
         const httpResult = $http.post('../api/timelion/run', {
           sheet: [expression],
           extended: {
             es: {
               filter: dashboardContext()
             }
           },
-          time: _.extend(timeFilter, {
+          time: _.extend(timeRange, {
             interval: vis.params.interval,
             timezone: timezone
           }),

diff --git a/src/core_plugins/vega/public/data_model/__tests__/time_cache.js b/src/core_plugins/vega/public/data_model/__tests__/time_cache.js
@@ -17,7 +17,7 @@ describe(`TimeCache`, () => {
       this._max = max;
     }
 
-    getBounds() {
+    calculateBounds() {
       this._accessCount++;
       return {
         min: { valueOf: () => this._min },

diff --git a/src/core_plugins/vega/public/data_model/time_cache.js b/src/core_plugins/vega/public/data_model/time_cache.js
@@ -58,13 +58,17 @@ export class TimeCache {
     return this._cachedBounds;
   }
 
+  setTimeRange(timeRange) {
+    this._timeRange = timeRange;
+  }
+
   /**
    * Get parsed min/max values
    * @returns {{min: number, max: number}}
    * @private
    */
   _getBounds() {
-    const bounds = this._timefilter.getBounds();
+    const bounds = this._timefilter.calculateBounds(this._timeRange);
     return {
       min: bounds.min.valueOf(),
       max: bounds.max.valueOf()

diff --git a/src/core_plugins/vega/public/vega_request_handler.js b/src/core_plugins/vega/public/vega_request_handler.js
@@ -13,7 +13,8 @@ export function VegaRequestHandlerProvider(Private, es, timefilter, serviceSetti
 
     name: 'vega',
 
-    handler(vis) {
+    handler(vis, { timeRange }) {
+      timeCache.setTimeRange(timeRange);
       const vp = new VegaParser(vis.params.spec, searchCache, timeCache, dashboardContext, serviceSettings);
       return vp.parseAsync();
     }

diff --git a/src/ui/public/agg_types/__tests__/buckets/date_histogram/_params.js b/src/ui/public/agg_types/__tests__/buckets/date_histogram/_params.js
@@ -36,10 +36,12 @@ describe('params', function () {
     setTimeBounds = function (n, units) {
       timefilter.enableAutoRefreshSelector();
       timefilter.enableTimeRangeSelector();
-      timefilter.getBounds = _.constant({
-        min: now.clone().subtract(n, units),
-        max: now.clone()
-      });
+      paramWriter.vis.filters = {
+        timeRange: {
+          from: now.clone().subtract(n, units),
+          to: now.clone()
+        }
+      };
     };
   }));
 

diff --git a/src/ui/public/agg_types/buckets/date_histogram.js b/src/ui/public/agg_types/buckets/date_histogram.js
@@ -27,12 +27,9 @@ export function AggTypesBucketsDateHistogramProvider(timefilter, config, Private
   }
 
   function getBounds(vis) {
-    if (!vis.getTimeRange) {
-      return timefilter.getActiveBounds();
+    if (timefilter.isTimeRangeSelectorEnabled && vis.filters) {
+      return timefilter.calculateBounds(vis.filters.timeRange);
     }
-
-    const timeRange = vis.getTimeRange();
-    return timefilter.calculateBounds(timeRange);
   }
 
   function setBounds(agg, force) {

diff --git a/src/ui/public/vis/request_handlers/courier.js b/src/ui/public/vis/request_handlers/courier.js
@@ -5,10 +5,46 @@ import { VisRequestHandlersRegistryProvider } from 'ui/registry/vis_request_hand
 const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
   const SearchSource = Private(SearchSourceProvider);
 
+  /**
+   * TODO: This code can be removed as soon as we got rid of inheritance in the
+   * searchsource and pass down every filter explicitly.
+   * we're only adding one range filter against the timeFieldName to ensure
+   * that our filter is the only one applied and override the global filters.
+   * this does rely on the "implementation detail" that filters are added first
+   * on the leaf SearchSource and subsequently on the parents.
+   */
+  function removeSearchSourceParentTimefilter(searchSource) {
+    searchSource.addFilterPredicate((filter, state) => {
+      if (!filter.range) {
+        return true;
+      }
+
+      const index = searchSource.index() || searchSource.getParent().index();
+      const timeFieldName = index.timeFieldName;
+      if (!timeFieldName) {
+        return true;
+      }
+
+      // Only check if we need to filter out this filter if it's actual a range filter
+      // on our time field and not any other field.
+      if (!filter.range[timeFieldName]) {
+        return true;
+      }
+
+      return !(state.filters || []).find(f => f.range && f.range[timeFieldName]);
+    });
+
+  }
+
   return {
     name: 'courier',
-    handler: function (vis, appState, uiState, queryFilter, searchSource) {
+    handler: function (vis, { appState, queryFilter, searchSource, timeRange }) {
+
+      searchSource.filter(() => {
+        return timefilter.get(searchSource.index(), timeRange);
+      });
 
+      removeSearchSourceParentTimefilter(searchSource, timeRange);
 
       if (queryFilter && vis.editorMode) {
         searchSource.set('filter', queryFilter.getFilters());
@@ -31,7 +67,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
         if (!_.isEqual(_.cloneDeep(searchSource.get('filter')), searchSource.lastQuery.filter)) return true;
         if (!_.isEqual(_.cloneDeep(searchSource.get('query')), searchSource.lastQuery.query)) return true;
         if (!_.isEqual(_.cloneDeep(copyAggs(vis.aggs)), searchSource.lastQuery.aggs)) return true;
-        if (!_.isEqual(_.cloneDeep(timefilter.time), searchSource.lastQuery.time)) return true;
+        if (!_.isEqual(_.cloneDeep(timeRange), searchSource.lastQuery.timeRange)) return true;
 
         return false;
       };
@@ -44,7 +80,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
               filter: _.cloneDeep(searchSource.get('filter')),
               query: _.cloneDeep(searchSource.get('query')),
               aggs: _.cloneDeep(copyAggs(vis.aggs)),
-              time: _.cloneDeep(timefilter.time)
+              timeRange: _.cloneDeep(timeRange)
             };
 
             searchSource.rawResponse = resp;

diff --git a/src/ui/public/visualize/visualize.js b/src/ui/public/visualize/visualize.js
@@ -59,32 +59,6 @@ uiModules
 
         $scope.vis.description = $scope.savedObj.description;
 
-        if ($scope.timeRange) {
-          $scope.vis.getTimeRange = () => $scope.timeRange;
-
-          const searchSource = $scope.savedObj.searchSource;
-          searchSource.filter(() => {
-            return timefilter.get(searchSource.index(), $scope.timeRange);
-          });
-
-          // we're only adding one range filter against the timeFieldName to ensure
-          // that our filter is the only one applied and override the global filters.
-          // this does rely on the "implementation detail" that filters are added first
-          // on the leaf SearchSource and subsequently on the parents
-          searchSource.addFilterPredicate((filter, state) => {
-            if (!filter.range) {
-              return true;
-            }
-
-            const timeFieldName = searchSource.index().timeFieldName;
-            if (!timeFieldName) {
-              return true;
-            }
-
-            return !(state.filters || []).find(f => f.range && f.range[timeFieldName]);
-          });
-        }
-
         $scope.editorMode = $scope.editorMode || false;
         $scope.vis.editorMode = $scope.editorMode;
 
@@ -96,8 +70,25 @@ uiModules
           // was still waiting for its debounce, in this case we don't want to start
           // fetching new data and rendering.
           if (!$scope.vis.initialized || !$scope.savedObj || destroyed) return;
+
+          // TODO: This should ALWAYS be passed into this component via the loader
+          // in the future. Unfortunately we need some refactoring in dashboard
+          // to make this working and correctly rerender, so for now we will either
+          // use the one passed in to us or look into the timefilter ourselfs (which
+          // will be removed in the future).
+          const timeRange = $scope.timeRange || timefilter.time;
+
+          $scope.vis.filters = { timeRange };
+
+          const handlerParams = {
+            appState: $scope.appState,
+            uiState: $scope.uiState,
+            queryFilter: queryFilter,
+            searchSource: $scope.savedObj.searchSource,
+            timeRange: timeRange,
+          };
           // searchSource is only there for courier request handler
-          requestHandler($scope.vis, $scope.appState, $scope.uiState, queryFilter, $scope.savedObj.searchSource)
+          requestHandler($scope.vis, handlerParams)
             .then(requestHandlerResponse => {
 
             //No need to call the response handler when there have been no data nor has been there changes