Hi, I have a few thoughts on the server side validations support in ember data through ActiveModelAdapter and wanted to see if anyone else thought similarly. @bradleypriest I think you may be the best person to ask about this as you worked on the ActiveModelAdapter in the first place, although perhaps not.
Mainly, I don’t think that the invalid state works that well. I’m loving the errors object that is automatically populated, however when validation errors come back the model is put into an invalid state. This makes sense but if you then try to submit the record again, you get an error like Attempted to handle event
willCommit on <DS.Model> while in state root.loaded.updated.invalid
.
I don’t think this semantic is right, because if we are delegating the validation to the server who’s to say it’s not valid now? I.e. because of a change external to this client, or e.g. a time based validation (only valid to create a record after 9am and the time just changed). It seems like it would be much better to allow willCommit in the invalid state, so that we can go back to the server and actually check if the record is valid now.
If you make any property change to the record, then the invalid state is exited. This means you are allowed to submit the record again, however again I think this is not the best way of handling things because you are now assuming any change makes the record valid when only the server can really know if that is the case. The main issue this causes is that if you are displaying some kind of error messages list above a form (something like rails’ error_messages_for), this list will disappear as soon as you change a field on the record, which is a jarring UX IMO.
I think a better approach would be:
- Allow willCommit in invalid states
- Invalid state persists until the record is saved again, at which point the errors are cleared and invalid state is exited unless server responds with more validation errors
Another minor issue that I have, although I’m not sure of an easy proposal to solve this, is that saving records that may have validation errors in an action is a bit ugly, because you need a no-op function in you then error handler to avoid everything breaking, e.g.:
actions: {
submit: function() {
record.save().then(null, function() { /* noop */});
}
}
Obivously, the promise has to reject if the save was unsuccessful otherwise the success handler will be called even if the record is invalid, however I find this noop pattern generally dissatisfying and would prefer if maybe action handlers rescued DS.InvalidError rather than causing the app to break if you forget the noop error handler? Not so sure about this one.
Happy to work up a pull request if consensus can be reached on anything raised here.