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 singleton, scheduling, telemetry/error sends), password-strength UI (CSS/JS toggle, rules, strength labels), WooCommerce Subscriptions compat, expanded pre-commit tooling (PHP + lint-staged for JS/CSS), improved template-switch error handling, many model meta‑key constant centralizations, new magic-link UI and subsite menu links, plus assorted UI/localization tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Tracker as "Tracker (WP_Ultimo\\Tracker)" rect rgba(56,138,112,0.5)
participant DB as "WP Options / DB" rect rgba(66,133,244,0.5)
participant Cron as "WP‑Cron" rect rgba(219,68,55,0.5)
participant API as "Telemetry API" rect rgba(244,180,0,0.5)
Tracker->>DB: init() — register hooks, read enable flag
Tracker->>Cron: create_weekly_schedule()
Cron-->>Tracker: trigger weekly maybe_send_tracking_data()
alt tracking enabled & interval elapsed
Tracker->>DB: gather tracking data (env, plugins, usage)
Tracker->>API: post telemetry (async/sync depending on type)
API-->>Tracker: response
Tracker->>DB: update last-send timestamp
end
Note right of API: Error telemetry uses same send pathway with type="error"
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 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; }
| 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.
Replace string literal meta keys with class constants across all model classes. This prevents bugs like the broken 'wu_available_addons' key that was accidentally split across lines, causing product addon saving to fail. Models updated: Product, Customer, Membership, Site, Payment, Discount_Code, Checkout_Form, Broadcast, Email Also fixes empty available_addons handling in Product edit admin page. 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 `@inc/debug/class-debug.php`:
- Around line 71-78: The anchor for "Pages" uses
wu_network_admin_url('wp-ultimo-debug-pages') but fails to echo and escape it,
leaving href empty; update the anchor in class-debug.php to echo esc_url(
wu_network_admin_url('wp-ultimo-debug-pages') ) so the returned URL is printed
and properly escaped (match the pattern used for the other links in this
method).
🧹 Nitpick comments (1)
inc/managers/class-rating-notice-manager.php (1)
77-88: Consider explicit integer cast for timestamp calculation.The
$installation_timestampretrieved from the network option might be stored as a string. While this works due to PHP's type juggling, an explicit cast improves clarity and robustness.♻️ Suggested improvement
protected function should_show_notice(): bool { $installation_timestamp = get_network_option(null, \WP_Ultimo::NETWORK_OPTION_SETUP_FINISHED); if (empty($installation_timestamp)) { return false; } - $days_since_installation = (time() - $installation_timestamp) / DAY_IN_SECONDS; + $days_since_installation = (time() - (int) $installation_timestamp) / DAY_IN_SECONDS; return $days_since_installation >= self::DAYS_BEFORE_NOTICE; }
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@inc/class-tracker.php`:
- Around line 468-519: The local variable $error_message in
customize_fatal_error_message is assigned but never used; remove the unused
assignment to eliminate the PHPMD warning (delete the sprintf(...) assignment
that creates $error_message inside customize_fatal_error_message) and keep the
subsequent prepare_error_data(...) and send_to_api(...) calls unchanged so
behavior is preserved.
In `@inc/tax/class-tax.php`:
- Around line 35-39: The admin page is always registered but the AJAX handlers
for wp_ajax_wu_get_tax_rates and wp_ajax_wu_save_tax_rates are only added when
$this->is_enabled(), causing the UI to break when taxes are disabled; fix by
registering the AJAX handlers unconditionally (move the add_action calls for the
handlers out of the is_enabled() branch) so the methods that serve GET and SAVE
(e.g., serve_taxes_rates_via_ajax and the method bound to
wp_ajax_wu_save_tax_rates) are always hooked, or alternatively add UI gating in
tax-rates.js to disable/avoid AJAX calls when $this->is_enabled() is false.
In `@views/taxes/list.php`:
- Line 9: The view is directly calling wu_get_setting('enable_taxes') which
bypasses the wu_enable_taxes filter used by WP_Ultimo\Tax\Tax::is_enabled(), so
update the logic that sets $taxes_enabled to use the same enablement check as
the runtime (either call Tax::is_enabled() or apply the wu_enable_taxes filter
around the setting) to ensure the UI notice in views/taxes/list.php reflects
actual Tax::is_enabled() behavior.
| */ | ||
| defined('ABSPATH') || exit; | ||
|
|
||
| $taxes_enabled = wu_get_setting('enable_taxes', false); |
There was a problem hiding this comment.
Use the same enablement logic as Tax::is_enabled().
wu_get_setting('enable_taxes') bypasses the wu_enable_taxes filter used in WP_Ultimo\Tax\Tax::is_enabled(), so the notice can disagree with actual runtime enablement. Consider applying the filter (or calling the helper) to keep UI and behavior aligned. (Line 9)
🔧 Suggested tweak
-$taxes_enabled = wu_get_setting('enable_taxes', false);
+$taxes_enabled = apply_filters('wu_enable_taxes', wu_get_setting('enable_taxes', false));📝 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.
| $taxes_enabled = wu_get_setting('enable_taxes', false); | |
| $taxes_enabled = apply_filters('wu_enable_taxes', wu_get_setting('enable_taxes', false)); |
🤖 Prompt for AI Agents
In `@views/taxes/list.php` at line 9, The view is directly calling
wu_get_setting('enable_taxes') which bypasses the wu_enable_taxes filter used by
WP_Ultimo\Tax\Tax::is_enabled(), so update the logic that sets $taxes_enabled to
use the same enablement check as the runtime (either call Tax::is_enabled() or
apply the wu_enable_taxes filter around the setting) to ensure the UI notice in
views/taxes/list.php reflects actual Tax::is_enabled() behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@readme.txt`:
- Around line 243-251: Update the Version [2.4.10] changelog entry in readme.txt
to reflect PR `#323` and fixes `#322`: replace the unrelated
password/telemetry/WooCommerce items with concise bullets describing the actual
fixes — e.g., "Fixed: template filtering by plan", "Fixed: type comparison for
template IDs", "Fixed: JavaScript error handling during template switching", and
"Fixed: typo in [relevant identifier]" — and replace the placeholder release
date `2026-01-XX` with the actual release date before merging.
| Version [2.4.10] - Released on 2026-01-XX | ||
| - New: Configurable minimum password strength setting with Medium, Strong, and Super Strong options. | ||
| - New: Super Strong password requirements include 12+ characters, uppercase, lowercase, numbers, and special characters - compatible with WPMU DEV Defender Pro rules. | ||
| - New: Real-time password requirement hints during checkout with translatable strings. | ||
| - New: Themed password field styling with visibility toggle and color fallbacks for page builders (Elementor, Kadence, Beaver Builder). | ||
| - New: Opt-in anonymous usage tracking to help improve the plugin. | ||
| - New: Rating reminder notice after 30 days of installation. | ||
| - New: WooCommerce Subscriptions compatibility layer for site duplication. | ||
| - Improved: JSON response handling for pending site creation in non-FastCGI environments. |
There was a problem hiding this comment.
Critical inconsistency: Changelog doesn't match PR objectives.
This PR (#323) is intended to fix template switching issues (fixes #322), but the changelog entries describe completely different features:
- PR fixes: Template filtering by plan, type comparison for template IDs, JavaScript error handling, and a typo
- Changelog describes: Password strength settings, telemetry, rating notices, WooCommerce compatibility
The changelog appears to document a different set of changes that aren't part of this PR. Ensure the changelog accurately reflects the template switching fixes and error handling improvements mentioned in the PR objectives.
Note: The placeholder date 2026-01-XX on line 243 should also be updated before release, as previously flagged.
🤖 Prompt for AI Agents
In `@readme.txt` around lines 243 - 251, Update the Version [2.4.10] changelog
entry in readme.txt to reflect PR `#323` and fixes `#322`: replace the unrelated
password/telemetry/WooCommerce items with concise bullets describing the actual
fixes — e.g., "Fixed: template filtering by plan", "Fixed: type comparison for
template IDs", "Fixed: JavaScript error handling during template switching", and
"Fixed: typo in [relevant identifier]" — and replace the placeholder release
date `2026-01-XX` with the actual release date before merging.
🔨 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: |
- Add new Nav_Menu_Subsite_Links class that provides a Subsites meta box in WordPress nav menus for easy subsite navigation - When subsites have mapped domains, links automatically include magic login tokens for seamless cross-domain authentication - Add WP Frontend Admin dashboard site support in magic link validation - Add early return for invalid SSO verify codes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Simplify staging mode detection by checking for wc_subscriptions_siteurl option instead of checking if plugin is active - Add filter to replace spaces with dots in WooCommerce customer usernames to prevent login issues caused by spaces in usernames Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change lock_site hook from 'init' to 'wp' for proper context availability - Add wp_is_rest_endpoint() check to prevent locking REST API endpoints Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Show user-friendly error messages when template switching fails - Handle both API errors and network errors gracefully - Unblock UI and reset state on failure to allow retry Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/ui/class-payment-methods-element.php (1)
193-195: Remove the literal debug output in the block render.Line 195 echoes
"lol", which will surface in the UI and suppress the actual payment methods output. This looks like a debug placeholder and breaks the block’s purpose. Please restore the real render logic (e.g., template rendering) instead.To verify the intended output target, you can locate the existing template or prior output implementation:
#!/bin/bash # Locate payment-methods templates or render targets rg -n "payment-methods" -S rg -n "Payment_Methods_Element" -S fd -a "payment" views
🤖 Fix all issues with AI agents
In `@inc/compat/class-woocommerce-subscriptions-compat.php`:
- Around line 87-97: The option existence check is being performed in the wrong
blog context inside reset_staging_mode; move the
get_option('wc_subscriptions_siteurl') call so it runs after
switch_to_blog($site_id) (and ensure restore_current_blog() is called before any
early returns) so the presence of wc_subscriptions_siteurl is checked on the
target site; update reset_staging_mode to switch_to_blog($site_id) first, then
call get_option, and always restore_current_blog() on exit.
In `@inc/managers/class-site-manager.php`:
- Around line 272-273: The conditional calls wp_is_rest_endpoint() unguarded
which can fatal on WP <6.5; update the condition to check
function_exists('wp_is_rest_endpoint') before calling it (e.g., replace
wp_is_rest_endpoint() with (function_exists('wp_is_rest_endpoint') &&
wp_is_rest_endpoint())) so the early-return in the method in
class-site-manager.php safely skips REST checks on older WP versions.
In `@inc/sso/class-nav-menu-subsite-links.php`:
- Around line 236-268: Move the logic that resolves the active site URL ($site =
wu_get_site($blog_id); $home_url = $site ? $site->get_active_site_url() :
get_home_url($blog_id); $site_domain = wp_parse_url($home_url, PHP_URL_HOST);)
above the early-return checks and replace all early returns that currently use
get_home_url($blog_id) with returning $home_url; specifically update
get_subsite_url_with_magic_link to compute $home_url/site_domain first, then use
$home_url when the user is anonymous or magic links are disabled, and keep the
existing magic-link generation call (Magic_Link::get_instance() and
generate_magic_link) unchanged.
In `@inc/ui/class-magic-link-url-element.php`:
- Around line 351-357: The fallback logic in the anonymous-user branch
incorrectly concatenates an already-absolute $redirect_to onto the site URL
(producing URLs like https://site/https://site/...), so update the block that
sets $url when !$url to detect absolute redirects (e.g., check for a scheme with
parse_url or a leading "http(s)://"); if $redirect_to is absolute, assign $url =
$redirect_to directly, otherwise continue to build $url =
trailingslashit($site->get_active_site_url()) . ltrim($redirect_to, '/') as
before; refer to the variables and helpers $url, $redirect_to,
$site->get_active_site_url(), trailingslashit and ltrim in your change.
♻️ Duplicate comments (1)
inc/class-tracker.php (1)
506-519: Remove unused$error_messagevariable.The
$error_messagevariable defined at line 507 is never used (also flagged by PHPMD). The$error_details['full']is passed directly toprepare_error_data()instead.♻️ Proposed fix
if ($this->is_tracking_enabled() && str_contains($error_file, 'ultimate-multisite')) { - $error_message = sprintf( - '[PHP %s] %s in %s on line %d', - $this->get_error_type_name($error['type'] ?? 0), - $error['message'] ?? 'Unknown error', - $error['file'] ?? 'unknown', - $error['line'] ?? 0 - ); - $error_data = $this->prepare_error_data('fatal', $error_details['full'], \Psr\Log\LogLevel::CRITICAL); // Send synchronously since we're about to die $this->send_to_api($error_data, 'error'); }
🧹 Nitpick comments (2)
inc/ui/class-invoices-element.php (1)
15-18: Stale class docblock references "Checkout Element".The class docblock describes "Checkout Element" but this is the
Invoices_Elementclass. This appears to be a copy-paste artifact.📝 Suggested fix
/** - * Adds the Checkout Element UI to the Admin Panel. + * Adds the Invoices Element UI to the Admin Panel. * * `@since` 2.0.0 */inc/sso/class-nav-menu-subsite-links.php (1)
94-101: Consider pagination or search for large networks.The hard limit of 100 subsites means larger networks won’t see all options in the meta box. A paged list or search/filter would avoid silent truncation.
| protected function get_subsite_url_with_magic_link(int $blog_id): string { | ||
|
|
||
| // Get current user - magic links only work for logged-in users. | ||
| $current_user_id = get_current_user_id(); | ||
|
|
||
| if ( ! $current_user_id) { | ||
| return get_home_url($blog_id); | ||
| } | ||
|
|
||
| // Check if magic links are enabled. | ||
| if ( ! wu_get_setting('enable_magic_links', true)) { | ||
| return get_home_url($blog_id); | ||
| } | ||
|
|
||
| // Get the magic link instance. | ||
| $magic_link = Magic_Link::get_instance(); | ||
|
|
||
| // Get the site's active URL (may include mapped domain). | ||
| $site = wu_get_site($blog_id); | ||
| $home_url = $site ? $site->get_active_site_url() : get_home_url($blog_id); | ||
|
|
||
| // Parse the URL to get the domain. | ||
| $site_domain = wp_parse_url($home_url, PHP_URL_HOST); | ||
|
|
||
| if ( ! $site_domain) { | ||
| return $home_url; | ||
| } | ||
|
|
||
| // Need a magic link - generate one. | ||
| $magic_link_url = $magic_link->generate_magic_link($current_user_id, $blog_id, $home_url); | ||
|
|
||
| return $magic_link_url ?: $home_url; | ||
| } |
There was a problem hiding this comment.
Use the active site URL for anonymous / magic-link-disabled paths.
Returning get_home_url($blog_id) can skip mapped domains, sending users to the internal network domain instead of the active custom domain. Use the same $home_url derived from get_active_site_url() for those early returns.
🐛 Proposed fix
- // Get current user - magic links only work for logged-in users.
- $current_user_id = get_current_user_id();
-
- if ( ! $current_user_id) {
- return get_home_url($blog_id);
- }
-
- // Check if magic links are enabled.
- if ( ! wu_get_setting('enable_magic_links', true)) {
- return get_home_url($blog_id);
- }
-
- // Get the magic link instance.
- $magic_link = Magic_Link::get_instance();
-
- // Get the site's active URL (may include mapped domain).
- $site = wu_get_site($blog_id);
- $home_url = $site ? $site->get_active_site_url() : get_home_url($blog_id);
+ // Get the site's active URL (may include mapped domain).
+ $site = wu_get_site($blog_id);
+ $home_url = $site ? $site->get_active_site_url() : get_home_url($blog_id);
+
+ // Get current user - magic links only work for logged-in users.
+ $current_user_id = get_current_user_id();
+
+ if ( ! $current_user_id) {
+ return $home_url;
+ }
+
+ // Check if magic links are enabled.
+ if ( ! wu_get_setting('enable_magic_links', true)) {
+ return $home_url;
+ }
+
+ // Get the magic link instance.
+ $magic_link = Magic_Link::get_instance();📝 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.
| protected function get_subsite_url_with_magic_link(int $blog_id): string { | |
| // Get current user - magic links only work for logged-in users. | |
| $current_user_id = get_current_user_id(); | |
| if ( ! $current_user_id) { | |
| return get_home_url($blog_id); | |
| } | |
| // Check if magic links are enabled. | |
| if ( ! wu_get_setting('enable_magic_links', true)) { | |
| return get_home_url($blog_id); | |
| } | |
| // Get the magic link instance. | |
| $magic_link = Magic_Link::get_instance(); | |
| // Get the site's active URL (may include mapped domain). | |
| $site = wu_get_site($blog_id); | |
| $home_url = $site ? $site->get_active_site_url() : get_home_url($blog_id); | |
| // Parse the URL to get the domain. | |
| $site_domain = wp_parse_url($home_url, PHP_URL_HOST); | |
| if ( ! $site_domain) { | |
| return $home_url; | |
| } | |
| // Need a magic link - generate one. | |
| $magic_link_url = $magic_link->generate_magic_link($current_user_id, $blog_id, $home_url); | |
| return $magic_link_url ?: $home_url; | |
| } | |
| protected function get_subsite_url_with_magic_link(int $blog_id): string { | |
| // Get the site's active URL (may include mapped domain). | |
| $site = wu_get_site($blog_id); | |
| $home_url = $site ? $site->get_active_site_url() : get_home_url($blog_id); | |
| // Get current user - magic links only work for logged-in users. | |
| $current_user_id = get_current_user_id(); | |
| if ( ! $current_user_id) { | |
| return $home_url; | |
| } | |
| // Check if magic links are enabled. | |
| if ( ! wu_get_setting('enable_magic_links', true)) { | |
| return $home_url; | |
| } | |
| // Get the magic link instance. | |
| $magic_link = Magic_Link::get_instance(); | |
| // Parse the URL to get the domain. | |
| $site_domain = wp_parse_url($home_url, PHP_URL_HOST); | |
| if ( ! $site_domain) { | |
| return $home_url; | |
| } | |
| // Need a magic link - generate one. | |
| $magic_link_url = $magic_link->generate_magic_link($current_user_id, $blog_id, $home_url); | |
| return $magic_link_url ?: $home_url; | |
| } |
🤖 Prompt for AI Agents
In `@inc/sso/class-nav-menu-subsite-links.php` around lines 236 - 268, Move the
logic that resolves the active site URL ($site = wu_get_site($blog_id);
$home_url = $site ? $site->get_active_site_url() : get_home_url($blog_id);
$site_domain = wp_parse_url($home_url, PHP_URL_HOST);) above the early-return
checks and replace all early returns that currently use get_home_url($blog_id)
with returning $home_url; specifically update get_subsite_url_with_magic_link to
compute $home_url/site_domain first, then use $home_url when the user is
anonymous or magic links are disabled, and keep the existing magic-link
generation call (Magic_Link::get_instance() and generate_magic_link) unchanged.
| // Fall back to regular site URL for anonymous users or if magic link fails | ||
| if ( ! $url) { | ||
| $url = $site->get_active_site_url(); | ||
|
|
||
| if ($redirect_to) { | ||
| $url = trailingslashit($url) . ltrim($redirect_to, '/'); | ||
| } |
There was a problem hiding this comment.
Handle absolute redirect targets in fallback URL assembly.
When $redirect_to is already absolute (the output method converts relative paths to absolute), the fallback branch appends it to the site URL, yielding https://site/https://site/.... That breaks links for anonymous users or when magic-link generation fails.
🐛 Proposed fix
- if ($redirect_to) {
- $url = trailingslashit($url) . ltrim($redirect_to, '/');
- }
+ if ($redirect_to) {
+ if (strpos($redirect_to, 'http') === 0) {
+ $url = $redirect_to;
+ } else {
+ $url = trailingslashit($url) . ltrim($redirect_to, '/');
+ }
+ }📝 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.
| // Fall back to regular site URL for anonymous users or if magic link fails | |
| if ( ! $url) { | |
| $url = $site->get_active_site_url(); | |
| if ($redirect_to) { | |
| $url = trailingslashit($url) . ltrim($redirect_to, '/'); | |
| } | |
| // Fall back to regular site URL for anonymous users or if magic link fails | |
| if ( ! $url) { | |
| $url = $site->get_active_site_url(); | |
| if ($redirect_to) { | |
| if (strpos($redirect_to, 'http') === 0) { | |
| $url = $redirect_to; | |
| } else { | |
| $url = trailingslashit($url) . ltrim($redirect_to, '/'); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@inc/ui/class-magic-link-url-element.php` around lines 351 - 357, The fallback
logic in the anonymous-user branch incorrectly concatenates an already-absolute
$redirect_to onto the site URL (producing URLs like
https://site/https://site/...), so update the block that sets $url when !$url to
detect absolute redirects (e.g., check for a scheme with parse_url or a leading
"http(s)://"); if $redirect_to is absolute, assign $url = $redirect_to directly,
otherwise continue to build $url = trailingslashit($site->get_active_site_url())
. ltrim($redirect_to, '/') as before; refer to the variables and helpers $url,
$redirect_to, $site->get_active_site_url(), trailingslashit and ltrim in your
change.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.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: |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.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: |
…by-plan' into fix/322-change-templates-filter-by-plan
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
Summary
Test plan
Fixes #322
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Developer tooling
Settings
✏️ Tip: You can customize this high-level summary in your review settings.