Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.6] Make inequality validation fail on different types rather than 500 #28168

Conversation

JonathanGawrych
Copy link
Contributor

There are four types of inequality validators: lt, lte, gt, gte. If these validators reference another attribute, the attribute must be of the same type. Before this change, if the payload has different types, a 500 error is given. However, the type is not determine by the rule configuration, but rather the HTTP request. It shouldn't be an Internal Server Error, when really the request was invalid.

There are four types of inequality validators: lt, lte, gt, gte.
If these validators reference another attribute, the attribute
must be of the same type. Before this change, if the payload has
different types, a 500 error is given. However, the type is not
determine by the rule configuration, but rather the HTTP request.
Instead of throwing an error, just fail the validation.
@JonathanGawrych JonathanGawrych force-pushed the 5.6-inequality-validation-fail-not-500 branch from 3c877f5 to 072dc30 Compare April 10, 2019 20:34
@devcircus
Copy link
Contributor

5.6 is no longer supported. If accepted, as a breaking change, master would need to be targeted.

If considered a bug, 5.5 could possibly be targeted since it is accepting bug fixes until August.

@taylorotwell
Copy link
Member

Please send to 5.8 if this is a bug fix or master if it is a breaking change improvement.

@JonathanGawrych
Copy link
Contributor Author

I considered it a bug, because I believe an invalid payload should cause a validation failure, rather than an error to be thrown, that if uncaught will return a 500 internal server error. Because I believe it to be a bug, I was going to target the 5.5 (LTS) branch like the collaboration docs said, but the inequality validators were introduced in 5.6, so I targeted it to 5.6 instead. Based on what taylorotwell said, I'll target 5.8 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants