-
-
Notifications
You must be signed in to change notification settings - Fork 67
support multinetwork #315
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
base: main
Are you sure you want to change the base?
support multinetwork #315
Conversation
📝 WalkthroughWalkthroughThree modifications across the admin pages, site query logic, and site list tables: a new widget registration hook method, early-return optimization for count queries, and bulk actions filter application for extensibility. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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. Comment |
🔨 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
inc/admin-pages/class-base-admin-page.phpinc/functions/site.phpinc/list-tables/class-site-list-table.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/functions/site.php (1)
inc/models/class-site.php (1)
Site(26-1942)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: PHP 8.1
- GitHub Check: PHP 8.0
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (3)
inc/list-tables/class-site-list-table.php (1)
380-388: LGTM! Good extensibility addition.The filter
wu_site_list_get_bulk_actionsis well-documented and allows external modification of bulk actions, improving extensibility for plugins and themes.inc/admin-pages/class-base-admin-page.php (2)
626-626: LGTM! New hook provides extensibility for widget registration.The new action hook at priority 21 enables addons to register widgets after the core registration completes.
734-756: LGTM! Well-documented extensibility method.The
fire_register_widgets_hook()method is properly documented and provides a clean extension point for page-specific widget registration via the dynamic actionwu_page_{$this->id}_register_widgets.
| // If we're just counting, skip the domain search merge logic | ||
| // and do a simple count query. | ||
| if ( ! empty($query['count'])) { | ||
| return \WP_Ultimo\Models\Site::query($query); | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and the specific lines mentioned
cat -n inc/functions/site.php | head -120Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3644
🏁 Script executed:
# Search for wu_get_sites calls with both count and search parameters
rg -n 'wu_get_sites\s*\(' --type=php -A 10 | head -100Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 4799
🏁 Script executed:
# More targeted search for count + search combination
rg -n "wu_get_sites" --type=php -B 2 -A 5 | rg -A 5 -B 2 "count|search"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 408
🏁 Script executed:
# Examine the list table implementation around the wu_get_sites call
cat -n inc/list-tables/class-site-list-table.php | sed -n '60,120p'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 2097
🏁 Script executed:
# Search specifically for cases where 'count' and 'search' appear together in query construction
rg -n "'count'|'search'" --type=php -B 3 -A 3 | grep -B 5 -A 5 "count.*search|search.*count"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Look at prepare_items method to see the full query construction
ast-grep --pattern $'function prepare_items() {
$$$
}'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Search for prepare_items method and how get_items is called
rg -n "prepare_items|get_items" inc/list-tables/class-site-list-table.php -B 2 -A 10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 735
🏁 Script executed:
# Check parent class to understand list table pagination
rg -n "class.*List_Table|prepare_items" inc/list-tables/class-site-list-table.php -A 20 | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 586
🏁 Script executed:
# Check the Base_List_Table to understand how pagination uses the count parameter
fd "class-base-list-table" --type=phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 251
🏁 Script executed:
# Search for pagination or prepare_items in the base class
rg -n "prepare_items|total_items|get_items" inc/list-tables/class-base-list-table.php -A 5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 2616
🏁 Script executed:
# Check how record_count is used in prepare_items
rg -n "record_count" inc/list-tables/class-base-list-table.php -B 5 -A 5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1415
🏁 Script executed:
# Verify the full flow: check if search parameter is indeed passed during count
rg -n "wu_request.*'s'|search.*wu_request" inc/list-tables/ -B 2 -A 2Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 2000
Fix count queries to include mapped domain matches when search filters are applied.
The list table pagination relies on record_count() which calls wu_get_sites() with both count=true and a search parameter (line 79 in class-site-list-table.php). However, the count query early returns at line 70-71 in inc/functions/site.php, bypassing the domain-search merge logic (lines 74-106). This causes pagination to report an incorrect total when searching text that matches mapped domains—the pagination count excludes those matches while the actual result set includes them.
The mismatch occurs because:
record_count()→get_items(..., true)→wu_get_sites(['count' => true, 'search' => ...])skips the domain mergeprepare_items()→get_items(..., false)→wu_get_sites(['count' => false, 'search' => ...])includes the domain merge
Apply the domain-search merge logic before returning the count so pagination totals remain consistent with actual result sets.
🤖 Prompt for AI Agents
In inc/functions/site.php around lines 68 to 72, the early return for count
queries skips the mapped-domain search merge logic, causing pagination totals to
differ from actual result sets when a search term matches mapped domains; move
or duplicate the domain-search merge logic so it runs even when $query['count']
is true (i.e., perform the mapped-domain lookup/merge into $query prior to
returning the count), ensuring the count query includes the same domain-based
filters as the non-count path and returns the correct total.
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.