Fix: WhenMissing doesn't work in Nested rules. #771
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #771 +/- ##
============================================
+ Coverage 94.40% 96.18% +1.78%
- Complexity 953 1037 +84
============================================
Files 108 122 +14
Lines 3018 3279 +261
============================================
+ Hits 2849 3154 +305
+ Misses 169 125 -44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This PR also fixes this issue #565. I can add a test from this issue. or maybe add it as a separate PR? |
There was a problem hiding this comment.
Pull Request Overview
Fix a bug where the WhenMissing condition was not properly handled in Nested rules.
- Added several tests in NestedTest.php to validate WhenMissing behavior under different scenarios.
- Updated NestedHandler.php to pass fallback data when validated values are missing, ensuring correct handling of nested rules.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/Rule/NestedTest.php | Added tests covering WhenMissing in nested rules. |
| src/Rule/NestedHandler.php | Modified value validation to address missing rule values. |
Comments suppressed due to low confidence (1)
tests/Rule/NestedTest.php:1264
- [nitpick] The error message 'Value not passed.' might benefit from clearer wording, such as 'Expected value is missing.', to enhance clarity for end users.
['value' => ['Value not passed.']],
|
Doesn't work with integer valuePath, needs to be further improved |
Currently, WhenMissing does not support rules with integer-based paths because the property is always set to null, causing the isPropertyMissing check to always return false. 😥 |
Yes, also rules with integer keys applies to whole value, not to property. |
Replaces null checks with MissingValue instance detection for more reliable validation of optional array paths.
| $property = end($valuePathList); | ||
| $itemResult = $context->validate([$property => $validatedValue], [$property => $rules]); | ||
| $itemResult = $context->validate( | ||
| ArrayHelper::pathExists($data, $valuePath) ? [$property => $validatedValue] : [], |
There was a problem hiding this comment.
| ArrayHelper::pathExists($data, $valuePath) ? [$property => $validatedValue] : [], | |
| ArrayHelper::keyExists($data, $valuePath) ? [$property => $validatedValue] : [], |
$valuePath is already an array of keys, so using keyExists() is a bit more efficient.
There was a problem hiding this comment.
Oh yes! Only then should check not $valuePath, but $valuePathList. Thank you, that's more correct. 👍
|
@Enjoyzz thank you! |
Fix #750