-
Notifications
You must be signed in to change notification settings - Fork 651
Automatically validate params based of the Schema data #1128
Conversation
…function for all validation
…t (default to 400)
|
@WP-API/amigos #reviewmerge |
|
Does it return all potential failures or just the first? |
|
@danielbachhuber The implementation of the We could change that and bulk them all or multiple - though that should be a PR on top of https://github.com/WP-API/WP-API/pull/989/files#diff-887d62e7b2ec4bf35ee08d99831b5b4bR681 I think. I think it could be a good idea - the only downside is slight performance hit I suppose. |
|
I think it makes sense to validate all at once. Consider submitting a form. If you did them one at a time, then the user would only get one error at a time and potentially hit 10+ error states before successfully submitting. If it's too much of a lift, we can come back to it later. At the very least though, it would be nice if the response could support one or more error messages. |
|
As the response is a |
This means clients can get lots of info at once, rather than one at a time, see #1128 (comment)
|
Created #1131 to address multiple errors at once. |
|
I also added the same for sanitizing defaults for the schema to this PR |
…lly want that as we want the validator to throw that error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about strings with HTML in them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, will remove this default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b344f5b
|
I like the approach here, but I don't think we can introduce another enhancement this late into Beta 1. Moving to Beta 2 |
…use correct formatting, I will use correct formatting
|
@WP-API/amigos ready for a #reviewmerge again |
|
Looks good to me. I'll wait for an assist from Ryan or Rachel.
|
|
@rmccue @rachelbaker fancy merging / any other thoughts? :) |
|
@joehoyle is going to resolve the merge conflicts so this can be merged. |
Conflicts: tests/test-rest-users-controller.php
Automatically validate params based of the Schema data
|
Merged #1128 |
The basic idea here is to provide to "for free" validation based off the item schema. Looking at enums and data types to start with.