Merge pull request #2716 from CartoDB/2623-fix-new-public-pagination

Fix pagination on new public pages

NEWS.md

@@ -19,6 +19,7 @@ Bugfixes:
 * Redirect to list on delete table vis [#2697](https://github.com/CartoDB/cartodb/pull/2697)
 * Fix account settings order [#2700](https://github.com/CartoDB/cartodb/pull/2700)
 * Fix geocoding by Lon/Lat: refresh the table [#2699](https://github.com/CartoDB/cartodb/pull/2699)
+* Fix new public pagination [#2716](https://github.com/CartoDB/cartodb/pull/2716)
 
 3.8.1 (2015-02-26)
 ------------------

config/frontend.yml

@@ -1,2 +1,2 @@
 #This file defines the version of the frontend code that is going to be loaded.
-3.7.22
+3.7.23

lib/assets/javascripts/cartodb/new_common/dialogs/create/listing/datasets/datasets_paginator_view.js

@@ -17,39 +17,39 @@ module.exports = cdb.core.View.extend({
   className: 'DatasetsPaginator',
 
   initialize: function() {
-    var self = this;
     this.routerModel = this.options.routerModel;
+    this.collection = this.options.collection;
     this.model = new PaginationModel({
-      current_page: this.routerModel.get('page'),
-      url_to:       function(page) { return '' }
+      current_page: this.routerModel.get('page')
     });
-    
+
     this._initBinds();
+    this._initViews();
   },
 
   render: function() {
     this.clearSubViews();
-    this._initViews();
+    this.$el.append(this.paginationView.render().el);
     return this;
   },
 
   _initBinds: function() {
     this.model.bind('change', this.render, this);
+    this.model.bind('change:current_page', function() {
+      this.routerModel.set('page', this.model.get('current_page'));
+    }, this);
     this.collection.bind('reset', this._updatePaginationModelByCollection, this);
-    this.routerModel.bind('change', this._updatePaginationModelByRouter, this);
+    this.routerModel.bind('change:page', this._updatePaginationModelByRouterModel, this);
+
     this.add_related_model(this.routerModel);
     this.add_related_model(this.collection);
+    this.add_related_model(this.model);
   },
 
   _initViews: function() {
     this.paginationView = new PaginationView({
       model: this.model
     });
-    this.paginationView.bind('pageClicked', function(p) {
-      this.routerModel.set('page', p);
-    }, this);
-    this.paginationView.render();
-    this.$el.append(this.paginationView.el);
     this.addView(this.paginationView);
   },
 
@@ -60,8 +60,8 @@ module.exports = cdb.core.View.extend({
     });
   },
 
-  _updatePaginationModelByRouter: function() {
+  _updatePaginationModelByRouterModel: function() {
     this.model.set('current_page', this.routerModel.get('page'));
   }
 
-});
+});

lib/assets/javascripts/cartodb/new_common/views/pagination/model.js

@@ -1,11 +1,6 @@
 var cdb = require('cartodb.js');
 var _ = require('underscore');
 
-var urlToStub = function() {
-  throw new TypeError('Provide a url-function when instantiating a pagination model, ' +
-    'it should return a URL for the given page argument.');
-};
-
 /**
  * View model intended to be responsible for pagination logic, and to be used in conjunction with a Pagination view.
  */
@@ -16,7 +11,7 @@ module.exports = cdb.core.Model.extend({
     current_page:         1,
     display_count:        5,
     extras_display_count: 1,
-    url_to:               urlToStub
+    url_to:               undefined
   },
 
   pagesCount: function() {
@@ -36,7 +31,13 @@ module.exports = cdb.core.Model.extend({
   },
 
   urlTo: function(page) {
-    return this.get('url_to')(page);
+    if (this.hasUrl()) {
+      return this.get('url_to')(page);
+    }
+  },
+
+  hasUrl: function() {
+    return typeof this.get('url_to') === 'function';
   },
 
   /**

lib/assets/javascripts/cartodb/new_common/views/pagination/view.js

@@ -28,6 +28,10 @@ module.exports = cdb.core.View.extend({
     this.template = cdb.templates.getTemplate('new_common/views/pagination/template');
     this.router = this.options.router;
 
+    if (this.router && !this.model.hasUrl()) {
+      throw new Error('since router is set the model must have a url method set too');
+    }
+
     this.model.bind('change', this.render, this);
   },
 
@@ -50,15 +54,13 @@ module.exports = cdb.core.View.extend({
   },
 
   _paginate: function(ev) {
-    if (this.router !== undefined) {
+    if (this.router) {
       navigateThroughRouter.apply(this, arguments);
+    } else if (!this.model.hasUrl()) {
+      this.killEvent(ev);
     }
-    if (ev) {
-      ev.preventDefault();
-    }
+
     var page = $(ev.target).data('page');
-    this.trigger('pageClicked', page, this);
     this.model.set('current_page', page);
-    this.render(); // forced since the change event is not triggered here for some reason
   }
 });

lib/assets/test/spec/cartodb/new_common/views/pagination/model.spec.js

@@ -120,4 +120,20 @@ describe('new_common/views/pagination/model', function() {
       expect(this.m.isCurrentPage(-1)).toBeFalsy();
     });
   });
+
+  describe('.hasUrl', function() {
+    beforeEach(function() {
+      this.newModel = function(attrs) {
+        return new PaginationModel(attrs);
+      };
+    });
+
+    it('should return true if there is a url_to method set', function() {
+      expect(this.newModel({}).hasUrl()).toBeFalsy();
+      expect(this.newModel({ url_to: true }).hasUrl()).toBeFalsy();
+      expect(this.newModel({ url_to: {} }).hasUrl()).toBeFalsy();
+
+      expect(this.newModel({ url_to: function() {} }).hasUrl()).toBeTruthy();
+    });
+  });
 });

lib/assets/test/spec/cartodb/new_common/views/pagination/view.spec.js

@@ -120,7 +120,7 @@ describe('new_common/views/pagination/view', function() {
       });
     });
 
-    describe('whne view is not created with a router', function() {
+    describe('when view is not created with a router', function() {
       beforeEach(function() {
         this.router = undefined;
         this.createView();
@@ -131,9 +131,29 @@ describe('new_common/views/pagination/view', function() {
       it('should update the current page', function() {
         expect(this.called).toBeTruthy();
       });
+
+      describe("when model doesn't have any url method", function() {
+        beforeEach(function() {
+          this.model.set('url_to', undefined);
+          spyOn(this.view, 'killEvent').and.callThrough();
+          spyOn(this.model, 'set');
+          this.view.$('a').click();
+        });
+
+        it('should prevent default click behaviour', function() {
+          expect(this.view.killEvent).toHaveBeenCalled();
+          expect(this.model.set).toHaveBeenCalled();
+        });
+
+        it('should update the model with new current page', function() {
+          expect(this.model.set).toHaveBeenCalled();
+          expect(this.model.set).toHaveBeenCalledWith('current_page', jasmine.any(Number));
+        });
+      });
     });
   });
 
+
   afterEach(function() {
     this.view.clean();
   });

package.json

@@ -1,6 +1,6 @@
 {
   "name": "cartodb-ui",
-  "version": "3.7.22",
+  "version": "3.7.23",
   "description": "CartoDB UI frontend",
   "repository": {
     "type": "git",