Fix jashkenas#1655 - sortBy & groupBy use attributes.

matthewyang · Sep 17, 2012 · c344201 · c344201
1 parent 07b88ef
commit c344201
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 13 deletions.
diff --git a/backbone.js b/backbone.js
@@ -735,13 +735,7 @@
         throw new Error('Cannot sort a set without a comparator');
       }
 
-      // If provided an attribute name, use it to sort the collection.
-      if (_.isString(this.comparator)) {
-        var attr = this.comparator;
-        this.comparator = function(model){ return model.get(attr); };
-      }
-
-      if (this.comparator.length === 1) {
+      if (_.isString(this.comparator) || this.comparator.length === 1) {
         this.models = this.sortBy(this.comparator, this);
       } else {
         this.models.sort(_.bind(this.comparator, this));
@@ -753,7 +747,7 @@
 
     // Pluck an attribute from each model in the collection.
     pluck: function(attr) {
-      return _.map(this.models, function(model){ return model.get(attr); });
+      return _.invoke(this.models, 'get', attr);
     },
 
     // When you have more items than you want to add or remove individually,
@@ -867,9 +861,9 @@
   var methods = ['forEach', 'each', 'map', 'collect', 'reduce', 'foldl',
     'inject', 'reduceRight', 'foldr', 'find', 'detect', 'filter', 'select',
     'reject', 'every', 'all', 'some', 'any', 'include', 'contains', 'invoke',
-    'max', 'min', 'sortBy', 'sortedIndex', 'toArray', 'size', 'first', 'head',
-    'take', 'initial', 'rest', 'tail', 'last', 'without', 'indexOf', 'shuffle',
-    'lastIndexOf', 'isEmpty', 'groupBy'];
+    'max', 'min', 'sortedIndex', 'toArray', 'size', 'first', 'head', 'take',
+    'initial', 'rest', 'tail', 'last', 'without', 'indexOf', 'shuffle',
+    'lastIndexOf', 'isEmpty'];
 
   // Mix in each Underscore method as a proxy to `Collection#models`.
   _.each(methods, function(method) {
@@ -880,6 +874,19 @@
     };
   });
 
+  // Underscore methods that take a property name as an argument.
+  var attributeMethods = ['groupBy', 'countBy', 'sortBy'];
+
+  // Use attributes instead of properties.
+  _.each(attributeMethods, function(method) {
+    Collection.prototype[method] = function(value, context) {
+      var iterator = _.isFunction(value) ? value : function(model) {
+        return model.get(value);
+      };
+      return _[method](this.models, iterator, context);
+    };
+  });
+
   // Backbone.Router
   // -------------------
 

diff --git a/test/collection.js b/test/collection.js
@@ -665,15 +665,15 @@ $(document).ready(function() {
     collection.create({id: 1});
   });
 
-  test("#1447 - create with wait adds model.", function() {
+  test("#1447 - create with wait adds model.", 1, function() {
     var collection = new Backbone.Collection;
     var model = new Backbone.Model;
     model.sync = function(method, model, options){ options.success(); };
     collection.on('add', function(){ ok(true); });
     collection.create(model, {wait: true});
   });
 
-  test("#1448 - add sorts collection after merge.", function() {
+  test("#1448 - add sorts collection after merge.", 1, function() {
     var collection = new Backbone.Collection([
       {id: 1, x: 1},
       {id: 2, x: 2}
@@ -682,4 +682,21 @@ $(document).ready(function() {
     collection.add({id: 1, x: 3}, {merge: true});
     deepEqual(collection.pluck('id'), [2, 1]);
   });
+
+  test("#1655 - groupBy can be used with a string argument.", 3, function() {
+    var collection = new Backbone.Collection([{x: 1}, {x: 2}]);
+    var grouped = collection.groupBy('x');
+    strictEqual(_.keys(grouped).length, 2);
+    strictEqual(grouped[1][0].get('x'), 1);
+    strictEqual(grouped[2][0].get('x'), 2);
+  });
+
+  test("#1655 - sortBy can be used with a string argument.", 1, function() {
+    var collection = new Backbone.Collection([{x: 3}, {x: 1}, {x: 2}]);
+    var values = _.map(collection.sortBy('x'), function(model) {
+      return model.get('x');
+    });
+    deepEqual(values, [1, 2, 3]);
+  });
+
 });