Fix duplicate fetch in Visualize (#41204)

* Fix duplicate fetch in Visualize * Don't refetch when disabled filters change * Use query filter fetches observable * Add back in watch on filters * Revert change completely * Update test to include non-disabled filter * Remove unnecessary checks
elastic · Aug 8, 2019 · 59c1eb9 · 59c1eb9
1 parent bf7ca4d
commit 59c1eb9
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 16 deletions.
diff --git a/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js b/src/legacy/core_plugins/kibana/public/visualize/editor/editor.js
@@ -402,30 +402,33 @@ function VisEditor(
     $scope.$listenAndDigestAsync(timefilter, 'timeUpdate', updateTimeRange);
     $scope.$listenAndDigestAsync(timefilter, 'refreshIntervalUpdate', updateRefreshInterval);
 
-    // update the searchSource when filters update
-    const filterUpdateSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), {
-      next: () => {
-        $scope.filters = queryFilter.getFilters();
-        $scope.fetch();
-      }
-    });
-
     // update the searchSource when query updates
     $scope.fetch = function () {
       $state.save();
       savedVis.searchSource.setField('query', $state.query);
       savedVis.searchSource.setField('filter', $state.filters);
-      $scope.globalFilters = queryFilter.getGlobalFilters();
       $scope.vis.forceReload();
     };
 
+    // update the searchSource when filters update
+    const filterUpdateSubscription = subscribeWithScope($scope, queryFilter.getUpdates$(), {
+      next: () => {
+        $scope.filters = queryFilter.getFilters();
+        $scope.globalFilters = queryFilter.getGlobalFilters();
+      }
+    });
+    const filterFetchSubscription = subscribeWithScope($scope, queryFilter.getFetches$(), {
+      next: $scope.fetch
+    });
+
     $scope.$on('$destroy', function () {
       if ($scope._handler) {
         $scope._handler.destroy();
       }
       savedVis.destroy();
       stateMonitor.destroy();
       filterUpdateSubscription.unsubscribe();
+      filterFetchSubscription.unsubscribe();
     });
 
     if (!$scope.chrome.getVisible()) {
@@ -441,9 +444,16 @@ function VisEditor(
   }
 
   $scope.updateQueryAndFetch = function ({ query, dateRange }) {
-    timefilter.setTime(dateRange);
+    const isUpdate = (
+      (query && !_.isEqual(query, $state.query)) ||
+      (dateRange && !_.isEqual(dateRange, $scope.timeRange))
+    );
+
     $state.query = query;
-    $scope.fetch();
+    timefilter.setTime(dateRange);
+
+    // If nothing has changed, trigger the fetch manually, otherwise it will happen as a result of the changes
+    if (!isUpdate) $scope.fetch();
   };
 
   $scope.onRefreshChange = function ({ isPaused, refreshInterval }) {

diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts
@@ -157,7 +157,7 @@ describe('EmbeddedVisualizeHandler', () => {
     });
 
     it('should call dataLoader.render with updated filters', () => {
-      const params = { filters: [{ foo: 'bar' }] };
+      const params = { filters: [{ meta: { disabled: false } }] };
       handler.update(params);
       jest.runAllTimers();
       expect(mockDataLoaderFetch).toHaveBeenCalledWith({ ...dataLoaderParams, ...params });

diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts
@@ -18,7 +18,7 @@
  */
 
 import { EventEmitter } from 'events';
-import { debounce, forEach, get } from 'lodash';
+import { debounce, forEach, get, isEqual } from 'lodash';
 import * as Rx from 'rxjs';
 import { share } from 'rxjs/operators';
 import { i18n } from '@kbn/i18n';
@@ -38,6 +38,7 @@ import { VisFiltersProvider } from '../../vis/vis_filters';
 import { PipelineDataLoader } from './pipeline_data_loader';
 import { visualizationLoader } from './visualization_loader';
 import { VisualizeDataLoader } from './visualize_data_loader';
+import { onlyDisabledFiltersChanged } from '../../../../core_plugins/data/public';
 
 import { DataAdapter, RequestAdapter } from '../../inspector/adapters';
 
@@ -236,15 +237,21 @@ export class EmbeddedVisualizeHandler {
     }
 
     let fetchRequired = false;
-    if (params.hasOwnProperty('timeRange')) {
+    if (
+      params.hasOwnProperty('timeRange') &&
+      !isEqual(this.dataLoaderParams.timeRange, params.timeRange)
+    ) {
       fetchRequired = true;
       this.dataLoaderParams.timeRange = params.timeRange;
     }
-    if (params.hasOwnProperty('filters')) {
+    if (
+      params.hasOwnProperty('filters') &&
+      !onlyDisabledFiltersChanged(this.dataLoaderParams.filters, params.filters)
+    ) {
       fetchRequired = true;
       this.dataLoaderParams.filters = params.filters;
     }
-    if (params.hasOwnProperty('query')) {
+    if (params.hasOwnProperty('query') && !isEqual(this.dataLoaderParams.query, params.query)) {
       fetchRequired = true;
       this.dataLoaderParams.query = params.query;
     }