-
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
[5.4] Allow the first validation error to be found on wildcard lookups #15217
[5.4] Allow the first validation error to be found on wildcard lookups #15217
Conversation
@@ -2585,6 +2585,7 @@ public function testValidateImplicitEachWithAsterisksConfirmed() | |||
$this->assertFalse($v->passes()); | |||
$this->assertTrue($v->messages()->has('foo.0.password')); | |||
$this->assertTrue($v->messages()->has('foo.1.password')); | |||
$this->assertTrue($v->messages()->has('foo.*.password')); |
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.
Test not related to the change?
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.
Yes, it's related. The above test causes the \ErrorException
without this PR. Can you suggest a better place for it?
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.
Support/SupportMessageBagTest.php
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.
@themsaid thank you 🍻
@themsaid what do you think about this? |
If a use case matters, I have a series of fieldsets containing multiple fields. I want to mark a
|
I think it's useful, but don't think it's breaking a breaking change. |
You don't think it's a breaking change or you do? :) |
So cryptic. |
haha I donno how I wrote that :D It's not a breaking change. |
Well the "but" threw me off, haha. Was expecting an "and" :) |
The entire phrase doesn't make much sense :D. Friday morning here is your Saturday morning, maybe that explains it :D |
With validation rules like
If an image and no caption are provided and we ask the messagebag if
photos.*
has any errorsan
\ErrorException
occurs:because the structure of the errors array in this situation is like
This PR flattens and filters empty validation messages regardless of depth, returning the first non-empty message if present.