Permalink
Browse files

Make Model's destroy event fully preventable.

Also preferring options.remove over options['delete'], although both
will work.
  • Loading branch information...
1 parent 23de80c commit 3f630e41ea190e5f918a371760404cce9f3caa13 @rgrove rgrove committed Feb 7, 2012
Showing with 25 additions and 21 deletions.
  1. +4 −0 src/app/HISTORY.md
  2. +1 −1 src/app/docs/model/index.mustache
  3. +17 −17 src/app/js/model.js
  4. +3 −3 src/app/tests/model-test.js
View
4 src/app/HISTORY.md
@@ -25,6 +25,10 @@ App Framework Change History
* Options passed to `set()` and `setAttrs()` are now correctly merged into the
event facade of the `change` event. [Ticket #2531492]
+* Model's `destroy` event is now fully preventable (previously it was possible
+ for the model to be deleted even if the `destroy` event was prevented by a
+ subscriber in the `on` phase).
+
### ModelList
* Added a `filter()` method that returns a filtered array of models. [Ticket
View
2 src/app/docs/model/index.mustache
@@ -748,7 +748,7 @@ If a model is new or if any of its attributes have changed since the model was l
</p>
<p>
-Finally, a model's `destroy()` method can be used to destroy the model instance. Calling `destroy()` with no arguments will destroy only the local model instance, while calling `destroy({'delete': true})` will both destroy the local model instance and call the sync layer with the "delete" action to delete the model from a persistence layer or server.
+Finally, a model's `destroy()` method can be used to destroy the model instance. Calling `destroy()` with no arguments will destroy only the local model instance, while calling `destroy({remove: true})` will both destroy the local model instance and call the sync layer with the "delete" action to delete the model from a persistence layer or server.
</p>
<p>
View
34 src/app/js/model.js
@@ -191,7 +191,7 @@ Y.Model = Y.extend(Model, Y.Base, {
Destroys this model instance and removes it from its containing lists, if
any.
- If `options['delete']` is `true`, then this method also delegates to the
+ If `options.remove` is `true`, then this method also delegates to the
`sync()` method to delete the model from the persistence layer, which is an
asynchronous action. Provide a _callback_ function to be notified of success
or failure.
@@ -200,7 +200,7 @@ Y.Model = Y.extend(Model, Y.Base, {
@param {Object} [options] Sync options. It's up to the custom sync
implementation to determine what options it supports or requires, if
any.
- @param {Boolean} [options.delete=false] If `true`, the model will be
+ @param {Boolean} [options.remove=false] If `true`, the model will be
deleted via the sync layer in addition to the instance being destroyed.
@param {callback} [callback] Called when the sync operation finishes.
@param {Error|null} callback.err If an error occurred, this parameter will
@@ -217,25 +217,25 @@ Y.Model = Y.extend(Model, Y.Base, {
options = {};
}
- function finish(err) {
- if (!err) {
- YArray.each(self.lists.concat(), function (list) {
- list.remove(self, options);
- });
+ self.onceAfter('destroy', function () {
+ function finish(err) {
+ if (!err) {
+ YArray.each(self.lists.concat(), function (list) {
+ list.remove(self, options);
+ });
+ }
- Model.superclass.destroy.call(self);
+ callback && callback.apply(null, arguments);
}
- callback && callback.apply(null, arguments);
- }
-
- if (options && options['delete']) {
- this.sync('delete', options, finish);
- } else {
- finish();
- }
+ if (options && (options.remove || options['delete'])) {
+ self.sync('delete', options, finish);
+ } else {
+ finish();
+ }
+ });
- return this;
+ return Model.superclass.destroy.call(self);
},
/**
View
6 src/app/tests/model-test.js
@@ -55,7 +55,7 @@ modelSuite.add(new Y.Test.Case({
Y.Mock.verify(mock);
},
- 'destroy() should delete the model if the `delete` option is truthy': function () {
+ 'destroy() should delete the model if the `remove` option is truthy': function () {
var calls = 0,
mock = Y.Mock(),
model = new Y.Model();
@@ -70,13 +70,13 @@ modelSuite.add(new Y.Test.Case({
Assert.areSame('delete', action, 'sync action should be "delete"');
Assert.isObject(options, 'options should be an object');
- Assert.isTrue(options['delete'], 'options.delete should be true');
+ Assert.isTrue(options.remove, 'options.delete should be true');
Assert.isFunction(callback, 'callback should be a function');
callback();
};
- model.destroy({'delete': true}, mock.callback);
+ model.destroy({remove: true}, mock.callback);
Y.Mock.verify(mock);
},

0 comments on commit 3f630e4

Please sign in to comment.