Fix #42 add event handlers are run before remove handlers under clien…

…t mode
backbone-paginator · Feb 11, 2013 · aca1b14 · aca1b14
1 parent a6d0e46
commit aca1b14
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 26 deletions.
diff --git a/lib/backbone-pageable.js b/lib/backbone-pageable.js
@@ -404,7 +404,7 @@
         var pageStart = currentPage * pageSize, pageEnd = pageStart + pageSize;
 
         if (event == "add") {
-          var fullIndex, addAt, colToAdd;
+          var fullIndex, addAt, colToAdd, options = options || {};
           if (collection == fullCol) {
             fullIndex = fullCol.indexOf(model);
             if (fullIndex >= pageStart && fullIndex < pageEnd) {
@@ -420,40 +420,60 @@
               fullIndex;
           }
 
+
           ++state.totalRecords;
           pageCol.state = pageCol._checkState(state);
 
           if (colToAdd) {
             colToAdd.add(model, _extend({}, options || {}, {at: addAt}));
-            if (pageCol.length > pageSize) pageCol.pop();
+            if (pageCol.length > pageSize) {
+              var addHandlers = collection._events.add,
+              popOptions = {onAdd: true};
+              if (addHandlers.length) {
+                var lastAddHandler = addHandlers[addHandlers.length - 1];
+                var oldCallback = lastAddHandler.callback;
+                lastAddHandler.callback = function () {
+                  try {
+                    oldCallback.apply(this, arguments);
+                    pageCol.pop(popOptions);
+                  }
+                  finally {
+                    lastAddHandler.callback = oldCallback;
+                  }
+                };
+              }
+              else pageCol.pop(popOptions);
+            }
           }
         }
 
         // remove the model from the other collection as well
         if (event == "remove") {
+          if (!options.onAdd) {
+            // decrement totalRecords and update totalPages and lastPage
+            if (!--state.totalRecords) {
+              state.totalRecords = null;
+              state.totalPages = null;
+            }
+            else {
+              var totalPages = state.totalPages = ceil(state.totalRecords / pageSize);
+              state.lastPage = firstPage === 0 ? totalPages - 1 : totalPages;
+              if (state.currentPage > totalPages) state.currentPage = state.lastPage;
+            }
+            pageCol.state = pageCol._checkState(state);
 
-          // decrement totalRecords and update totalPages and lastPage
-          if (!--state.totalRecords) {
-            state.totalRecords = null;
-            state.totalPages = null;
-          }
-          else {
-            var totalPages = state.totalPages = ceil(state.totalRecords / pageSize);
-            state.lastPage = firstPage === 0 ? totalPages - 1 : totalPages;
-            if (state.currentPage > totalPages) state.currentPage = state.lastPage;
-          }
-          pageCol.state = pageCol._checkState(state);
-
-          var nextModel;
-          if (collection == pageCol) {
-            if (nextModel = fullCol.at(pageEnd)) pageCol.push(nextModel);
-            fullCol.remove(model);
-          }
-          else if (options.index >= pageStart && options.index < pageEnd) {
-            pageCol.remove(model);
-            nextModel = fullCol.at(currentPage * (pageSize + options.index));
-            if (nextModel) pageCol.push(nextModel);
+            var nextModel, removedIndex = options.index;
+            if (collection == pageCol) {
+              if (nextModel = fullCol.at(pageEnd)) pageCol.push(nextModel);
+              fullCol.remove(model);
+            }
+            else if (removedIndex >= pageStart && removedIndex < pageEnd) {
+              pageCol.remove(model);
+              nextModel = fullCol.at(currentPage * (pageSize + removedIndex));
+              if (nextModel) pageCol.push(nextModel);
+            }
           }
+          else delete options.onAdd;
         }
 
         if (event == "reset" || event == "sort") {
@@ -680,7 +700,7 @@
       else if (self.links) delete self.links;
 
       return options.fetch ?
-        self.fetch(_omit(options, ["fetch", "resetState"])) :
+        self.fetch(_omit(options, "fetch", "resetState")) :
         self;
     },
 

diff --git a/test/client-pageable.js b/test/client-pageable.js
@@ -122,7 +122,7 @@ $(document).ready(function () {
     strictEqual(col.fullCollection.at(2).get("name"), "c");
   });
 
-  test("add", 48, function () {
+  test("add", 41, function () {
     var col = new Backbone.PageableCollection(models, {
       state: {
         pageSize: 2
@@ -132,7 +132,6 @@ $(document).ready(function () {
 
     var lastTotalRecords = col.state.totalRecords;
     var onAdd = function () {
-      ok(true);
       strictEqual(col.state.totalRecords, lastTotalRecords + 1);
     };
     col.fullCollection.on("add", onAdd);
@@ -142,6 +141,7 @@ $(document).ready(function () {
     col.add(d);
     strictEqual(col.state.totalRecords, 4);
     strictEqual(col.state.totalPages, 2);
+    strictEqual(col.fullCollection.size(), 4);
     strictEqual(col.size(), 2);
     strictEqual(col.at(0).get("name"), "a");
     strictEqual(col.at(1).get("name"), "c");
@@ -155,6 +155,7 @@ $(document).ready(function () {
     col.fullCollection.push(e);
     strictEqual(col.state.totalRecords, 5);
     strictEqual(col.state.totalPages, 3);
+    strictEqual(col.fullCollection.size(), 5);
     strictEqual(col.size(), 2);
     strictEqual(col.at(0).get("name"), "a");
     strictEqual(col.at(1).get("name"), "c");
@@ -263,6 +264,27 @@ $(document).ready(function () {
     strictEqual(col.fullCollection.size(), 0);
   });
 
+  test("add handlers are run before remove handlers", 2, function () {
+    var addRan = false;
+    var onAdd = function () {
+      addRan = true;
+    };
+    var onRemove = function () {
+      strictEqual(addRan, true);
+      addRan = false;
+    };
+    var col = new Backbone.PageableCollection(models, {
+      state: {
+        pageSize: 1
+      },
+      mode: "client"
+    });
+    col.on("add", onAdd);
+    col.on("remove", onRemove);
+    col.unshift(new Backbone.Model({name: "d"}));
+    col.fullCollection.unshift(new Backbone.Model({name: "e"}));
+  });
+
   test("change", 6, function () {
     var col = new Backbone.PageableCollection(models, {
       state: {