Fix issue with overwritten filters (elastic#17713)

* Fix issue with overwritten filters

* Add tests

* Only add timerange filter predicate if index exists

src/ui/public/courier/data_source/__tests__/search_source.js

@@ -6,6 +6,12 @@ import { requestQueue } from '../../_request_queue';
 import { SearchSourceProvider } from '../search_source';
 import StubIndexPatternProv from 'test_utils/stub_index_pattern';
 
+function timeout() {
+  return new Promise(resolve => {
+    setTimeout(resolve);
+  });
+}
+
 describe('SearchSource', function () {
   require('test_utils/no_digest_promises').activateForSuite();
 
@@ -154,6 +160,48 @@ describe('SearchSource', function () {
     });
   });
 
+  describe('#onRequestStart()', () => {
+    it('should be called when starting a request', async () => {
+      const source = new SearchSource();
+      const fn = sinon.spy();
+      source.onRequestStart(fn);
+      const request = {};
+      source.requestIsStarting(request);
+      await timeout();
+      expect(fn.calledWith(source, request)).to.be(true);
+    });
+
+    it('should not be called on parent searchSource', async () => {
+      const parent = new SearchSource();
+      const source = new SearchSource().inherits(parent);
+
+      const fn = sinon.spy();
+      source.onRequestStart(fn);
+      const parentFn = sinon.spy();
+      parent.onRequestStart(parentFn);
+      const request = {};
+      source.requestIsStarting(request);
+      await timeout();
+      expect(fn.calledWith(source, request)).to.be(true);
+      expect(parentFn.notCalled).to.be(true);
+    });
+
+    it('should be called on parent searchSource if callParentStartHandlers is true', async () => {
+      const parent = new SearchSource();
+      const source = new SearchSource().inherits(parent, { callParentStartHandlers: true });
+
+      const fn = sinon.spy();
+      source.onRequestStart(fn);
+      const parentFn = sinon.spy();
+      parent.onRequestStart(parentFn);
+      const request = {};
+      source.requestIsStarting(request);
+      await timeout();
+      expect(fn.calledWith(source, request)).to.be(true);
+      expect(parentFn.calledWith(source, request)).to.be(true);
+    });
+  });
+
   describe('#_mergeProp', function () {
     describe('filter', function () {
       let source;

src/ui/public/courier/data_source/search_source.js

@@ -134,6 +134,7 @@ export function SearchSourceProvider(Promise, PromiseEmitter, Private, config) {
 
       this.history = [];
       this._requestStartHandlers = [];
+      this._inheritOptions = {};
 
       this._filterPredicates = [
         (filter) => {
@@ -197,8 +198,9 @@ export function SearchSourceProvider(Promise, PromiseEmitter, Private, config) {
      * @param  {SearchSource} searchSource - the parent searchSource
      * @return {this} - chainable
      */
-    inherits(parent) {
+    inherits(parent, options = {}) {
       this._parent = parent;
+      this._inheritOptions = options;
       return this;
     }
 
@@ -458,8 +460,19 @@ export function SearchSourceProvider(Promise, PromiseEmitter, Private, config) {
       this.activeFetchCount = (this.activeFetchCount || 0) + 1;
       this.history = [request];
 
+      const handlers = [...this._requestStartHandlers];
+      // If callparentStartHandlers has been set to true, we also call all
+      // handlers of parent search sources.
+      if (this._inheritOptions.callParentStartHandlers) {
+        let searchSource = this.getParent();
+        while (searchSource) {
+          handlers.push(...searchSource._requestStartHandlers);
+          searchSource = searchSource.getParent();
+        }
+      }
+
       return Promise
-        .map(this._requestStartHandlers, fn => fn(this, request))
+        .map(handlers, fn => fn(this, request))
         .then(_.noop);
     }
 

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

@@ -20,8 +20,8 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
       }
 
       const index = searchSource.index() || searchSource.getParent().index();
-      const timeFieldName = index.timeFieldName;
-      if (!timeFieldName) {
+      const timeFieldName = index && index.timeFieldName;
+      if (!index || !timeFieldName) {
         return true;
       }
 
@@ -40,11 +40,32 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
     name: 'courier',
     handler: function (vis, { appState, queryFilter, searchSource, timeRange }) {
 
-      searchSource.filter(() => {
+      // Create a new search source that inherits the original search source
+      // but has the propriate timeRange applied via a filter.
+      // This is a temporary solution until we properly pass down all required
+      // information for the request to the request handler (https://github.com/elastic/kibana/issues/16641).
+      // Using callParentStartHandlers: true we make sure, that the parent searchSource
+      // onSearchRequestStart will be called properly even though we use an inherited
+      // search source.
+      const requestSearchSource = new SearchSource().inherits(searchSource, { callParentStartHandlers: true });
+
+      // For now we need to mirror the history of the passed search source, since
+      // the spy panel wouldn't work otherwise.
+      Object.defineProperty(requestSearchSource, 'history', {
+        get() {
+          return requestSearchSource._parent.history;
+        },
+        set(history) {
+          return requestSearchSource._parent.history = history;
+        }
+      });
+
+      // Add the explicit passed timeRange as a filter to the requestSearchSource.
+      requestSearchSource.filter(() => {
         return timefilter.get(searchSource.index(), timeRange);
       });
 
-      removeSearchSourceParentTimefilter(searchSource, timeRange);
+      removeSearchSourceParentTimefilter(requestSearchSource);
 
       if (queryFilter && vis.editorMode) {
         searchSource.set('filter', queryFilter.getFilters());
@@ -75,7 +96,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
       return new Promise((resolve, reject) => {
         if (shouldQuery()) {
           delete vis.reload;
-          searchSource.onResults().then(resp => {
+          requestSearchSource.onResults().then(resp => {
             searchSource.lastQuery = {
               filter: _.cloneDeep(searchSource.get('filter')),
               query: _.cloneDeep(searchSource.get('query')),
@@ -88,7 +109,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) {
             return _.cloneDeep(resp);
           }).then(async resp => {
             for (const agg of vis.getAggConfig()) {
-              const nestedSearchSource = new SearchSource().inherits(searchSource);
+              const nestedSearchSource = new SearchSource().inherits(requestSearchSource);
               resp = await agg.type.postFlightRequest(resp, vis.aggs, agg, nestedSearchSource);
             }