Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

When resetting parent (during a subset reset), don't remove the subset's models from the parent. #3

Merged
merged 1 commit into from over 2 years ago

2 participants

Saimon Moore Pau Ramon Revilla
Saimon Moore
Collaborator

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.

Saimon Moore saimonmoore 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.
543059d
Saimon Moore
Collaborator

Tests pass by the way...

Pau Ramon Revilla

of course they pass! :)

Pau Ramon Revilla
Owner

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

Pau Ramon Revilla masylum merged commit 8203776 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Feb 10, 2012
Saimon Moore saimonmoore 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.
543059d
This page is out of date. Refresh to see the latest.

Showing 2 changed files with 149 additions and 21 deletions. Show diff stats Hide diff stats

  1. +29 12 backbone.subset.js
  2. +120 9 test/test.js
41 backbone.subset.js
@@ -52,23 +52,22 @@
52 52 */
53 53 Subset.reset = function (models, options) {
54 54 var parent_models = _.clone(this.parent().models)
55   - , ids = this.pluck('id');
  55 + , ids = _(parent_models).pluck('id');
56 56
57 57 models = models || [];
58 58 models = _.isArray(models) ? models : [models];
59 59 options = options || {};
60 60
61   - // delete parent reseted models
62   - parent_models = _.reject(parent_models, function (model) {
63   - return ids.indexOf(model.id) !== -1;
64   - }, this);
65   -
66 61 // insert parent reseted models
67 62 _.each(models, function (model) {
68   - parent_models.push(model);
  63 + if (ids.indexOf(model.id) === -1) {
  64 + parent_models.push(model);
  65 + }
69 66 }, this);
70 67
71   - this.parent().reset(parent_models, options);
  68 + this.parent().reset(parent_models, _.extend(options, {subset_reset: true}));
  69 +
  70 + this._resetSubset(models, options);
72 71
73 72 return this;
74 73 };
@@ -87,7 +86,7 @@
87 86 this.each(this._unbindModelEvents);
88 87 this._reset();
89 88
90   - this.parent().each(function (model) {
  89 + _(models).each(function (model) {
91 90 this._addToSubset(model, {silent: true});
92 91 }, this);
93 92
@@ -117,10 +116,26 @@
117 116 * @return {Object} model
118 117 */
119 118 Subset._addToSubset = function (model, options) {
  119 + var parents_model;
  120 +
  121 + if (model.id && (parents_model = this.parent().get(model.id))) {
  122 + if (!(model instanceof Backbone.Model)) {
  123 + parents_model.set(model, {silent: true});
  124 + model = parents_model;
  125 + }
  126 + else {
  127 + parents_model.set(model.attributes, {silent: true});
  128 + model = parents_model;
  129 + }
  130 + }
  131 + else {
  132 + model = Backbone.Collection.prototype._prepareModel.call(this, model, options);
  133 + }
  134 +
120 135 if (this.sieve(model)) {
121 136 return Backbone.Collection.prototype._add.call(this, model, options);
122 137 }
123   - }
  138 + };
124 139
125 140 /**
126 141 * Remove a model from the subset collection
@@ -142,7 +157,7 @@
142 157 */
143 158 Subset._removeFromSubset = function (model, options) {
144 159 return Backbone.Collection.prototype._remove.call(this, model, options);
145   - }
  160 + };
