Skip to content

Fix #752: Allow access to sibling properties in nested context - #791

Open
WarLikeLaux wants to merge 12 commits into
yiisoft:masterfrom
WarLikeLaux:fix-nested-context-sibling-access
Open

Fix #752: Allow access to sibling properties in nested context#791
WarLikeLaux wants to merge 12 commits into
yiisoft:masterfrom
WarLikeLaux:fix-nested-context-sibling-access

Conversation

@WarLikeLaux

@WarLikeLauxWarLikeLaux commented Mar 21, 2026

Copy link
Copy Markdown
Contributor
QA
Is bugfix?✔️
New feature?
Tests added?✔️
Breaks BC?
Fixed issues#752

What does this PR do?

Pass full scope data (with sibling properties) to context->validate() in NestedHandler, so when callbacks can access siblings via $context->getDataSet()->getPropertyValue().

Coverage

Tests: 89 → 93. Line coverage: 66/66 (100%) → 73/73 (100%). MSI: 96% (55/57) → 95% (65/68).

Summary by CodeRabbit

Bug Fixes

  • Fixed an issue preventing validation rules from accessing sibling properties within nested validation contexts.

Review Change Stack

@codecov

codecovBot commented Mar 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.30%. Comparing base (fe63447) to head (dc39ec0).
⚠️ Report is 117 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #791 +/- ##
============================================
+ Coverage 94.40% 96.30% +1.90% - Complexity 953 1157 +204 
============================================
Files 108 124 +16 Lines 3018 3549 +531 ============================================
+ Hits 2849 3418 +569 + Misses 169 131 -38 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes nested validation context scoping so when callbacks in nested rules can access sibling properties (e.g., isEnabled when validating content) via ValidationContext::getDataSet().

Changes:

  • Update NestedHandler to pass the full relevant scope data into context->validate() for nested property validation.
  • Add regression tests covering sibling access in when callbacks (including inside Each + Nested combinations) and a missing-intermediate-key dotted path case.
  • Add a changelog entry documenting the bugfix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

FileDescription
src/Rule/NestedHandler.phpAdjusts nested validation to validate against the parent scope dataset so sibling properties are available in context.
tests/Rule/Nested/NestedTest.phpAdds tests proving sibling access works in when callbacks across nested structures.
CHANGELOG.mdDocuments the bugfix in the upcoming release notes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadCHANGELOG.md Outdated
Comment threadsrc/Rule/NestedHandler.php
Comment threadCHANGELOG.md Outdated
@WarLikeLauxWarLikeLaux changed the title Allow access to sibling properties in nested context (#752)Fix #752: Allow access to sibling properties in nested contextMay 30, 2026
@WarLikeLaux
WarLikeLaux requested a review from samdarkMay 30, 2026 13:10
@WarLikeLaux

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai full review

@coderabbitai

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@coderabbitai

coderabbitaiBot commented May 30, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR enables sibling property access in nested validation contexts. NestedHandler reworks how it validates non-integer property paths by parsing them into segments, walking through intermediate keys to build a scoped data structure, and validating rules against that scope. Tests verify sibling-driven conditional skipping and missing key handling. The changelog documents the fix for Bug #752.

Changes

Sibling property access in nested validation

Layer / File(s)Summary
NestedHandler sibling-aware validation logic
src/Rule/NestedHandler.php
array_key_exists and count imports are added. Non-integer property path validation is refactored to parse the path into segments, walk through intermediate segments to build a scopeData subtree (or empty array if traversal fails), and validate rules against that computed scope instead of wrapping a single fetched value.
Test cases for sibling access and missing keys
tests/Rule/Nested/NestedTest.php
New "validation passed" entries test sibling-driven rule skipping via when callbacks in direct and nested Each contexts. New "validation failed" entries confirm when callbacks can read sibling fields and control nested rules, and that missing intermediate keys in dotted paths produce errors at the full path.
Changelog entry for bug fix #752
CHANGELOG.md
Bug #752 is documented under the "2.5.2 under development" section, describing the change that enables sibling property access in nested validation contexts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A nested hop through sibling lands,
Where paths now follow walkers' commands,
Each segment parsed with careful care,
Scope data built and validated fair!
Siblings whisper through the when,
And tests ensure it works again! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'Fix #752: Allow access to sibling properties in nested context' directly and clearly summarizes the main change: enabling access to sibling properties in nested validation contexts.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@WarLikeLaux

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai full review

@coderabbitai

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@WarLikeLaux

Copy link
Copy Markdown
ContributorAuthor

@coderabbitai full review

@coderabbitai

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Rule/NestedHandler.php`:
- Around line 98-109: The code can pass a scalar as the scope to
context->validate when an intermediate path segment exists but is not an array
(e.g. ['push'=>'on'] with path push.content); after the for-loop that walks
$valuePathList in NestedHandler.php, ensure $scopeData is an array and if not
set $scopeData = [] so context->validate always receives an array (empty-scope
fallback) instead of a scalar; locate the variables $scopeData, $valuePathList
and the call to $context->validate and add the normalization check just before
the validate call.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a34e8aa5-ff03-43bc-8834-3d217bc1b576

📥 Commits

Reviewing files that changed from the base of the PR and between 2779afc and 0cb3cc2.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/Rule/NestedHandler.php
  • tests/Rule/Nested/NestedTest.php

Comment threadsrc/Rule/NestedHandler.php
Sign up for freeto 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

@WarLikeLaux@samdark