Merge pull request #2118 from CartoDB/onpremise-510

Avoid problems with cancelled requests in dataviews
CartoDB · May 21, 2018 · d58105d · d58105d
2 parents 4c2fa70 + c4865e5
commit d58105d
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 7 deletions.
diff --git a/src/dataviews/dataview-model-base.js b/src/dataviews/dataview-model-base.js
@@ -72,6 +72,8 @@ module.exports = Model.extend({
     opts = opts || {};
     util.checkRequiredOpts(opts, REQUIRED_OPTS, 'DataviewModelBase');
 
+    this._hasBinds = false;
+
     this._engine = opts.engine;
 
     if (!attrs.source) throw new Error('source is a required attr');
@@ -102,9 +104,9 @@ module.exports = Model.extend({
       this._checkBBoxFilter();
       if (this.syncsOnBoundingBoxChanges() && !this._bboxFilter.areBoundsAvailable()) {
         // wait until map gets bounds from view
-        this.listenTo(this._bboxFilter, 'boundsChanged', this._initialFetch);
+        this.listenTo(this._bboxFilter, 'boundsChanged', this._fetch);
       } else {
-        this._initialFetch();
+        this._fetch();
       }
     });
 
@@ -123,31 +125,39 @@ module.exports = Model.extend({
     this.on('change:url', function (model, value, opts) {
       this._newDataAvailable = true;
       if (this._shouldFetchOnURLChange(opts && _.pick(opts, ['forceFetch', 'sourceId']))) {
-        this.fetch();
+        this.refresh();
       }
     }, this);
 
     this.on('change:enabled', function (mdl, isEnabled) {
       if (isEnabled && this._newDataAvailable) {
-        this.fetch();
+        this.refresh();
         this._newDataAvailable = false;
       }
     }, this);
   },
 
   _onMapBoundsChanged: function () {
     if (this._shouldFetchOnBoundingBoxChange()) {
-      this.fetch();
+      // If the widget is the first one created it changes the map bounds
+      // and cacels the first ._fetch request so we have to call ._fetch here
+      // instead of .refresh to set the binds if they're not set up yet
+      this._fetch();
     }
 
     if (this.syncsOnBoundingBoxChanges()) {
       this._newDataAvailable = true;
     }
   },
 
-  _initialFetch: function () {
+  _fetch: function () {
     this.fetch({
-      success: this._onChangeBinds.bind(this)
+      success: function () {
+        if (!this._hasBinds) {
+          this._hasBinds = true;
+          this._onChangeBinds();
+        }
+      }.bind(this)
     });
   },
 

diff --git a/test/spec/dataviews/dataview-model-base.spec.js b/test/spec/dataviews/dataview-model-base.spec.js
@@ -602,6 +602,45 @@ describe('dataviews/dataview-model-base', function () {
       expect(error.message).toEqual('an error');
     });
   });
+
+  describe('._fetch', function () {
+    describe('when request is success', function () {
+      beforeEach(function () {
+        this.model.fetch = function (opts) {
+          opts.success();
+        };
+      });
+
+      it('sets _hasBinds to true', function () {
+        expect(this.model._hasBinds).toBe(false);
+
+        this.model._fetch();
+
+        expect(this.model._hasBinds).toBe(true);
+      });
+
+      it('calls ._onChangeBinds', function () {
+        spyOn(this.model, '_onChangeBinds');
+
+        this.model._fetch();
+
+        expect(this.model._onChangeBinds).toHaveBeenCalled();
+      });
+    });
+  });
+
+  describe('._onMapBoundsChanged', function () {
+    describe('when _shouldFetchOnBoundingBoxChange is true', function () {
+      it('calls ._fetch', function () {
+        spyOn(this.model, '_shouldFetchOnBoundingBoxChange').and.returnValue(true);
+        spyOn(this.model, '_fetch');
+
+        this.model._onMapBoundsChanged();
+
+        expect(this.model._fetch).toHaveBeenCalled();
+      });
+    });
+  });
 });
 
 function sharedTestsForAnalysisEvents () {