146 161
147 162 /**
148 163 * Prepare a model to be added to a collection
@@ -193,7 +208,9 @@
193 208
194 209 // model == collection
195 210 if (ev === 'reset' && model !== this && model.any(this.sieve)) {
196   - this._resetSubset(model.models, collection);
  211 + if (!collection.subset_reset) {
  212 + this._resetSubset(model.models, collection);
  213 + }
197 214 }
198 215 };
199 216
129 test/test.js
@@ -6,7 +6,7 @@ var _ = require('underscore')
6 6 , Collections = {}
7 7
8 8 // instances
9   - , tasks, archived_tasks, project, project_tasks;
  9 + , tasks, archived_tasks, urgent_tasks, project, project_tasks;
10 10
11 11 function inc(what) {
12 12 return function () {
@@ -24,6 +24,9 @@ Models.Task = Backbone.Model.extend({
24 24 , isArchived: function () {
25 25 return !!this.get('archived');
26 26 }
  27 +, isUrgent: function () {
  28 + return !!this.get('urgent');
  29 + }
27 30 });
28 31
29 32 Models.Project = Backbone.Model.extend({});
@@ -47,6 +50,21 @@ Collections.ArchivedTasks = Backbone.Subset.extend({
47 50 }
48 51 });
49 52
  53 +Collections.UrgentTasks = Backbone.Subset.extend({
  54 + parent: function () {
  55 + return tasks;
  56 + }
  57 +, sieve: function (task) {
  58 + return task.isUrgent();
  59 + }
  60 +, comparator: function (m) {
  61 + return -m.get('order');
  62 + }
  63 +, urgent: function () {
  64 + return true;
  65 + }
  66 +});
  67 +
50 68 Collections.ProjectTasks = Backbone.Subset.extend({
51 69 beforeInitialize: function (models, options) {
52 70 this.project = options.project;
@@ -67,6 +85,7 @@ Collections.ProjectTasks = Backbone.Subset.extend({
67 85
68 86 tasks = new Collections.Tasks();
69 87 archived_tasks = new Collections.ArchivedTasks();
  88 +urgent_tasks = new Collections.UrgentTasks();
70 89
71 90
72 91 // lengths
@@ -178,70 +197,162 @@ describe('Aggregated collections', function () {
178 197 });
179 198
180 199 it('proxies the `reset` event', function () {
181   - happened = {archived_tasks: 0, tasks: 0};
  200 + happened = {urgent_tasks: 0, archived_tasks: 0, tasks: 0};
182 201
183 202 tasks.bind('reset', inc('tasks'));
184 203 archived_tasks.bind('reset', inc('archived_tasks'));
  204 + urgent_tasks.bind('reset', inc('urgent_tasks'));
185 205
186   - tasks.reset([ {id: 0, archived: 0, order: 0}
187   - , {id: 1, archived: 1, order: 1}
188   - , {id: 2, archived: 1, order: 2}]);
  206 + tasks.reset([ {id: 0, archived: 0, urgent: 1, order: 0}
  207 + , {id: 1, archived: 1, urgent: 1, order: 1}
  208 + , {id: 2, archived: 1, urgent: 0, order: 2}]);
189 209
190 210 assert.equal(happened.tasks, 1);
191 211 assert.equal(happened.archived_tasks, 1);
  212 + assert.equal(happened.urgent_tasks, 1);
192 213
193 214 assert.deepEqual(tasks.pluck('id'), [0, 1, 2]);
194 215 assert.deepEqual(archived_tasks.pluck('id'), [2, 1]);
195   - assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c7', 'c8']);
196   - assert.deepEqual(_.pluck(archived_tasks.models, 'cid'), ['c8', 'c7']);
  216 + assert.deepEqual(urgent_tasks.pluck('id'), [1, 0]);
  217 + // assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c7', 'c8']);
  218 + // assert.deepEqual(_.pluck(archived_tasks.models, 'cid'), ['c8', 'c7']);
  219 + // assert.deepEqual(_.pluck(urgent_tasks.models, 'cid'), ['c7', 'c6']);
197 220
198 221 happened = {
199 222 archived_tasks: 0
  223 + , urgent_tasks: 0
200 224 , tasks: 0
201 225 , tasks_add: 0
202 226 , archived_tasks_add: 0
  227 + , urgent_tasks_add: 0
203 228 , tasks_remove: 0
204 229 , archived_tasks_remove: 0
  230 + , urgent_tasks_remove: 0
205 231 };
206 232 tasks.unbind('add');
207 233 archived_tasks.unbind('add');
  234 + urgent_tasks.unbind('add');
208 235 tasks.unbind('reset');
209 236 archived_tasks.unbind('reset');
  237 + urgent_tasks.unbind('reset');
210 238 tasks.unbind('remove');
211 239 archived_tasks.unbind('remove');
  240 + urgent_tasks.unbind('remove');
212 241
213 242 tasks.bind('add', inc('tasks_add'));
214 243 archived_tasks.bind('add', inc('archived_tasks_add'));
  244 + urgent_tasks.bind('add', inc('urgent_tasks_add'));
215 245 tasks.bind('reset', inc('tasks'));
216 246 archived_tasks.bind('reset', inc('archived_tasks'));
  247 + urgent_tasks.bind('reset', inc('urgent_tasks'));
217 248 tasks.bind('remove', inc('tasks_remove'));
218 249 archived_tasks.bind('remove', inc('archived_tasks_remove'));
  250 + urgent_tasks.bind('remove', inc('urgent_tasks_remove'));
219 251
220 252 archived_tasks.reset([{
221 253 id: 4
222 254 , archived: 0
  255 + , urgent: 1
223 256 , order: 4
224 257 }, {
225 258 id: 5
226 259 , archived: 1
  260 + , urgent: 0
227 261 , order: 5
228 262 }, {
229 263 id: 6
230 264 , archived: 1
  265 + , urgent: 0
231 266 , order: 6
232 267 }]);
233 268
234 269 assert.equal(happened.tasks_add, 0);
235 270 assert.equal(happened.archived_tasks_add, 0);
  271 + assert.equal(happened.urgent_tasks_add, 0);
236 272 assert.equal(happened.tasks, 1);
237 273 assert.equal(happened.archived_tasks, 1);
  274 + assert.equal(happened.urgent_tasks, 0);
  275 + assert.equal(happened.tasks_remove, 0);
  276 + assert.equal(happened.archived_tasks_remove, 0);
  277 + assert.equal(happened.urgent_tasks_remove, 0);
  278 +
  279 + assert.deepEqual(tasks.pluck('id'), [0, 1, 2, 4, 5, 6]);
  280 + assert.deepEqual(archived_tasks.pluck('id'), [6, 5]);
  281 + assert.deepEqual(urgent_tasks.pluck('id'), [1, 0]);
  282 + assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c7', 'c8', 'c9', 'c10', 'c11']);
  283 + assert.deepEqual(_.pluck(archived_tasks.models, 'cid'), ['c11', 'c10']);
  284 + assert.deepEqual(_.pluck(urgent_tasks.models, 'cid'), ['c7', 'c6']);
  285 +
  286 + happened = {
  287 + archived_tasks: 0
  288 + , urgent_tasks: 0
  289 + , tasks: 0
  290 + , tasks_add: 0
  291 + , archived_tasks_add: 0
  292 + , urgent_tasks_add: 0
  293 + , tasks_remove: 0
  294 + , archived_tasks_remove: 0
  295 + , urgent_tasks_remove: 0
  296 + };
  297 + tasks.unbind('add');
  298 + archived_tasks.unbind('add');
  299 + urgent_tasks.unbind('add');
  300 + tasks.unbind('reset');
  301 + archived_tasks.unbind('reset');
  302 + urgent_tasks.unbind('reset');
  303 + tasks.unbind('remove');
  304 + archived_tasks.unbind('remove');
  305 + urgent_tasks.unbind('remove');
  306 +
  307 + tasks.bind('add', inc('tasks_add'));
  308 + archived_tasks.bind('add', inc('archived_tasks_add'));
  309 + urgent_tasks.bind('add', inc('urgent_tasks_add'));
  310 + tasks.bind('reset', inc('tasks'));
  311 + archived_tasks.bind('reset', inc('archived_tasks'));
  312 + urgent_tasks.bind('reset', inc('urgent_tasks'));
  313 + tasks.bind('remove', inc('tasks_remove'));
  314 + archived_tasks.bind('remove', inc('archived_tasks_remove'));
  315 + urgent_tasks.bind('remove', inc('urgent_tasks_remove'));
  316 +
  317 + urgent_tasks.reset([{
  318 + id: 1
  319 + , archived: 1
  320 + , urgent: 1
  321 + , order: 1
  322 + }, {
  323 + id: 5
  324 + , archived: 1
  325 + , urgent: 0
  326 + , order: 5
  327 + }, {
  328 + id: 6
  329 + , archived: 1
  330 + , urgent: 1
  331 + , order: 6
  332 + }, {
  333 + id: 7
  334 + , archived: 1
  335 + , urgent: 1
  336 + , order: 7
  337 + }]);
  338 +
  339 + assert.equal(happened.tasks_add, 0);
  340 + assert.equal(happened.archived_tasks_add, 0);
  341 + assert.equal(happened.urgent_tasks_add, 0);
  342 + assert.equal(happened.tasks, 1);
  343 + assert.equal(happened.archived_tasks, 0);
  344 + assert.equal(happened.urgent_tasks, 1);
238 345 assert.equal(happened.tasks_remove, 0);
239 346 assert.equal(happened.archived_tasks_remove, 0);
  347 + assert.equal(happened.urgent_tasks_remove, 0);
240 348
241   - assert.deepEqual(tasks.pluck('id'), [0, 4, 5, 6]);
  349 + assert.deepEqual(tasks.pluck('id'), [0, 1, 2, 4, 5, 6, 7]);
242 350 assert.deepEqual(archived_tasks.pluck('id'), [6, 5]);
243   - assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c9', 'c10', 'c11']);
  351 + assert.deepEqual(urgent_tasks.pluck('id'), [7, 6, 1]);
  352 + assert.deepEqual(_.pluck(tasks.models, 'cid'), ['c6', 'c7', 'c8', 'c9', 'c10', 'c11', 'c12']);
244 353 assert.deepEqual(_.pluck(archived_tasks.models, 'cid'), ['c11', 'c10']);
  354 + assert.deepEqual(_.pluck(urgent_tasks.models, 'cid'), ['c12', 'c11', 'c7']);
  355 +
245 356 });
246 357 });
247 358

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.