GH#1716: optimize front-end My Sites toolbar - #1719
Conversation
📝 WalkthroughWalkthroughThe PR adds a disabled-by-default Sites setting for frontend My Sites toolbar optimization. When enabled for eligible authenticated frontend toolbar requests, the site manager returns only the current site and preserves existing behavior in excluded contexts. ChangesFrontend My Sites toolbar optimization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WP_Admin_Bar
participant Site_Manager
participant Current_Site
WP_Admin_Bar->>Site_Manager: request frontend toolbar sites
Site_Manager->>Current_Site: get current site
Current_Site-->>Site_Manager: current site data
Site_Manager-->>WP_Admin_Bar: return current site only
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Merge summary
Verification
|
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![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
🤖 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 `@inc/managers/class-site-manager.php`:
- Around line 1166-1168: Update the optimized branch in pre_get_blogs_of_user so
the result from get_current_site_for_frontend_my_sites_toolbar is passed through
the same get_blogs_of_user filter as the normal path before returning. Add a
regression test covering the frontend My Sites toolbar optimization and
verifying that the filter can modify the returned site list.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fbbced2f-f4e5-4ca5-83dd-a8b2fa780b12
📒 Files selected for processing (4)
inc/class-settings.phpinc/managers/class-site-manager.phptests/WP_Ultimo/Managers/Site_Manager_Test.phptests/WP_Ultimo/Settings_Test.php
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/WP_Ultimo/Managers/Site_Manager_Test.php (2)
15-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsume the test override parameter.
PHPMD reports
$backtraceas an unused formal parameter. Keep the parameter for signature compatibility, then addunset($backtrace);or a targeted suppression.Proposed fix
protected function is_frontend_my_sites_toolbar_request($backtrace = null) { + unset($backtrace); + return true; }Based on static analysis, PHPMD reports
UnusedFormalParameterfor$backtrace.🤖 Prompt for 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. In `@tests/WP_Ultimo/Managers/Site_Manager_Test.php` around lines 15 - 17, Update is_frontend_my_sites_toolbar_request to consume the intentionally retained $backtrace parameter by unsetting it before returning true, preserving signature compatibility while resolving PHPMD’s UnusedFormalParameter warning.Source: Linters/SAST tools
2979-2979: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
wu_create_*()helpers for test users.The three new tests call
$this->factory()->user->create(). Replace these calls with the applicable project helper that returns the required user ID. Keep the super-admin grant and cleanup unchanged.As per coding guidelines, tests must use
wu_create_*()helper functions to set up test data.Also applies to: 3010-3010, 3043-3043
🤖 Prompt for 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. In `@tests/WP_Ultimo/Managers/Site_Manager_Test.php` at line 2979, Replace the three test user setups in the affected tests with the applicable wu_create_*() helper that returns an administrator user ID. Preserve the existing super-admin grant and cleanup logic unchanged.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@tests/WP_Ultimo/Managers/Site_Manager_Test.php`:
- Around line 15-17: Update is_frontend_my_sites_toolbar_request to consume the
intentionally retained $backtrace parameter by unsetting it before returning
true, preserving signature compatibility while resolving PHPMD’s
UnusedFormalParameter warning.
- Line 2979: Replace the three test user setups in the affected tests with the
applicable wu_create_*() helper that returns an administrator user ID. Preserve
the existing super-admin grant and cleanup logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 908cf389-40b4-4e44-948c-eec983f22d3c
📒 Files selected for processing (2)
inc/managers/class-site-manager.phptests/WP_Ultimo/Managers/Site_Manager_Test.php
🚧 Files skipped from review as they are similar to previous changes (1)
- inc/managers/class-site-manager.php
1 participant
Summary
Verification
vendor/bin/phpcs inc/class-settings.php inc/managers/class-site-manager.php tests/WP_Ultimo/Managers/Site_Manager_Test.php tests/WP_Ultimo/Settings_Test.phpvendor/bin/phpunit --filter test_frontend_my_sites_toolbar_optimization --debugvendor/bin/phpunit --filter Settings_TestKnown verification limitation
vendor/bin/phpunit --filter Site_Manager_Teststops when an existing test accesses unavailable database tables; the three new focused Site_Manager tests pass.Resolves #1716
Summary by CodeRabbit