test: Add verification for attribute refresh() method after promoting node to root in NestedSetsBehaviorTest.
#42
Conversation
…ng node to root in `NestedSetsBehaviorTest`.
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NestedSetsBehavior
participant Model (Owner)
participant Database
User->>NestedSetsBehavior: makeRoot()
NestedSetsBehavior->>Model (Owner): save()
Model (Owner)->>Database: Save changes
Database-->>Model (Owner): Save result
NestedSetsBehavior->>Model (Owner): refresh()
Model (Owner)->>Database: Fetch latest attributes
Database-->>Model (Owner): Updated attributes
NestedSetsBehavior-->>User: Return result
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2)src/NestedSetsBehavior.phpWarning: PHP Startup: Invalid date.timezone value 'UTC', using 'UTC' instead in Unknown on line 0 Fatal error: Uncaught Error: Class "Phar" not found in /usr/local/bin/phpcs:3 tests/NestedSetsBehaviorTest.phpWarning: PHP Startup: Invalid date.timezone value 'UTC', using 'UTC' instead in Unknown on line 0 Fatal error: Uncaught Error: Class "Phar" not found in /usr/local/bin/phpcs:3 ✨ 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 #42 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 90 91 +1
===========================================
Files 2 2
Lines 470 474 +4
===========================================
+ Hits 470 474 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/NestedSetsBehaviorTest.php (1)
2326-2455: Excellent comprehensive test for therefresh()behavior aftermakeRoot().This test effectively validates that the
refresh()call in themakeRoot()method properly updates model attributes immediately after the database operation. The test logic is sound and the assertions comprehensively cover the expected behavior changes.However, the method length (130 lines) exceeds recommended guidelines. While this is an integration test that benefits from testing the complete workflow, consider if some verification logic could be extracted into helper methods to improve readability.
Optional refactor suggestion:
Consider extracting some assertion groups into private helper methods:
+ private function assertNodeAttributesAfterMakeRoot(MultipleTree $node, array $originalAttributes): void + { + self::assertTrue($node->isRoot(), "Node should be identified as root after 'makeRoot()' - this requires 'refresh()' to work."); + self::assertEquals(1, $node->getAttribute('lft'), "Root node left value should be '1' after 'makeRoot()' - requires 'refresh()' to see updated value."); + self::assertEquals(4, $node->getAttribute('rgt'), "Root node right value should be '4' after 'makeRoot()' - requires 'refresh()' to see updated value."); + self::assertEquals(0, $node->getAttribute('depth'), "Root node depth should be '0' after 'makeRoot()' - requires 'refresh()' to see updated value."); + self::assertEquals($node->getAttribute('id'), $node->getAttribute('tree'), "Tree attribute should equal node ID for new root - requires 'refresh()' to see updated value."); + } + + private function assertAttributesChangedFromOriginal(MultipleTree $node, array $originalAttributes): void + { + self::assertNotEquals($originalAttributes['lft'], $node->getAttribute('lft'), "Left value should have changed from original after 'makeRoot()'."); + self::assertNotEquals($originalAttributes['rgt'], $node->getAttribute('rgt'), "Right value should have changed from original after 'makeRoot()'."); + self::assertNotEquals($originalAttributes['depth'], $node->getAttribute('depth'), "Depth should have changed from original after 'makeRoot()'."); + self::assertNotEquals($originalAttributes['tree'], $node->getAttribute('tree'), "Tree should have changed from original after 'makeRoot()'."); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/NestedSetsBehavior.php(3 hunks)tests/NestedSetsBehaviorTest.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)
src/NestedSetsBehavior.php (1)
src/NestedSetsQueryBehavior.php (1)
getOwner(136-143)
tests/NestedSetsBehaviorTest.php (3)
tests/TestCase.php (1)
createDatabase(93-127)tests/support/model/MultipleTree.php (1)
MultipleTree(18-59)src/NestedSetsBehavior.php (2)
makeRoot(845-858)isRoot(753-756)
🪛 PHPMD (2.15.0)
tests/NestedSetsBehaviorTest.php
2326-2455: The method testMakeRootRefreshIsNecessaryForCorrectAttributeValues() has 130 lines of code. Current threshold is set to 100. Avoid really long methods. (Code Size Rules)
(ExcessiveMethodLength)
2346-2346: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\MultipleTree' in method 'testMakeRootRefreshIsNecessaryForCorrectAttributeValues'. (Clean Code Rules)
(StaticAccess)
2413-2413: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\MultipleTree' in method 'testMakeRootRefreshIsNecessaryForCorrectAttributeValues'. (Clean Code Rules)
(StaticAccess)
2430-2430: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\MultipleTree' in method 'testMakeRootRefreshIsNecessaryForCorrectAttributeValues'. (Clean Code Rules)
(StaticAccess)
🔇 Additional comments (4)
src/NestedSetsBehavior.php (4)
812-816: Excellent documentation enhancement.The added specificity about nested set attribute values (
left=1,right=2,depth=0) and the clarification about adjusting affected nodes makes the method behavior much clearer for developers.
821-822: Good documentation of the refresh behavior.Accurately documents the new automatic refresh functionality, which is important for developers to understand the post-operation state of the model.
834-841: Valuable usage examples added.The examples clearly demonstrate both use cases (creating new root and moving existing node to root), which enhances developer understanding of the method's versatility.
849-857: Excellent fix for model state consistency.This implementation ensures that nested set attributes reflect their current database values after the operation. The approach is well-designed:
- Properly captures and checks the save result
- Only calls
refresh()on successful save operations- Uses local variable to avoid multiple
getOwner()calls- Maintains the original return behavior
This addresses a real issue where accessing nested set attributes immediately after
makeRoot()could return stale values.
1 participant
Summary by CodeRabbit
Documentation
Bug Fixes
Tests