Skip to content

Commit

Permalink
Fix #2446 save should always set with validate: true by default
Browse files Browse the repository at this point in the history
  • Loading branch information
caseywebdev committed Mar 29, 2013
1 parent 0e7ac5a commit 8e7208e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
6 changes: 3 additions & 3 deletions backbone.js
Expand Up @@ -453,11 +453,11 @@
(attrs = {})[key] = val;
}

// If we're not waiting and attributes exist, save acts as `set(attr).save(null, opts)`.
if (attrs && (!options || !options.wait) && !this.set(attrs, options)) return false;

options = _.extend({validate: true}, options);

// If we're not waiting and attributes exist, save acts as `set(attr).save(null, opts)`.
if (attrs && !options.wait && !this.set(attrs, options)) return false;

// Do not persist invalid models.
if (!this._validate(attrs, options)) return false;

This comment has been minimized.

Copy link
@philfreo

philfreo Mar 29, 2013

Contributor

isn't _validate going to validate twice now?

This comment has been minimized.

Copy link
@caseywebdev

caseywebdev Mar 29, 2013

Author Collaborator

Yea this save logic is a little wonky...

This comment has been minimized.

Copy link
@philfreo

philfreo Mar 29, 2013

Contributor

make this "if" an "else" instead?

This comment has been minimized.

Copy link
@caseywebdev

caseywebdev Mar 29, 2013

Author Collaborator

Yup, reworking it now.

This comment has been minimized.

Copy link
@tgriesser

tgriesser Mar 29, 2013

Collaborator

@caseywebdev the way it worked before was correct, see my comment on #2446.


Expand Down
7 changes: 7 additions & 0 deletions test/model.js
Expand Up @@ -705,6 +705,13 @@ $(document).ready(function() {
ok(this.syncArgs.model === model);
});

test("save without `wait` doesn't set invalid attributes", function () {
var model = new Backbone.Model();
model.validate = function () { return 1; }
model.save({a: 1});
equal(model.get('a'), void 0);
});

test("`hasChanged` for falsey keys", 2, function() {
var model = new Backbone.Model();
model.set({x: true}, {silent: true});
Expand Down

0 comments on commit 8e7208e

Please sign in to comment.