-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add TreeWithStrictValidation model with strict validation rules. Update tests to include validation scenarios for appendTo method.
#25
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
Conversation
…es. Update tests to include validation scenarios for `appendTo` method.
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughA new model class with strict validation rules was introduced, and corresponding tests were added to verify the behavior of the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as NestedSetsQueryBehaviorTest
participant Model as TreeWithStrictValidation
participant DB as Database
Test->>Model: Create invalid node (invalid name)
Test->>Model: appendTo(parent, runValidation=true)
Model->>Model: Validate (fails)
Model-->>Test: Returns false, has errors
Test->>Model: Create another invalid node
Test->>Model: appendTo(parent, runValidation=false)
Model->>Model: Skip validation
Model->>DB: Persist node
Model-->>Test: Returns true, no errors
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 90 90
===========================================
Files 2 2
Lines 469 469
===========================================
Hits 469 469 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/NestedSetsQueryBehaviorTest.php (1)
74-125: Comprehensive test coverage for appendTo validation scenarios.The test method effectively validates both code paths of the
appendTomethod - with and without validation enabled. The test logic correctly uses invalid data ('x') that fails the strict validation rules (minimum length 5 and uppercase first letter) to verify the expected behavior in each scenario.However, consider improving the assertion message clarity:
- self::assertNotNull( - $persistedNode, - "Node with ID '{$persistedNode?->id}' should exist after appending to target node.", - ); + self::assertNotNull( + $persistedNode, + "Node should exist in database after appending to target node with validation disabled.", + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/NestedSetsQueryBehaviorTest.php(2 hunks)tests/phpstan-config.php(2 hunks)tests/support/model/TreeWithStrictValidation.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#13
File: src/NestedSetsBehavior.php:145-145
Timestamp: 2025-06-29T11:22:41.839Z
Learning: The user terabytesoftw prefers to keep PRs focused and small, implementing enhancements in separate dedicated PRs rather than bundling multiple changes together.
🧬 Code Graph Analysis (2)
tests/support/model/TreeWithStrictValidation.php (1)
tests/support/model/Tree.php (1)
Tree(17-64)
tests/NestedSetsQueryBehaviorTest.php (4)
tests/support/model/Tree.php (1)
Tree(17-64)tests/support/model/TreeWithStrictValidation.php (1)
TreeWithStrictValidation(14-24)tests/TestCase.php (1)
generateFixtureTree(168-202)src/NestedSetsBehavior.php (1)
appendTo(294-306)
🪛 PHPMD (2.15.0)
tests/NestedSetsQueryBehaviorTest.php
78-78: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAppendToWithRunValidationParameterUsingStrictValidation'. (Clean Code Rules)
(StaticAccess)
114-114: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\TreeWithStrictValidation' in method 'testAppendToWithRunValidationParameterUsingStrictValidation'. (Clean Code Rules)
(StaticAccess)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
🔇 Additional comments (2)
tests/support/model/TreeWithStrictValidation.php (1)
14-24: Well-designed strict validation model for testing purposes.The
TreeWithStrictValidationclass effectively extends the baseTreemodel with comprehensive validation rules that will enable thorough testing of validation scenarios. The three-tiered validation approach (required, minimum length, pattern matching) with custom error messages provides good coverage for testing different validation failure cases.tests/phpstan-config.php (1)
7-13: Proper PHPStan configuration update for the new model.The configuration correctly adds
TreeWithStrictValidationto both the import statement and behavior mapping, following the established pattern for other model classes in the project.Also applies to: 35-37
…Test` class and clean up imports.
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.
Pull Request Overview
This PR adds a new TreeWithStrictValidation model enforcing stronger name validation rules and updates tests and configuration to cover appendTo() behavior with validation enabled or skipped.
- Introduce
TreeWithStrictValidationsubclass with required, min-length, and uppercase-start rules - Register the new model in
phpstan-config.phpfor correct behavior typing - Add tests in
NestedSetsBehaviorTest.phpcoveringappendTo()with and without validation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/support/model/TreeWithStrictValidation.php | New model subclass defining strict name validation rules |
| tests/phpstan-config.php | Added import and behavior mapping for TreeWithStrictValidation |
| tests/NestedSetsBehaviorTest.php | New test method for appendTo() under strict validation scenarios |
Comments suppressed due to low confidence (1)
tests/NestedSetsBehaviorTest.php:1797
- Consider adding a positive test case where a valid
name(e.g., starting uppercase and at least 5 characters) is used withrunValidation=trueto assertappendTo()returnstrue, has no errors, and persists the node.
$invalidNode = new TreeWithStrictValidation(['name' => 'x']);
Summary by CodeRabbit