refactor: Update save logic in NestedSetsBehavior class to refresh node after saving. Add test for left and right attribute adjustment when appending child to root.
#24
Conversation
…node after saving. Add test for `left` and `right` attribute adjustment when appending child to root.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 89 90 +1
===========================================
Files 2 2
Lines 466 469 +3
===========================================
+ Hits 466 469 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 37 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)
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant NestedSetsBehavior
participant Node
Caller->>NestedSetsBehavior: appendTo(node)
NestedSetsBehavior->>NestedSetsBehavior: save()
alt save successful
NestedSetsBehavior->>Node: refresh()
end
NestedSetsBehavior-->>Caller: return result
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 (
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…node only on successful save.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the save logic in the NestedSetsBehavior class so that the parent node is refreshed after a successful save and adds a new test to verify that left/right attributes are correctly adjusted when appending a child to a root.
- Updated the save logic in NestedSetsBehavior to refresh the node after saving.
- Introduced a new test in NestedSetsBehaviorTest to validate left/right attribute adjustments for appended child nodes.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/NestedSetsBehavior.php | Added logic to refresh the passed node after a successful save. |
| tests/NestedSetsBehaviorTest.php | Added a new test to validate left/right attribute values post-append. |
Comments suppressed due to low confidence (1)
src/NestedSetsBehavior.php:302
- Consider adding a brief comment explaining why the passed node is refreshed after a successful save operation.
$node->refresh();
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/NestedSetsBehavior.php (1)
299-305: LGTM! Proper implementation of target node refresh after save.The implementation correctly stores the save result, guards the refresh call to only execute on successful saves, and preserves the original return value. The strict comparison
=== trueis more explicit and safer than the previously suggestedif ($result), though both are functionally equivalent forActiveRecord::save().Potential consistency consideration for future enhancement:
Other similar methods like
insertAfter,insertBefore, andprependTodon't refresh their target nodes, which could lead to stale data in memory after these operations. Consider addressing this in a separate PR to maintain consistency across all node manipulation methods.tests/NestedSetsBehaviorTest.php (1)
1755-1764: Remove redundant assertions.These assertions checking that
child.lft != 0andchild.rgt != 1are redundant since the exact values are already asserted above (lines 1746-1754). The specific value assertions provide stronger validation.Remove the redundant assertions:
- self::assertNotEquals( - 0, - $child->lft, - 'Child node left value should not be \'0\' after \'appendTo\' operation.', - ); - self::assertNotEquals( - 1, - $child->rgt, - 'Child node right value should not be \'1\' after \'appendTo\' operation.', - );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/NestedSetsBehavior.php(1 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 (1)
tests/NestedSetsBehaviorTest.php (3)
tests/TestCase.php (1)
createDatabase(93-127)tests/support/model/Tree.php (1)
Tree(17-64)src/NestedSetsBehavior.php (2)
makeRoot(837-842)appendTo(294-306)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.3-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
🔇 Additional comments (1)
tests/NestedSetsBehaviorTest.php (1)
1723-1765: Well-structured test for nested set boundary validation.The test effectively validates the core functionality of nested set left/right boundary adjustment when appending a child to a root node. The assertions correctly verify the expected nested set structure (root: 1-4, child: 2-3) and complement the
appendTomethod changes.
1 participant
Summary by CodeRabbit
Bug Fixes
Tests