[6.x] Replace vis.reload by forceFetch requestHandler param (#19296) | Fix breaking tests on master (#19344) (#19345)

* Replace vis.reload by forceFetch requestHandler param (#19296)

* Fix breaking tests on master (#19344)

src/ui/public/vis/request_handlers/courier.js

@@ -20,7 +20,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
 
   return {
     name: 'courier',
-    handler: function (vis, { appState, queryFilter, searchSource, timeRange }) {
+    handler: function (vis, { appState, queryFilter, searchSource, timeRange, forceFetch }) {
 
       // Create a new search source that inherits the original search source
       // but has the propriate timeRange applied via a filter.
@@ -63,7 +63,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
       }
 
       const shouldQuery = () => {
-        if (!searchSource.lastQuery || vis.reload) return true;
+        if (!searchSource.lastQuery || forceFetch) return true;
         if (!_.isEqual(_.cloneDeep(searchSource.get('filter')), searchSource.lastQuery.filter)) return true;
         if (!_.isEqual(_.cloneDeep(searchSource.get('query')), searchSource.lastQuery.query)) return true;
         if (!_.isEqual(calculateObjectHash(vis.getAggConfig()), searchSource.lastQuery.aggs)) return true;
@@ -74,7 +74,6 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
 
       return new Promise((resolve, reject) => {
         if (shouldQuery()) {
-          delete vis.reload;
           requestSearchSource.onResults().then(resp => {
             searchSource.lastQuery = {
               filter: _.cloneDeep(searchSource.get('filter')),

src/ui/public/visualize/__tests__/visualize.js

@@ -1,10 +1,12 @@
 import $ from 'jquery';
 import expect from 'expect.js';
 import ngMock from 'ng_mock';
+import sinon from 'sinon';
 import { VisProvider } from '../../vis';
 import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern';
 import FixturesStubbedSearchSourceProvider from 'fixtures/stubbed_search_source';
 import MockState from 'fixtures/mock_state';
+import { PersistedState } from '../../persisted_state';
 
 describe('visualize directive', function () {
   let $rootScope;
@@ -16,6 +18,7 @@ describe('visualize directive', function () {
   let fixtures;
   let searchSource;
   let appState;
+  let uiState;
 
   beforeEach(ngMock.module('kibana', 'kibana/table_vis'));
   beforeEach(ngMock.inject(function (Private, $injector) {
@@ -25,11 +28,11 @@ describe('visualize directive', function () {
     Vis = Private(VisProvider);
     appState = new MockState({ filters: [] });
     appState.toJSON = () => { return {}; };
+    uiState = new PersistedState({});
     indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider);
     searchSource = Private(FixturesStubbedSearchSourceProvider);
 
-    const requiresSearch = false;
-    init(new CreateVis(null, requiresSearch), fixtures.oneRangeBucket);
+    init(new CreateVis(null), fixtures.oneRangeBucket);
   }));
 
   afterEach(() => {
@@ -42,7 +45,7 @@ describe('visualize directive', function () {
 
     $rootScope.vis = vis;
     $rootScope.esResponse = esResponse;
-    $rootScope.uiState = require('fixtures/mock_ui_state');
+    $rootScope.uiState = uiState;
     $rootScope.appState = appState;
     $rootScope.appState.vis = vis.getState();
     $rootScope.searchSource = searchSource;
@@ -57,7 +60,7 @@ describe('visualize directive', function () {
     $scope = $el.isolateScope();
   }
 
-  function CreateVis(params, requiresSearch) {
+  function CreateVis(params, requestHandler = 'none') {
     const vis = new Vis(indexPattern, {
       type: 'table',
       params: params || {},
@@ -77,7 +80,7 @@ describe('visualize directive', function () {
       ]
     });
 
-    vis.type.requestHandler = requiresSearch ? 'default' : 'none';
+    vis.type.requestHandler = requestHandler;
     vis.type.responseHandler = 'none';
     vis.type.requiresSearch = false;
     return vis;
@@ -103,9 +106,79 @@ describe('visualize directive', function () {
     expect($scope.appState.vis).to.not.equal({});
   });
 
-  it('sets force flag on force event', () => {
-    $scope.vis.emit('reload');
-    expect($scope.vis.reload).to.equal(true);
+  describe('request handler', () => {
+
+    const requestHandler = sinon.stub().resolves();
+
+    /**
+     * Asserts that a specific parameter had a specific value in the last call to the requestHandler.
+     */
+    function assertParam(obj) {
+      sinon.assert.calledWith(requestHandler, sinon.match.any, sinon.match(obj));
+    }
+
+    /**
+     * Wait for the next $scope.fetch call.
+     * Since we use an old lodash version we cannot use fake timers here.
+     */
+    function waitForFetch() {
+      return new Promise(resolve => { setTimeout(resolve, 150); });
+    }
+
+    beforeEach(() => {
+      init(new CreateVis(null, requestHandler), fixtures.oneRangeBucket);
+    });
+
+    afterEach(() => {
+      requestHandler.resetHistory();
+    });
+
+    describe('forceFetch param', () => {
+      it('should be true if triggered via vis.forceReload', async () => {
+        $scope.vis.forceReload();
+        await waitForFetch();
+        sinon.assert.calledOnce(requestHandler);
+        assertParam({ forceFetch: true });
+      });
+
+      it('should be true if triggered via courier:searchRefresh event', async () => {
+        $scope.$emit('courier:searchRefresh');
+        await waitForFetch();
+        sinon.assert.calledOnce(requestHandler);
+        assertParam({ forceFetch: true });
+      });
+
+      it('should be true if triggered via fetch event', async () => {
+        $scope.$emit('fetch');
+        await waitForFetch();
+        sinon.assert.calledOnce(requestHandler);
+        assertParam({ forceFetch: true });
+      });
+
+      it('should be false if triggered via resize event', async () => {
+        $el.width(400);
+        $el.height(500);
+        await waitForFetch();
+        sinon.assert.calledOnce(requestHandler);
+        assertParam({ forceFetch: false });
+      });
+
+      it('should be false if triggered via uiState change', async () => {
+        uiState.set('foo', 'bar');
+        await waitForFetch();
+        sinon.assert.calledOnce(requestHandler);
+        assertParam({ forceFetch: false });
+      });
+
+      it('should be true if at least one trigger required it to be true', async () => {
+        $el.width(400);
+        $scope.vis.forceReload(); // This requires forceFetch to be true
+        uiState.set('foo', 'bar');
+        await waitForFetch();
+        sinon.assert.calledOnce(requestHandler);
+        assertParam({ forceFetch: true });
+      });
+    });
   });
 
 });

src/ui/public/visualize/visualize.js

@@ -40,6 +40,7 @@ uiModules
       template: visualizeTemplate,
       link: async function ($scope, $el) {
         let destroyed = false;
+        let forceFetch = false;
         if (!$scope.savedObj) throw(`saved object was not provided to <visualize> directive`);
         if (!$scope.appState) $scope.appState = getAppState();
 
@@ -87,7 +88,12 @@ uiModules
             queryFilter: queryFilter,
             searchSource: $scope.savedObj.searchSource,
             timeRange: timeRange,
+            forceFetch,
           };
+
+          // Reset forceFetch flag, since we are now executing our forceFetch in case it was true
+          forceFetch = false;
+
           // searchSource is only there for courier request handler
           requestHandler($scope.vis, handlerParams)
             .then(requestHandlerResponse => {
@@ -135,7 +141,7 @@ uiModules
 
 
         const reload = () => {
-          $scope.vis.reload = true;
+          forceFetch = true;
           $scope.fetch();
         };
         $scope.vis.on('reload', reload);