[9.x] Patch nested array validation rule regression bug #43897
Merged
+44
−38
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Awesome Steve. Thanks so much for adding the additional tests as well! 😀 |
|
Hi @stevebauman. I've updated to 9.27 version and getting error for this rule. My rule: |
|
|
|
Hi @siarheipashkevich -- as shown in the above PR, you can no longer nest one or many $rules = [
'conditions.*.items.*.item_id' => ['required', 'integer', Rule::forEach('...')], // <-- Will not work.
];You must refactor the validation rule like so by returning all your validation rules inside of the $promotionItemIdRule = Rule::forEach(function (mixed $value, string $attribute, array $data) use (
$conditionItemIds,
) {
$type = Arr::get($data, Str::replaceLast('.item_id', '.item_type', $attribute));
return ['required', 'integer', Rule::in($conditionItemIds->get($type, []))]; // <-- All rules returned here.
});
// ...
$rules = [
'conditions.*.items.*.item_id' => $promotionItemIdRule,
]; |
|
@stevebauman thank you so much. |
|
Happy to help @siarheipashkevich. Also you may be able to use something like this to clean it up a bit (correct me if I'm wrong): $rules = [
'conditions.*.items' => ['required', 'array'],
'conditions.*.items.*.item_id' => ['required', 'integer'],
'conditions.*.items.*.item_type' => ['required', 'string'],
'conditions.*.items.*' => Rule::forEach(function ($value) use ($conditionItemIds) {
return [
'item_id' => Rule::in($conditionItemIds->get($value['item_type'], [])),
'item_type' => Rule::in(PromotionItemType::all()),
];
}),
]; |
|
@stevebauman in your example the rules will be combined? |
|
@siarheipashkevich I'm not sure what you mean, can you elaborate? |
|
@stevebauman in your example we will have the next rules: because |
|
@siarheipashkevich The rules will not be merged. They will be evaluated individually. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #43779
Closes #43751
Related #43192
Related #40924
This PR resolves a regression bug that I introduced in
9.xin theRule::forEach()validation feature, where nested array validation rules can no longer be used, due to flattening them via anArr::flatten($rules).For example, this can no longer be done, whereas it used to work in prior versions of Laravel:
Arr::flatten()was used to be able to determine if a ruleset contains aNestedRulesinstance, and allowed validation rules to be infinitely nested, as they would be flattened before being parsed:framework/src/Illuminate/Validation/ValidationRuleParser.php
Lines 154 to 155 in 2a1a55c
This PR will break the below (however unlikely) implementation for developers -- but will resolve the regression bug, while keeping the
Rule::forEachfeature:Developers will have instead have to use a single
Rule::forEach()(non-nested), as this will no longer be supported:I've added test to ensure we have some coverage over the nested array rule feature so regression will not occur again in the future.
Thanks so much for your time, and I apologize for this issue that has caused grief for devs upgrading to 9.x ❤️