Bindings and failed validation #201
Conversation
|
Strictly speaking you're right - that's a breaking change. However, I'm not sure that {validate:true} was usable at all with the previous behavior, as it eventually ended up with inconsistent state of view vs model. Still it's safer to have it as a configurable option for 1.x (so we can easily upgrade the library in the existing projects), and may be turn it on by default in the (future) 2.x release. |
|
@amakhrov Besides @theironcook I don't know who the collaborators and other decision-makers might be. If there are others, care to pull them in? |
|
@amakhrov Since you opened the original issue, did you have a chance to see if this fixes your issue? (Ignoring for the moment that it might be a "breaking change") |
|
@theironcook Could you take a look at this and weigh in on if my proposal is too much of a "breaking change"? As @amakhrov has said, using ModelBinder with model validation is pretty much useless at this point anyway. |
Fixes #178. Note that this is potentially a breaking change.
The basic idea is that if changing the view causes a model set, and that model set fails (ostensibly due to validation errors), the previous model attribute should be copied back over into the view so that the model and view are back in sync.
If this is deemed to be too much of a breaking change, I can augment the pull request to support a new ModelBinder option to enable this behavior (e.g.,
revertViewOnValidationErroror something similar). Just let me know.Patch impact:
If this change is rejected outright (even with a ModelBinder option), I'll probably create a new pull request with just the test file (minus the last assertion), so that we have some coverage around ModelBinder plus validation. Presumably that would only need a minor version update.