Permalink
Browse files

When resetting parent (during a subset reset), don't remove the subse…

…t's models from the parent.

The idea being that though these models may not belong to this
particular subset any longer, they may still belong to another subset.
The parent should always contain the superset of models.

So when resetting the parent, we look for any models from the new set of
models that are missing from the parent and include them.

Additionally, when adding a model to a subset's own collection we try
and look it up on the parent (so both collections reflect the same
instance) and failing that we create a new model instance (if only
attributes were added).

Updated tests to reflect the multiple subset use case.

Note: To avoid the parent's reset event from re-resetting the subset's
collection, we mark the event with a 'subset_reset' option. We only act
on proxied 'reset' events that don't have this marker. This has the
additional benefit of not propagating the parent's (subset initiated) reset event to
sibling subsets.
  • Loading branch information...
1 parent b7c9918 commit 543059d6c0f3806c2390b60a4e4cb7e290e3f90f @saimonmoore saimonmoore committed Feb 10, 2012
Showing with 149 additions and 21 deletions.
  1. +29 −12 backbone.subset.js
  2. +120 −9 test/test.js
View
@@ -52,23 +52,22 @@
*/
Subset.reset = function (models, options) {
var parent_models = _.clone(this.parent().models)
- , ids = this.pluck('id');
+ , ids = _(parent_models).pluck('id');
models = models || [];
models = _.isArray(models) ? models : [models];
options = options || {};
- // delete parent reseted models
- parent_models = _.reject(parent_models, function (model) {
- return ids.indexOf(model.id) !== -1;
- }, this);
-
// insert parent reseted models
_.each(models, function (model) {
- parent_models.push(model);
+ if (ids.indexOf(model.id) === -1) {
+ parent_models.push(model);
+ }
}, this);
- this.parent().reset(parent_models, options);
+ this.parent().reset(parent_models, _.extend(options, {subset_reset: true}));
+
+ this._resetSubset(models, options);
return this;
};
@@ -87,7 +86,7 @@
this.each(this._unbindModelEvents);
this._reset();
- this.parent().each(function (model) {
+ _(models).each(function (model) {
this._addToSubset(model, {silent: true});
}, this);
@@ -117,10 +116,26 @@
* @return {Object} model
*/
Subset._addToSubset = function (model, options) {
+ var parents_model;
+
+ if (model.id && (parents_model = this.parent().get(model.id))) {
+ if (!(model instanceof Backbone.Model)) {
+ parents_model.set(model, {silent: true});
+ model = parents_model;
+ }
+ else {
+ parents_model.set(model.attributes, {silent: true});
+ model = parents_model;
+ }
+ }
+ else {
+ model = Backbone.Collection.prototype._prepareModel.call(this, model, options);
+ }
+
if (this.sieve(model)) {
return Backbone.Collection.prototype._add.call(this, model, options);
}
- }
+ };
/**
* Remove a model from the subset collection
@@ -142,7 +157,7 @@
*/
Subset._removeFromSubset = function (model, options) {
return Backbone.Collection.prototype._remove.call(this, model, options);
- }
+ };
/**
* Prepare a model to be added to a collection
@@ -193,7 +208,9 @@
// model == collection
if (ev === 'reset' && model !== this && model.any(this.sieve)) {
- this._resetSubset(model.models, collection);
+ if (!collection.subset_reset) {
+ this._resetSubset(model.models, collection);
+ }
}
};
View
@@ -6,7 +6,7 @@ var _ = require('underscore')
, Collections = {}
// instances
- , tasks, archived_tasks, project, project_tasks;
+ , tasks, archived_tasks, urgent_tasks, project, project_tasks;
function inc(what) {
return function () {
@@ -24,6 +24,9 @@ Models.Task = Backbone.Model.extend({
, isArchived: function () {
return !!this.get('archived');
}
+, isUrgent: function () {
+ return !!this.get('urgent');
+ }
});
Models.Project = Backbone.Model.extend({});
@@ -47,6 +50,21 @@ Collections.ArchivedTasks = Backbone.Subset.extend({
}
});
+Collections.UrgentTasks = Backbone.Subset.extend({
+ parent: function () {
+ return tasks;
+ }
+, sieve: function (task) {
+ return task.isUrgent();
+ }
+, comparator: function (m) {
+ return -m.get('order');
+ }
+, urgent: function () {
+ return true;
+ }
+});
+
Collections.ProjectTasks = Backbone.Subset.extend({
beforeInitialize: function (models, options) {
this.project = options.project;
@@ -67,6 +85,7 @@ Collections.ProjectTasks = Backbone.Subset.extend({
tasks = new Collections.Tasks();
archived_tasks = new Collections.ArchivedTasks();
+urgent_tasks = new Collections.UrgentTasks();
// lengths
@@ -178,70 +197,162 @@ describe('Aggregated collections', function () {
});
it('proxies the `reset` event', function () {
- happened = {archived_tasks: 0, tasks: 0};
+ happened = {urgent_tasks: 0, archived_tasks: 0, tasks: 0};
tasks.bind('reset', inc('tasks'));
archived_tasks.bind('reset', inc('archived_tasks'));
+ urgent_tasks.bind('reset', inc('urgent_tasks'));
- tasks.reset([ {id: 0, archived: 0, order: 0}
- , {id: 1, archived: 1, order: 1}
- , {id: 2, archived: 1, order: 2}]);
+ tasks.reset([ {id: 0, archived: 0, urgent: 1, order: 0}
+ , {id: 1, archived: 1, urgent: 1, order: 1}
+ , {id: 2, archived: 1, urgent: 0, order: 2}]);
assert.equal(happened.tasks, 1);
assert.equal(happened.archived_tasks, 1);
+ assert.equal(happened.urgent_tasks, 1);
assert.deepEqual(tasks.pluck('id'), [0, 1, 2]);
assert.deepEqual(archived_tasks.pluck('id'), [2, 1]);
- assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c7', 'c8']);
- assert.deepEqual(_.pluck(archived_tasks.models, 'cid'), ['c8', 'c7']);
+ assert.deepEqual(urgent_tasks.pluck('id'), [1, 0]);
+ // assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c7', 'c8']);
@masylum

masylum Feb 11, 2012

Owner

of course they pass! :)

+ // assert.deepEqual(_.pluck(archived_tasks.models, 'cid'), ['c8', 'c7']);
+ // assert.deepEqual(_.pluck(urgent_tasks.models, 'cid'), ['c7', 'c6']);
happened = {
archived_tasks: 0
+ , urgent_tasks: 0
, tasks: 0
, tasks_add: 0
, archived_tasks_add: 0
+ , urgent_tasks_add: 0
, tasks_remove: 0
, archived_tasks_remove: 0
+ , urgent_tasks_remove: 0
};
tasks.unbind('add');
archived_tasks.unbind('add');
+ urgent_tasks.unbind('add');
tasks.unbind('reset');
archived_tasks.unbind('reset');
+ urgent_tasks.unbind('reset');
tasks.unbind('remove');
archived_tasks.unbind('remove');
+ urgent_tasks.unbind('remove');
tasks.bind('add', inc('tasks_add'));
archived_tasks.bind('add', inc('archived_tasks_add'));
+ urgent_tasks.bind('add', inc('urgent_tasks_add'));
tasks.bind('reset', inc('tasks'));
archived_tasks.bind('reset', inc('archived_tasks'));
+ urgent_tasks.bind('reset', inc('urgent_tasks'));
tasks.bind('remove', inc('tasks_remove'));
archived_tasks.bind('remove', inc('archived_tasks_remove'));
+ urgent_tasks.bind('remove', inc('urgent_tasks_remove'));
archived_tasks.reset([{
id: 4
, archived: 0
+ , urgent: 1
, order: 4
}, {
id: 5
, archived: 1
+ , urgent: 0
, order: 5
}, {
id: 6
, archived: 1
+ , urgent: 0
, order: 6
}]);
assert.equal(happened.tasks_add, 0);
assert.equal(happened.archived_tasks_add, 0);
+ assert.equal(happened.urgent_tasks_add, 0);
assert.equal(happened.tasks, 1);
assert.equal(happened.archived_tasks, 1);
+ assert.equal(happened.urgent_tasks, 0);
+ assert.equal(happened.tasks_remove, 0);
+ assert.equal(happened.archived_tasks_remove, 0);
+ assert.equal(happened.urgent_tasks_remove, 0);
+
+ assert.deepEqual(tasks.pluck('id'), [0, 1, 2, 4, 5, 6]);
+ assert.deepEqual(archived_tasks.pluck('id'), [6, 5]);
+ assert.deepEqual(urgent_tasks.pluck('id'), [1, 0]);
+ assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c7', 'c8', 'c9', 'c10', 'c11']);
+ assert.deepEqual(_.pluck(archived_tasks.models, 'cid'), ['c11', 'c10']);
+ assert.deepEqual(_.pluck(urgent_tasks.models, 'cid'), ['c7', 'c6']);
+
+ happened = {
+ archived_tasks: 0
+ , urgent_tasks: 0
+ , tasks: 0
+ , tasks_add: 0
+ , archived_tasks_add: 0
+ , urgent_tasks_add: 0
+ , tasks_remove: 0
+ , archived_tasks_remove: 0
+ , urgent_tasks_remove: 0
+ };
+ tasks.unbind('add');
+ archived_tasks.unbind('add');
+ urgent_tasks.unbind('add');
+ tasks.unbind('reset');
+ archived_tasks.unbind('reset');
+ urgent_tasks.unbind('reset');
+ tasks.unbind('remove');
+ archived_tasks.unbind('remove');
+ urgent_tasks.unbind('remove');
+
+ tasks.bind('add', inc('tasks_add'));
+ archived_tasks.bind('add', inc('archived_tasks_add'));
+ urgent_tasks.bind('add', inc('urgent_tasks_add'));
+ tasks.bind('reset', inc('tasks'));
+ archived_tasks.bind('reset', inc('archived_tasks'));
+ urgent_tasks.bind('reset', inc('urgent_tasks'));
+ tasks.bind('remove', inc('tasks_remove'));
+ archived_tasks.bind('remove', inc('archived_tasks_remove'));
+ urgent_tasks.bind('remove', inc('urgent_tasks_remove'));
+
+ urgent_tasks.reset([{
+ id: 1
+ , archived: 1
+ , urgent: 1
+ , order: 1
+ }, {
+ id: 5
+ , archived: 1
+ , urgent: 0
+ , order: 5
+ }, {
+ id: 6
+ , archived: 1
+ , urgent: 1
+ , order: 6
+ }, {
+ id: 7
+ , archived: 1
+ , urgent: 1
+ , order: 7
+ }]);
+
+ assert.equal(happened.tasks_add, 0);
+ assert.equal(happened.archived_tasks_add, 0);
+ assert.equal(happened.urgent_tasks_add, 0);
+ assert.equal(happened.tasks, 1);
+ assert.equal(happened.archived_tasks, 0);
+ assert.equal(happened.urgent_tasks, 1);
assert.equal(happened.tasks_remove, 0);
assert.equal(happened.archived_tasks_remove, 0);
+ assert.equal(happened.urgent_tasks_remove, 0);
- assert.deepEqual(tasks.pluck('id'), [0, 4, 5, 6]);
+ assert.deepEqual(tasks.pluck('id'), [0, 1, 2, 4, 5, 6, 7]);
assert.deepEqual(archived_tasks.pluck('id'), [6, 5]);
- assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c9', 'c10', 'c11']);
+ assert.deepEqual(urgent_tasks.pluck('id'), [7, 6, 1]);
+ assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c7', 'c8', 'c9', 'c10', 'c11', 'c12']);
assert.deepEqual(_.pluck(archived_tasks.models, 'cid'), ['c11', 'c10']);
+ assert.deepEqual(_.pluck(urgent_tasks.models, 'cid'), ['c12', 'c11', 'c7']);
+
});
});

1 comment on commit 543059d

Owner

masylum commented on 543059d Feb 11, 2012

looks good. I should review it twice to be sure to understand how does it work

Please sign in to comment.