powmedia / backbone-forms

Form framework for BackboneJS with nested forms, editable lists and validation
MIT License
2.17k stars 413 forks source link

Validation options for form.commit() don't work. #264

Closed erikcw closed 10 years ago

erikcw commented 11 years ago

The README says:

var errors = form.commit(); // runs schema validation
or

var errors = form.commit({ validate: true }); // runs schema and model validation

However passing {validate: false} to form.commit() doesn't prevent the model validation from running. form.commit() calls form.validate() without any options and before commit() is able to set the value of model.validate.

Something like form.validate({model: false}) should be added to form.validate() to maximize flexibility.

https://github.com/powmedia/backbone-forms/blob/master/src/form.js#L290

philfreo commented 11 years ago

Are you on Backbone 1.0.0 (behavior in older versions was different) and the latest Backbone-Forms?

erikcw commented 11 years ago

@philfreo Yes, Backbone 1.0.0 and Backbone-Forms 0.12.0.

philfreo commented 11 years ago

hmm ya this does appear to be broken - the option isn't passed from commit to validate (nor is it checked in validate)

cmaher commented 11 years ago

This isn't too hard of a fix to add, but it does pose a question. Is the behavior supposed to be as specified in the documentation ({validate: true} validates model) or as coded (model is validated all the time)? The documentation in question was added in commit 5e9a9d7 by @philfreo/.

An alternative to what is specified by the documentation that is compatible with existing code would be to pass { validate: false} so as not to change the default behavior (which would still validate the model) and break existing code.

exussum12 commented 11 years ago

http://backbonejs.org/#Model-validate is where the {validate:true} comes from it seems. The docs read to me as schema validation runs regardless

Calling `form.commit()` will run schema
+level validation by default, and can also run model validation if `{ validate: true }` is passed.

Schema validation is what backbone form does as far as im aware

philfreo commented 11 years ago

The behavior should be match the existing documentation (pull requests welcome). The behavior should have been added alongside the documentation in e0f7be23a202d87f7803d4e878061fc0fb4cffe7 (unless there was a bug). If it's broken now it probably got changed in the major 2.0 update.

The idea is to keep things in line with how Backbone.js does validate with Model#set, as @exussum12 mentioned.

reustle commented 10 years ago

I think this issue can be closed now