Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds an optional Footer Credits feature with per-site limits, an Action Scheduler option-lock compat class, a “Recommended Plugins” setup-wizard installer, network plugin/theme usage columns, multiple helper functions for site URL/title generation, various docs/UI tweaks, removal of the commit-msg githook, and version bumps to 2.4.5. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant WP as WordPress
participant Credits as WP_Ultimo\Credits
participant Settings as WP Settings API
participant User as Admin/Visitor
rect rgba(230,240,255,0.5)
note over WP,Credits: Initialization & Settings
WP->>Credits: get_instance()/init()
Credits->>Settings: register_settings()
end
alt Admin footer (non-network)
WP->>Credits: filter_admin_footer_text(text)
Credits->>Credits: site_allows_credit() / build_credit_html()
Credits-->>WP: return replaced footer text
else Frontend/Login (non-admin)
WP->>Credits: wp_footer / login_footer
Credits->>Credits: site_allows_credit() / build_credit_html()
Credits-->>WP: echo credit HTML
end
sequenceDiagram
autonumber
participant Wizard as Setup Wizard
participant Installer as Recommended_Plugins_Installer
participant WPAPI as WordPress Plugin API
participant Upgrader as Plugin_Upgrader
participant User as Admin
Wizard->>Installer: get_instance()/get_steps()
Wizard->>User: render "Recommended Plugins" step
User->>Wizard: request install (AJAX)
Wizard->>Installer: handle(install_plugin_<slug>)
Installer->>WPAPI: plugins_api(slug) -> download_link
Installer->>Upgrader: Plugin_Upgrader->install(download_link)
Upgrader-->>Installer: success/error
Installer-->>Wizard: return result / log errors
sequenceDiagram
autonumber
participant ActionScheduler as ActionScheduler
participant Compat as ActionScheduler_OptionLock_UM
participant Options as WP Options Table
ActionScheduler->>Compat: lock->set(lock_type)
Compat->>Options: get_existing_lock(key)
alt no existing or expired
Compat->>Options: add/update option (unique|expiration)
Compat-->>ActionScheduler: return true
else still valid
Compat-->>ActionScheduler: return false
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
inc/class-credits.php (2)
148-156: Add a filter to allow developers to adjust final credit HTMLProvides simple extensibility without new settings.
Apply:
- return $label . $link; + $html = $label . $link; + /** + * Filter the final credits HTML. + * + * @param string $html + */ + return apply_filters('wu_credits_html', $html);
221-229: Expose render controls and wrapper customization via filtersLets integrators suppress or restyle output without overriding the class.
Apply:
public function render_frontend_footer(): void { if (is_admin()) { return; } $credit = $this->build_credit_html(); if (! $credit || ! $this->site_allows_credit()) { return; } - echo '<div class="wu-powered-by" style="text-align:center;opacity:.7;font-size:12px;margin:10px 0;">' . $credit . '</div>'; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + if (! apply_filters('wu_credits_should_render', true)) { + return; + } + $html = '<div class="wu-powered-by" style="text-align:center;opacity:.7;font-size:12px;margin:10px 0;">' . $credit . '</div>'; + /** + * Filter the rendered credits wrapper HTML. + * + * @param string $html Full HTML to echo. + * @param string $credit Inner credits HTML. + */ + echo apply_filters('wu_credits_render_html', $html, $credit); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inc/class-credits.php(1 hunks)inc/class-wp-ultimo.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
inc/class-wp-ultimo.php (2)
inc/class-credits.php (1)
Credits(22-231)inc/deprecated/deprecated.php (1)
get_instance(353-356)
inc/class-credits.php (3)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-916)inc/functions/settings.php (2)
wu_register_settings_field(89-92)wu_get_setting(39-42)inc/functions/site.php (1)
wu_get_current_site(18-27)
🪛 GitHub Check: Code Quality Checks
inc/class-credits.php
[failure] 174-174:
Doc comment for parameter "$text" missing
⏰ 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). (2)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (2)
inc/class-wp-ultimo.php (1)
599-603: Credits initialization placement looks goodSingleton init fits the existing extra components pattern and will hook settings on
init.inc/class-credits.php (1)
7-8: Version annotation mismatches current plugin VERSIONFile says
@since 2.4.5whileWP_Ultimo::VERSIONis2.4.4. Align before release or bump the constant with the release commit.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/class-wp-ultimo.php (1)
34-42: Keep VERSION constant aligned with the 2.4.5 release metadataWe bumped the project to 2.4.5 elsewhere, but the core constant still reports 2.4.4. That causes asset cache busting/version checks to continue advertising the older build. Please update the constant (and the derived
$versionproperty) to 2.4.5.- const VERSION = '2.4.4'; + const VERSION = '2.4.5';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
composer.json(1 hunks)inc/admin-pages/class-setup-wizard-admin-page.php(4 hunks)inc/class-scripts.php(1 hunks)inc/class-whitelabel.php(2 hunks)inc/class-wp-ultimo.php(3 hunks)inc/compat/class-edit-users-compat.php(1 hunks)package.json(1 hunks)readme.txt(2 hunks)ultimate-multisite.php(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- ultimate-multisite.php
- package.json
👮 Files not reviewed due to content moderation or server errors (1)
- readme.txt
🧰 Additional context used
🧬 Code graph analysis (4)
inc/class-scripts.php (1)
inc/functions/url.php (1)
wu_ajax_url(73-95)
inc/class-whitelabel.php (1)
inc/functions/settings.php (1)
wu_get_setting(39-42)
inc/admin-pages/class-setup-wizard-admin-page.php (1)
inc/installers/class-base-installer.php (2)
all_done(48-59)get_steps(36-39)
inc/class-wp-ultimo.php (1)
inc/class-credits.php (1)
Credits(22-231)
🪛 GitHub Actions: Unit & Integration Tests
composer.json
[warning] 1-1: Lock file is not up to date with composer.json. It is recommended to run 'composer update'.
inc/class-wp-ultimo.php
[error] 141-141: Bootstrap error: Class "WP_Ultimo\Installers\Recommended_Plugins_Installer" not found.
🪛 GitHub Check: Code Quality Checks
inc/admin-pages/class-setup-wizard-admin-page.php
[failure] 142-142:
Tabs must be used to indent lines; spaces are not allowed
[failure] 141-141:
Tabs must be used to indent lines; spaces are not allowed
[failure] 140-140:
Tabs must be used to indent lines; spaces are not allowed
[failure] 139-139:
Tabs must be used to indent lines; spaces are not allowed
[failure] 364-364:
Tabs must be used to indent lines; spaces are not allowed
[failure] 492-492:
Tabs must be used to indent lines; spaces are not allowed
[failure] 491-491:
Tabs must be used to indent lines; spaces are not allowed
[failure] 490-490:
Tabs must be used to indent lines; spaces are not allowed
[failure] 489-489:
Tabs must be used to indent lines; spaces are not allowed
🪛 PHPMD (2.15.0)
inc/class-wp-ultimo.php
631-631: Avoid unused parameters such as '$class_name'. (undefined)
(UnusedFormalParameter)
⏰ 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). (1)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (3)
inc/class-scripts.php (1)
223-228: Init-scoped AJAX endpoint looks good
Passing'init'towu_ajax_url()ensures the localized endpoint includes the newwu_whencontext while leaving nonce generation intact. Nicely done.inc/compat/class-edit-users-compat.php (1)
143-143: Correct escaping helper
Switching toesc_html_e()removes the fatal typo and keeps the header string safely escaped.inc/class-whitelabel.php (1)
68-133: Precomputing rename map on init is solid
Running the rename setup oninit(priority 1) and only wiringgettextwhen replacements exist avoids the previous lazy-init race and keeps the allowed-domain guard nice and tight. Looks great.
| add_action('wu_handle_ajax_installers', [Core_Installer::get_instance(), 'handle'], 10, 3); | ||
| add_action('wu_handle_ajax_installers', [Default_Content_Installer::get_instance(), 'handle'], 10, 3); | ||
| add_action('wu_handle_ajax_installers', [Recommended_Plugins_Installer::get_instance(), 'handle'], 10, 3); | ||
| add_action('wu_handle_ajax_installers', [Migrator::get_instance(), 'handle'], 10, 3); | ||
|
|
There was a problem hiding this comment.
Missing installer class breaks setup wizard
Recommended_Plugins_Installer::get_instance() is invoked here, but the class is not present, so autoloading fails. This is exactly what the CI bootstrap error reports (Class "WP_Ultimo\Installers\Recommended_Plugins_Installer" not found). Please add the new installer implementation (or guard these calls) so the wizard can load.
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 142-142:
Tabs must be used to indent lines; spaces are not allowed
[failure] 141-141:
Tabs must be used to indent lines; spaces are not allowed
[failure] 140-140:
Tabs must be used to indent lines; spaces are not allowed
[failure] 139-139:
Tabs must be used to indent lines; spaces are not allowed
🤖 Prompt for AI Agents
inc/admin-pages/class-setup-wizard-admin-page.php around lines 139 to 143: the
code calls Recommended_Plugins_Installer::get_instance() but that class is
missing causing autoload/CI failures; either implement the
Recommended_Plugins_Installer class in the WP_Ultimo\Installers namespace (with
get_instance() and handle() methods) and ensure it is autoloaded, or guard the
add_action call with a
class_exists('WP_Ultimo\\Installers\\Recommended_Plugins_Installer') check (or
require the file) so the hook is only registered when the class is available.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
inc/class-wp-ultimo.php (2)
34-42: Version constant not bumped (PR claims 2.4.5).Update the constant to avoid version mismatch across UI, headers, and caches.
- const VERSION = '2.4.4'; + const VERSION = '2.4.5';
138-146: Critical: Missing Recommended_Plugins_Installer class file
Setup_Wizard_Admin_Page referencesWP_Ultimo\Installers\Recommended_Plugins_Installer(inc/admin-pages/class-setup-wizard-admin-page.php) but no corresponding class file was found and composer.json lacks a PSR-4 mapping for it.
- Add
WP_Ultimo\Installers\Recommended_Plugins_Installer(e.g. create inc/installers/class-recommended-plugins-installer.php) and update composer.json autoload.psr-4.- Or guard all usages of
Recommended_Plugins_Installer::get_instance()withclass_exists()to prevent fatal errors.
🧹 Nitpick comments (4)
phpstan.neon.dist (2)
5-8: Good addition: WordPress bootstrap and Action Scheduler scan.These help PHPStan resolve WP symbols and AS types. Minor: align list indent under scanDirectories for readability.
Apply:
bootstrapFiles: - vendor/szepeviktor/phpstan-wordpress/bootstrap.php scanDirectories: - - vendor/woocommerce/action-scheduler + - vendor/woocommerce/action-scheduler
18-19: Regex is too specific; relax quotes to match PHPStan message reliably.Current pattern assumes double-quotes inside the error text. Make it robust regardless of quoting.
- message: '#Path in require_once\(\) "\./wp-admin/includes/.*" is not a file or it does not exist\.#' + message: '#Path in require_once\(\)\s+[\'"]?\./wp-admin/includes/.*[\'"]?\s+is not a file or it does not exist\.#'inc/class-wp-ultimo.php (1)
629-634: Silence PHPMD unused parameter; avoid arrow fn here.PHPMD flags $class_name as unused. Switch to a closure and unset the param.
- add_filter( - 'action_scheduler_lock_class', - // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter - fn ($class_name) => \WP_Ultimo\Compat\ActionScheduler_OptionLock_UM::class - ); + add_filter( + 'action_scheduler_lock_class', + /** + * @param string $class_name Provided by Action Scheduler; unused here. + */ + function ($class_name) { + // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter + unset($class_name); + return \WP_Ultimo\Compat\ActionScheduler_OptionLock_UM::class; + } + );inc/compat/class-actionscheduler-optionlock-um.php (1)
25-27: Doc nit: option is not autoloaded.The comment says “autoloaded option” but insert uses autoload 'no'. Tweak wording.
- * Using an autoloaded option avoids running database queries or other resource intensive tasks + * Using an option avoids running database queries or other resource intensive tasks
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.githooks/commit-msg(0 hunks)inc/class-wp-ultimo.php(4 hunks)inc/compat/class-actionscheduler-optionlock-um.php(1 hunks)phpstan.neon.dist(1 hunks)
💤 Files with no reviewable changes (1)
- .githooks/commit-msg
🧰 Additional context used
🧬 Code graph analysis (2)
inc/class-wp-ultimo.php (2)
inc/class-credits.php (1)
Credits(22-231)inc/compat/class-actionscheduler-optionlock-um.php (1)
ActionScheduler_OptionLock_UM(20-144)
inc/compat/class-actionscheduler-optionlock-um.php (1)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-937)
🪛 PHPMD (2.15.0)
inc/class-wp-ultimo.php
632-632: Avoid unused parameters such as '$class_name'. (undefined)
(UnusedFormalParameter)
🪛 GitHub Actions: Unit & Integration Tests
inc/class-wp-ultimo.php
[error] 141-141: WP_Ultimo: Class 'WP_Ultimo\Installers\Recommended_Plugins_Installer' not found during setup.
🔇 Additional comments (2)
inc/class-wp-ultimo.php (1)
566-570: LGTM: Network Usage Columns loader.Non-invasive addition; singleton init pattern matches surrounding code.
inc/compat/class-actionscheduler-optionlock-um.php (1)
31-63: Concurrency guard mirrors upstream; looks correct.Null-check insert and compare-and-swap style update prevent stomping. Casting DB writes to bool is fine.
| /* | ||
| * Optional Footer Credits (opt-in, defaults OFF) | ||
| */ | ||
| \WP_Ultimo\Credits::get_instance(); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
LGTM: Optional Credits init; confirm autoload.
Looks safe (opt‑in, hooks self). Verify autoload maps WP_Ultimo\Credits to inc/class-credits.php so the singleton is resolvable.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=php 'namespace\s+WP_Ultimo\b.*class\s+Credits\b' -C1
jq -r '.autoload."psr-4"' composer.jsonLength of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== Inspect class-credits.php for namespace and class ====="
rg -n --type=php 'namespace\s+WP_Ultimo\b' -C2 inc/class-credits.php
rg -n --type=php 'class\s+Credits\b' -C2 inc/class-credits.php
echo
echo "===== PSR-4 autoload in composer.json ====="
jq '.autoload["psr-4"]' composer.jsonLength of output: 296
Add PSR-4 autoload for WP_Ultimo namespace
composer.json has no autoload.psr-4 entry, so \WP_Ultimo\Credits won’t be found at runtime. Add e.g.
"autoload": {
"psr-4": {
"WP_Ultimo\\": "inc/"
}
}or otherwise ensure inc/class-credits.php is loaded before calling Credits::get_instance().
🤖 Prompt for AI Agents
In inc/class-wp-ultimo.php around lines 605-609 the code calls
\WP_Ultimo\Credits::get_instance() but there is no PSR-4 autoload mapping for
the WP_Ultimo namespace so the class may not be found at runtime; fix by adding
an "autoload.psr-4" entry in composer.json mapping "WP_Ultimo\\" to "inc/" and
run composer dump-autoload, or alternatively ensure the legacy file
inc/class-credits.php is required/loaded before this call (e.g. require_once or
an include loader) so Credits is defined prior to invoking get_instance().
| bootstrapFiles: | ||
| - vendor/szepeviktor/phpstan-wordpress/bootstrap.php |
There was a problem hiding this comment.
🧩 Analysis chain
Verify dev-dependency presence for the bootstrap path.
Ensure composer requires szepeviktor/phpstan-wordpress so the bootstrap file exists on CI.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check composer dev dependency and that the bootstrap file exists after install.
rg -n '"szepeviktor/phpstan-wordpress"' composer.json -C2 || true
jq -r '.["require-dev"] // {}' composer.json | rg -n 'szepeviktor/phpstan-wordpress' || true
test -f vendor/szepeviktor/phpstan-wordpress/bootstrap.php && echo "Bootstrap OK" || echo "Missing bootstrap file"Length of output: 292
Make bootstrap path available on CI
szepeviktor/phpstan-wordpressis only in require-dev and CI installs without dev dependencies by default, sovendor/szepeviktor/phpstan-wordpress/bootstrap.phpis missing. Move it to composer.json require or enable installing dev dependencies on CI.
🤖 Prompt for AI Agents
phpstan.neon.dist around lines 5-6: the bootstrapFiles entry points to
vendor/szepeviktor/phpstan-wordpress/bootstrap.php which lives in require-dev
and is absent on CI; fix by either (A) moving the package into composer.json
"require" (add package to require, run composer update, commit composer.json and
composer.lock) so the file exists in production installs, or (B) change the CI
composer install step to include dev dependencies (remove --no-dev or use
composer install --with-dev) so the vendor path is present during CI runs; apply
one of these fixes and re-run CI.
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
inc/checkout/class-checkout.php (1)
2089-2098: Bug: process_checkout now returns false on gateway errors; caller only treats WP_Error as failuremaybe_process_checkout checks is_wp_error($results) to redirect with error context. Returning false leaves the user on the page with no redirect/feedback. Return a WP_Error (like process_order) and remove $this->errors usage here.
Suggested fix:
- } elseif ($gateway->get_id() === 'free') { - $this->errors = new \WP_Error('no-gateway', __('Payment gateway not registered.', 'ultimate-multisite')); - - return false; - } - - if ( ! $gateway) { - $this->errors = new \WP_Error('no-gateway', __('Payment gateway not registered.', 'ultimate-multisite')); - - return false; - } + } elseif ($gateway->get_id() === 'free') { + return new \WP_Error('no-gateway', __('Payment gateway not registered.', 'ultimate-multisite')); + } + + if ( ! $gateway) { + return new \WP_Error('no-gateway', __('Payment gateway not registered.', 'ultimate-multisite')); + }inc/checkout/signup-fields/class-signup-field-site-url.php (1)
270-276: Update the description to match the new implementation.The
auto_generate_site_urlfield value was changed from'username'to'site_title', but the description on line 173 still says "based on the username of the customer." This is now inconsistent with the implementation.Apply this diff to update the description:
- 'desc' => __('Check this option to auto-generate this field based on the username of the customer.', 'ultimate-multisite'), + 'desc' => __('Check this option to auto-generate this field based on the site title.', 'ultimate-multisite'),inc/checkout/signup-fields/class-signup-field-site-title.php (1)
165-174: Update the description to reflect the new behavior.Similar to the site URL field, the description on line 168 states the field is "auto-generate[d] based on the username of the customer," but the implementation has changed. Given the value is now
'autogenerate', the description should be updated to match the actual generation logic.Apply this diff if the generation is now based on a different source:
- 'desc' => __('Check this option to auto-generate this field based on the username of the customer.', 'ultimate-multisite'), + 'desc' => __('Check this option to auto-generate the site title automatically.', 'ultimate-multisite'),inc/installers/class-base-installer.php (1)
74-102: Propagate installer status; don’t return void from a filter handlerapply_filters('wu_handle_ajax_installers', ...) relies on return values. Void returns mask failures and cause always-success JSON.
- * @return void + * @return bool|\WP_Error @@ - public function handle($status, $installer, $wizard) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter + public function handle($status, $installer, $wizard) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter @@ - if ( ! is_callable($callable)) { - return; - } + if ( ! is_callable($callable)) { + return $status; + } @@ - } catch (\Throwable $e) { + } catch (\Throwable $e) { $wpdb->query('ROLLBACK'); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching - wu_log_add(\WP_Ultimo::LOG_HANDLE, $e->getMessage(), LogLevel::ERROR); - return; + wu_log_add(\WP_Ultimo::LOG_HANDLE, $e->getMessage(), LogLevel::ERROR); + return new \WP_Error('installer_failed', $e->getMessage()); } - $wpdb->query('COMMIT'); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $wpdb->query('COMMIT'); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + return $status;inc/admin-pages/class-setup-wizard-admin-page.php (1)
259-266: Harden status handling for installer chainDefensive guard: treat null/false as errors so void-return handlers don’t yield false success. Keeps the wizard robust even if a handler forgets to return.
- $status = apply_filters('wu_handle_ajax_installers', true, $installer, $this); - - if (is_wp_error($status)) { + $status = apply_filters('wu_handle_ajax_installers', true, $installer, $this); + if ($status === null || $status === false) { + $status = new \WP_Error('installer-missing-return', __('Installer did not return a status.', 'ultimate-multisite')); + } + if (is_wp_error($status)) { wp_send_json_error($status); }
♻️ Duplicate comments (3)
phpstan.neon.dist (1)
5-6: Bootstrap file availability issue persists.This issue was already flagged in a previous review: the bootstrap file from
szepeviktor/phpstan-wordpresslives in require-dev and won't be available on CI unless dev dependencies are installed.inc/class-wp-ultimo.php (1)
613-616: Autoload concern for Credits class.The initialization of the optional footer credits component is appropriate. However, a previous review has already flagged the missing PSR-4 autoload configuration for the
WP_Ultimonamespace. Ensure the autoload issue is addressed as noted in the earlier review.inc/admin-pages/class-setup-wizard-admin-page.php (1)
490-502: Convert indentation to tabs (WP coding standards)This block uses spaces; CI previously failed on this. Replace leading spaces with tabs.
- $sections['recommended-plugins'] = [ - 'title' => __('Recommended Plugins', 'ultimate-multisite'), - 'description' => __('Optionally install helpful plugins. We will install them one by one and report progress.', 'ultimate-multisite'), - 'next_label' => Recommended_Plugins_Installer::get_instance()->all_done() ? __('Go to the Next Step →', 'ultimate-multisite') : __('Install', 'ultimate-multisite'), - 'disable_next' => true, - 'fields' => [ - 'plugins' => [ - 'type' => 'note', - 'desc' => fn() => $this->render_installation_steps(Recommended_Plugins_Installer::get_instance()->get_steps()), - ], - ], - ]; + $sections['recommended-plugins'] = [ + 'title' => __('Recommended Plugins', 'ultimate-multisite'), + 'description' => __('Optionally install helpful plugins. We will install them one by one and report progress.', 'ultimate-multisite'), + 'next_label' => Recommended_Plugins_Installer::get_instance()->all_done() ? __('Go to the Next Step →', 'ultimate-multisite') : __('Install', 'ultimate-multisite'), + 'disable_next' => true, + 'fields' => [ + 'plugins' => [ + 'type' => 'note', + 'desc' => fn() => $this->render_installation_steps(Recommended_Plugins_Installer::get_instance()->get_steps()), + ], + ], + ];
🧹 Nitpick comments (22)
phpstan.neon.dist (1)
17-19: Consider using WordPress constants for require_once paths.This suppression rule correctly handles the fact that PHPStan can't resolve WordPress core paths at analysis time. However, in the actual code, ensure that
require_oncestatements use proper WordPress path constants (e.g.,ABSPATH . 'wp-admin/includes/...') rather than relative paths to avoid runtime errors.inc/checkout/signup-fields/class-signup-field-pricing-table.php (1)
282-288: Optional: declare closure static and explicit return type.
Since the closure doesn’t use $this or return a value, marking it static and using a void return can aid static analysis with zero runtime impact.- $desc = function () use ($template_attributes, $template_class) { + $desc = static function () use ($template_attributes, $template_class): void {views/checkout/templates/template-selection/clean.php (1)
105-105: Use strict equality (===) for template_id comparison in clean.php. Replace==with===in the:classbinding (e.g. at line 105) to make the comparison explicit and avoid implicit coercion.inc/checkout/class-checkout.php (2)
2128-2130: Behavior change: returning true when gateway defersPreviously “simply return;” implied null. If any hooks/consumers check for strict null, this may be observable. Consider keeping it as a bare return for backward compatibility, or confirm no callers rely on a null here.
1224-1239: Auto‑generation: solid additions; minor nits
- Good fallbacks from email/username and early uniqueness for 'autogenerate' site_url.
- Consider normalizing auto-generated $site_title to meet your min length rule (min:4) when it’s derived from username (e.g., very short handles). Optional guard:
if (strlen((string)$site_title) < 4) { $site_title .= ' Site'; }inc/checkout/signup-fields/field-templates/period-selection/class-legacy-period-selection-field-template.php (3)
78-81: Normalize legacy version string and add a return typeSmall copy nit and consistency with other methods using typed returns.
- public function get_description() { - - return __('Implementation of the layout that shipped with WP Ultimo < 1.10.X.', 'ultimate-multisite'); + public function get_description(): string { + return __('Implementation of the layout that shipped with WP Ultimo < 1.10.x.', 'ultimate-multisite'); }
65-68: Add return type to get_title for consistencyOther methods in this class declare return types; mirror that here.
- public function get_title() { + public function get_title(): string { return __('Legacy', 'ultimate-multisite'); }
3-4: Docblock title mismatch with class nameBoth headers say “Template Selection Clean” but the class is “Legacy…”. Rename for clarity.
- * Template Selection Clean + * Legacy Period SelectionAlso applies to: 18-21
scripts/translate.php (1)
1-318: Consider addressing static analysis hints for documentation.The static analysis tools flagged a few documentation issues that could improve code maintainability:
- Missing
@vartags for member variable comments (lines 16, 55)- Missing doc comment for
__construct()method (line 98)- Blank line after class comment (line 9)
These are minor quality improvements that can enhance IDE support and developer experience.
views/base/responsive-table-row.php (1)
108-116: Add callable support to second_row items.First_row’s URL and non-URL branches guard against callables (via is_callable) and invoke them; second_row always echoes the value. In views/base/responsive-table-row.php (around lines 113–125), wrap the $item['value'] output in an is_callable check—calling the value for callables and falling back to wp_kses()—to match first_row’s behavior.
inc/admin-pages/class-base-admin-page.php (1)
556-579: Switching to admin_body_class filter is correct; consider de-duping classesUsing add_filter('admin_body_class', ...) and returning the augmented string is the correct approach. Minor nit: folded may be appended twice (fold_menu + remove_frame). Consider building an array and joining to avoid duplicates.
- add_filter( - 'admin_body_class', - function ($classes) { - if ($this->hide_admin_notices) { $classes .= ' wu-hide-admin-notices'; } - if ($this->fold_menu) { $classes .= ' folded'; } - if ($this->remove_frame) { $classes .= ' wu-remove-frame folded'; } - if (is_network_admin()) { $classes .= ' wu-network-admin'; } - return "$classes wu-page-{$this->id} wu-styling hover:wu-styling first:wu-styling odd:wu-styling"; - } - ); + add_filter( + 'admin_body_class', + function ($classes) { + $extra = ['wu-page-' . $this->id, 'wu-styling', 'hover:wu-styling', 'first:wu-styling', 'odd:wu-styling']; + if ($this->hide_admin_notices) { $extra[] = 'wu-hide-admin-notices'; } + if ($this->fold_menu) { $extra[] = 'folded'; } + if ($this->remove_frame) { $extra[] = 'wu-remove-frame'; $extra[] = 'folded'; } + if (is_network_admin()) { $extra[] = 'wu-network-admin'; } + return trim($classes . ' ' . implode(' ', array_unique($extra))); + } + );inc/managers/class-limitation-manager.php (2)
65-67: Type-hint return to match doc (and async_switch_theme)async_handle_plugins is documented as returning void but lacks a return type. Align it with async_switch_theme for clarity.
- public function async_handle_plugins($action, $site_id, $plugins, $network_wide = false, $silent = true) { + public function async_handle_plugins($action, $site_id, $plugins, $network_wide = false, $silent = true): void {
99-101: Minor: consistent error handling intentSince deactivate_plugins doesn’t return a WP_Error, consider scoping the is_wp_error($results) check to the 'activate' branch to reflect intent explicitly.
- if (is_wp_error($results)) { - wu_log_add('plugins', $results, LogLevel::ERROR); - } + if ('activate' === $action && is_wp_error($results)) { + wu_log_add(\WP_Ultimo::LOG_HANDLE, $results, LogLevel::ERROR); + }inc/objects/class-limitations.php (1)
240-241: Prefer instanceof for object checksStylistic nit: using instanceof self is more idiomatic and slightly faster than is_a with a class string.
- if (is_a($limitation, self::class)) { + if ($limitation instanceof self) {inc/admin/class-network-usage-columns.php (3)
307-354: Consider performance impact of fetching all site options.Lines 321-346 iterate over all sites (up to
sites_limit) and callget_blog_option()twice per site. For networks approaching the 199-site limit, this could result in ~400 database queries when the transient is cold. Consider using a single query with$wpdbto fetch all active plugins and themes in one pass.Example approach using direct database queries:
// Instead of looping and calling get_blog_option() per site: global $wpdb; // Fetch all active_plugins options in one query $plugins_results = $wpdb->get_results( $wpdb->prepare( "SELECT blog_id, option_value FROM {$wpdb->options} WHERE option_name = 'active_plugins' AND blog_id IN (%s)", implode(',', array_map('intval', $blogs)) ) ); // Fetch all stylesheet options in one query $themes_results = $wpdb->get_results( $wpdb->prepare( "SELECT blog_id, option_value FROM {$wpdb->options} WHERE option_name = 'stylesheet' AND blog_id IN (%s)", implode(',', array_map('intval', $blogs)) ) );This reduces N queries to 2-3 queries total, significantly improving performance on larger networks.
362-364: Address unused parameters flagged by static analysis.The
clear_site_transientmethod accepts$pluginand$network_wideparameters but doesn't use them. While the phpcs:ignore comment suppresses the warning, consider whether you need to add logic that uses these parameters for selective cache invalidation, or remove them if they're not needed.If selective invalidation isn't planned, simplify the signature:
-public function clear_site_transient($plugin, $network_wide = false): void { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed +public function clear_site_transient(): void { delete_site_transient(self::SITE_TRANSIENT_BLOGS_PLUGINS); }And update the hook registrations at lines 63-66 to pass fewer arguments if the action supports it.
39-39: Document defaultwu_sites_column_limitbehavior and guidanceDefault
sites_limitis set to 199, so networks with more sites will see incomplete usage data. Add guidance (e.g., in README or admin UI) explaining how to override this via the existingwu_sites_column_limitfilter.inc/functions/site.php (2)
223-256: Consider internationalization and multibyte character handling.The function uses
strtolower()andsubstr()which are not multibyte-safe. For site titles containing non-ASCII characters (e.g., "Café", "日本"), these functions may produce incorrect results or corrupt multibyte sequences.Use multibyte-safe alternatives:
- $slug = strtolower(html_entity_decode(trim((string) $site_title), ENT_QUOTES, 'UTF-8')); + $slug = mb_strtolower(html_entity_decode(trim((string) $site_title), ENT_QUOTES, 'UTF-8'), 'UTF-8'); // ... (character replacements) // Ensure it starts with a letter (WordPress requirement) - if (! empty($slug) && is_numeric(substr($slug, 0, 1))) { + if (! empty($slug) && is_numeric(mb_substr($slug, 0, 1, 'UTF-8'))) { $slug = 'site' . $slug; }However, note that non-ASCII characters are stripped by the regex on line 243 anyway, so the practical impact is limited. Consider whether you want to support transliteration (e.g., "Café" → "cafe") using a library or WordPress's
remove_accents().
346-377: Infinite loop protection is present but consider logging.The safety net at line 370-373 prevents infinite loops, which is good. However, if this condition is hit, it indicates either a logic error or an unusual collision scenario that should be investigated. Consider logging when the counter exceeds a threshold.
Add logging when the safety net is triggered:
// Safety net to prevent infinite loops if ($counter > 9999) { + if (function_exists('wu_log_event')) { + wu_log_event('site_url_collision_limit', [ + 'base_url' => $base_url, + 'domain' => $domain, + 'counter' => $counter, + ]); + } $site_url = $base_url . wp_rand(10000, 99999); break; }inc/installers/class-recommended-plugins-installer.php (2)
96-106: Remove unused$datato satisfy static analysisIterate keys only.
- foreach ($installed as $file => $data) { + foreach (array_keys($installed) as $file) {
158-166: Avoid localized message matching; check upgrader result (optionally activate)Relying on skin messages is brittle across locales. Check the boolean result and optionally activate the plugin.
- $messages = $upgrader->skin->get_upgrade_messages(); - - if (! in_array($upgrader->strings['process_success'], $messages, true)) { - $error_message = array_pop($messages); - return new \WP_Error('installation-failed', $error_message ?: __('Installation failed.', 'ultimate-multisite')); - } + if ($results === false) { + return new \WP_Error('installation-failed', __('Installation failed.', 'ultimate-multisite')); + } + // Optional: auto-activate common case. + if (function_exists('activate_plugin')) { + $plugin_file = $plugin_slug . '/' . $plugin_slug . '.php'; + if (file_exists(WP_PLUGIN_DIR . '/' . $plugin_file)) { + @activate_plugin($plugin_file); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged + } + }inc/admin-pages/class-base-customer-facing-admin-page.php (1)
400-407: Harden return typing and avoid empty() pitfalls in get_settingsempty($saved) treats '0' as empty; screen_layout could be '0' on some setups. Also normalize types per option to prevent subtle UI bugs.
- public function get_settings($result, $option, $user) { - - $option = wu_replace_dashes($option); - - $saved = wu_get_setting($option); - - return empty($saved) ? $result : $saved; - } + public function get_settings($result, $option, $user) { + $key = wu_replace_dashes($option); + $saved = wu_get_setting($key, null); // null distinguishes from falsy values like 0. + + if ($saved === null) { + return $result; + } + + if (0 === strpos($key, 'screen_layout_')) { + return is_scalar($saved) ? (int) $saved : $result; + } + + if (0 === strpos($key, 'meta_box_order_')) { + return is_array($saved) ? $saved : $result; + } + + return $saved; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
assets/css/admin.css(1 hunks)assets/js/thank-you.js(1 hunks)inc/admin-pages/class-base-admin-page.php(8 hunks)inc/admin-pages/class-base-customer-facing-admin-page.php(2 hunks)inc/admin-pages/class-customer-edit-admin-page.php(3 hunks)inc/admin-pages/class-email-template-customize-admin-page.php(1 hunks)inc/admin-pages/class-product-edit-admin-page.php(1 hunks)inc/admin-pages/class-setup-wizard-admin-page.php(4 hunks)inc/admin-pages/class-site-edit-admin-page.php(1 hunks)inc/admin-pages/class-system-info-admin-page.php(1 hunks)inc/admin/class-network-usage-columns.php(1 hunks)inc/checkout/class-checkout.php(11 hunks)inc/checkout/class-legacy-checkout.php(4 hunks)inc/checkout/signup-fields/class-signup-field-order-bump.php(1 hunks)inc/checkout/signup-fields/class-signup-field-order-summary.php(1 hunks)inc/checkout/signup-fields/class-signup-field-period-selection.php(1 hunks)inc/checkout/signup-fields/class-signup-field-pricing-table.php(1 hunks)inc/checkout/signup-fields/class-signup-field-site-title.php(1 hunks)inc/checkout/signup-fields/class-signup-field-site-url.php(1 hunks)inc/checkout/signup-fields/class-signup-field-steps.php(1 hunks)inc/checkout/signup-fields/field-templates/period-selection/class-legacy-period-selection-field-template.php(1 hunks)inc/class-credits.php(1 hunks)inc/class-hooks.php(2 hunks)inc/class-sunrise.php(1 hunks)inc/class-wp-ultimo.php(5 hunks)inc/functions/limitations.php(2 hunks)inc/functions/site.php(1 hunks)inc/installers/class-base-installer.php(4 hunks)inc/installers/class-migrator.php(2 hunks)inc/installers/class-recommended-plugins-installer.php(1 hunks)inc/integrations/host-providers/class-base-host-provider.php(1 hunks)inc/integrations/host-providers/class-closte-host-provider.php(5 hunks)inc/limitations/class-limit-domain-mapping.php(0 hunks)inc/limitations/class-limit-site-templates.php(0 hunks)inc/limitations/class-limit.php(3 hunks)inc/managers/class-limitation-manager.php(4 hunks)inc/models/class-customer.php(2 hunks)inc/objects/class-limitations.php(3 hunks)inc/ui/class-account-summary-element.php(1 hunks)inc/ui/class-base-element.php(2 hunks)phpstan.neon.dist(1 hunks)scripts/translate.php(1 hunks)views/base/addons.php(2 hunks)views/base/dash.php(1 hunks)views/base/edit.php(6 hunks)views/base/list.php(2 hunks)views/base/responsive-table-row.php(1 hunks)views/base/settings.php(2 hunks)views/checkout/templates/template-selection/clean.php(2 hunks)views/checkout/templates/template-selection/legacy.php(2 hunks)views/checkout/templates/template-selection/minimal.php(2 hunks)views/dashboard-widgets/domain-mapping.php(2 hunks)
💤 Files with no reviewable changes (2)
- inc/limitations/class-limit-site-templates.php
- inc/limitations/class-limit-domain-mapping.php
✅ Files skipped from review due to trivial changes (10)
- inc/checkout/signup-fields/class-signup-field-order-bump.php
- inc/checkout/signup-fields/class-signup-field-steps.php
- inc/checkout/signup-fields/class-signup-field-period-selection.php
- inc/checkout/signup-fields/class-signup-field-order-summary.php
- inc/checkout/class-legacy-checkout.php
- inc/integrations/host-providers/class-closte-host-provider.php
- inc/ui/class-account-summary-element.php
- views/base/settings.php
- inc/ui/class-base-element.php
- views/base/edit.php
🚧 Files skipped from review as they are similar to previous changes (1)
- inc/class-credits.php
🧰 Additional context used
🧬 Code graph analysis (13)
inc/class-hooks.php (2)
inc/functions/helper.php (1)
wu_log_add(208-211)inc/class-wp-ultimo.php (1)
WP_Ultimo(23-945)
inc/limitations/class-limit.php (1)
inc/objects/class-limitations.php (3)
__serialize(115-118)to_array(369-371)__unserialize(129-131)
views/dashboard-widgets/domain-mapping.php (2)
inc/models/class-domain.php (1)
is_primary_domain(261-264)inc/functions/markup-helpers.php (1)
wu_tooltip(120-135)
inc/admin-pages/class-system-info-admin-page.php (2)
inc/class-wp-ultimo.php (1)
WP_Ultimo(23-945)inc/class-settings.php (1)
Settings(23-1684)
inc/class-wp-ultimo.php (3)
inc/admin/class-network-usage-columns.php (1)
Network_Usage_Columns(23-365)inc/class-credits.php (1)
Credits(24-306)inc/compat/class-actionscheduler-optionlock-um.php (1)
ActionScheduler_OptionLock_UM(20-144)
views/checkout/templates/template-selection/minimal.php (1)
inc/ui/class-base-element.php (1)
get_id(961-964)
inc/admin-pages/class-setup-wizard-admin-page.php (2)
inc/installers/class-recommended-plugins-installer.php (1)
Recommended_Plugins_Installer(24-167)inc/installers/class-base-installer.php (2)
all_done(50-61)get_steps(38-41)
inc/installers/class-base-installer.php (2)
inc/functions/helper.php (1)
wu_log_add(208-211)inc/class-wp-ultimo.php (1)
WP_Ultimo(23-945)
inc/admin-pages/class-customer-edit-admin-page.php (1)
inc/functions/url.php (1)
wu_network_admin_url(53-60)
views/base/responsive-table-row.php (2)
inc/functions/helper.php (2)
wu_get_isset(66-73)wu_kses_allowed_html(326-589)inc/functions/markup-helpers.php (1)
wu_tooltip_text(144-147)
inc/checkout/class-checkout.php (2)
inc/models/class-customer.php (1)
get_username(258-267)inc/functions/site.php (3)
wu_generate_site_title_from_email(269-332)wu_generate_unique_site_url(346-377)wu_generate_site_url_from_title(223-256)
inc/installers/class-recommended-plugins-installer.php (2)
inc/installers/class-base-installer.php (1)
Base_Installer(22-103)inc/functions/helper.php (1)
wu_log_add(208-211)
inc/managers/class-limitation-manager.php (4)
inc/functions/helper.php (1)
wu_get_main_site_id(81-86)inc/models/interfaces/interface-limitable.php (1)
get_limitations(36-36)inc/models/traits/trait-limitable.php (1)
get_limitations(47-101)inc/limitations/class-limit.php (1)
is_enabled(235-238)
🪛 GitHub Actions: Unit & Integration Tests
inc/class-sunrise.php
[error] 162-162: PHPUnit test suite encountered a fatal error due to a missing dependency file: Failed to require_once the file 'inc/limitations/class-limit-hide-footer-credits.php'.
🪛 GitHub Check: Code Quality Checks
inc/admin-pages/class-customer-edit-admin-page.php
[failure] 110-110:
$_GET['_wpnonce'] not unslashed before sanitization. Use wp_unslash() or similar
🪛 PHPMD (2.15.0)
inc/class-wp-ultimo.php
640-640: Avoid unused parameters such as '$class_name'. (undefined)
(UnusedFormalParameter)
inc/installers/class-recommended-plugins-installer.php
70-70: Avoid unused parameters such as '$status'. (undefined)
(UnusedFormalParameter)
70-70: Avoid unused parameters such as '$wizard'. (undefined)
(UnusedFormalParameter)
99-99: Avoid unused local variables such as '$data'. (undefined)
(UnusedLocalVariable)
inc/admin/class-network-usage-columns.php
171-171: Avoid unused local variables such as '$child_context'. (undefined)
(UnusedLocalVariable)
362-362: Avoid unused parameters such as '$plugin'. (undefined)
(UnusedFormalParameter)
362-362: Avoid unused parameters such as '$network_wide'. (undefined)
(UnusedFormalParameter)
⏰ 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). (2)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
🔇 Additional comments (47)
inc/integrations/host-providers/class-base-host-provider.php (1)
195-195: LGTM! UI refinement aligns with flexbox layout.The change from
<span>to<div>with flexbox utility classes (wu-self-center) is semantically appropriate and improves the layout structure for the "Activated" indicator.inc/limitations/class-limit.php (1)
81-81: LGTM! Good extensibility improvement.The visibility change from
privatetoprotectedenables subclasses to customize the default enabled value, which aligns with the broader refactoring across other limitation modules.inc/admin-pages/class-customer-edit-admin-page.php (1)
485-496: Good security improvement withesc_url().The addition of
esc_url()on line 494 properly escapes the delete URL before embedding it in HTML, preventing potential XSS vulnerabilities. The multi-line formatting ofadd_query_argalso improves readability.phpstan.neon.dist (1)
7-8: No changes needed to scanDirectories path.Action Scheduler is managed via Composer (
woocommerce/action-schedulerin composer.json and composer.lock), sovendor/woocommerce/action-schedulerwill be available after runningcomposer install.inc/checkout/signup-fields/class-signup-field-pricing-table.php (1)
282-288: LGTM: PSR-12-compliant closure spacing; behavior unchanged.views/checkout/templates/template-selection/clean.php (1)
138-138: Dynamic class binding looks good.The Vue binding correctly applies the
wu-selected-template-buttonclass when this template is selected, providing visual feedback. The implementation is consistent with the conditional text rendering below (lines 140-142).views/checkout/templates/template-selection/legacy.php (2)
194-194: Dynamic class binding implementation looks correct.The Vue
:classbinding properly applies the selection state class. Note that this uses loose equality (==), consistent with the pattern in clean.php and the conditional rendering later in this file (line 235).
230-230: Button state binding looks good.The dynamic class application for the button selection state is correctly implemented and aligns with the conditional text rendering (lines 235-237).
views/checkout/templates/template-selection/minimal.php (3)
97-97: Container selection state binding looks correct.The dynamic class binding properly reflects the selection state, consistent with the implementations in clean.php and legacy.php.
120-120: Label styling for selected state looks good.This template adds an additional visual indicator by styling the label element when selected. This is unique to the minimal template and provides extra visual feedback beyond what's in the other template variants.
124-124: Button state class binding implemented correctly.The anchor's selected state styling aligns with the pattern in the other template files. The
@click.prevent=""prevents navigation while the label's radio input (line 122) handles the actual selection viav-model.inc/checkout/class-checkout.php (3)
653-657: Good: consistent early error return for missing/invalid gatewayReturning a WP_Error here aligns with handle_order_submission’s flow (it propagates and triggers rollback/JSON error). LGTM.
1041-1046: Docblocks and param names tidy‑upSwitching @param from $this to $checkout and importing WP_User improves clarity and IDE help. LGTM.
Also applies to: 1095-1101, 1127-1133, 1696-1701, 1820-1824
21-24: Imports match updated annotationsAdding Customer and WP_User imports aligns with the docblocks and avoids FQCN noise. LGTM.
inc/checkout/signup-fields/field-templates/period-selection/class-legacy-period-selection-field-template.php (1)
67-68: No action needed: “ultimate-multisite” is the canonical text domain. Plugin header in ultimate-multisite.php declares Text Domain: ultimate-multisite, and no instances of ‘wp-ultimo’ appear in translation calls—existing usage is correct.inc/admin-pages/class-email-template-customize-admin-page.php (1)
710-725: LGTM! Improved defensive programming.The addition of a fallback
return $default_value;statement ensures that the method always returns a value even when$settingis falsy (empty string, null, false, etc.). This prevents potential undefined return value issues.inc/checkout/signup-fields/class-signup-field-site-url.php (1)
277-282: Downstream handling of ‘autogenerate’ is correctly implemented
The checkout logic ininc/checkout/class-checkout.php(around lines 1234–1238) detects the'autogenerate'sentinel and callswu_generate_unique_site_urlas intended, so no further changes are needed.inc/checkout/signup-fields/class-signup-field-site-title.php (1)
190-202: Auto-generation logic validated
Site titles are generated first (lines 1225–1232) and then used to produce site URLs (lines 1234–1238), matching the hidden-field defaults in the signup classes—no changes needed.inc/admin-pages/class-site-edit-admin-page.php (1)
133-133: Approve hook type change. No instances ofapply_filtersoradd_filterforwu_page_{$this->id}_loadwere found in the codebase, so switching toadd_actionis safe.views/base/addons.php (1)
52-52: LGTM! Documentation improvement.Adding the parameter name
$pageto the PHPDoc annotations improves IDE support and follows PHPDoc best practices.Also applies to: 282-282
inc/models/class-customer.php (2)
18-18: LGTM! Import added for clarity.Adding the explicit
use WP_User;import improves code clarity and IDE support.
133-133: LGTM! Appropriate PHPCS suppression.The phpcs:ignore comment is correctly applied here since
$_useris intentionally public to allow dependency injection for testing/mocking purposes, as documented in the property's docblock.assets/css/admin.css (1)
5360-5370: LGTM! Minor UI/UX improvements.These CSS additions provide useful enhancements:
- Images in addon descriptions will scale properly to fit their container
- Consistent column width for auto-updates in the multisite plugins table
- Better UX with cursor feedback on expandable details/summary elements
assets/js/thank-you.js (1)
72-78: LGTM! Correct AJAX payload format.Switching from JSON to URL-encoded form data is the correct approach for WordPress admin-ajax.php requests. This ensures the
actionand_ajax_nonceparameters are properly received by the server-side handler.views/base/responsive-table-row.php (1)
58-91: LGTM! Good addition of callable value support.The changes properly add support for callable values in the first row rendering, allowing dynamic content generation while maintaining proper escaping with
wp_kses().views/dashboard-widgets/domain-mapping.php (2)
10-10: LGTM: phpcs ignore comment added.The phpcs ignore directive is appropriate for handling the camelCase
$classNamevariable in legacy code.
101-108: Approve: Callable values are supported inwu_responsive_table_row
Verified that theresponsive-table-rowtemplate checksis_callable($item['value'])and invokes closures, so the refactor is safe as-is.views/base/dash.php (1)
47-47: LGTM: Documentation improvement.The PHPDoc update adds the parameter variable name
$page, improving documentation clarity and consistency with PHPDoc standards.views/base/list.php (2)
48-48: LGTM: Documentation improvement.The PHPDoc update adds the parameter variable name
$page, improving documentation clarity.
113-113: LGTM: Documentation improvement.The PHPDoc update adds the parameter variable name
$page, consistent with the earlier change in this file.inc/class-wp-ultimo.php (3)
36-42: LGTM: Log handle constant added.The
LOG_HANDLEconstant provides a consistent identifier for core logging operations.
574-577: LGTM: Network usage columns component added.The initialization of
Network_Usage_Columnsfollows the established singleton pattern and adds useful functionality for tracking plugin/theme usage across sites.
637-641: LGTM: Action Scheduler lock class filter added.The filter correctly overrides the Action Scheduler lock implementation with the custom option-based lock class. The phpcs ignore for the unused parameter is appropriate since the filter signature requires it.
inc/admin-pages/class-system-info-admin-page.php (1)
739-739: LGTM: Singleton pattern fix.The change from instantiating a new
Settingsobject to usingget_instance()correctly respects the singleton pattern implemented by theSettingsclass. This ensures consistent access to the shared settings instance.inc/class-hooks.php (1)
59-60: Consistent log handle — good changeSwitching to WP_Ultimo::LOG_HANDLE standardizes logging context and avoids string drift. LGTM.
Also applies to: 103-104
inc/admin-pages/class-base-admin-page.php (1)
202-205: Docblocks updated with additional params — LGTMThe extra params (page_hook, page_id, page) make the hooks clearer for consumers.
Also applies to: 362-364, 372-374, 387-389, 397-399, 617-618, 643-645
inc/managers/class-limitation-manager.php (1)
485-507: New “Hide Credits” limitation section — wiring looks correctSection state and fields map to $object_model->get_limitations()->hide_credits and v-model allow_hide_credits. Pairs with repository addition in inc/objects/class-limitations.php. LGTM.
inc/objects/class-limitations.php (1)
24-35: Expanded @property-read annotations — nice DX improvementThese help IDEs and static analyzers understand available modules (including hide_credits). LGTM.
inc/admin-pages/class-product-edit-admin-page.php (1)
98-99: Correct hook type
The wu_page_{$this->id}_load hook is fired via do_action; switching to add_action aligns with the emitter and avoids filter return-value confusion. LGTM.inc/admin/class-network-usage-columns.php (3)
68-70: Good early return for non-network-admin context.The early return prevents unnecessary hook registration outside the network admin, which is efficient and correct.
99-101: Verify 'dropins' and 'mustuse' are the only excluded statuses.The exclusion logic only checks for 'dropins' and 'mustuse' plugin types. Confirm whether other plugin types (e.g., 'recently_activated') should also be excluded from the Usage column.
197-228: Efficient parent-child theme lookup with caching.The implementation correctly builds a cache map in O(n) on first access and provides O(1) lookups afterward. The sorting of children lists ensures stable output. Well done!
inc/functions/limitations.php (1)
53-53: Documentation now accurately reflects WP_Error returns.The updated
@returntypes correctly document that both functions can returnWP_Errorwhen the site is not found (lines 68, 107). This improves IDE support and developer understanding.Also applies to: 96-96
inc/functions/site.php (3)
269-332: Robust email-to-title generation with good fallbacks.The function handles edge cases well: validates email format, avoids generic prefixes, filters numeric parts, provides domain fallback, and adds "Site" suffix for short titles. The logic is sound.
346-377: No changes needed for domain_exists usage. Coredomain_exists( $domain, $path )correctly distinguishes subdomain vs subdirectory installs by checking both parameters.
223-256: Potential security issue: preg_replace without error handling.Line 243 uses
preg_replace()without checking for errors. In rare cases (e.g., PCRE backtrack limit exceeded),preg_replace()can returnnull, which would cause subsequent operations to fail silently.Add error handling after the preg_replace call:
// Remove any remaining non-alphanumeric characters $slug = preg_replace('/[^a-z0-9]/', '', $slug); + + if ($slug === null) { + $slug = 'site' . wp_rand(1000, 9999); + return $slug; + }Likely an incorrect or invalid review comment.
inc/admin-pages/class-base-customer-facing-admin-page.php (1)
341-349: Correct use of add_filter for admin_body_classGood fix; returning the modified class string is required. The conditional append is safe and scoped via load-$this->page_hook.
| add_filter("get_user_option_meta-box-order_{$this->page_hook}", [$this, 'get_settings'], 10, 3); | ||
|
|
||
| add_action("get_user_option_screen_layout_{$this->page_hook}", [$this, 'get_settings'], 10, 3); | ||
| add_filter("get_user_option_screen_layout_{$this->page_hook}", [$this, 'get_settings'], 10, 3); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Filters added correctly, but screen layout isn’t persisted and timing may be late
- You now filter both meta-box-order and screen_layout, but save_settings only runs when action=meta-box-order, so screen_layout updates won’t be captured. Also, relying on the request action misses non-AJAX screen option saves.
- Registering these filters inside load-$this->page_hook can be late on some WP versions where screen layout is read during set_current_screen. Please verify they take effect on first load.
Proposed fix: persist both keys based on the meta key itself (not the request action) and ensure it’s for this page hook.
--- a/inc/admin-pages/class-base-customer-facing-admin-page.php
+++ b/inc/admin-pages/class-base-customer-facing-admin-page.php
@@ -420,22 +420,22 @@
public function save_settings($meta_id, $user_id, $meta_key, $_meta_value): void {
-
- if ('meta-box-order' !== wu_request('action')) {
- return;
- }
-
- $is_this_page = str_contains((string) wu_request('page'), $this->id);
-
- if ( ! $is_this_page) {
- return;
- }
+ // Capture both meta-box-order_* and screen_layout_* updates for this page.
+ $relevant = (0 === strpos($meta_key, 'meta-box-order_')) || (0 === strpos($meta_key, 'screen_layout_'));
+ $for_this_page = false !== strpos($meta_key, '_' . $this->page_hook);
+ if ( ! $relevant || ! $for_this_page) {
+ return;
+ }
if ( ! user_can($user_id, $this->edit_capability)) {
return;
}
$meta_key = wu_replace_dashes($meta_key);
wu_save_setting($meta_key, $_meta_value);
}QA check to verify timing: open the target screen with a fresh user and confirm columns/layout reflect the global value on the first render (no reload). If not, register the two add_filter lines earlier (e.g., on current_screen) or unconditionally at init with guard logic inside get_settings.
🏁 Script executed:
#!/bin/bash
# Locate filter registrations
rg -n "'add_filter" inc/admin-pages/class-base-customer-facing-admin-page.php
# Inspect get_settings implementation
rg -nP "function get_settings" -C5 inc/admin-pages/class-base-customer-facing-admin-page.php
# Inspect save_settings implementation
rg -nP "function save_settings" -C5 inc/admin-pages/class-base-customer-facing-admin-page.phpLength of output: 770
🏁 Script executed:
#!/bin/bash
# Show filter registration and surrounding code
sed -n '280,360p' inc/admin-pages/class-base-customer-facing-admin-page.php
# Show get_settings and save_settings functions
sed -n '380,460p' inc/admin-pages/class-base-customer-facing-admin-page.php
# Find add_action hooking save_settings
rg -n "add_action.*save_settings" -C3 inc/admin-pages/class-base-customer-facing-admin-page.phpLength of output: 4089
Persist both meta-box-order and screen_layout in save_settings
save_settings()only runs whenwu_request('action') === 'meta-box-order', so updates toscreen_layout_{page_hook}(which are submitted via the “screen-options” action) never get saved. Instead of guarding on the request action, detect the meta key itself:
--- a/inc/admin-pages/class-base-customer-facing-admin-page.php
+++ b/inc/admin-pages/class-base-customer-facing-admin-page.php
@@ public function save_settings($meta_id, $user_id, $meta_key, $_meta_value): void {
- if ('meta-box-order' !== wu_request('action')) {
- return;
- }
+ // Only persist meta-box-order_* and screen_layout_* for this page hook
+ $pattern = '/^(meta-box-order|screen_layout)_' . preg_quote($this->page_hook, '/') . '$/';
+ if (! preg_match($pattern, $meta_key)) {
+ return;
+ }- This ensures both ordering and layout changes are captured regardless of the form action or AJAX endpoint.
Optional improvements:
- In
get_settings(), replaceempty($saved)with a stricter check (e.g.isset($saved)) if you need to preserve values like0. - Register your
get_user_option_*filters earlier (e.g. oncurrent_screen) if you encounter timing issues with WordPress reading screen options beforeload-{$page_hook}.
📝 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.
| add_filter("get_user_option_meta-box-order_{$this->page_hook}", [$this, 'get_settings'], 10, 3); | |
| add_action("get_user_option_screen_layout_{$this->page_hook}", [$this, 'get_settings'], 10, 3); | |
| add_filter("get_user_option_screen_layout_{$this->page_hook}", [$this, 'get_settings'], 10, 3); | |
| public function save_settings( $meta_id, $user_id, $meta_key, $_meta_value ): void { | |
| // Only persist meta-box-order_* and screen_layout_* for this page hook | |
| $pattern = '/^(meta-box-order|screen_layout)_' | |
| . preg_quote( $this->page_hook, '/' ) | |
| . '$/'; | |
| if ( ! preg_match( $pattern, $meta_key ) ) { | |
| return; | |
| } | |
| // …the rest of the save logic remains unchanged… | |
| update_user_meta( $user_id, $meta_key, $_meta_value ); | |
| } |
🤖 Prompt for AI Agents
inc/admin-pages/class-base-customer-facing-admin-page.php lines 312-315:
save_settings currently only runs when wu_request('action') === 'meta-box-order'
so screen_layout_{page_hook} updates are never persisted; change the
save_settings logic to detect the incoming meta key (inspect the $meta_key or
request param name) and persist both meta-box-order_{page_hook} and
screen_layout_{page_hook} when present rather than guarding on the request
action, and update get_settings to use isset($saved) instead of empty($saved) to
preserve falsy but valid values like 0.
|
|
||
| $meta_key = sanitize_key($_GET['delete_meta_key']); | ||
| $nonce = sanitize_text_field($_GET['_wpnonce']); | ||
| $nonce = sanitize_text_field($_GET['_wpnonce']); |
There was a problem hiding this comment.
Add wp_unslash() before sanitizing the nonce.
Per WordPress coding standards, superglobal array values should be unslashed before sanitization. Without wp_unslash(), nonce verification may fail in environments where WordPress adds slashes.
Apply this diff:
- $nonce = sanitize_text_field($_GET['_wpnonce']);
+ $nonce = sanitize_text_field(wp_unslash($_GET['_wpnonce']));📝 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.
| $nonce = sanitize_text_field($_GET['_wpnonce']); | |
| $nonce = sanitize_text_field(wp_unslash($_GET['_wpnonce'])); |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 110-110:
$_GET['_wpnonce'] not unslashed before sanitization. Use wp_unslash() or similar
🤖 Prompt for AI Agents
In inc/admin-pages/class-customer-edit-admin-page.php around line 110, the nonce
is being sanitized directly from the $_GET superglobal; per WP standards you
must call wp_unslash() first to remove added slashes before sanitization.
Replace the direct sanitize_text_field($_GET['_wpnonce']) usage with
sanitize_text_field( wp_unslash( $_GET['_wpnonce'] ) ) (also ensure the index
exists before using it to avoid notices).
| if ( ! function_exists('is_plugin_active_for_network') ) { | ||
| require_once ABSPATH . '/wp-admin/includes/plugin.php'; | ||
| } |
There was a problem hiding this comment.
Missing capability check for security.
The function requires plugin.php to check network activation status but doesn't verify that the current user has appropriate capabilities to view this information. Network admin pages typically require manage_network_plugins capability.
Consider adding a capability check at the beginning of the method:
public function manage_plugins_custom_column(string $column_name, string $plugin_file, array $plugin_data): void {
+ if ( ! current_user_can('manage_network_plugins') ) {
+ return;
+ }
if ( 'active_blogs' !== $column_name ) {
return;
}📝 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.
| if ( ! function_exists('is_plugin_active_for_network') ) { | |
| require_once ABSPATH . '/wp-admin/includes/plugin.php'; | |
| } | |
| public function manage_plugins_custom_column(string $column_name, string $plugin_file, array $plugin_data): void { | |
| if ( ! current_user_can('manage_network_plugins') ) { | |
| return; | |
| } | |
| if ( 'active_blogs' !== $column_name ) { | |
| return; | |
| } | |
| if ( ! function_exists('is_plugin_active_for_network') ) { | |
| require_once ABSPATH . '/wp-admin/includes/plugin.php'; | |
| } | |
| // …rest of the method… | |
| } |
🤖 Prompt for AI Agents
In inc/admin/class-network-usage-columns.php around lines 121-123, add a
capability check before requiring plugin.php and performing network-related
checks: verify the current user has the network capability (e.g.
current_user_can('manage_network_plugins')) and return early (or skip the
network check) if they do not; only proceed to require ABSPATH .
'/wp-admin/includes/plugin.php' and call is_plugin_active_for_network() when the
capability check passes (and optionally ensure is_multisite() if relevant).
| $child_context = ''; | ||
| if ( $theme_data->parent() ) { | ||
| echo '<br>' . sprintf( | ||
| // Translators: The placeholder will be replaced by the name of the parent theme. | ||
| esc_attr__('This is a child theme of %s.', 'multisite-enhancements'), | ||
| '<strong>' . esc_attr($theme_data->parent()->Name) . '</strong>' | ||
| ); | ||
| } |
There was a problem hiding this comment.
Remove unused variable flagged by static analysis.
Line 171 declares $child_context but never uses it. This appears to be leftover from development.
Apply this diff:
- // Check, if is a child theme and return parent.
- $child_context = '';
if ( $theme_data->parent() ) {Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 PHPMD (2.15.0)
171-171: Avoid unused local variables such as '$child_context'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In inc/admin/class-network-usage-columns.php around lines 171 to 178, the
variable $child_context is declared on line 171 but never used; remove the
unused declaration so no dead variable remains and the subsequent conditional
that outputs the parent theme string remains unchanged.
| private function output_blog_list(array $blogs): void { | ||
|
|
||
| if ( ! $blogs ) { | ||
| // Translators: The plugin is not activated, the string is for each plugin possible. | ||
| echo '<span style="white-space:nowrap">' . esc_html__('Not Activated', 'ultimate-multisite') . '</span>'; | ||
| } else { | ||
| $active_count = count($blogs); | ||
| echo '<details ' . ($active_count > 4 ? '' : 'open') . ' ><summary class="non-breaking">'; | ||
| printf( | ||
| // Translators: The placeholder will be replaced by the count and the toggle link of sites there use that plugin. | ||
| esc_html(_n('Active on %1$d site', 'Active on %1$d sites', $active_count, 'ultimate-multisite')), | ||
| esc_html($active_count), | ||
| ); | ||
| echo '</summary>'; | ||
| echo '<ul>'; | ||
| foreach ($blogs as $blog_id => $blog) { | ||
| // Check the site for archived and deleted. | ||
| $class = ''; | ||
| $hint = ''; | ||
| if ($blog['archived']) { | ||
| $class = 'site-archived'; | ||
| $hint = ', ' . esc_attr__('Archived', 'ultimate-multisite'); | ||
| } | ||
| if ($blog['deleted']) { | ||
| $class = 'site-deleted'; | ||
| $hint .= ', ' . esc_attr__('Deleted', 'ultimate-multisite'); | ||
| } | ||
| echo '<li class="' . esc_attr($class) . '" title="Blog ID: ' . esc_attr($blog_id . $hint) . '">'; | ||
| echo '<span><a href="' . esc_attr(get_admin_url($blog_id)) . 'plugins.php">' | ||
| . esc_html(trim($blog['name']) ?: $blog['path']) . '</a>' . esc_html($hint) . '</span></li>'; | ||
| } | ||
| echo '</ul></details>'; | ||
| } | ||
| } |
There was a problem hiding this comment.
Review security: output escaping is mostly correct, but check esc_attr usage.
Lines 258, 262, 264, 266 use esc_attr() for escaping content that appears in HTML element content (not attributes). Use esc_html() instead for text content and reserve esc_attr() strictly for attribute values.
Apply this diff to correct the escaping:
if ($blog['archived']) {
$class = 'site-archived';
- $hint = ', ' . esc_attr__('Archived', 'ultimate-multisite');
+ $hint = ', ' . esc_html__('Archived', 'ultimate-multisite');
}
if ($blog['deleted']) {
$class = 'site-deleted';
- $hint .= ', ' . esc_attr__('Deleted', 'ultimate-multisite');
+ $hint .= ', ' . esc_html__('Deleted', 'ultimate-multisite');
}
echo '<li class="' . esc_attr($class) . '" title="Blog ID: ' . esc_attr($blog_id . $hint) . '">';
- echo '<span><a href="' . esc_attr(get_admin_url($blog_id)) . 'plugins.php">'
+ echo '<span><a href="' . esc_url(get_admin_url($blog_id)) . 'plugins.php">'
. esc_html(trim($blog['name']) ?: $blog['path']) . '</a>' . esc_html($hint) . '</span></li>';Note: Line 265 should use esc_url() for the href attribute, not esc_attr().
📝 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.
| private function output_blog_list(array $blogs): void { | |
| if ( ! $blogs ) { | |
| // Translators: The plugin is not activated, the string is for each plugin possible. | |
| echo '<span style="white-space:nowrap">' . esc_html__('Not Activated', 'ultimate-multisite') . '</span>'; | |
| } else { | |
| $active_count = count($blogs); | |
| echo '<details ' . ($active_count > 4 ? '' : 'open') . ' ><summary class="non-breaking">'; | |
| printf( | |
| // Translators: The placeholder will be replaced by the count and the toggle link of sites there use that plugin. | |
| esc_html(_n('Active on %1$d site', 'Active on %1$d sites', $active_count, 'ultimate-multisite')), | |
| esc_html($active_count), | |
| ); | |
| echo '</summary>'; | |
| echo '<ul>'; | |
| foreach ($blogs as $blog_id => $blog) { | |
| // Check the site for archived and deleted. | |
| $class = ''; | |
| $hint = ''; | |
| if ($blog['archived']) { | |
| $class = 'site-archived'; | |
| $hint = ', ' . esc_attr__('Archived', 'ultimate-multisite'); | |
| } | |
| if ($blog['deleted']) { | |
| $class = 'site-deleted'; | |
| $hint .= ', ' . esc_attr__('Deleted', 'ultimate-multisite'); | |
| } | |
| echo '<li class="' . esc_attr($class) . '" title="Blog ID: ' . esc_attr($blog_id . $hint) . '">'; | |
| echo '<span><a href="' . esc_attr(get_admin_url($blog_id)) . 'plugins.php">' | |
| . esc_html(trim($blog['name']) ?: $blog['path']) . '</a>' . esc_html($hint) . '</span></li>'; | |
| } | |
| echo '</ul></details>'; | |
| } | |
| } | |
| foreach ($blogs as $blog_id => $blog) { | |
| // Check the site for archived and deleted. | |
| $class = ''; | |
| $hint = ''; | |
| if ($blog['archived']) { | |
| $class = 'site-archived'; | |
| $hint = ', ' . esc_html__('Archived', 'ultimate-multisite'); | |
| } | |
| if ($blog['deleted']) { | |
| $class = 'site-deleted'; | |
| $hint .= ', ' . esc_html__('Deleted', 'ultimate-multisite'); | |
| } | |
| echo '<li class="' . esc_attr($class) . '" title="Blog ID: ' . esc_attr($blog_id . $hint) . '">'; | |
| echo '<span><a href="' . esc_url(get_admin_url($blog_id)) . 'plugins.php">' | |
| . esc_html(trim($blog['name']) ?: $blog['path']) . '</a>' . esc_html($hint) . '</span></li>'; | |
| } |
🤖 Prompt for AI Agents
In inc/admin/class-network-usage-columns.php around lines 237 to 270, the output
uses esc_attr() for values that are rendered as element content and for a URL;
change esc_attr() to esc_html() where the value is displayed inside HTML (lines
~258, ~262, ~264, ~266) and change the esc_attr() used for the plugins.php link
href (line ~265) to esc_url(); keep esc_attr() only for actual attribute values
(e.g., class and title portions that remain attributes), ensuring all visible
text is escaped with esc_html() and the link URL is escaped with esc_url().
| * @return void | ||
| */ | ||
| public function handle($status, $installer, $wizard) { | ||
|
|
There was a problem hiding this comment.
Do not change handle() contract to void; the wizard expects a status
The AJAX flow (setup_install) checks for WP_Error/bool. Switching to void hides failures. Revert the return type in docblock to match behavior.
- * @return void
+ * @return bool|\WP_Error🧰 Tools
🪛 PHPMD (2.15.0)
464-464: Avoid unused parameters such as '$status'. (undefined)
(UnusedFormalParameter)
464-464: Avoid unused parameters such as '$wizard'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In inc/installers/class-migrator.php around lines 462-465, the docblock was
changed to indicate the handle() method returns void but the setup_install AJAX
flow expects a status (WP_Error or bool); revert the docblock return to match
actual behavior (e.g. @return WP_Error|bool or @return mixed) and ensure the
method signature/documentation reflect that it returns the status value rather
than void so callers can detect failures.
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Preserve filter value on early return
Returning void here drops the filter value, leading to false “success” responses. Return the incoming $status.
- return;
+ return $status;📝 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.
| return; | |
| } | |
| return $status; | |
| } |
🤖 Prompt for AI Agents
In inc/installers/class-migrator.php around lines 481 to 483, the early return
currently returns nothing and discards the incoming $status causing false
“success” responses; change the early return to return the incoming $status
(i.e., replace the bare return with return $status;) so the original filter
value is preserved and propagated back to the caller.
| public function handle($status, $installer, $wizard) { | ||
|
|
||
| if (strpos($installer, 'install_plugin_') !== 0) { | ||
| return; // Not ours. | ||
| } | ||
|
|
||
| $plugin_slug = substr($installer, strlen('install_plugin_')); | ||
|
|
||
| try { | ||
| $result = $this->install_wporg_plugin($plugin_slug); | ||
|
|
||
| if (is_wp_error($result)) { | ||
| return; | ||
| } | ||
| } catch (\Throwable $e) { | ||
| wu_log_add(\WP_Ultimo::LOG_HANDLE, $e->getMessage(), LogLevel::ERROR); | ||
| } | ||
| } |
There was a problem hiding this comment.
Return a status from handle() and ignore unused params for PHPCS
Propagate $status/WP_Error so the wizard can react; add PHPCS ignore for unused args.
- public function handle($status, $installer, $wizard) {
+ public function handle($status, $installer, $wizard) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter
@@
- if (strpos($installer, 'install_plugin_') !== 0) {
- return; // Not ours.
+ if (strpos($installer, 'install_plugin_') !== 0) {
+ return $status; // Not ours.
}
@@
- try {
- $result = $this->install_wporg_plugin($plugin_slug);
-
- if (is_wp_error($result)) {
- return;
- }
- } catch (\Throwable $e) {
- wu_log_add(\WP_Ultimo::LOG_HANDLE, $e->getMessage(), LogLevel::ERROR);
- }
+ try {
+ $result = $this->install_wporg_plugin($plugin_slug);
+ if (is_wp_error($result)) {
+ return $result;
+ }
+ return $status;
+ } catch (\Throwable $e) {
+ wu_log_add(\WP_Ultimo::LOG_HANDLE, $e->getMessage(), LogLevel::ERROR);
+ return new \WP_Error('installer-exception', $e->getMessage());
+ }📝 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.
| public function handle($status, $installer, $wizard) { | |
| if (strpos($installer, 'install_plugin_') !== 0) { | |
| return; // Not ours. | |
| } | |
| $plugin_slug = substr($installer, strlen('install_plugin_')); | |
| try { | |
| $result = $this->install_wporg_plugin($plugin_slug); | |
| if (is_wp_error($result)) { | |
| return; | |
| } | |
| } catch (\Throwable $e) { | |
| wu_log_add(\WP_Ultimo::LOG_HANDLE, $e->getMessage(), LogLevel::ERROR); | |
| } | |
| } | |
| public function handle($status, $installer, $wizard) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter | |
| if (strpos($installer, 'install_plugin_') !== 0) { | |
| return $status; // Not ours. | |
| } | |
| $plugin_slug = substr($installer, strlen('install_plugin_')); | |
| try { | |
| $result = $this->install_wporg_plugin($plugin_slug); | |
| if (is_wp_error($result)) { | |
| return $result; | |
| } | |
| return $status; | |
| } catch (\Throwable $e) { | |
| wu_log_add(\WP_Ultimo::LOG_HANDLE, $e->getMessage(), LogLevel::ERROR); | |
| return new \WP_Error('installer-exception', $e->getMessage()); | |
| } | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
70-70: Avoid unused parameters such as '$status'. (undefined)
(UnusedFormalParameter)
70-70: Avoid unused parameters such as '$wizard'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In inc/installers/class-recommended-plugins-installer.php around lines 70 to 87,
update handle() to propagate the status or WP_Error and mark unused parameters
for PHPCS: change the function so it returns either the original $status on
success or the WP_Error returned by install_wporg_plugin(); when
install_wporg_plugin($plugin_slug) yields is_wp_error($result) return $result
immediately; on success return $status; in the catch block log the exception as
before but then return a WP_Error constructed from the exception so the wizard
can react; add PHPCS ignore annotations (e.g. /* phpcs:ignore
Squiz.Commenting.FunctionComment */ or appropriate inline phpcs ignore) or
prefix unused parameter names with $_ to silence unused-parameter warnings for
$installer/$wizard if they remain unused.
| public function __serialize() { | ||
|
|
||
| return serialize($this->to_array()); | ||
| return serialize($this->to_array()); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize | ||
| } |
There was a problem hiding this comment.
Critical: Incorrect __serialize() implementation causes double serialization.
The __serialize() magic method should return an array, not a serialized string. PHP's serialization mechanism automatically handles the array returned by __serialize(). Wrapping to_array() in serialize() creates double serialization, which will break deserialization.
Compare with the parent Limitations class (lines 114-117 in inc/objects/class-limitations.php), which correctly returns the array directly:
public function __serialize() {
return $this->to_array();
}Apply this diff to fix the serialization:
- public function __serialize() {
-
- return serialize($this->to_array()); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize
- }
+ public function __serialize() {
+
+ return $this->to_array();
+ }The docblock at line 98 should also be updated:
/**
* Prepare for serialization.
*
* @since 2.0.0
- * @return string
+ * @return array
*/🤖 Prompt for AI Agents
In inc/limitations/class-limit.php around lines 98 to 103, the __serialize()
method wrongly returns a serialized string (serialize($this->to_array()))
causing double-serialization; change the method to return the array directly
(return $this->to_array();) and remove the phpcs ignore for serialize, and
update the docblock at line 98 to state that __serialize() returns an array
instead of a string.
| public function __unserialize($data) { | ||
|
|
||
| $this->setup(unserialize($data)); | ||
| $this->setup(unserialize($data)); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_unserialize | ||
| } |
There was a problem hiding this comment.
Critical: Incorrect __unserialize() implementation.
The __unserialize() magic method receives an array from PHP's deserialization mechanism, not a serialized string. Calling unserialize() on the array parameter will fail or produce unexpected results.
Compare with the parent Limitations class (lines 128-130 in inc/objects/class-limitations.php), which correctly uses the data directly:
public function __unserialize($modules_data) {
$this->raw_module_data = $modules_data;
}Apply this diff to fix the unserialization:
- public function __unserialize($data) {
-
- $this->setup(unserialize($data)); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_unserialize
- }
+ public function __unserialize($data) {
+
+ $this->setup($data);
+ }🤖 Prompt for AI Agents
In inc/limitations/class-limit.php around lines 113 to 116, the __unserialize()
method incorrectly calls unserialize() on the $data parameter (which is already
an array); replace that call with using the array directly by calling
$this->setup($data) (and remove the phpcs:ignore comment), so the method accepts
the provided array from PHP’s deserialization mechanism rather than trying to
unserialize it again.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
inc/limitations/class-limit-hide-footer-credits.php (2)
55-57: Consider removing redundant enabled check.The
is_enabled()check on line 55 is redundant because the parent class'sallowed()method already performs this check before callingcheck(). The parent only invokescheck()when the module is enabled.Apply this diff to simplify the logic:
public function check($value_to_check, $limit, $type = ''): bool { - if (! $this->is_enabled()) { - return false; - } - // For boolean limits (enabled/disabled to hide credits) if (is_bool($limit)) { return $limit;
53-53: Retain required parameters; optional remove redundantis_enabled()
$value_to_checkand$typeare mandated by the abstractLimit::check($value_to_check, $limit, $type = '')signature (invoked byLimit::allowed()), so PHPMD’s unused-parameter warnings are false positives.- Optional: remove the
is_enabled()guard at the start ofcheck()(line 55) sinceLimit::allowed()already performs that check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
inc/limitations/class-limit-hide-footer-credits.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
inc/limitations/class-limit-hide-footer-credits.php (2)
inc/objects/class-limitations.php (1)
Limitations(36-501)inc/limitations/class-limit.php (1)
Limit(20-346)
🪛 PHPMD (2.15.0)
inc/limitations/class-limit-hide-footer-credits.php
53-53: Avoid unused parameters such as '$value_to_check'. (undefined)
(UnusedFormalParameter)
53-53: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (3)
inc/limitations/class-limit-hide-footer-credits.php (3)
15-22: LGTM! Clear documentation and structure.The class declaration and documentation clearly explain the purpose of this limitation module.
24-38: LGTM! Appropriate default values.The
enabled_default_value = falseis intentional and appropriate, ensuring that the ability to hide footer credits is opt-in rather than enabled by default.
74-80: LGTM! Appropriate default state.The default state correctly initializes both
enabledandlimittofalse, which is appropriate for a boolean limitation that should be opt-in.
1 participant
Summary by CodeRabbit