-
-
Notifications
You must be signed in to change notification settings - Fork 69
Add configurable password strength settings with Super Strong level #321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
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>
📝 WalkthroughWalkthroughThis PR introduces password strength UI with themed styling and validation rules, adds opt-in telemetry tracking with error reporting, implements WooCommerce Subscriptions compatibility for site duplication, adds a 30-day post-installation rating reminder, updates script registration, and refactors pre-commit hooks from PHP-focused to lint-staged-based linting. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Browser
participant Form as Password Input
participant Strength as WU_PasswordStrength
participant Settings as Settings/Defender
participant UI as Strength Meter
User->>Form: Enter password
Form->>Strength: checkStrength(password)
Strength->>Settings: getSettings() + Defender detection
Strength->>Strength: Evaluate strength level
Strength->>Strength: checkPasswordRules(password)
alt enforce_rules enabled
Strength->>Strength: Validate rules, populate failedRules
Strength->>Strength: getRulesHint(failedRules)
end
Strength->>UI: updateUI(strength, label, color)
UI->>User: Display strength indicator & hints
sequenceDiagram
participant Client as Browser
participant Server as PHP Handler
participant FastCGI as FastCGI
Client->>Server: POST publish_pending_site
Server->>Server: Set Content-Type: application/json
Server->>Client: Echo JSON response
Server->>FastCGI: fastcgi_finish_request()
FastCGI->>Client: Close connection
Server->>Server: Continue site updates (background)
sequenceDiagram
participant Admin as Network Admin
participant Tracker as Tracker
participant Cron as WordPress Cron
participant API as Telemetry API
Admin->>Tracker: Update enable_tracking setting
Tracker->>Tracker: maybe_send_initial_data()
Tracker->>API: send_to_api_async(initial_data)
Cron->>Cron: Weekly schedule fires
Cron->>Tracker: maybe_send_tracking_data()
Tracker->>Tracker: Aggregate & sanitize data
Tracker->>API: send_to_api(tracking_data)
sequenceDiagram
participant Dup as Site_Duplicator
participant Hook as WordPress Hooks
participant Compat as WooCommerce_Subscriptions_Compat
participant WCS as WC Subscriptions
Dup->>Dup: Duplicate site
Dup->>Hook: Fire wu_duplicate_site
Hook->>Compat: reset_staging_mode_on_duplication()
Compat->>WCS: Switch & update siteurl
Note over Compat,WCS: Also fires when domain becomes primary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes This PR introduces substantial new infrastructure (Tracker class with 690 lines of telemetry logic, WooCommerce_Subscriptions_Compat with 162 lines, Rating_Notice_Manager with 151 lines) alongside significant password strength validation enhancements (222 new lines with rule enforcement). The changes are heterogeneous—spanning new classes, telemetry hooks, settings, CSS/JS, database models, and script registration—requiring separate reasoning per functional area. Logic density is moderate to high in the Tracker class (API communication, error sanitization, scheduling) and password validation layer (rule checking, hint generation). While individual components are well-modularized, the breadth of touches and interaction patterns between new systems (hooks, cron, telemetry) increases review complexity. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@assets/js/wu-password-strength.js`:
- Around line 351-384: In checkPasswordRules(), the character-class used when
settings.require_special is true omits the backslash; update the regex used in
that branch (the /[!@#$%^&*()_+\-={};:'",.<>?~\[\]\/|`]/ pattern) to include an
escaped backslash (i.e. add \\ inside the class), and ensure you escape it
correctly for JavaScript/any surrounding string context so the require_special
check will match backslashes as a valid special character.
In `@inc/managers/class-membership-manager.php`:
- Around line 108-115: The code only echoes the JSON response when
fastcgi_finish_request() exists, causing no response if it doesn't; move the
header('Content-Type: ...') and echo wp_json_encode(['status'=>'creating-site'])
to run unconditionally before the if-block (so the client always gets the JSON)
and leave fastcgi_finish_request() call inside the existing if
(function_exists('fastcgi_finish_request')) { ... } block; do not use
wp_send_json since it exits, and ensure async_publish_pending_site continues to
run after the unconditional echo.
🧹 Nitpick comments (2)
assets/css/password.css (1)
157-160: Consider using a CSS variable for the "good" state color.The "good" state uses a hardcoded color
#d88a00while other states reference CSS variables (--wu-password-strength-weak,--wu-password-strength-strong). For consistency and easier theming, consider adding a medium/good variable.Proposed change
In the
:rootblock (around line 60):--wu-password-strength-weak: `#dc3232`; - --wu-password-strength-medium: `#f0b849`; + --wu-password-strength-medium: `#d88a00`; --wu-password-strength-strong: `#46b450`;Then at line 159:
`#pass-strength-result.good` { background-color: `#fff8e1`; - color: `#d88a00`; + color: var(--wu-password-strength-medium); }assets/js/wu-password-strength.js (1)
239-279: Consider cachingcheckPasswordRulesresult to avoid duplicate validation.Both
updateUI()(line 266) andupdateValidity()(line 335) callcheckPasswordRules()with the same password. This results in redundant regex evaluations on every keystroke.♻️ Suggested optimization
Move the rules check to
checkStrength()and pass the result to both methods:checkStrength: function() { var pass1 = this.options.pass1.val(); var pass2 = this.options.pass2 ? this.options.pass2.val() : ''; // Reset classes this.options.result.attr('class', 'wu-py-2 wu-px-4 wu-block wu-text-sm wu-border-solid wu-border wu-mt-2'); if (!pass1) { this.options.result.addClass('wu-bg-gray-100 wu-border-gray-200').html(this.getStrengthLabel('empty')); this.setValid(false); return; } // Get disallowed list from WordPress var disallowedList = this.getDisallowedList(); var strength = wp.passwordStrength.meter(pass1, disallowedList, pass2); + + // Check rules once and pass to both methods + var rulesResult = null; + if (this.settings.enforce_rules && strength >= this.options.minStrength && strength !== 5) { + rulesResult = this.checkPasswordRules(pass1); + } - this.updateUI(strength); - this.updateValidity(strength); + this.updateUI(strength, rulesResult); + this.updateValidity(strength, rulesResult); },Also applies to: 324-343
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
assets/css/password.cssassets/css/password.min.cssassets/js/wu-password-reset.jsassets/js/wu-password-strength.jsinc/checkout/class-checkout.phpinc/class-addon-repository.phpinc/class-scripts.phpinc/class-settings.phpinc/class-sunrise.phpinc/managers/class-membership-manager.phpinc/models/class-base-model.phpinc/models/class-membership.phpinc/stuff.phpinc/ui/class-login-form-element.phpviews/admin-pages/fields/field-password.phpviews/checkout/fields/field-password.php
🧰 Additional context used
🧬 Code graph analysis (2)
inc/class-addon-repository.php (1)
inc/models/class-site.php (1)
set_transient(1301-1306)
inc/class-scripts.php (1)
inc/functions/settings.php (1)
wu_get_setting(37-40)
⏰ 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). (3)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (27)
inc/stuff.php (1)
3-4: Credential rotation acknowledged.The encrypted client credentials have been updated. This change appears unrelated to the PR's stated objectives (password strength settings) but the implementation pattern remains consistent.
Consider documenting credential rotations separately or in the PR description to maintain clear change history.
inc/class-addon-repository.php (2)
96-100: Good defensive improvement for token handling.The added guard prevents caching empty or missing access tokens, which could otherwise cause issues when the token response is malformed or the server returns an error. This properly validates the response before persisting.
104-104: Safe fallback return value.The
?: ''ensures a consistent string return type when no valid token is available, preventing potential type errors in callers.inc/class-sunrise.php (1)
164-164: New dependency addition is correct.The
class-domain-stage.phpfile exists at the specified path and the require_once statement follows the established pattern in theload_dependencies()method, using consistent__DIR__relative path resolution.inc/checkout/class-checkout.php (1)
2567-2568: LGTM! Clean separation of password styles.The
wu-passwordstyle is properly registered ininc/class-scripts.phpwithdashiconsas a dependency, exactly as the comment indicates. This change appropriately moves password-specific styles to a dedicated stylesheet (password.css), aligning with the PR's objective.inc/models/class-base-model.php (1)
302-302: Good refactor removing reflection overhead.Replacing
ReflectionObjectwith a directget_columns()call is cleaner and more performant. Theget_columns()method is public on theQuerybase class (inc/database/engine/class-query.php:110) and inherited by all query classes used throughout the codebase.inc/class-settings.php (1)
866-890: LGTM! Well-structured settings addition.The new password strength settings follow the existing patterns in this file correctly. The default value of
'strong'is a sensible security-conscious choice, and the three-tier option structure (medium, strong, super_strong) provides appropriate flexibility for administrators.inc/models/class-membership.php (1)
1937-1940: LGTM! Correct simplification of the async request logic.Removing the obsolete
version_compare(phpversion(), '7.0.16', '<')check is appropriate since PHP 7.0 reached EOL years ago and WordPress now requires PHP 7.4+. The simplified condition correctly setsblocking = falseonly whenfastcgi_finish_requestis unavailable.assets/css/password.css (1)
1-197: LGTM! Well-structured and documented password field styles.The CSS is well-organized with clear documentation, comprehensive theme fallbacks for page builders, and proper accessibility handling via
:focus-visible. The layered CSS variable cascade is a clever solution for supporting multiple theming systems.assets/css/password.min.css (1)
1-1: LGTM! Comprehensive password field styling with good theme compatibility.The CSS custom property fallback cascade smartly handles multiple page builders (Elementor, Kadence, Beaver Builder) and WordPress core themes. The focus states properly use
:focus-visiblefor accessibility. The minified file (2,236 bytes) is correctly generated from the source CSS (4,324 bytes) with proper size reduction from minification.inc/class-scripts.php (4)
161-176: LGTM! Well-structured script registration with proper i18n support.The password strength script is properly registered with
wp-i18ndependency, translations are set, and settings are localized with a sensible merge pattern.
400-401: LGTM! Style registration looks correct.The
wu-passwordstyle is properly registered withdashiconsas a dependency, which is needed for the password toggle visibility icons.
476-589: Well-designed password requirements configuration with comprehensive filter support.The implementation provides good extensibility through multiple filters and properly handles the Defender Pro integration. A few observations:
Line 490: The null coalescing on array access (
$strength_map[ $strength_setting ] ?? 4) is fine in PHP 7+, but consider validating$strength_settingagainst known keys first for clarity.The filter documentation is thorough and follows WordPress standards.
597-621: Robust Defender Pro detection with proper defensive coding.The try/catch block around
wd_di()->get()is a good practice for handling potential issues with external plugin APIs. The multiple guard clauses ensure graceful degradation when Defender is not available.assets/js/wu-password-strength.js (5)
23-55: LGTM! Clean settings retrieval with sensible defaults.The
getSettings()andgetDefaultMinStrength()functions properly handle missing localized data with fallback defaults, ensuring the script works even ifwu_password_strength_settingsis undefined.
68-84: LGTM! Constructor properly initializes state.The constructor correctly initializes settings from the merged configuration and sets up the
failedRulesarray for tracking validation state.
98-104: Good defensive handling for missing DOM elements.Returning early when the strength result element cannot be found prevents errors in edge cases where the expected DOM structure isn't present.
170-182: LGTM! Translation helper with proper fallback.The
__method correctly checks forwp.i18navailability and falls back gracefully, ensuring translations work across different WordPress environments.
289-317: LGTM! Hint generation with i18n support.The
getRulesHint()method properly uses the translation helper for all user-facing strings. The string replacement pattern for the minimum length message works correctly.assets/js/wu-password-reset.js (1)
28-34: LGTM! Simplified initialization uses centralized defaults.Removing the explicit
minStrengthoption allows the password reset form to inherit settings from the admin configuration viawu_minimum_password_strengthfilter, ensuring consistent behavior across all password forms.inc/ui/class-login-form-element.php (2)
305-311: LGTM! Consolidated password asset loading.The change from
dashiconstowu-passwordstyle properly consolidates password-related CSS, andwu-passwordalready includesdashiconsas a dependency (as registered in class-scripts.php line 401).
697-704: Null coalescing prevents undefined variable warnings but may hide errors.The
$rp_key ?? ''and$rp_login ?? ''changes prevent PHP warnings when these variables aren't set. However, if the cookie parsing fails (line 627), the form will render with empty hidden fields, and the user won't see an error until form submission fails server-side.This is acceptable as the existing flow (lines 632-658) already handles invalid/expired reset keys with proper redirects, and the empty string case would be caught by the server-side validation.
views/checkout/fields/field-password.php (2)
26-40: LGTM! Clean template structure with proper CSS class hooks.The updated markup correctly:
- Adds
wu-password-field-containerfor the CSS positioning context- Adds
wu-password-inputclass for input styling- Removes inline styles from the toggle button in favor of CSS classes (
wu-pwd-toggle)This improves maintainability by moving presentation to CSS.
42-48: LGTM! Strength meter wrapper structure.The
wu-password-strength-wrapperclass provides a proper hook for CSS styling, and the simplified inner span structure is cleaner.views/admin-pages/fields/field-password.php (3)
30-31: LGTM!The new container wrapper and input class additions properly support the CSS-based styling approach. Moving from inline styles to class-based positioning is a cleaner pattern that aligns with the new
password.cssstylesheet.
38-43: LGTM!Good cleanup - moving positioning styles to CSS classes improves maintainability. The accessibility attributes (
aria-label,aria-hidden) are correctly preserved.
46-52: LGTM!The simplified conditional and new wrapper class align with the CSS-based theming approach. The
pass-strength-resultID is correctly preserved for JavaScript targeting by the strength meter scripts.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@assets/js/wu-password-strength.js`:
- Around line 212-214: Add a null guard when reading
this.settings.i18n.super_strong to avoid TypeError: change the return to use a
guarded access or fallback, e.g. return (this.settings.i18n &&
this.settings.i18n.super_strong) || /* default string */; or use optional
chaining: return this.settings.i18n?.super_strong || /* default string */;
update the case 'super_strong' return in the same switch where case 5 follows so
behavior matches the guarded checks at lines ~198-200.
- Around line 276-301: getRulesHint can throw when this.settings.i18n is
undefined; add a null guard at the top of getRulesHint (use the existing symbol
name getRulesHint and this.settings.i18n) — for example obtain i18n then if
(!i18n) return this.getStrengthLabel('super_strong'); or alternatively set const
i18n = this.settings.i18n || {} and ensure you handle missing keys with sensible
defaults (e.g. fall back to getStrengthLabel('super_strong') or a default
required string) so accessing i18n.min_length / i18n.required never throws.
♻️ Duplicate comments (1)
assets/js/wu-password-strength.js (1)
359-362: Special character regex is missing the backslash character.The regex
/[!@#$%^&*()_+\-={};:'",.<>?~\[\]\/|]/omits the backslash` from the standard US keyboard special characters documented by Defender Pro. Add an escaped backslash (\\) to the character class.🐛 Proposed fix
// Check for special character (matches Defender Pro's pattern) - if (settings.require_special && ! /[!@#$%^&*()_+\-={};:'",.<>?~\[\]\/|`]/.test(password)) { + if (settings.require_special && ! /[!@#$%^&*()_+\-={};:'",.<>?~\[\]\/|`\\]/.test(password)) { failedRules.push('special'); }
🧹 Nitpick comments (1)
inc/class-scripts.php (1)
603-627: Consider logging Defender detection failures for debugging.The try-catch silently returns
falsewhen Defender integration fails. While this is safe, logging the exception would help diagnose integration issues.🔧 Suggested improvement
try { $settings = wd_di()->get('WP_Defender\Model\Setting\Strong_Password'); if ($settings && method_exists($settings, 'is_active')) { return $settings->is_active(); } } catch (\Exception $e) { // Defender class not available or error occurred. + if (defined('WP_DEBUG') && WP_DEBUG) { + error_log('[WP Ultimo] Defender Strong Password detection failed: ' . $e->getMessage()); + } return false; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/js/wu-password-strength.jsinc/class-scripts.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/class-scripts.php (1)
inc/functions/settings.php (1)
wu_get_setting(37-40)
⏰ 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). (4)
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: PHP 8.3
🔇 Additional comments (7)
inc/class-scripts.php (3)
163-182: LGTM! Well-structured localization with proper i18n.The localized settings are properly merged with password requirements, and all translatable strings use the correct text domain. The
min_lengthplaceholder%dis appropriately designed for client-side replacement.
406-407: LGTM! Appropriate dependency for password field styling.Adding
dashiconsas a dependency is correct for the password visibility toggle icon support.
482-595: LGTM! Well-documented method with comprehensive filter hooks.The
get_password_requirements()method is well-structured with:
- Clear docblocks for each filter
- Proper type casting (
absint(),(bool)) on all return values- Sensible defaults that align with Defender Pro requirements
- Good separation of strength mapping from rule enforcement logic
assets/js/wu-password-strength.js (4)
24-56: LGTM! Clean settings retrieval with sensible defaults.Good defensive programming with the
typeofcheck before accessing global settings, and$.extendproperly merges defaults with localized values.
91-105: LGTM! Good defensive return on missing result element.The early return at line 103 prevents errors when the strength meter element cannot be found, which is appropriate for graceful degradation.
335-368: LGTM! Clean rule validation logic.The
checkPasswordRules()method properly validates each requirement independently and returns a clear result object with both validity status and failed rules for UI feedback.
308-327: LGTM! Proper integration of rule enforcement with strength validation.The
updateValidity()method correctly chains strength check with rules enforcement when enabled, and properly tracks failed rules for external access.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…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>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.githooks/pre-commit:
- Around line 14-19: The script currently always echoes "Pre-commit checks
passed!" even when npx is missing and lint-staged was skipped; modify the logic
so the final success message (the echo "Pre-commit checks passed!") is only
printed when lint-staged actually ran and returned success, and print a distinct
message indicating checks were skipped when npx is not found; locate the branch
that tests for npx and the echo "Pre-commit checks passed!" line in
.githooks/pre-commit and change it to conditionally output success on
lint-staged success and a clear "checks skipped" warning otherwise.
In `@assets/js/enhance-integration.js`:
- Around line 94-101: In populateWebsites, avoid building option HTML with
string concatenation using website.name and website.id (which can contain
malicious content); instead create option elements via DOM APIs (e.g.,
document.createElement or jQuery $('<option>')) and set the option's value and
text via safe setters like option.value / $option.val(…) and option.textContent
/ $option.text(…) so both website.id and website.name are assigned without
injecting raw HTML; replace the current $select.append('<option value="' +
website.id + '">'+ website.name +'</option>') usage with these safe API calls.
- Around line 69-81: The error path assumes response.data exists before
accessing response.data.message, which can throw; update the else branch that
sets $status (used alongside populateWebsites and `#wu_enhance_website_id`) to
safely read the message by checking for response.data and response.data.message
(or falling back to response.message and wu_enhance_data.i18n.websites_failed)
and then set the text and color accordingly so the status always renders even
when response.data is missing.
In `@assets/js/payment-status-poll.js`:
- Around line 146-162: startPolling() can create multiple concurrent intervals
causing runaway polling; guard against that by checking the module-scoped
pollTimer before starting a new interval: if pollTimer is already set, either
return early or clearInterval(pollTimer) first, then set pollTimer =
setInterval(...). Ensure stopPolling() clears the interval and sets pollTimer =
null, and that any early-return path keeps pollTimer state consistent; reference
functions/variables: startPolling, stopPolling, pollTimer, checkPaymentStatus,
ensureStatusElement, config.
- Around line 41-47: Add a complete backend for the AJAX polling: register both
ajax hooks wp_ajax_wu_check_payment_status and
wp_ajax_nopriv_wu_check_payment_status in the Payment Manager class, implement
the handler function (named e.g. handle_wu_check_payment_status) to verify a
localized nonce (sent from JS), validate that the provided payment_hash belongs
to the current user/session before returning JSON status, and exit with proper
WP_Error on failure; enqueue the frontend script payment-status-poll.js during
the Thank You page setup (the existing enqueue function in the Payment Manager
or thank-you setup) and localize the script with a wu_payment_poll object
containing ajaxUrl (admin_url('admin-ajax.php')), paymentHash, nonce
(wp_create_nonce for your action), polling params and messages so the JS $.ajax
call can include the nonce and polling config for CSRF protection and ownership
checks.
In `@inc/managers/class-rating-notice-manager.php`:
- Line 125: The $review_url in class-rating-notice-manager.php is using the
placeholder plugin slug 'developer-developer'; update the URL assigned to
$review_url so it uses the actual WordPress.org plugin slug (e.g., replace
'developer-developer' with 'wp-ultimo' or the correct slug for this plugin) so
the review link points to the real plugin review page.
🧹 Nitpick comments (8)
inc/class-logger.php (1)
112-112: Document the updatedwu_log_addhook args and remind listeners to opt in to 3 params.
Passing the new arg is fine, but consumers won’t see it unless they register withaccepted_args = 3. Consider adding a hook docblock to prevent confusion.📝 Suggested docblock addition
- do_action('wu_log_add', $handle, $message, $log_level); + /** + * Fires when a log entry is added. + * + * `@param` string $handle Log file handle. + * `@param` string $message Log message. + * `@param` string $log_level PSR-3 log level. + */ + do_action('wu_log_add', $handle, $message, $log_level);inc/ui/class-site-actions-element.php (2)
891-894: Consider adding a comment explaining the purpose of this hidden field.The
wu-whenfield with a base64-encoded value appears to serve some form-processing or timing purpose, but its intent isn't clear from the code. A brief comment would help future maintainers understand why this field exists.Note: base64 encoding provides no security benefit here—the value is trivially decoded.
1060-1063: Fix array alignment for consistency.The static analysis tool flagged inconsistent array double arrow alignment. This field should match the alignment style used elsewhere in the
$fieldsarray (18 spaces between key and=>).🔧 Suggested fix
- 'wu-when' => [ + 'wu-when' => [ 'type' => 'hidden', 'value' => base64_encode('init'), // phpcs:ignore ],lang/ultimate-multisite.pot (1)
8840-8858: Avoid hard-coding the “12+” requirement in the settings description.
The help text currently embeds “12+” directly. Consider using a placeholder tied to the same constant used for validation so the string doesn’t drift if the minimum changes.inc/class-tracker.php (3)
315-368: Consider adding table existence checks before querying.The direct database queries assume the custom tables exist. If the plugin is in a partially installed state or tables are missing, these queries could throw errors or return unexpected results.
💡 Suggested defensive check
protected function get_usage_data(): array { global $wpdb; $table_prefix = $wpdb->base_prefix; + + // Check if tables exist before querying + $sites_table = $table_prefix . 'wu_sites'; + if ($wpdb->get_var($wpdb->prepare("SHOW TABLES LIKE %s", $sites_table)) !== $sites_table) { + return [ + 'sites_count' => '0', + 'customers_count' => '0', + 'memberships_count' => '0', + 'active_memberships_count' => '0', + 'products_count' => '0', + 'payments_count' => '0', + 'domains_count' => '0', + ]; + }
460-467: File path check may miss some plugin-related errors.The check for
wu-prefix is quite broad and could potentially match unrelated files. Consider using a more specific pattern.💡 More specific pattern suggestion
if (strpos($error_file, 'ultimate-multisite') === false && - strpos($error_file, 'wp-multisite-waas') === false && - strpos($error_file, 'wu-') === false) { + strpos($error_file, 'wp-multisite-waas') === false) { return $should_handle; }The
wu-prefix check could match files from other plugins. If needed, consider checking for the full plugin directory path instead.
654-660: Consider avoiding potential infinite loop when logging tracker failures.If
send_to_apifails and logs the error viaLogger::add, and if that log triggersmaybe_send_error, you could potentially enter a loop (though the log level filtering likely prevents this). Consider adding a guard or using a different logging approach.💡 Add a static flag to prevent recursion
+ /** + * Flag to prevent recursive error sending. + * + * `@var` bool + */ + private static bool $sending = false; public function maybe_send_error(string $handle, string $message, string $log_level = ''): void { if ( ! $this->is_tracking_enabled()) { return; } + // Prevent recursive calls + if (self::$sending || 'tracker' === $handle) { + return; + } // Only send error-level messages if ( ! in_array($log_level, self::ERROR_LOG_LEVELS, true)) { return; } + self::$sending = true; $error_data = $this->prepare_error_data($handle, $message, $log_level); // Send asynchronously to avoid blocking $this->send_to_api_async($error_data, 'error'); + self::$sending = false; }assets/js/payment-status-poll.js (1)
86-122: LGTM with optional enhancement.Good use of optional chaining for message fallbacks and proper class management. The
attemptparameter is passed but unused—consider displaying it (e.g., "Verifying payment... (attempt 3 of 20)") to give users feedback on polling progress, but this is optional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
assets/js/checkout.min.jsis excluded by!**/*.min.jsassets/js/enhance-integration.min.jsis excluded by!**/*.min.jsassets/js/payment-status-poll.min.jsis excluded by!**/*.min.jsassets/js/wu-password-reset.min.jsis excluded by!**/*.min.jsassets/js/wu-password-strength.min.jsis excluded by!**/*.min.jsassets/js/wu-password-toggle.min.jsis excluded by!**/*.min.jspackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.githooks/pre-commitassets/js/enhance-integration.jsassets/js/payment-status-poll.jsbin/setup-hooks.shinc/admin-pages/class-setup-wizard-admin-page.phpinc/class-logger.phpinc/class-settings.phpinc/class-tracker.phpinc/class-wp-ultimo.phpinc/compat/class-woocommerce-subscriptions-compat.phpinc/helpers/class-site-duplicator.phpinc/managers/class-rating-notice-manager.phpinc/models/class-domain.phpinc/ui/class-site-actions-element.phplang/ultimate-multisite.potpackage.jsonreadme.txtviews/dashboard-widgets/thank-you.phpviews/settings/widget-settings-body.php
💤 Files with no reviewable changes (2)
- inc/admin-pages/class-setup-wizard-admin-page.php
- inc/helpers/class-site-duplicator.php
✅ Files skipped from review due to trivial changes (2)
- readme.txt
- bin/setup-hooks.sh
🧰 Additional context used
🧬 Code graph analysis (3)
inc/compat/class-woocommerce-subscriptions-compat.php (1)
inc/class-wp-ultimo.php (2)
WP_Ultimo(23-1002)init(115-215)
inc/class-wp-ultimo.php (3)
inc/compat/class-woocommerce-subscriptions-compat.php (1)
WooCommerce_Subscriptions_Compat(28-162)inc/class-tracker.php (1)
Tracker(23-690)inc/managers/class-rating-notice-manager.php (1)
Rating_Notice_Manager(23-151)
inc/class-tracker.php (3)
inc/functions/settings.php (1)
wu_get_setting(37-40)inc/functions/helper.php (1)
wu_get_version(21-24)inc/class-logger.php (1)
add(54-113)
🪛 GitHub Check: Code Quality Checks
inc/compat/class-woocommerce-subscriptions-compat.php
[warning] 72-72:
The method parameter $was_new is never used
inc/ui/class-site-actions-element.php
[warning] 1060-1060:
Array double arrow not aligned correctly; expected 18 space(s) between "'wu-when'" and double arrow, but found 7.
🪛 PHPMD (2.15.0)
inc/compat/class-woocommerce-subscriptions-compat.php
72-72: Avoid unused parameters such as '$domain'. (undefined)
(UnusedFormalParameter)
72-72: Avoid unused parameters such as '$was_new'. (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). (3)
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (27)
inc/models/class-domain.php (1)
546-560: LGTM! Well-documented action hook for domain primary status changes.The new
wu_domain_became_primaryaction hook is correctly placed after the old primary domains are queued for removal and provides useful parameters for consumers. The triggering condition on line 535 properly detects both new primary domains and existing domains that become primary.One minor point: the
@since 2.0.0version tag may need updating if this feature is being released in a newer version of the plugin.package.json (2)
97-104: LGTM!The lint-staged configuration is well-structured and consistent with the existing lint scripts. The glob patterns correctly target JS and CSS files under
assets/, and the--ignore-patternflags properly exclude minified files.
26-26: The lint-staged version^16.2.7is a valid published version on npm.views/dashboard-widgets/thank-you.php (1)
255-258: LGTM! Good accessibility improvement.Adding the
altattribute to the site thumbnail image improves accessibility for screen reader users. The generic text is acceptable given the context.inc/managers/class-rating-notice-manager.php (1)
54-59: LGTM! Clean initialization pattern.The hook registrations are appropriate—storing the timestamp on activation and checking for notice display on admin_init is a sensible approach.
inc/compat/class-woocommerce-subscriptions-compat.php (4)
95-133: Good defensive pattern with try/finally.The
try/finallyblock correctly ensuresrestore_current_blog()is always called, even when earlyreturnstatements are hit inside thetryblock. This prevents blog context leaks.
72-75: Unused parameters are acceptable here.The
$domainand$was_newparameters are required by thewu_domain_became_primaryhook signature but aren't needed for this handler's logic. This is a common pattern when hooking into actions with fixed signatures.Consider adding a brief comment or
@phpstan-ignore-next-lineto silence static analysis warnings if they become noisy.
38-43: LGTM! Hook registrations are appropriate.The hooks for site duplication and primary domain changes correctly target the scenarios where WooCommerce Subscriptions staging mode needs to be reset.
97-133: Critical: Early returns bypassrestore_current_blog(), causing blog context leak.After
switch_to_blog($site_id)on line 95, the earlyreturnstatements on lines 101, 107, and 118 exit without callingrestore_current_blog(). This leaves the global blog context incorrectly switched, which can cause data corruption or unexpected behavior in subsequent code.🐛 Proposed fix: Use a flag to track success and always restore
switch_to_blog($site_id); try { $site_url = get_site_url(); if (empty($site_url) || ! is_string($site_url)) { - return; + return; // finally block will handle restore } $scheme = wp_parse_url($site_url, PHP_URL_SCHEME); if (empty($scheme) || ! is_string($scheme)) { - return; + return; // finally block will handle restore } /* * Generate the obfuscated key that WooCommerce Subscriptions uses. * It inserts '_[wc_subscriptions_siteurl]_' in the middle of the URL. */ $scheme_with_separator = $scheme . '://'; $site_url_without_scheme = str_replace($scheme_with_separator, '', $site_url); if (empty($site_url_without_scheme) || ! is_string($site_url_without_scheme)) { - return; + return; // finally block will handle restore }Actually, wait—the
finallyblock does execute on early returns in PHP. The current code is correct! Thefinallyblock on line 131-133 will always run, even whenreturnis called inside thetryblock. My apologies for the false alarm.Correction: After re-analysis, the
finallyblock in PHP executes even whenreturnis called insidetry. The current implementation is actually correct—restore_current_blog()will always be called.lang/ultimate-multisite.pot (2)
1-12: POT metadata update looks good.
No concerns with the 2026 header and creation date refresh.
8501-8531: Verify strength-hint strings stay aligned with actual enforcement.
These UI hints (“Strength indicator”, “Required:”, “uppercase letter”, etc.) should reflect the exact server-side rules (and any Defender Pro override). Please confirm they stay in sync.inc/class-tracker.php (6)
23-26: LGTM on the singleton implementation.The class properly implements the Singleton interface and uses the trait, following the established pattern in the codebase.
54-59: LGTM on error log level filtering.Good use of PSR-3 log levels for filtering which errors to report. The selection of ERROR, CRITICAL, ALERT, and EMERGENCY is appropriate for telemetry.
67-86: LGTM on hook registration.The initialization properly registers all necessary hooks for weekly cron, error reporting, fatal error handling, and initial data sending.
206-212: Site hash implementation looks good for privacy.Using SHA-256 with site URL and AUTH_KEY provides good anonymization while maintaining consistency for deduplication.
607-625: Good sanitization implementation for privacy protection.The error message sanitization properly removes file paths, URLs, domains, emails, and IP addresses. The 1000 character limit is reasonable for error telemetry.
670-688: Non-blocking request with 0.01s timeout is appropriate.Using a very short timeout with
blocking => falseis the correct approach for async telemetry that shouldn't impact user experience.inc/class-wp-ultimo.php (3)
613-616: LGTM on WooCommerce Subscriptions compatibility initialization.Properly placed in the compatibility section of
load_extra_components().
654-657: LGTM on Tracker initialization.The Tracker is appropriately initialized in
load_extra_components()after other core components are loaded, ensuring settings and other dependencies are available.
942-945: LGTM on Rating Notice Manager initialization.Properly placed in
load_managers()with appropriate comment.views/settings/widget-settings-body.php (1)
278-288: LGTM on privacy-focused telemetry messaging.The updated labels and description clearly communicate what data is collected and link to the privacy policy. Proper use of
esc_html_e()for internationalization and escaping.inc/class-settings.php (2)
866-890: LGTM on password strength settings.The new password strength configuration provides clear options with appropriate defaults. The description for "Super Strong" properly explains the requirements (12+ chars, mixed case, numbers, symbols).
1737-1744: Good change to opt-in telemetry by default."The feature should be disabled by default and the admin can make an active choice to participate." Changing the default from1to0aligns with WordPress community best practices for telemetry, ensuring users must explicitly opt-in. The privacy-focused description with a link to the privacy policy provides necessary transparency.assets/js/payment-status-poll.js (4)
10-28: LGTM!Good defensive setup with the guard clause for missing config, sensible defaults with fallbacks, and proper
parseIntusage with radix.
127-141: LGTM!Good defensive approach to ensure the status element exists, with sensible fallback selectors for various page structures.
175-180: LGTM!Proper guards ensure polling only starts when appropriate conditions are met.
182-188: Good debugging utility.Exposing the controller is helpful for debugging and external control. With the guard against multiple intervals (flagged above), this will be safe to use.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
.githooks/pre-commit
Outdated
| else | ||
| echo "Warning: npx not found. Skipping lint-staged." | ||
| echo "Run 'npm install' to set up the development environment." | ||
| fi | ||
|
|
||
| echo -e "${GREEN}All pre-commit checks passed!${NC}" | ||
| exit 0 | ||
| echo "Pre-commit checks passed!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading success message when lint-staged is skipped.
When npx is not found, the script skips lint-staged but still prints "Pre-commit checks passed!" at line 19. This is inaccurate since no checks actually ran.
Suggested fix
# Check if lint-staged is available
if command -v npx &> /dev/null; then
echo "Running lint-staged..."
npx lint-staged
+ echo "Pre-commit checks passed!"
else
echo "Warning: npx not found. Skipping lint-staged."
echo "Run 'npm install' to set up the development environment."
fi
-
-echo "Pre-commit checks passed!"📝 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.
| else | |
| echo "Warning: npx not found. Skipping lint-staged." | |
| echo "Run 'npm install' to set up the development environment." | |
| fi | |
| echo -e "${GREEN}All pre-commit checks passed!${NC}" | |
| exit 0 | |
| \ No newline at end of file | |
| echo "Pre-commit checks passed!" | |
| # Check if lint-staged is available | |
| if command -v npx &> /dev/null; then | |
| echo "Running lint-staged..." | |
| npx lint-staged | |
| echo "Pre-commit checks passed!" | |
| else | |
| echo "Warning: npx not found. Skipping lint-staged." | |
| echo "Run 'npm install' to set up the development environment." | |
| fi |
🤖 Prompt for AI Agents
In @.githooks/pre-commit around lines 14 - 19, The script currently always
echoes "Pre-commit checks passed!" even when npx is missing and lint-staged was
skipped; modify the logic so the final success message (the echo "Pre-commit
checks passed!") is only printed when lint-staged actually ran and returned
success, and print a distinct message indicating checks were skipped when npx is
not found; locate the branch that tests for npx and the echo "Pre-commit checks
passed!" line in .githooks/pre-commit and change it to conditionally output
success on lint-staged success and a clear "checks skipped" warning otherwise.
assets/js/enhance-integration.js
Outdated
| }).done(function(response) { | ||
| if (response.success && response.data.websites) { | ||
| self.populateWebsites(response.data.websites); | ||
| $('#wu_enhance_website_id').prop('disabled', false); | ||
| $status.text(wu_enhance_data.i18n.websites_loaded).css('color', 'green'); | ||
|
|
||
| // If only one website, auto-select it | ||
| if (response.data.websites.length === 1) { | ||
| $('#wu_enhance_website_id').val(response.data.websites[ 0 ].id); | ||
| } | ||
| } else { | ||
| $status.text(response.data.message || wu_enhance_data.i18n.websites_failed).css('color', 'red'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against missing response.data in the error path.
If the API returns { success: false } without data, response.data.message throws and the status message never renders.
🛠️ Proposed fix
- }).done(function(response) {
+ }).done(function(response) {
if (response.success && response.data.websites) {
self.populateWebsites(response.data.websites);
$('#wu_enhance_website_id').prop('disabled', false);
$status.text(wu_enhance_data.i18n.websites_loaded).css('color', 'green');
@@
- } else {
- $status.text(response.data.message || wu_enhance_data.i18n.websites_failed).css('color', 'red');
+ } else {
+ const message = (response && response.data && response.data.message)
+ ? response.data.message
+ : wu_enhance_data.i18n.websites_failed;
+ $status.text(message).css('color', 'red');
}
}).fail(function() {📝 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.
| }).done(function(response) { | |
| if (response.success && response.data.websites) { | |
| self.populateWebsites(response.data.websites); | |
| $('#wu_enhance_website_id').prop('disabled', false); | |
| $status.text(wu_enhance_data.i18n.websites_loaded).css('color', 'green'); | |
| // If only one website, auto-select it | |
| if (response.data.websites.length === 1) { | |
| $('#wu_enhance_website_id').val(response.data.websites[ 0 ].id); | |
| } | |
| } else { | |
| $status.text(response.data.message || wu_enhance_data.i18n.websites_failed).css('color', 'red'); | |
| } | |
| }).done(function(response) { | |
| if (response.success && response.data.websites) { | |
| self.populateWebsites(response.data.websites); | |
| $('#wu_enhance_website_id').prop('disabled', false); | |
| $status.text(wu_enhance_data.i18n.websites_loaded).css('color', 'green'); | |
| // If only one website, auto-select it | |
| if (response.data.websites.length === 1) { | |
| $('#wu_enhance_website_id').val(response.data.websites[ 0 ].id); | |
| } | |
| } else { | |
| const message = (response && response.data && response.data.message) | |
| ? response.data.message | |
| : wu_enhance_data.i18n.websites_failed; | |
| $status.text(message).css('color', 'red'); | |
| } |
🤖 Prompt for AI Agents
In `@assets/js/enhance-integration.js` around lines 69 - 81, The error path
assumes response.data exists before accessing response.data.message, which can
throw; update the else branch that sets $status (used alongside populateWebsites
and `#wu_enhance_website_id`) to safely read the message by checking for
response.data and response.data.message (or falling back to response.message and
wu_enhance_data.i18n.websites_failed) and then set the text and color
accordingly so the status always renders even when response.data is missing.
assets/js/enhance-integration.js
Outdated
| populateWebsites(websites) { | ||
| const $select = $('#wu_enhance_website_id'); | ||
| $select.empty().append('<option value="">' + wu_enhance_data.i18n.select_website + '</option>'); | ||
|
|
||
| $.each(websites, function(i, website) { | ||
| $select.append( | ||
| '<option value="' + website.id + '">' + website.name + '</option>' | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Escape website names to prevent XSS in the dropdown.
website.name and website.id come from an external API; building raw HTML can inject scripts into the admin UI. Use DOM APIs to set text/value safely.
🔒 Proposed fix
- $.each(websites, function(i, website) {
- $select.append(
- '<option value="' + website.id + '">' + website.name + '</option>'
- );
- });
+ $.each(websites, function(i, website) {
+ $select.append(
+ $('<option>').val(website.id).text(website.name)
+ );
+ });📝 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.
| populateWebsites(websites) { | |
| const $select = $('#wu_enhance_website_id'); | |
| $select.empty().append('<option value="">' + wu_enhance_data.i18n.select_website + '</option>'); | |
| $.each(websites, function(i, website) { | |
| $select.append( | |
| '<option value="' + website.id + '">' + website.name + '</option>' | |
| ); | |
| populateWebsites(websites) { | |
| const $select = $('#wu_enhance_website_id'); | |
| $select.empty().append('<option value="">' + wu_enhance_data.i18n.select_website + '</option>'); | |
| $.each(websites, function(i, website) { | |
| $select.append( | |
| $('<option>').val(website.id).text(website.name) | |
| ); | |
| }); |
🤖 Prompt for AI Agents
In `@assets/js/enhance-integration.js` around lines 94 - 101, In populateWebsites,
avoid building option HTML with string concatenation using website.name and
website.id (which can contain malicious content); instead create option elements
via DOM APIs (e.g., document.createElement or jQuery $('<option>')) and set the
option's value and text via safe setters like option.value / $option.val(…) and
option.textContent / $option.text(…) so both website.id and website.name are
assigned without injecting raw HTML; replace the current $select.append('<option
value="' + website.id + '">'+ website.name +'</option>') usage with these safe
API calls.
assets/js/payment-status-poll.js
Outdated
| $.ajax({ | ||
| url: config.ajaxUrl, | ||
| type: 'POST', | ||
| data: { | ||
| action: 'wu_check_payment_status', | ||
| payment_hash: config.paymentHash, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find PHP files related to payment handling
fd -e php -t f | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 991
🏁 Script executed:
#!/bin/bash
# Search for the AJAX handler function
rg -n "wu_check_payment_status" --type php -A 15Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for wu_payment_poll localization
rg -n "wu_payment_poll" --type php -B 3 -A 10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for nonce validation patterns in PHP
rg -n "check_ajax_referer|wp_verify_nonce" --type php -B 2 -A 5 | head -100Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 6605
🏁 Script executed:
#!/bin/bash
# Search for files containing "payment" in name
fd -e php payment | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 982
🏁 Script executed:
#!/bin/bash
# Search more broadly for wu_check_payment_status across all files
rg "wu_check_payment_status" -A 15Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3337
🏁 Script executed:
#!/bin/bash
# Search for where AJAX actions are registered
rg "add_action.*ajax" --type php | grep -i paymentRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Check the actual JS file to see what data structure it expects
cat -n assets/js/payment-status-poll.jsRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 6174
🏁 Script executed:
#!/bin/bash
# Search for wu_check_payment_status handler in payment manager
cat -n inc/managers/class-payment-manager.php | grep -A 20 "wu_check_payment_status"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for the handler registration - look for add_action with ajax suffix
rg "add_action.*wu_check_payment_status" -A 5 -B 5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search where wu_payment_poll is localized - look for wp_localize_script
rg "wu_payment_poll" -B 10 -A 10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 7896
🏁 Script executed:
#!/bin/bash
# Check if there's a REST API endpoint instead of traditional AJAX
rg "check_payment_status" --type php -B 5 -A 10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for where payment-status-poll.js is enqueued
rg "payment-status-poll" --type php -B 5 -A 5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for thank you page or payment confirmation related code
rg -i "thank.?you|payment.*confirmation|payment.*complete" --type php | head -30Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 2936
🏁 Script executed:
#!/bin/bash
# Look in functions/payment.php for handlers
head -100 inc/functions/payment.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 2089
🏁 Script executed:
#!/bin/bash
# Search for all add_action calls that might be related to payment
rg "add_action.*payment" --type php | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1742
🏁 Script executed:
#!/bin/bash
# Search in class-payment-manager.php for show_pending_payments which enqueues the script
rg -n "show_pending_payments" --type php -A 30 inc/managers/class-payment-manager.phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 2304
🏁 Script executed:
#!/bin/bash
# Search for REST API endpoints for payment status
rg "register_rest_route" --type php | grep -i paymentRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search more broadly for AJAX handlers - look for wp_ajax hooks
rg "wp_ajax" --type php | head -20Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 2351
🏁 Script executed:
#!/bin/bash
# Search for checkout form and thank you page implementations
rg "thank_you_page|payment_hash" --type php | head -30Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 2876
🏁 Script executed:
#!/bin/bash
# Search in class-checkout.php for thank you page handlers
cat -n inc/checkout/class-checkout.php | head -150Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3452
🏁 Script executed:
#!/bin/bash
# Look for REST API payment endpoints
rg "register_rest_route.*payment" --type php -A 10Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for class-checkout-element.php to see thank you page handling
cat -n inc/ui/class-checkout-element.php | head -200Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 5699
🏁 Script executed:
#!/bin/bash
# Search where payment-status-poll.js file is located and check git history
ls -lah assets/js/payment-status-poll.* 2>/dev/nullRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 248
🏁 Script executed:
#!/bin/bash
# Search for any incomplete handlers or TODOs related to payment polling
rg "TODO|FIXME|payment.*poll" --type php -iRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 5985
🏁 Script executed:
#!/bin/bash
# Look for where thank you page is rendered in checkout
rg "is_thank_you_page" --type php -B 5 -A 15Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3083
🏁 Script executed:
#!/bin/bash
# Find Thank_You_Element class
fd -e php -t f | xargs grep -l "class Thank_You_Element"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Check Thank_You_Element to see if it enqueues payment-status-poll.js
cat -n inc/ui/class-thank-you-element.php 2>/dev/null | head -150Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 4586
🏁 Script executed:
#!/bin/bash
# Check git log to see when payment-status-poll.js was added
git log --oneline --all -- assets/js/payment-status-poll.js 2>/dev/null | head -5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Check for any pending/incomplete feature branches or comments about this feature
rg "payment.*poll|payment.*status.*check" --type php --type js -iRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 4299
Incomplete AJAX handler and script enqueue for payment status polling.
The JavaScript file exists and references a wu_check_payment_status AJAX endpoint, but the backend implementation is missing entirely:
- No AJAX handler – The
wp_ajax_wu_check_payment_statusaction is never registered - No script enqueue –
payment-status-poll.jsis never enqueued - No localization – The
wu_payment_pollobject is never localized from PHP to JavaScript - Security concern – Even if implemented, the AJAX request lacks a nonce for CSRF protection
The feature currently cannot function. Implement the complete backend:
- Register the AJAX handler in the Payment Manager
- Enqueue
payment-status-poll.jsin the Thank You page setup - Localize
wp_payment_pollwith necessary data (payment hash, AJAX URL, nonce, polling parameters, messages) - Ensure the handler validates the nonce and verifies the payment hash ownership
🤖 Prompt for AI Agents
In `@assets/js/payment-status-poll.js` around lines 41 - 47, Add a complete
backend for the AJAX polling: register both ajax hooks
wp_ajax_wu_check_payment_status and wp_ajax_nopriv_wu_check_payment_status in
the Payment Manager class, implement the handler function (named e.g.
handle_wu_check_payment_status) to verify a localized nonce (sent from JS),
validate that the provided payment_hash belongs to the current user/session
before returning JSON status, and exit with proper WP_Error on failure; enqueue
the frontend script payment-status-poll.js during the Thank You page setup (the
existing enqueue function in the Payment Manager or thank-you setup) and
localize the script with a wu_payment_poll object containing ajaxUrl
(admin_url('admin-ajax.php')), paymentHash, nonce (wp_create_nonce for your
action), polling params and messages so the JS $.ajax call can include the nonce
and polling config for CSRF protection and ownership checks.
assets/js/payment-status-poll.js
Outdated
| function startPolling() { | ||
| if (! config.paymentHash || ! config.ajaxUrl) { | ||
| return; | ||
| } | ||
|
|
||
| // Ensure the status element exists | ||
| ensureStatusElement(); | ||
|
|
||
| // Initial status update | ||
| updateStatusMessage('pending', 0); | ||
|
|
||
| // Start polling | ||
| pollTimer = setInterval(checkPaymentStatus, config.pollInterval); | ||
|
|
||
| // Do first check immediately | ||
| checkPaymentStatus(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against multiple concurrent polling loops.
If startPolling() is called multiple times (via the exposed controller or double-initialization), multiple intervals will be created. Only the last one would be cleared by stopPolling(), causing runaway polling and excessive server requests.
🔧 Proposed fix to prevent multiple intervals
function startPolling() {
if (! config.paymentHash || ! config.ajaxUrl) {
return;
}
+ // Prevent multiple polling loops
+ if (pollTimer) {
+ return;
+ }
+
// Ensure the status element exists
ensureStatusElement();
// Initial status update
updateStatusMessage('pending', 0);
// Start polling
pollTimer = setInterval(checkPaymentStatus, config.pollInterval);
// Do first check immediately
checkPaymentStatus();
}📝 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.
| function startPolling() { | |
| if (! config.paymentHash || ! config.ajaxUrl) { | |
| return; | |
| } | |
| // Ensure the status element exists | |
| ensureStatusElement(); | |
| // Initial status update | |
| updateStatusMessage('pending', 0); | |
| // Start polling | |
| pollTimer = setInterval(checkPaymentStatus, config.pollInterval); | |
| // Do first check immediately | |
| checkPaymentStatus(); | |
| } | |
| function startPolling() { | |
| if (! config.paymentHash || ! config.ajaxUrl) { | |
| return; | |
| } | |
| // Prevent multiple polling loops | |
| if (pollTimer) { | |
| return; | |
| } | |
| // Ensure the status element exists | |
| ensureStatusElement(); | |
| // Initial status update | |
| updateStatusMessage('pending', 0); | |
| // Start polling | |
| pollTimer = setInterval(checkPaymentStatus, config.pollInterval); | |
| // Do first check immediately | |
| checkPaymentStatus(); | |
| } |
🤖 Prompt for AI Agents
In `@assets/js/payment-status-poll.js` around lines 146 - 162, startPolling() can
create multiple concurrent intervals causing runaway polling; guard against that
by checking the module-scoped pollTimer before starting a new interval: if
pollTimer is already set, either return early or clearInterval(pollTimer) first,
then set pollTimer = setInterval(...). Ensure stopPolling() clears the interval
and sets pollTimer = null, and that any early-return path keeps pollTimer state
consistent; reference functions/variables: startPolling, stopPolling, pollTimer,
checkPaymentStatus, ensureStatusElement, config.
| */ | ||
| protected function add_rating_notice(): void { | ||
|
|
||
| $review_url = 'https://wordpress.org/support/plugin/developer-developer/reviews/#new-post'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect plugin slug in review URL.
The review URL contains developer-developer which appears to be a placeholder. This should be updated to the actual WordPress.org plugin slug before release.
🐛 Suggested fix
- $review_url = 'https://wordpress.org/support/plugin/developer-developer/reviews/#new-post';
+ $review_url = 'https://wordpress.org/support/plugin/wp-ultimo/reviews/#new-post';Replace wp-ultimo with the correct plugin slug if different.
📝 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.
| $review_url = 'https://wordpress.org/support/plugin/developer-developer/reviews/#new-post'; | |
| $review_url = 'https://wordpress.org/support/plugin/wp-ultimo/reviews/#new-post'; |
🤖 Prompt for AI Agents
In `@inc/managers/class-rating-notice-manager.php` at line 125, The $review_url in
class-rating-notice-manager.php is using the placeholder plugin slug
'developer-developer'; update the URL assigned to $review_url so it uses the
actual WordPress.org plugin slug (e.g., replace 'developer-developer' with
'wp-ultimo' or the correct slug for this plugin) so the review link points to
the real plugin review page.
- 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>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@assets/js/wu-password-strength.js`:
- Around line 133-144: In checkStrength(), when handling the early-return for an
empty password (inside the checkStrength method where pass1 is falsy),
reset/clear the component's failedRules state so getFailedRules() won't return
stale entries; locate the checkStrength method and add logic to set failedRules
(or the equivalent property used to store rule failures) to an empty
array/object before calling setValid(false) and returning.
- Around line 146-150: checkStrength() currently calls
wp.passwordStrength.meter(pass1, disallowedList, pass2) without verifying
wp.passwordStrength exists; mirror the defensive check used in
getDisallowedList(): before invoking wp.passwordStrength.meter in the
checkStrength method, confirm wp && wp.passwordStrength are defined (or
bail/return a safe default strength value) to avoid runtime errors when the
WordPress password-strength-meter script is not loaded; update checkStrength()
accordingly to perform this guard.
🧹 Nitpick comments (1)
inc/managers/class-membership-manager.php (1)
107-119: LGTM! Correctly addresses the missing response issue.The refactor now sends the JSON response unconditionally before the
fastcgi_finish_requestcheck, ensuring all clients receive a response regardless of server configuration. This properly resolves the previous review concern.Optional enhancement: For non-fastcgi environments, consider explicitly flushing output buffers after the
echoto ensure the client receives the response immediately rather than waiting for the full operation to complete:♻️ Optional: Flush output buffers
header('Content-Type: application/json; charset=' . get_option('blog_charset')); echo wp_json_encode(['status' => 'creating-site']); +if (ob_get_level()) { + ob_flush(); +} +flush(); // Don't make the request block till we finish, if possible.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/js/wu-password-strength.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (6)
.githooks/pre-commitassets/js/wu-password-strength.jsinc/managers/class-membership-manager.phpinc/managers/class-rating-notice-manager.phpreadme.txtultimate-multisite.php
✅ Files skipped from review due to trivial changes (1)
- ultimate-multisite.php
🚧 Files skipped from review as they are similar to previous changes (2)
- inc/managers/class-rating-notice-manager.php
- .githooks/pre-commit
🧰 Additional context used
🧬 Code graph analysis (1)
inc/managers/class-membership-manager.php (1)
inc/class-helper.php (1)
get_option(108-113)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: PHP 8.0
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: Build Plugin for Testing
- GitHub Check: PHP 7.4
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (15)
readme.txt (3)
8-8: LGTM: Version bump is consistent with changelog.The stable tag update to 2.4.10 correctly matches the new changelog entry.
243-252: Changelog is comprehensive and aligns with PR objectives.The version 2.4.10 changelog accurately documents all major features from the PR:
- Password strength settings with Super Strong level and WPMU DEV Defender Pro compatibility
- Real-time password hints with i18n support
- Themed password field styling with page builder support
- Opt-in telemetry, rating reminder, and WooCommerce compatibility
Note: The release date "2026-01-XX" is a placeholder—remember to update it before the actual release.
214-224: All documentation claims have been verified against the implementation ininc/class-tracker.php. The telemetry feature is accurately described:✓ API endpoint URL matches the implementation constant
✓ Weekly frequency confirmed (const SEND_INTERVAL = WEEK_IN_SECONDS)
✓ Opt-in disabled by default confirmed (enable_error_reportingsetting defaults to false)
✓ Error reporting on demand confirmed (hooked intowu_log_add)
✓ Privacy policy URLs are correct (subdomain difference is intentional—separate services have separate policies)No issues found with the documentation.
assets/js/wu-password-strength.js (12)
24-56: Solid defaults and settings merge.Clean separation of defaults and localized overrides; the fallback behavior is sensible.
69-85: Constructor state setup looks good.Initializing
settings,minStrength, andfailedRulesin one place keeps state predictable.
91-128: Init flow is robust.Early exits when required elements are missing prevent DOM errors and unintended behavior.
160-169: Compatibility handling is solid.The old/new WordPress API fallback is clean and safe.
177-221: Labeling logic is clear and well-guarded.The fallback labels and i18n checks are implemented cleanly.
228-268: UI update logic is clear and consistent.Nice handling of the Super Strong state and color progression.
278-307: Hint construction is straightforward.The rule-to-hint mapping is easy to follow and localized.
314-333: Validity calculation is correct.The strength gate plus rule enforcement aligns with the intended policy.
341-374: Rule checks are clean and Defender-compatible.The regex and rule gates are clear and match the stated requirements.
381-383: Useful external accessor.Exposing
failedRulesenables clean UI integration.
390-402: Submit handling and callback logic look correct.
409-411: Simple and correct accessor.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| checkStrength() { | ||
| const pass1 = this.options.pass1.val(); | ||
| const pass2 = this.options.pass2 ? this.options.pass2.val() : ''; | ||
|
|
||
| // Reset classes | ||
| this.options.result.attr('class', 'wu-py-2 wu-px-4 wu-block wu-text-sm wu-border-solid wu-border wu-mt-2'); | ||
|
|
||
| if (!pass1) { | ||
| if (! pass1) { | ||
| this.options.result.addClass('wu-bg-gray-100 wu-border-gray-200').html(this.getStrengthLabel('empty')); | ||
| this.setValid(false); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clear failedRules when password is empty to avoid stale state.
If the user clears the password, getFailedRules() can still expose the previous rules because failedRules isn’t reset on the early return.
🐛 Proposed fix
if (! pass1) {
+ this.failedRules = [];
this.options.result.addClass('wu-bg-gray-100 wu-border-gray-200').html(this.getStrengthLabel('empty'));
this.setValid(false);
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.
| checkStrength() { | |
| const pass1 = this.options.pass1.val(); | |
| const pass2 = this.options.pass2 ? this.options.pass2.val() : ''; | |
| // Reset classes | |
| this.options.result.attr('class', 'wu-py-2 wu-px-4 wu-block wu-text-sm wu-border-solid wu-border wu-mt-2'); | |
| if (!pass1) { | |
| if (! pass1) { | |
| this.options.result.addClass('wu-bg-gray-100 wu-border-gray-200').html(this.getStrengthLabel('empty')); | |
| this.setValid(false); | |
| return; | |
| } | |
| checkStrength() { | |
| const pass1 = this.options.pass1.val(); | |
| const pass2 = this.options.pass2 ? this.options.pass2.val() : ''; | |
| // Reset classes | |
| this.options.result.attr('class', 'wu-py-2 wu-px-4 wu-block wu-text-sm wu-border-solid wu-border wu-mt-2'); | |
| if (! pass1) { | |
| this.failedRules = []; | |
| this.options.result.addClass('wu-bg-gray-100 wu-border-gray-200').html(this.getStrengthLabel('empty')); | |
| this.setValid(false); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In `@assets/js/wu-password-strength.js` around lines 133 - 144, In
checkStrength(), when handling the early-return for an empty password (inside
the checkStrength method where pass1 is falsy), reset/clear the component's
failedRules state so getFailedRules() won't return stale entries; locate the
checkStrength method and add logic to set failedRules (or the equivalent
property used to store rule failures) to an empty array/object before calling
setValid(false) and returning.
| // Get disallowed list from WordPress | ||
| var disallowedList = this.getDisallowedList(); | ||
| const disallowedList = this.getDisallowedList(); | ||
|
|
||
| var strength = wp.passwordStrength.meter(pass1, disallowedList, pass2); | ||
| const strength = wp.passwordStrength.meter(pass1, disallowedList, pass2); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate script registration/enqueue for wu-password-strength and check dependency ordering
echo "=== Searching for wu-password-strength registration ==="
rg -n -C3 "wu-password-strength"
echo -e "\n=== Searching for password-strength-meter and passwordStrength references ==="
rg -n -C3 "password-strength-meter|passwordStrength"Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 16365
Add defensive check before calling wp.passwordStrength.meter() for consistency.
The password-strength-meter dependency is correctly registered in inc/class-scripts.php:161 and properly enqueued on pages that use this script. However, the getDisallowedList() method (line 161) already includes a defensive check for undefined wp.passwordStrength, while the checkStrength() method (line 149) directly calls wp.passwordStrength.meter() without verification. Add a matching safety check before the meter call to maintain consistent defensive programming, or document that the code explicitly relies on the WordPress dependency system to guarantee availability.
🤖 Prompt for AI Agents
In `@assets/js/wu-password-strength.js` around lines 146 - 150, checkStrength()
currently calls wp.passwordStrength.meter(pass1, disallowedList, pass2) without
verifying wp.passwordStrength exists; mirror the defensive check used in
getDisallowedList(): before invoking wp.passwordStrength.meter in the
checkStrength method, confirm wp && wp.passwordStrength are defined (or
bail/return a safe default strength value) to avoid runtime errors when the
WordPress password-strength-meter script is not loaded; update checkStrength()
accordingly to perform this guard.
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes – Version 2.4.10
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.