Fix template switching showing too many templates and improve error handling #323
Conversation
- Add admin setting for minimum password strength (Medium, Strong, Super Strong) - Super Strong requires 12+ chars, uppercase, lowercase, numbers, and special characters - Integrate with WPMU DEV Defender Pro password rules when active - Add translatable strings using wp.i18n for password requirement hints - Create dedicated password.css with theme color fallbacks for page builders - Update password field templates to use new shared styles Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove wp.i18n dependency and helper method, read localized strings directly from settings.i18n object passed via wp_localize_script. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…UI improvements - Add Tracker class for anonymous usage data and error reporting (opt-in, disabled by default) - Update Logger to pass log level to wu_log_add action for better error filtering - Add WooCommerce Subscriptions compatibility to prevent staging mode on site duplication - Add Rating Notice Manager for user feedback collection - Add payment status polling and enhance integration JS files - Update setup wizard to show telemetry opt-in checkbox - Update readme.txt with usage tracking documentation - Various UI improvements to settings and thank-you pages Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add backslash to special character regex for Defender Pro compatibility - Add null guards for i18n object in password strength JS - Fix pre-commit hook to only show success when lint-staged runs - Fix plugin slug in rating notice manager review URL - Send JSON response unconditionally in publish_pending_site for non-fastcgi - Remove unused enhance-integration and payment-status-poll JS files - Update changelog and version to 2.4.10 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…andling
- Filter template selection to only show templates allowed by the customer's plan
- Fix type comparison issues when validating template IDs (int vs string)
- Add proper error handling in JavaScript to show errors to users instead of infinite loading
- Fix typo in error message ("allow" -> "allowed")
Fixes #322
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds opt-in telemetry (Tracker), password-strength UI and rule checks, WooCommerce Subscriptions compatibility class, lint-staged pre-commit hooks, improved template-switch error handling, a 30-day rating notice manager, and assorted UI/script/back-end adjustments and fixes. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as "WP_Ultimo::Tracker"
participant DB as "WP Options / DB"
participant Cron as "WP-Cron"
participant API as "Telemetry API"
Plugin->>DB: init() (register hooks, store opts)
Plugin->>Cron: create_weekly_schedule()
Cron-->>Plugin: weekly event triggers maybe_send_tracking_data()
alt tracking enabled & interval elapsed
Plugin->>DB: get_tracking_data() / get_site_hash()
Plugin->>DB: get_environment_data(), get_plugin_data(), get_usage_data()
Plugin->>API: send_to_api_async(data, "track")
API-->>Plugin: async response (success/fail)
Plugin->>DB: update last-send timestamp on success
end
Note over Plugin,API: Error telemetry flow
Plugin->>Plugin: maybe_send_error(handle,message,level)
Plugin->>API: send_to_api_async(error_payload, "error")
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ultimate-multisite.php (1)
7-33: Keep header and docblock versions in sync.
The header is 2.4.10 but the docblock still says 2.4.9, which can confuse tooling/readers.✅ Suggested fix
- * `@version` 2.4.9 + * `@version` 2.4.10
🤖 Fix all issues with AI agents
In `@inc/class-tracker.php`:
- Around line 654-657: The is_wp_error branch in the tracker (the block calling
Logger::add when is_wp_error($response)) can re-trigger telemetry via the
wu_log_add action and maybe_send_error; to fix add a guard to prevent recursion:
modify the error handling so that when Logger::add is called from the tracker
you either (a) call Logger::add with a non-telemetry level (e.g. 'warning'
instead of 'error') or (b) pass/implement a flag/parameter to Logger::add to
suppress firing the wu_log_add action, or (c) implement a static/in-class
reentrancy guard in maybe_send_error (e.g. a private static $sending flag
checked/set before sending) and return early if already sending; update the
is_wp_error($response) branch to use one of these approaches and ensure the
guard symbol (maybe_send_error, Logger::add) is referenced so the change avoids
recursive calls.
In `@inc/managers/class-rating-notice-manager.php`:
- Around line 123-134: The review URL in add_rating_notice uses the wrong slug;
update the $review_url value to use the correct plugin slug "ultimate-multisite"
instead of "developer" (locate the $review_url variable in the add_rating_notice
method and replace
'https://wordpress.org/support/plugin/developer/reviews/#new-post' with
'https://wordpress.org/support/plugin/ultimate-multisite/reviews/#new-post').
In `@inc/models/class-domain.php`:
- Around line 545-560: The hook wu_domain_became_primary currently fires
immediately after scheduling wu_async_remove_old_primary_domains which can leave
listeners seeing multiple primaries; update the do_action call to include the
$old_primary_domains array as an additional parameter
(do_action('wu_domain_became_primary', $this, $this->blog_id, $was_new,
$old_primary_domains)) and update the docblock for wu_domain_became_primary in
class-domain.php to state that removal is scheduled asynchronously and that
$old_primary_domains lists the previously primary domains; alternatively, if you
prefer listeners to only see a single primary, move firing of do_action to after
the async removal completes, but prefer adding the extra $old_primary_domains
parameter and docblock note for backward-compatible behavior.
In `@readme.txt`:
- Line 243: Update the placeholder release date in the version header so it
contains the actual release date instead of "2026-01-XX"; locate the version
line containing "Version [2.4.10] - Released on 2026-01-XX" in readme.txt and
replace the "2026-01-XX" token with the real YYYY-MM-DD release date before
merging.
In `@views/settings/widget-settings-body.php`:
- Line 288: The external link using target="_blank" around the esc_html_e('Learn
more', 'ultimate-multisite') output must include rel="noopener noreferrer" to
prevent window.opener access; update the anchor element (the <a> tag that links
to https://developer.ultimatemultisite.com/privacy-policy/ and calls esc_html_e)
to add rel="noopener noreferrer" while keeping target="_blank".
🧹 Nitpick comments (10)
inc/class-addon-repository.php (2)
96-101: Good defensive check, butexpires_inis not validated.The guard for
access_tokenis a solid improvement. However,$response['expires_in']at line 99 is accessed without validation. If the API returns a malformed response withaccess_tokenbut missingexpires_in, this could trigger a PHP notice/warning.♻️ Suggested improvement
$response = json_decode($body, true); -if ( ! empty($response['access_token'])) { +if ( ! empty($response['access_token']) && isset($response['expires_in'])) { $access_token = $response['access_token']; set_transient('wu-access-token', $response['access_token'], $response['expires_in']); }Alternatively, provide a sensible default for
expires_in:$response = json_decode($body, true); if ( ! empty($response['access_token'])) { $access_token = $response['access_token']; - set_transient('wu-access-token', $response['access_token'], $response['expires_in']); + set_transient('wu-access-token', $response['access_token'], $response['expires_in'] ?? 3600); }
229-233: Inconsistent validation withget_access_token.The
save_access_tokenmethod directly accesses$response['access_token'],$response['expires_in'], and$response['refresh_token']without the same defensive checks added toget_access_token. A malformed API response could cause issues here as well.♻️ Suggested improvement
if (200 === absint($code) && 'OK' === $message) { $response = json_decode($body, true); - - set_transient('wu-access-token', $response['access_token'], $response['expires_in']); - wu_save_option('wu-refresh-token', $response['refresh_token']); + if ( ! empty($response['access_token']) && ! empty($response['refresh_token'])) { + set_transient('wu-access-token', $response['access_token'], $response['expires_in'] ?? 3600); + wu_save_option('wu-refresh-token', $response['refresh_token']); + } else { + wp_admin_notice( + __('Invalid response from UltimateMultisite.com.', 'ultimate-multisite'), + [ + 'type' => 'error', + 'dismissible' => true, + ] + ); + return; + } wp_admin_notice(views/dashboard-widgets/thank-you.php (1)
255-258: Use site-specific alt text for better accessibility.
Static alt text loses context when multiple sites are listed; consider including the site title.♻️ Proposed tweak
- alt="Thumbnail of Site" /> + alt="<?php echo esc_attr(sprintf(__('Thumbnail of %s', 'ultimate-multisite'), $site->get_title())); ?>" />inc/limits/class-site-template-limits.php (1)
180-182: Potential type mismatch in strictin_arraycomparison.The
$template_idis cast to(int)at line 164, but$available_templatesfromget_available_site_templates()may contain string values (array keys are typically strings). The strictin_array(..., true)comparison could fail due to type mismatch.Consider applying the same integer casting pattern used in
maybe_filter_template_selection_options:♻️ Suggested fix
} else { - $available_templates = $limits->site_templates->get_available_site_templates(); + $available_templates = array_map('intval', $limits->site_templates->get_available_site_templates()); return in_array($template_id, $available_templates, true); }inc/compat/class-woocommerce-subscriptions-compat.php (1)
62-75: Suppress or document unused parameters for hook signature compatibility.The
$domainand$was_newparameters are flagged as unused by static analysis, but they're required to match thewu_domain_became_primaryaction signature. Consider adding a suppression annotation or documenting the intent.📝 Suggested documentation
/** * Resets WooCommerce Subscriptions staging mode when a primary domain is set. * * `@since` 2.0.0 * - * `@param` \WP_Ultimo\Models\Domain $domain The domain that became primary. + * `@param` \WP_Ultimo\Models\Domain $domain The domain that became primary (unused, required for hook signature). * `@param` int $blog_id The blog ID of the affected site. - * `@param` bool $was_new Whether this is a newly created domain. + * `@param` bool $was_new Whether this is a newly created domain (unused, required for hook signature). * `@return` void + * + * `@SuppressWarnings`(PHPMD.UnusedFormalParameter) */ public function reset_staging_mode_on_primary_domain_change($domain, int $blog_id, bool $was_new): void {inc/admin-pages/class-email-template-customize-admin-page.php (1)
272-286: Consider adding hide_logo check to custom_logo visibility.The
custom_logofield's visibility condition (line 280) only checksuse_custom_logobut nothide_logo. If a user setsuse_custom_logo = true, then later setshide_logo = true, the custom logo selector would still be visible despite being irrelevant.♻️ Suggested enhancement
'custom_logo' => [ 'type' => 'image', 'stacked' => true, 'title' => __('Custom Logo', 'ultimate-multisite'), 'desc' => __('The custom logo is used in the email header, if HTML emails are used.', 'ultimate-multisite'), 'value' => $custom_logo, 'img' => $custom_logo_url, 'wrapper_html_attr' => [ - 'v-show' => 'require("tab", "header") && require("use_custom_logo", true)', + 'v-show' => 'require("tab", "header") && require("hide_logo", false) && require("use_custom_logo", true)', 'v-cloak' => 1, ],views/broadcast/emails/base.php (1)
17-21: Duplicate ABSPATH check.Line 17 already exits if
ABSPATHis not defined. The check at lines 19-21 is redundant and can be removed.🧹 Suggested fix
defined('ABSPATH') || exit; - -if ( ! defined('ABSPATH')) { - exit; // Exit if accessed directly -}inc/class-tracker.php (2)
607-625: Good PII sanitization, but consider additional patterns.The sanitization covers common PII (paths, URLs, emails, IPs). Consider whether database table names, usernames, or API keys might appear in error messages and need redaction.
463-467: Broad pattern match may capture unrelated errors.The check for
'wu-'in the file path is quite generic and could match files from unrelated plugins (e.g.,wu-something.phpin another plugin). Consider using a more specific pattern like checking for the plugin directory.🛡️ Suggested improvement
-if (strpos($error_file, 'ultimate-multisite') === false && - strpos($error_file, 'wp-multisite-waas') === false && - strpos($error_file, 'wu-') === false) { +$plugin_indicators = ['ultimate-multisite', 'wp-multisite-waas', 'wp-ultimo']; +$is_plugin_error = false; +foreach ($plugin_indicators as $indicator) { + if (strpos($error_file, $indicator) !== false) { + $is_plugin_error = true; + break; + } +} +if (!$is_plugin_error) { return $should_handle; }assets/css/password.css (1)
76-78: Consider documenting the!importantoverride rationale.The
!importantonpadding-rightis likely needed to override form framework styles that may set their own padding. Consider adding a brief comment explaining why this override is necessary for maintainability.Suggested comment
/** * Password input with space for toggle. + * + * Uses !important to ensure space for toggle button + * regardless of form framework padding rules. */ .wu-password-input { padding-right: 40px !important; }
|
|
||
| if (is_wp_error($response)) { | ||
| Logger::add('tracker', 'Failed to send tracking data: ' . $response->get_error_message()); | ||
| } |
There was a problem hiding this comment.
Potential infinite loop when logging tracker failures.
If Logger::add triggers the wu_log_add action (which it does per the relevant code snippets), and logging fails at ERROR level, this could recursively call maybe_send_error, potentially causing an infinite loop or excessive API calls.
Consider adding a guard or using a different log level that won't trigger telemetry.
🛡️ Suggested fix
if (is_wp_error($response)) {
- Logger::add('tracker', 'Failed to send tracking data: ' . $response->get_error_message());
+ // Use INFO level to avoid triggering telemetry recursively
+ Logger::add('tracker', 'Failed to send tracking data: ' . $response->get_error_message(), \Psr\Log\LogLevel::INFO);
}🤖 Prompt for AI Agents
In `@inc/class-tracker.php` around lines 654 - 657, The is_wp_error branch in the
tracker (the block calling Logger::add when is_wp_error($response)) can
re-trigger telemetry via the wu_log_add action and maybe_send_error; to fix add
a guard to prevent recursion: modify the error handling so that when Logger::add
is called from the tracker you either (a) call Logger::add with a non-telemetry
level (e.g. 'warning' instead of 'error') or (b) pass/implement a flag/parameter
to Logger::add to suppress firing the wu_log_add action, or (c) implement a
static/in-class reentrancy guard in maybe_send_error (e.g. a private static
$sending flag checked/set before sending) and return early if already sending;
update the is_wp_error($response) branch to use one of these approaches and
ensure the guard symbol (maybe_send_error, Logger::add) is referenced so the
change avoids recursive calls.
| protected function add_rating_notice(): void { | ||
|
|
||
| $review_url = 'https://wordpress.org/support/plugin/developer/reviews/#new-post'; | ||
|
|
||
| $message = sprintf( | ||
| /* translators: %1$s opening strong tag, %2$s closing strong tag, %3$s review link opening tag, %4$s link closing tag */ | ||
| __('Hey there! You\'ve been using %1$sUltimate Multisite%2$s for a while now. If it\'s been helpful for your network, we\'d really appreciate a quick review on WordPress.org. Your feedback helps other users discover the plugin and motivates us to keep improving it. %3$sLeave a review%4$s', 'ultimate-multisite'), | ||
| '<strong>', | ||
| '</strong>', | ||
| '<a href="' . esc_url($review_url) . '" target="_blank" rel="noopener">', | ||
| ' →</a>' | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the main plugin file and readme
echo "=== Looking for plugin metadata ==="
fd -type f \( -name "*.php" -o -name "readme.txt" -o -name "README.md" \) | head -20
echo ""
echo "=== Searching for plugin URI and text domain ==="
fd -type f -name "*.php" | xargs grep -h "Plugin URI\|Text Domain\|Domain Path" | head -10
echo ""
echo "=== Checking package.json or composer.json ==="
fd -type f \( -name "package.json" -o -name "composer.json" \) -exec cat {} \;
echo ""
echo "=== Checking readme files ==="
fd -type f \( -name "readme.txt" -o -name "README.md" \) -exec cat {} \; 2>/dev/null | head -30Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 775
🏁 Script executed:
#!/bin/bash
echo "=== Listing PHP files in root ==="
ls -la *.php 2>/dev/null || echo "No PHP files in root"
echo ""
echo "=== Looking for main plugin file ==="
fd "\.php$" -d 2 | head -20
echo ""
echo "=== Searching for Plugin URI and Text Domain ==="
find . -maxdepth 3 -name "*.php" -type f 2>/dev/null | xargs grep -h "Plugin URI\|Text Domain\|Domain Path" 2>/dev/null | head -15
echo ""
echo "=== Checking for readme files ==="
find . -maxdepth 2 -name "readme.*" -o -name "README.*" 2>/dev/null | xargs cat 2>/dev/null | head -50
echo ""
echo "=== Repository info from git ==="
git remote -v 2>/dev/null || echo "Not a git repo"
echo ""
echo "=== Checking plugin slug context ==="
rg "wordpress.org/plugin" --max-count 20 2>/dev/nullRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 4352
Review URL contains incorrect plugin slug.
The review URL uses developer which is incorrect. Based on the plugin's text domain and slug convention, it should be ultimate-multisite.
🔧 Suggested fix
- $review_url = 'https://wordpress.org/support/plugin/developer/reviews/#new-post';
+ $review_url = 'https://wordpress.org/support/plugin/ultimate-multisite/reviews/#new-post';🤖 Prompt for AI Agents
In `@inc/managers/class-rating-notice-manager.php` around lines 123 - 134, The
review URL in add_rating_notice uses the wrong slug; update the $review_url
value to use the correct plugin slug "ultimate-multisite" instead of "developer"
(locate the $review_url variable in the add_rating_notice method and replace
'https://wordpress.org/support/plugin/developer/reviews/#new-post' with
'https://wordpress.org/support/plugin/ultimate-multisite/reviews/#new-post').
| do_action('wu_async_remove_old_primary_domains', $old_primary_domains); | ||
|
|
||
| /** | ||
| * Fires when a domain becomes the primary domain for a site. | ||
| * | ||
| * This action is triggered when a domain's primary_domain flag is set to true, | ||
| * either when creating a new primary domain or when updating an existing domain | ||
| * to become primary. | ||
| * | ||
| * @since 2.0.0 | ||
| * | ||
| * @param \WP_Ultimo\Models\Domain $domain The domain that became primary. | ||
| * @param int $blog_id The blog ID of the affected site. | ||
| * @param bool $was_new Whether this is a newly created domain. | ||
| */ | ||
| do_action('wu_domain_became_primary', $this, $this->blog_id, $was_new); |
There was a problem hiding this comment.
Clarify hook timing vs. async removal of old primaries.
wu_domain_became_primary fires right after scheduling wu_async_remove_old_primary_domains, so listeners may still see multiple primaries. Either move the hook to after actual removal completes, or document that removal is pending (and/or pass $old_primary_domains).
📝 Suggested doc/param tweak
- /**
- * Fires when a domain becomes the primary domain for a site.
- *
- * This action is triggered when a domain's primary_domain flag is set to true,
- * either when creating a new primary domain or when updating an existing domain
- * to become primary.
- *
- * `@since` 2.0.0
- *
- * `@param` \WP_Ultimo\Models\Domain $domain The domain that became primary.
- * `@param` int $blog_id The blog ID of the affected site.
- * `@param` bool $was_new Whether this is a newly created domain.
- */
- do_action('wu_domain_became_primary', $this, $this->blog_id, $was_new);
+ /**
+ * Fires when a domain is marked as primary for a site.
+ *
+ * Note: old primary domains are scheduled for async removal and may still
+ * be present at this point.
+ *
+ * `@since` 2.0.0
+ *
+ * `@param` \WP_Ultimo\Models\Domain $domain The domain that became primary.
+ * `@param` int $blog_id The blog ID of the affected site.
+ * `@param` bool $was_new Whether this is a newly created domain.
+ * `@param` array $old_primary_domains IDs scheduled for removal.
+ */
+ do_action('wu_domain_became_primary', $this, $this->blog_id, $was_new, $old_primary_domains);📝 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.
| do_action('wu_async_remove_old_primary_domains', $old_primary_domains); | |
| /** | |
| * Fires when a domain becomes the primary domain for a site. | |
| * | |
| * This action is triggered when a domain's primary_domain flag is set to true, | |
| * either when creating a new primary domain or when updating an existing domain | |
| * to become primary. | |
| * | |
| * @since 2.0.0 | |
| * | |
| * @param \WP_Ultimo\Models\Domain $domain The domain that became primary. | |
| * @param int $blog_id The blog ID of the affected site. | |
| * @param bool $was_new Whether this is a newly created domain. | |
| */ | |
| do_action('wu_domain_became_primary', $this, $this->blog_id, $was_new); | |
| do_action('wu_async_remove_old_primary_domains', $old_primary_domains); | |
| /** | |
| * Fires when a domain is marked as primary for a site. | |
| * | |
| * Note: old primary domains are scheduled for async removal and may still | |
| * be present at this point. | |
| * | |
| * `@since` 2.0.0 | |
| * | |
| * `@param` \WP_Ultimo\Models\Domain $domain The domain that became primary. | |
| * `@param` int $blog_id The blog ID of the affected site. | |
| * `@param` bool $was_new Whether this is a newly created domain. | |
| * `@param` array $old_primary_domains IDs scheduled for removal. | |
| */ | |
| do_action('wu_domain_became_primary', $this, $this->blog_id, $was_new, $old_primary_domains); |
🤖 Prompt for AI Agents
In `@inc/models/class-domain.php` around lines 545 - 560, The hook
wu_domain_became_primary currently fires immediately after scheduling
wu_async_remove_old_primary_domains which can leave listeners seeing multiple
primaries; update the do_action call to include the $old_primary_domains array
as an additional parameter (do_action('wu_domain_became_primary', $this,
$this->blog_id, $was_new, $old_primary_domains)) and update the docblock for
wu_domain_became_primary in class-domain.php to state that removal is scheduled
asynchronously and that $old_primary_domains lists the previously primary
domains; alternatively, if you prefer listeners to only see a single primary,
move firing of do_action to after the async removal completes, but prefer adding
the extra $old_primary_domains parameter and docblock note for
backward-compatible behavior.
|
|
||
| == Changelog == | ||
|
|
||
| Version [2.4.10] - Released on 2026-01-XX |
There was a problem hiding this comment.
Placeholder date should be updated before release.
The version date 2026-01-XX contains a placeholder. Ensure this is updated to the actual release date before merging to main.
🤖 Prompt for AI Agents
In `@readme.txt` at line 243, Update the placeholder release date in the version
header so it contains the actual release date instead of "2026-01-XX"; locate
the version line containing "Version [2.4.10] - Released on 2026-01-XX" in
readme.txt and replace the "2026-01-XX" token with the real YYYY-MM-DD release
date before merging.
| manually report anything. No sensitive data gets collected, only environmental stuff (e.g. if this is | ||
| this is a subdomain network, etc). | ||
| <?php esc_html_e('Allow Ultimate Multisite to collect anonymous usage data and error reports to help us improve the plugin. We collect: PHP version, WordPress version, plugin version, network type, aggregate counts, active gateways, and error logs. We never collect personal data, customer information, or domain names.', 'ultimate-multisite'); ?> | ||
| <a href="https://developer.ultimatemultisite.com/privacy-policy/" target="_blank"><?php esc_html_e('Learn more', 'ultimate-multisite'); ?></a>. |
There was a problem hiding this comment.
Add rel="noopener noreferrer" to external link for security.
External links with target="_blank" should include rel="noopener noreferrer" to prevent the opened page from accessing window.opener.
Proposed fix
- <a href="https://developer.ultimatemultisite.com/privacy-policy/" target="_blank"><?php esc_html_e('Learn more', 'ultimate-multisite'); ?></a>.
+ <a href="https://developer.ultimatemultisite.com/privacy-policy/" target="_blank" rel="noopener noreferrer"><?php esc_html_e('Learn more', 'ultimate-multisite'); ?></a>.📝 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.
| <a href="https://developer.ultimatemultisite.com/privacy-policy/" target="_blank"><?php esc_html_e('Learn more', 'ultimate-multisite'); ?></a>. | |
| <a href="https://developer.ultimatemultisite.com/privacy-policy/" target="_blank" rel="noopener noreferrer"><?php esc_html_e('Learn more', 'ultimate-multisite'); ?></a>. |
🤖 Prompt for AI Agents
In `@views/settings/widget-settings-body.php` at line 288, The external link using
target="_blank" around the esc_html_e('Learn more', 'ultimate-multisite') output
must include rel="noopener noreferrer" to prevent window.opener access; update
the anchor element (the <a> tag that links to
https://developer.ultimatemultisite.com/privacy-policy/ and calls esc_html_e) to
add rel="noopener noreferrer" while keeping target="_blank".
The previous commit replaced the PHP quality checks (PHPCS, PHPStan) with JS/CSS linting instead of adding to them. This restores the original PHP checks and properly integrates the new lint-staged functionality. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.githooks/pre-commit:
- Around line 23-25: The for-loops iterate unquoted STAGED_PHP_FILES and
STAGED_JS_CSS_FILES, causing word-splitting on filenames with whitespace;
replace the current pipeline with NUL-delimited output (use git diff --name-only
--diff-filter=ACM -z) and populate bash arrays (e.g., readarray -d ''
STAGED_PHP_FILES_ARRAY or while IFS= read -r -d '' f; do ...; done) then iterate
using quoted array expansion (for f in "${STAGED_PHP_FILES_ARRAY[@]}"; do ...)
and ensure all expansions of these variables are quoted to safely handle
filenames with spaces or special characters.
| # Get list of staged PHP files | ||
| STAGED_PHP_FILES=$(git diff --cached --name-only --diff-filter=ACM | grep '\.php$' | grep -v '^vendor/' | grep -v '^tests/' || true) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .githooks/pre-commitRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 6114
🏁 Script executed:
git ls-files | awk '/[[:space:]]/'Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
Use quoted variable expansion and bash arrays for safer filename handling.
The unquoted $STAGED_PHP_FILES and $STAGED_JS_CSS_FILES variables in for loops (lines 40, 54, 73, 88, 95) will split on whitespace. While your repository currently has no tracked files with whitespace, this remains a best practice issue for robustness. Consider using NUL-delimited lists with arrays as shown in the proposed fix to handle any filenames safely.
🤖 Prompt for AI Agents
In @.githooks/pre-commit around lines 23 - 25, The for-loops iterate unquoted
STAGED_PHP_FILES and STAGED_JS_CSS_FILES, causing word-splitting on filenames
with whitespace; replace the current pipeline with NUL-delimited output (use git
diff --name-only --diff-filter=ACM -z) and populate bash arrays (e.g., readarray
-d '' STAGED_PHP_FILES_ARRAY or while IFS= read -r -d '' f; do ...; done) then
iterate using quoted array expansion (for f in "${STAGED_PHP_FILES_ARRAY[@]}";
do ...) and ensure all expansions of these variables are quoted to safely handle
filenames with spaces or special characters.
Summary
Test plan
Fixes #322
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Settings
✏️ Tip: You can customize this high-level summary in your review settings.