-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[6.x] Fix required_if boolean validation #36969
[6.x] Fix required_if boolean validation #36969
Conversation
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.
Looks good to me. Sorry for the headache!
Thanks @jessarcher. Don't worry about it 🙂 |
This now breaks nullable values:
Previously if (Using Laravel 8.x) From documentation:
This is no longer correct since validator considers null values as |
@aromka I'm looking into this now. |
@aromka would appreciate your review on that one. |
@driesvints This is still a breaking change. Every piece of validation that used 0/1 instead of false/true in required_if just stopped working as expected. |
I don't like posting "me too" replies, but I agree with @oprypkhantc, we also have this problem and it's fixed by changing |
Re-send of #36967
This should fix some regressions for the changed behavior introduced in #36504. The PR now takes into account the
boolean
validation rule when combining rules likerequired_if
. As reported in #36952 atm the validator will incorrectly return incorrect error messages when a boolean value of0
is used. By first checking if the rules for the subject ofrequired_if
containsboolean
we can safely know that it should be converted to a boolean. This doesn't breaks previous tests and all tests introduced by @jessarcher still pass.I'd appreciate a second pair of eyes on this @jessarcher @timacdonald @SjorsO @taylorotwell.
Fixes #36952