Fix multinetwork #316
Fix multinetwork #316
Conversation
|
Warning Rate limit exceeded@superdav42 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 59 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)
📝 WalkthroughWalkthroughAdds ESLint and Stylelint configs and npm lint scripts; introduces a reusable product-type helper; replaces literal plan-type checks with the helper; adds an admin hook-fire method and a bulk-actions filter; and short-circuits site query domain-merge for certain queries. Changes
Sequence Diagram(s)(omitted — changes do not introduce a new multi-component sequential flow needing visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.stylelintrc.js (1)
1-36: Consider a gradual rule enablement roadmap.The permissive configuration is appropriate for introducing linting to legacy code. However, consider creating a roadmap to gradually re-enable rules over time, starting with those that catch common issues:
- Phase 1 (current): All rules disabled for baseline
- Phase 2: Re-enable critical rules like
no-duplicate-selectorsanddeclaration-block-no-duplicate-properties- Phase 3: Enable specificity and organization rules
- Phase 4: Enforce formatting consistency
This approach allows incremental improvement without blocking immediate adoption.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.eslintrc.js.stylelintrc.jsinc/admin-pages/class-base-admin-page.phpinc/checkout/class-cart.phpinc/functions/product.phpinc/functions/site.phpinc/list-tables/class-site-list-table.phppackage.json
🧰 Additional context used
🧬 Code graph analysis (3)
inc/functions/product.php (6)
inc/models/class-site.php (1)
get_type(1144-1157)inc/models/class-customer.php (1)
get_type(722-725)inc/models/class-product.php (1)
get_type(844-847)inc/checkout/class-line-item.php (1)
get_type(357-360)inc/checkout/signup-fields/class-signup-field-order-summary.php (1)
get_type(32-35)inc/models/class-discount-code.php (1)
get_type(359-362)
inc/functions/site.php (1)
inc/functions/domain.php (1)
wu_get_domains(35-38)
inc/checkout/class-cart.php (3)
inc/models/class-product.php (1)
get_type(844-847)inc/checkout/class-line-item.php (1)
get_type(357-360)inc/functions/product.php (1)
wu_is_plan_type(184-199)
⏰ 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). (7)
- GitHub Check: PHP 8.3
- GitHub Check: PHP 8.0
- GitHub Check: PHP 8.5
- GitHub Check: PHP 7.4
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (11)
inc/admin-pages/class-base-admin-page.php (2)
626-627: LGTM! Clean hook registration.The new hook registration follows established patterns and correctly sequences after the
register_widgetshook (priority 20) to allow addons to augment widget registration for specific admin pages.
734-756: LGTM! Well-structured extension point.The new method provides a clean, page-specific hook for addons to register widgets, following the same pattern as existing
wu_page_{$this->id}_loadhooks. The implementation is well-documented and consistent with the codebase conventions.inc/list-tables/class-site-list-table.php (1)
372-389: LGTM! Good extensibility pattern.The addition of the
wu_site_list_get_bulk_actionsfilter provides a clean extension point for addons to modify available bulk actions. The implementation is well-documented and follows WordPress conventions.inc/functions/site.php (1)
64-66: LGTM! Cleaner empty check.The use of
empty()for checkingquery['number']is idiomatic and consistent with PHP best practices.inc/functions/product.php (2)
172-199: LGTM! Excellent refactoring for extensibility.The new
wu_is_plan_type()helper centralizes plan-type detection and allows addons to register additional plan types via thewu_plan_product_typesfilter. The use of strict comparison (in_arraywithtrueflag) prevents type coercion issues.
222-222: LGTM! Consistent use of the new helper.Replacing the direct type comparison with
wu_is_plan_type()ensures consistent plan-type detection and benefits from the extensibility provided by the new helper.inc/checkout/class-cart.php (2)
647-647: LGTM! Consistent adoption of centralized helper.Replacing the direct plan-type comparison with
wu_is_plan_type($product->get_type())ensures consistent plan detection when recovering payments and allows the cart logic to benefit from the filter-based extensibility.
1610-1610: LGTM! Proper use of the refactored helper.Using
wu_is_plan_type($product->get_type())in theadd_productmethod ensures consistent plan identification and allows addons to extend plan types via the filter, maintaining consistency across the checkout flow.package.json (1)
52-59: LGTM! Well-structured linting workflow.The lint scripts are well-organized with:
- Parallel execution for efficiency (
run-p)- Separate commands for each linter type (PHP, JS, CSS)
- Automatic fix variants for each linter
- Appropriate ignore patterns for minified files
.stylelintrc.js (1)
37-43: LGTM! Appropriate ignore patterns.The
ignoreFilesconfiguration correctly excludes minified files, dependencies, and vendor code from linting..eslintrc.js (1)
1-7: LGTM! Well-structured ESLint configuration.The configuration provides:
- Appropriate extends and environment settings for WordPress
- Comprehensive globals list preventing false "undefined" warnings
- Correct ignore patterns for generated and third-party code
Also applies to: 8-35, 65-71
| rules: { | ||
| // Allow tabs for indentation (matches PHP coding standards) | ||
| indent: ['error', 'tab', { SwitchCase: 1 }], | ||
| // Disable prettier - too strict for legacy code | ||
| 'prettier/prettier': 'off', | ||
| // Relax some rules for legacy code compatibility | ||
| 'no-unused-vars': ['warn', { vars: 'all', args: 'none', ignoreRestSiblings: true }], | ||
| // Allow console for development | ||
| 'no-console': 'warn', | ||
| // Allow var for legacy code | ||
| 'no-var': 'warn', | ||
| // Prefer const but don't enforce strictly for legacy code | ||
| 'prefer-const': 'warn', | ||
| // Object shorthand is nice but not required for legacy code | ||
| 'object-shorthand': 'warn', | ||
| // Allow snake_case for WP compatibility | ||
| camelcase: 'off', | ||
| // Allow redeclaring globals (we define them above) | ||
| 'no-redeclare': 'off', | ||
| // Disable strict formatting rules for legacy code | ||
| 'space-in-parens': 'off', | ||
| 'comma-dangle': 'off', | ||
| quotes: 'off', | ||
| semi: 'off', | ||
| 'padded-blocks': 'off', | ||
| 'eol-last': 'off', | ||
| 'space-before-function-paren': 'off', | ||
| 'space-before-blocks': 'off', | ||
| }, |
There was a problem hiding this comment.
Reconsider disabling no-redeclare rule.
While the permissive configuration is appropriate for legacy code adoption, the no-redeclare: 'off' rule (line 54) is concerning. The comment states "Allow redeclaring globals (we define them above)", but this rule catches ALL variable redeclarations, not just globals. Disabling it can mask bugs where variables are accidentally redeclared in the same scope.
Consider changing it to 'warn' instead of 'off' to catch potential issues without blocking linting.
🔎 Suggested fix
- // Allow redeclaring globals (we define them above)
- 'no-redeclare': 'off',
+ // Warn on redeclaring variables (including globals)
+ 'no-redeclare': 'warn',Additionally, similar to the Stylelint configuration, consider creating a roadmap to gradually re-enable rules over time for incremental code quality improvements.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rules: { | |
| // Allow tabs for indentation (matches PHP coding standards) | |
| indent: ['error', 'tab', { SwitchCase: 1 }], | |
| // Disable prettier - too strict for legacy code | |
| 'prettier/prettier': 'off', | |
| // Relax some rules for legacy code compatibility | |
| 'no-unused-vars': ['warn', { vars: 'all', args: 'none', ignoreRestSiblings: true }], | |
| // Allow console for development | |
| 'no-console': 'warn', | |
| // Allow var for legacy code | |
| 'no-var': 'warn', | |
| // Prefer const but don't enforce strictly for legacy code | |
| 'prefer-const': 'warn', | |
| // Object shorthand is nice but not required for legacy code | |
| 'object-shorthand': 'warn', | |
| // Allow snake_case for WP compatibility | |
| camelcase: 'off', | |
| // Allow redeclaring globals (we define them above) | |
| 'no-redeclare': 'off', | |
| // Disable strict formatting rules for legacy code | |
| 'space-in-parens': 'off', | |
| 'comma-dangle': 'off', | |
| quotes: 'off', | |
| semi: 'off', | |
| 'padded-blocks': 'off', | |
| 'eol-last': 'off', | |
| 'space-before-function-paren': 'off', | |
| 'space-before-blocks': 'off', | |
| }, | |
| rules: { | |
| // Allow tabs for indentation (matches PHP coding standards) | |
| indent: ['error', 'tab', { SwitchCase: 1 }], | |
| // Disable prettier - too strict for legacy code | |
| 'prettier/prettier': 'off', | |
| // Relax some rules for legacy code compatibility | |
| 'no-unused-vars': ['warn', { vars: 'all', args: 'none', ignoreRestSiblings: true }], | |
| // Allow console for development | |
| 'no-console': 'warn', | |
| // Allow var for legacy code | |
| 'no-var': 'warn', | |
| // Prefer const but don't enforce strictly for legacy code | |
| 'prefer-const': 'warn', | |
| // Object shorthand is nice but not required for legacy code | |
| 'object-shorthand': 'warn', | |
| // Allow snake_case for WP compatibility | |
| camelcase: 'off', | |
| // Warn on redeclaring variables (including globals) | |
| 'no-redeclare': 'warn', | |
| // Disable strict formatting rules for legacy code | |
| 'space-in-parens': 'off', | |
| 'comma-dangle': 'off', | |
| quotes: 'off', | |
| semi: 'off', | |
| 'padded-blocks': 'off', | |
| 'eol-last': 'off', | |
| 'space-before-function-paren': 'off', | |
| 'space-before-blocks': 'off', | |
| }, |
🤖 Prompt for AI Agents
In .eslintrc.js around lines 36 to 64, change the 'no-redeclare' rule from 'off'
to 'warn' so accidental redeclarations in the same scope are flagged without
failing linting; if you truly need to allow specific globals declared elsewhere,
keep them in the top-level "globals" ESLint config or use file-level /* global
*/ directives rather than disabling the rule entirely.
| // If we're just counting, skip the domain search merge logic | ||
| // and do a simple count query. | ||
| if ( ! empty($query['count'])) { | ||
| if ( ! empty($query['search'])) { | ||
| $domain_count = wu_get_domains( | ||
| [ | ||
| 'number' => $query['number'], | ||
| 'search' => '*' . $query['search'] . '*', | ||
| 'count' => true, | ||
| 'blog_id__in' => $query['blog_id__in'] ?? false, | ||
| ] | ||
| ); | ||
| } else { | ||
| $domain_count = 0; | ||
| } | ||
| return \WP_Ultimo\Models\Site::query($query) + $domain_count; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the structure of the site.php file and understand the full context
head -n 120 inc/functions/site.php | tail -n +60Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1952
🏁 Script executed:
# Find the wu_get_domains function definition
rg -n "function wu_get_domains" --type=phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 147
🏁 Script executed:
# Find the Site class and understand how query() works
rg -n "class Site" --type=php | grep -E "(^|/)class-site"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1720
🏁 Script executed:
# Search for how Site::query implements search
rg -n "search" inc/models/class-site.php -A 3 -B 1 --type=php | head -n 50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1085
🏁 Script executed:
# Check Site_Query to see what fields are searched
rg -n "search" inc/database/sites/class-site-query.php -B 2 -A 5 --type=php | head -n 80Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check the wu_get_domains function implementation to see what it searches
sed -n '35,150p' inc/functions/domain.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 2525
🏁 Script executed:
# Look for where search parameter is processed in Site_Query
ast-grep --pattern $'function query($$$) {
$$$
}'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 660
🏁 Script executed:
# Read the Site_Query class to understand what fields it searches
cat -n inc/database/sites/class-site-query.php | head -n 200Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3099
🏁 Script executed:
# Find and read the Domain_Query class
find . -name "*domain*query*" -type f | head -n 5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 126
🏁 Script executed:
# Read the Domain_Query class
cat -n ./inc/database/domains/class-domain-query.php | head -n 150Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 2892
🏁 Script executed:
# Find the parent Query class
find . -path "*/database/engine/class-query.php" -o -path "*/database/engine/Query.php" | head -n 1Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 118
🏁 Script executed:
# Search for searchable_columns or search handling in parent classes
rg -n "searchable_columns\|search" inc/database/engine/ --type=php -B 2 -A 5 | head -n 100Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Read the parent Query class to understand search implementation
cat -n ./inc/database/engine/class-query.php | head -n 300Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3525
🏁 Script executed:
# Look specifically for search-related methods in Query class
rg -n "function.*search\|searchable_columns" ./inc/database/engine/class-query.php --type=phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check the Sites_Schema to see what columns are searchable
cat -n inc/database/sites/class-sites-schema.php | head -n 200Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3694
🏁 Script executed:
# Check the Domains_Schema to see what columns are searchable
cat -n inc/database/domains/class-domains-schema.php | head -n 200Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3145
The count logic will double-count sites that match both domain and site search criteria.
When counting, the code adds site count and domain count without deduplication. A site will be counted twice if it matches the search term in both:
- The sites table (via
domainorpathcolumn) - The domain_mappings table (via mapped
domain)
The non-count logic explicitly handles this deduplication (lines 86-120) by getting domains, then excluding those blog_ids from the main site query. The count path must do the same: subtract domain-matched sites from the site count, or exclude their blog_ids before adding the domain count.
| "@eslint/js": "^9.39.2", | ||
| "@wordpress/env": "^10.27.0", | ||
| "@wordpress/eslint-plugin": "^23.0.0", | ||
| "@wordpress/stylelint-config": "^23.29.0", | ||
| "clean-css-cli": "^5.6.3", | ||
| "cypress": "^14.5.4", | ||
| "cypress-mailpit": "^1.4.0", | ||
| "eslint": "^8.57.1", | ||
| "globals": "^16.5.0", | ||
| "npm-run-all": "^4.1.5", | ||
| "stylelint": "^16.26.1", | ||
| "stylelint-config-standard": "^39.0.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat package.json | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1845
🏁 Script executed:
#!/bin/bash
# Query npm for peer dependencies and versions
echo "=== @eslint/js peer dependencies ==="
npm view @eslint/js@9.39.2 peerDependencies 2>/dev/null || echo "Failed to fetch"
echo -e "\n=== ESLint peer dependencies ==="
npm view eslint@8.57.1 peerDependencies 2>/dev/null || echo "Failed to fetch"
echo -e "\n=== Check ESLint 9 compatibility ==="
npm view eslint@latest version
npm view @eslint/js@latest version
echo -e "\n=== Security check ==="
npm audit --production --json 2>/dev/null | jq -r '.vulnerabilities | length' || echo "Could not check vulnerabilities"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 229
🌐 Web query:
ESLint 9 migration @eslint/js eslint 8 compatibility
💡 Result:
Do you want (A) migration steps to ESLint 9, (B) whether @eslint/js is compatible with ESLint 8, or (C) help resolving peer‑dependency/plugin errors when upgrading? Which one should I search and answer first?
🌐 Web query:
@eslint/js 9 compatible with ESLint 8.x package configuration
💡 Result:
Short answer: No — @eslint/js v9 targets ESLint v9 (flat config) and is not guaranteed to work with ESLint 8.x; you will hit peer-dependency / config incompatibilities. [1][2][3]
Details & options:
- @eslint/js v9 is intended for the ESLint v9 ecosystem (flat eslint.config.js and v9 APIs). Upgrading/downgrading across major ESLint versions can cause peerDependency errors and runtime/config differences. [1][3]
- If you must stay on ESLint 8, either use the @eslint/js release that matches ESLint 8 (or stick to the legacy "eslint:recommended" behavior) or use compatibility tooling such as @eslint/compat to try to bridge differences — but that package does not guarantee every plugin/config will work. [2][6]
Sources:
- ESLint migration to v9 (flat config / v9 requirements). [1]
- @eslint/compat (compatibility utilities to help v8→v9 differences). [2]
- Common peer-dependency/install issues when mixing v8 plugins with ESLint v9 (example issues). [3]
If you want, tell me your current package.json (eslint and @eslint/js versions) and I’ll give exact next steps.
🏁 Script executed:
#!/bin/bash
# Check available @eslint/js versions for ESLint 8 compatibility
echo "=== Available @eslint/js versions ==="
npm view @eslint/js versions --json | jq -r '.[] | select(startswith("8"))' | tail -10
echo -e "\n=== ESLint 8.x info ==="
npm view eslint@8.57.1 description
echo -e "\n=== Check if @eslint/js 8.x exists ==="
npm view @eslint/js@8 2>/dev/null || echo "@eslint/js 8.x does not appear to exist"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 16342
Downgrade @eslint/js or upgrade eslint to resolve version mismatch.
The configuration has @eslint/js@9.39.2 with eslint@8.57.1. ESLint 9.x is incompatible with ESLint 8.x. Either downgrade @eslint/js to ^8.57.1 to match your current ESLint version, or upgrade eslint to ^9.39.2 to match the newer configuration package.
🤖 Prompt for AI Agents
In package.json around lines 17 to 28, there is a version mismatch between
@eslint/js (9.39.2) and eslint (8.57.1); ESLint v9 plugins require ESLint v9.
Fix by either downgrading @eslint/js to a v8-compatible range (e.g., ^8.x) to
match eslint@8.57.1, or upgrade eslint to a v9 release that matches @eslint/js
(e.g., ^9.39.2), then run npm install and update lockfile accordingly.
🔨 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: |
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.