Merged
Conversation
When password_strength_meter form setting is disabled ('0'), the
#pass-strength-result element is not rendered. The checkout JS was
still setting valid_password=false and trying to initialize the
strength checker, which silently failed without the result element.
This caused all form submissions to be blocked with "Password too weak".
Also fix $className → $class_name in thank-you.php (PHPCS).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds invoice send/resend flows and pay-invoice checkout, payment-method UI and gateway Stripe portal integration, registers invoice/payment_failed/membership_expired events and email templates, tightens many helper type hints, adds domainmeta table and loader entry, refactors admin handle_save return contracts, removes a subscription-cancellation wiki doc, and numerous UI/UX and integration tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Network Admin
participant UI as Payment Admin UI
participant Server as Server (Form Handler)
participant Payment as Payment Model
participant Events as Event System
participant Email as Email Manager
Admin->>UI: Open "Send Invoice" modal
Admin->>UI: Submit invoice form
UI->>Server: AJAX POST -> handle_send_invoice_modal
Server->>Payment: Validate customer/products, create pending payment
Payment-->>Server: Payment created
Server->>Events: Emit "invoice_sent" with payload
Events->>Email: Trigger invoice email(s)
Email->>Admin: Send email(s)
Server-->>UI: Respond with redirect to payment page
UI-->>Admin: Redirect user
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
assets/js/integration-test.js (1)
15-32:⚠️ Potential issue | 🟡 MinorAdd a
timeoutto the AJAX call to preventloadingfrom getting permanently stuck.If the server hangs or the network drops without triggering a proper HTTP error, the
errorcallback will never fire andthat.loadingwill remaintrueindefinitely, leaving the UI frozen in a loading state.⏱️ Proposed fix: add a timeout
$.ajax({ url: ajaxurl, method: 'POST', + timeout: 30000, data: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/js/integration-test.js` around lines 15 - 32, The AJAX request for action 'wu_test_hosting_integration' can hang and leave that.loading true; add a timeout option to the $.ajax call (e.g., timeout: 10000) and ensure the error callback sets that.loading = false and handles timeout cases (textStatus === 'timeout') to set that.success = false and an appropriate that.results message using wu_integration_test_data.error_message or a timeout-specific message; update the options object passed to $.ajax in integration-test.js where the AJAX call is created.views/wizards/host-integrations/configuration.php (1)
35-51:⚠️ Potential issue | 🟡 MinorSubmit button left-aligns when no back URL is present.
wu-flex wu-justify-betweenrequires two children to push the submit button to the right. When$back_urlis falsy the<a>is omitted, leaving only the<span>, so the button drifts to the left instead of staying right-aligned.Consider rendering an empty placeholder element (or applying
wu-justify-end) when the back button is absent:🎨 Proposed fix
<div class="wu-flex wu-justify-between wu-bg-gray-100 wu--m-in wu-mt-4 wu-p-4 wu-overflow-hidden wu-border-t wu-border-solid wu-border-l-0 wu-border-r-0 wu-border-b-0 wu-border-gray-300"> <?php if ($back_url) : ?> <a href="<?php echo esc_url($back_url); ?>" class="wu-self-center button button-large wu-float-left" > <?php esc_html_e('← Go Back', 'ultimate-multisite'); ?> </a> - <?php endif; ?> + <?php else : ?> + <span></span> + <?php endif; ?>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@views/wizards/host-integrations/configuration.php` around lines 35 - 51, The container using classes "wu-flex wu-justify-between" drifts the Test Configuration button left when $back_url is falsy because the left node is omitted; fix by rendering a placeholder element when $back_url is empty (e.g. an empty <div> or <span> with the same sizing/visibility as the link) or by conditionally switching the container class to "wu-flex wu-justify-end" when $back_url is falsy; update the markup around the $back_url conditional (the <a> block and the surrounding container) so the submit <button> inside the <span> remains right-aligned in both cases.inc/sso/auth-functions.php (1)
192-202:⚠️ Potential issue | 🟠 MajorUnsanitized
$request_urifed intowp_redirect(set_url_scheme(...))— pre-existing open-redirect risk now more visible.Removing
sanitize_text_field()is the right call — that function strips tags and converts HTML entities, which corrupts legitimate URL paths and query strings.wp_redirect()→wp_sanitize_redirect()already strips CRLF, so header injection is mitigated.However, lines 197 and 228 pass the raw (attacker-controlled) REQUEST_URI directly through
set_url_scheme()and intowp_redirect()whenever the URI starts withhttp. A crafted request such asGET http://evil.com/wp-admin/foo HTTP/1.1would redirect the user tohttps://evil.com/wp-admin/foo. This is a pre-existing open redirect, but the phpcs suppression comment here now makes it a documented choice rather than an oversight — worth ensuring it is intentional and that the project threat model accepts it.Consider adding a host-validation guard before the redirect:
🛡️ Proposed guard against open redirect on the https-upgrade path
if ( str_starts_with($request_uri, 'http') ) { + $parsed_host = wp_parse_url($request_uri, PHP_URL_HOST); + if ( $parsed_host && strtolower($parsed_host) !== strtolower($host) ) { + wp_die(esc_html__('Invalid redirect.', 'ultimate-multisite')); + } wp_redirect(set_url_scheme($request_uri, 'https')); // phpcs:ignore WordPress.Security.SafeRedirect.wp_redirect_wp_redirect exit; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/sso/auth-functions.php` around lines 192 - 202, The code currently passes raw $request_uri through set_url_scheme() into wp_redirect() when it starts with 'http', enabling an open-redirect; modify the https-upgrade branch in the function that uses $request_uri, $host, set_url_scheme, wp_redirect and is_ssl to validate the parsed URL host before redirecting: when str_starts_with($request_uri, 'http') parse the URL (extract host) and only allow the redirect if the parsed host equals the sanitized $host (or is in a configured allowlist); if the host does not match, instead redirect to a safe local path (e.g. '/' or the path component only) or refuse the redirect; always run the final redirect URL through wp_sanitize_redirect() before calling wp_redirect().inc/functions/helper.php (1)
67-74:⚠️ Potential issue | 🔴 CriticalAdding
stringtype hint to$keywill break existing callers using numeric array indices.The function has three call sites in the codebase that pass numeric keys (e.g.,
wu_get_isset($array, 0, $default)), which is valid for PHP arrays. The newstringtype hint will causeTypeErrorexceptions at runtime:
inc/ui/class-jumper.php:wu_get_isset($title, 0, '')inc/managers/class-site-manager.php:wu_get_isset($logo, 0, false)inc/class-current.php:wu_get_isset($memberships, 0, false)Either update these call sites to cast keys to strings or change the type hint to accept both
string|int.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/functions/helper.php` around lines 67 - 74, The function wu_get_isset currently type-hints $key as string which breaks existing callers that pass numeric indices; update the parameter to accept both strings and integers (e.g., change the type to string|int or remove the strict string hint) so callers like wu_get_isset($title, 0, ''), wu_get_isset($logo, 0, false), and wu_get_isset($memberships, 0, false) no longer throw TypeError, keeping the rest of wu_get_isset (including casting $array_or_obj to array and returning $array_or_obj[$key] ?? $default_value) unchanged.inc/class-cron.php (1)
255-275:⚠️ Potential issue | 🟠 MajorGuard event dispatch on failed membership save.
renewal_payment_createdis dispatched even if the membership status update fails, because$savedis never validated before building and sending the event payload.💡 Suggested fix
$membership->set_status(Membership_Status::ON_HOLD); $saved = $membership->save(); + + if (is_wp_error($saved) || ! $saved) { + return; + } $payment_url = add_query_arg(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/class-cron.php` around lines 255 - 275, The code builds and dispatches the renewal_payment_created event regardless of whether the membership status update succeeded because the return value $saved from $membership->save() is ignored; modify the flow in the block that calls $membership->save() to check $saved (or the truthiness of $membership->save()) and only construct $payload and call wu_do_event('renewal_payment_created', $payload) when the save succeeded, otherwise bail out or log an error; specifically, guard the payload construction and the wu_do_event call behind a conditional that verifies $saved (or handles the failure) so renewal_payment_created is not emitted on failed membership saves.inc/gateways/class-paypal-gateway.php (1)
1595-1605:⚠️ Potential issue | 🔴 CriticalStop execution after invalid checkout details.
The method echoes an error when checkout details are invalid, but still continues and dereferences
$checkout_details['pending_payment'], which can crash the flow.💡 Suggested fix
if ( ! is_array($checkout_details)) { $error = is_wp_error($checkout_details) ? $checkout_details->get_error_message() : __('Invalid response code from PayPal', 'ultimate-multisite'); // translators: %s is the paypal error message. echo '<p>' . sprintf(esc_html__('An unexpected PayPal error occurred. Error message: %s.', 'ultimate-multisite'), esc_html($error)) . '</p>'; + return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/gateways/class-paypal-gateway.php` around lines 1595 - 1605, The code echoes an error when $checkout_details is not an array but then continues and dereferences $checkout_details['pending_payment'], which can cause a crash; update the error branch in the method in class-paypal-gateway.php to immediately stop further processing by returning (or otherwise exiting the current method) after echoing the error so the subsequent use of $checkout_details['pending_payment'] (and $customer = $checkout_details['pending_payment']->get_customer()) only runs when $checkout_details is a valid array/object.
🧹 Nitpick comments (9)
assets/js/integration-test.js (1)
27-31: LGTM — the error handler is a solid addition.The fallback to
'Connection test failed. Please try again.'whenerror_messageis absent is a clean defensive pattern. Optionally, capturing the jQuery AJAX error parameters (jqXHR, textStatus, errorThrown) would improve future debuggability if more granular error reporting is ever needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/js/integration-test.js` around lines 27 - 31, Update the jQuery AJAX error callback in assets/js/integration-test.js (the anonymous function that sets that.loading, that.success, and that.results) to accept the standard parameters (jqXHR, textStatus, errorThrown), log them (e.g., console.error(jqXHR, textStatus, errorThrown)) and include a concise diagnostic in that.results when available (for example append textStatus/errorThrown to the fallback message or use jqXHR.responseText if present) while preserving the existing fallback to wu_integration_test_data.error_message || 'Connection test failed. Please try again.' so debuggability is improved without changing the current behavior.inc/admin-pages/customer-panel/class-account-admin-page.php (1)
141-147: Normalize and whitelistupdatedbefore using it as filter context.
$update_typeis request-derived and is passed through on Line 147. Restricting values avoids unexpected success notices and reduces risky assumptions in filter callbacks.♻️ Suggested hardening
- if ('payment_method' === $update_type) { + $update_type = sanitize_key((string) $update_type); + $allowed_update_types = ['account', 'payment_method']; + + if (!in_array($update_type, $allowed_update_types, true)) { + return; + } + + if ('payment_method' === $update_type) { $update_message = __('Your payment method was successfully updated.', 'ultimate-multisite'); } else { $update_message = __('Your account was successfully updated.', 'ultimate-multisite'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/admin-pages/customer-panel/class-account-admin-page.php` around lines 141 - 147, Normalize and whitelist the request-derived $update_type before passing it into apply_filters('wu_account_update_message', ...) to avoid untrusted values reaching filter callbacks: sanitize the incoming value (e.g., cast to string and use sanitize_key or similar), check it against an allowlist of expected types (e.g., 'payment_method', 'password', 'email' — include a sensible default like 'general' or '' when not matched), then use that sanitized/whitelisted variable in the apply_filters call (referencing the $update_type variable and the wu_account_update_message filter) so only known contexts are ever provided to filter handlers.inc/sso/class-sso.php (1)
385-389:sanitize_text_field()removal is consistent and correct; note a pre-existing fragility in the dummy-URL trick.The removal of
sanitize_text_field()aligns with the same fix inauth-functions.phpand is justified for the same reason (it would corrupt URL characters).However, the dummy-base approach carries a latent bug: the outer
str_replace('https://a.com/', '', ...)replaces the first occurrence, so ifREQUEST_URIhappens to contain the literal stringhttps://a.com/as a query-parameter value, thestr_replacewould corrupt the result. A safer construction passes only the path/query portion:♻️ Suggested refactor — avoid dummy base URL
-$_SERVER['REQUEST_URI'] = str_replace( - 'https://a.com/', - '', - remove_query_arg('sso', 'https://a.com/' . wp_unslash($_SERVER['REQUEST_URI'] ?? '')) // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -); +$raw_uri = wp_unslash($_SERVER['REQUEST_URI'] ?? ''); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized +// remove_query_arg requires a full URL; reconstruct one, strip the SSO arg, then restore only the path+query. +$site_url = home_url('/'); +$full_uri = $site_url . ltrim($raw_uri, '/'); +$cleaned = remove_query_arg('sso', $full_uri); +$_SERVER['REQUEST_URI'] = '/' . ltrim(str_replace($site_url, '', $cleaned), '/');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/sso/class-sso.php` around lines 385 - 389, The current replacement uses a dummy base and str_replace which can accidentally remove the literal "https://a.com/" if it appears in the query string; change the logic so you do not prepend a dummy URL and instead extract only the path+query portion before calling remove_query_arg and assigning back to $_SERVER['REQUEST_URI']—use wp_unslash($_SERVER['REQUEST_URI'] ?? '') to get the raw request, parse or trim any leading scheme/host if present (e.g. via wp_parse_url or parsing only path+query), call remove_query_arg('sso', <path+query>) and then set $_SERVER['REQUEST_URI'] to that result (no outer str_replace). Ensure the code references the same symbols: $_SERVER['REQUEST_URI'], wp_unslash, remove_query_arg and remove the str_replace dummy-base approach.inc/admin-pages/class-top-admin-nav-menu.php (1)
211-223: Consider extracting duplicated settings-tab node construction.Line 211 to Line 223 and Line 239 to Line 250 duplicate the same node shape with only
parentchanging. A tiny helper/closure would reduce drift risk.Refactor sketch
+ $add_settings_tab_node = static function(string $tab, array $tab_info, string $parent) use ($wp_admin_bar): void { + $wp_admin_bar->add_node( + [ + 'id' => 'wp-ultimo-settings-' . $tab, + 'parent' => $parent, + 'title' => $tab_info['title'], + 'href' => network_admin_url('admin.php?page=wp-ultimo-settings&tab=') . $tab, + 'meta' => [ + 'class' => 'wp-ultimo-top-menu', + 'title' => __('Go to the settings page', 'ultimate-multisite'), + ], + ] + ); + }; ... - $wp_admin_bar->add_node([...]); + $add_settings_tab_node($tab, $tab_info, 'wp-ultimo-settings'); ... - $wp_admin_bar->add_node([...]); + $add_settings_tab_node($tab, $tab_info, 'wp-ultimo-settings-addons');Also applies to: 239-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/admin-pages/class-top-admin-nav-menu.php` around lines 211 - 223, Duplicate construction of the settings-tab node used in calls to $wp_admin_bar->add_node (the array with keys 'id' => 'wp-ultimo-settings-' . $tab, 'title' => $tab_info['title'], 'href' => network_admin_url(...), 'meta' => ['class' => 'wp-ultimo-top-menu','title' => __('Go to the settings page','ultimate-multisite')]) should be extracted into a small helper or closure inside the class (e.g., a private method build_settings_tab_node($tab, $tab_info, $parent) or $make_node = function($tab,$tab_info,$parent){...}) that returns the node array, then replace both $wp_admin_bar->add_node(...) calls to call $wp_admin_bar->add_node($this->build_settings_tab_node($tab,$tab_info,'wp-ultimo-settings')) and the other with the different parent; keep the same id/title/href/meta values (including 'wp-ultimo-top-menu') so behavior is unchanged.inc/integrations/capabilities/interface-node-management-capability.php (2)
138-138: Nit:install_depsdeviates from the fully-spelled naming convention used throughout this and sibling interfaces.Every other method here (
detect_node,destroy_app,get_app_status,list_apps) and all methods inDomain_Selling_Capabilityuse full, unabbreviated names. Considerinstall_dependenciesfor consistency.♻️ Proposed rename
- public function install_deps(string $app_id): array; + public function install_dependencies(string $app_id): array;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/integrations/capabilities/interface-node-management-capability.php` at line 138, The method name install_deps is inconsistent with the project's full-word naming convention; rename the interface method to install_dependencies and update all references and implementations (e.g., any classes implementing the interface and any callers of install_deps) to use install_dependencies, adjust method signatures (public function install_dependencies(string $app_id): array) and run tests/grep to ensure no remaining install_deps usages remain.
24-139: AddIDconstant for consistency with sibling capability interface.
Domain_Selling_Capabilitydefinespublic const ID = 'domain-selling'as a documented reference.Node_Management_Capabilityshould follow the same pattern for consistency, even though the capability registry resolves capabilities by calling theget_capability_id()method from implementations, not via the constant.Suggested addition
interface Node_Management_Capability { + + public const ID = 'node-management'; + /** * Detect available Node.js installations on the server.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/integrations/capabilities/interface-node-management-capability.php` around lines 24 - 139, Add a public constant ID to the Node_Management_Capability interface for consistency with sibling interfaces (e.g., Domain_Selling_Capability); declare public const ID = 'node-management' inside the Node_Management_Capability interface (near the top, after the interface declaration) so implementations can reference the capability by a stable name while keeping get_capability_id() behavior unchanged.inc/models/class-checkout-form.php (1)
1262-1312: Payment lookup logic duplicated fromfinish_checkout_form_fields().Lines 1264–1276 are a verbatim copy of the same block in
finish_checkout_form_fields()(Lines 1204–1216). This means any future fix (e.g., adding a new lookup strategy) must be applied in two places.Consider extracting a shared private helper:
♻️ Suggested refactor
+ /** + * Resolves a payment from the current request, with admin fallback. + * + * `@since` 2.5.0 + * `@return` \WP_Ultimo\Models\Payment|false + */ + private static function resolve_payment_from_request() { + + $payment = wu_get_payment_by_hash(wu_request('payment')); + + if ( ! $payment && wu_request('payment_id')) { + $payment = wu_get_payment(wu_request('payment_id')); + } + + if ( ! $payment && current_user_can('manage_options')) { + $payment = wu_mock_payment(); + } + + return $payment ?: false; + }Then in both methods:
- $payment = wu_get_payment_by_hash(wu_request('payment')); - - if ( ! $payment && wu_request('payment_id')) { - $payment = wu_get_payment(wu_request('payment_id')); - } - - if ( ! $payment && current_user_can('manage_options')) { - $payment = wu_mock_payment(); - } - - if ( ! $payment) { - return []; - } + $payment = self::resolve_payment_from_request(); + + if ( ! $payment) { + return []; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/models/class-checkout-form.php` around lines 1262 - 1312, The payment lookup logic duplicated in pay_invoice_form_fields() (lines where wu_get_payment_by_hash, wu_get_payment, current_user_can and wu_mock_payment are used) should be extracted into a private static helper, e.g., private static function resolve_payment_from_request(): ?Payment (or appropriate return type); move the three lookup steps into that helper and replace the duplicated block in both pay_invoice_form_fields() and finish_checkout_form_fields() to call self::resolve_payment_from_request(); ensure the helper returns null when no payment found so existing early-return checks (if (! $payment) return []) remain valid.assets/js/checkout.js (1)
789-794: Repeated re-invocation ofinit_password_strengthon every update cycle.When
#pass-strength-resultis absent, this early return prevents settingthis.password_strength_checker. Theupdated()hook (Line 1119) re-callsinit_password_strength()whenever!this.password_strength_checker && jQuery('#field-password').length— so on every Vue update the DOM is queried twice unnecessarily.Consider setting a sentinel so
updated()stops retrying:♻️ Proposed fix
// If the strength meter element doesn't exist, skip validation if (! jQuery('#pass-strength-result').length) { + this.password_strength_checker = false; // sentinel: prevents repeated retries return; } // end if;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/js/checkout.js` around lines 789 - 794, The early return in init_password_strength when jQuery('#pass-strength-result') is missing leaves this.password_strength_checker unset, causing updated() to repeatedly re-query the DOM; fix by setting a sentinel (e.g. this.password_strength_checker = 'missing' or this._password_strength_checked = true) inside init_password_strength when the element is absent and adjust the updated() guard (which currently tests !this.password_strength_checker && jQuery('#field-password').length) to only call init_password_strength when the sentinel is truly uninitialized (e.g. check for undefined or null), so the DOM is not polled on every update.inc/debug/class-debug.php (1)
61-61: Scope footer debug UI to privileged users.This currently injects the button/script on every frontend footer when debug mode is enabled. Consider gating to network admins to reduce accidental exposure.
🔧 Suggested guard
public function add_additional_hooks(): void { add_action('wu_header_left', [$this, 'add_debug_links']); - add_action('wp_footer', [$this, 'render_checkout_autofill_button']); + if (is_user_logged_in() && current_user_can('manage_network')) { + add_action('wp_footer', [$this, 'render_checkout_autofill_button']); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/debug/class-debug.php` at line 61, The footer debug UI is being injected unconditionally via add_action('wp_footer', [$this, 'render_checkout_autofill_button']); — restrict this to privileged users by only registering the hook (or early-returning in render_checkout_autofill_button) when the current user is a network admin/super admin: wrap the add_action call (or add a capability check at the top of render_checkout_autofill_button) with is_user_logged_in() && (current_user_can('manage_network') || is_super_admin()), so the button/script is only output for privileged users such as network admins.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/admin-pages/class-payment-list-admin-page.php`:
- Around line 358-497: In handle_send_invoice_modal: sanitize the incoming
products_raw (cast to string and run through sanitize_text_field before explode)
and validate membership_id when non-zero by loading the membership (e.g.
$membership = wu_get_membership($membership_id)) and returning a
wp_send_json_error if the membership doesn't exist or does not belong to the
selected $customer (compare its owner/customer id to $customer->get_id()); also
replace the uniqid() used for fee line 'hash' with a less collision-prone call
such as wp_unique_id() or uniqid('', true). Ensure you reference $products_raw,
membership_id, handle_send_invoice_modal, and the Line_Item creation block when
making these changes.
In `@inc/admin-pages/class-top-admin-nav-menu.php`:
- Around line 198-251: Short-circuit creation of settings/addon admin bar nodes
when the current user lacks the settings read capability by wrapping the entire
block that iterates $settings_tabs and the subsequent if ($addon_tabs) block in
a check for wu_read_settings() (or return early if !wu_read_settings()).
Specifically, guard the foreach ($settings_tabs as $tab => $tab_info) { ... }
loop and the addon-tabs generation that follows so nodes are only added when
wu_read_settings() is true; reference the $settings_tabs/$addon_tabs variables
and the $wp_admin_bar->add_node(...) calls in this file/class to locate and
protect the code.
In `@inc/debug/class-debug.php`:
- Around line 975-990: The code currently relies on the private property
form.__vue__ and hard-codes plan ID 2 when calling vue.add_plan(2); update it to
avoid using Vue internals by first verifying form and that form.__vue__ (or a
safer public ref if available) exists and that vue.add_plan is a callable
function, then determine a plan ID dynamically (e.g., inspect vue.plans or
vue.availablePlans and pick the first entry's id, or fall back to using a safe
default only if none found) before calling vue.add_plan(planId); also keep the
subsequent assignments to vue.username, vue.email_address, vue.site_title,
vue.subdomain_url and vue.gateway unchanged but guard them behind the same
existence check for vue to avoid runtime errors.
In `@inc/gateways/class-base-stripe-gateway.php`:
- Around line 829-836: The Stripe portal session creation can fail when the
cached option wu_stripe_portal_config_id is stale; modify the code around the
portal session creation (the logic that reads
get_option('wu_stripe_portal_config_id') and calls Stripe to create a portal
session) to wrap the session creation in a try/catch, and on catching a
config-specific Stripe error (invalid or not_found for the portal config) call
delete_option('wu_stripe_portal_config_id') to invalidate the cache and then
retry the session creation one time; if the retry still fails, surface the
original error as before. Ensure you reference the existing option key
wu_stripe_portal_config_id and only retry once to avoid loops.
- Around line 2096-2101: The code is reading
$s_line_item->taxes[0]->tax_behavior without ensuring taxes exists which can
cause an NPE; in the array construction that sets 'tax_inclusive' (inside
class-base-stripe-gateway.php where $s_line_item is used) guard that access by
checking the taxes array/object first (e.g. use !empty($s_line_item->taxes) or
isset($s_line_item->taxes[0]->tax_behavior)) and only compare to 'inclusive'
when present, otherwise default 'tax_inclusive' to false.
In `@inc/gateways/class-stripe-checkout-gateway.php`:
- Around line 342-361: The downgrade branch sets
$subscription_data['subscription_data']['trial_end'] based on
$this->order->get_billing_next_charge_date(), but it's currently always
overwritten by the subsequent if ($this->order->has_trial()) which assigns
$this->order->get_billing_start_date(); decide the intended precedence and
implement it: if downgrade should win, change the trial block to elseif
($this->order->has_trial()) so it only runs when cart type is not 'downgrade';
if trial should win, remove or guard the downgrade block to avoid dead code (or
explicitly document the precedence) — update the conditional around
$subscription_data['subscription_data']['trial_end'] accordingly.
- Around line 334-335: The code currently always sets
$subscription_data['subscription_data'] = [] causing an empty object to be sent
to Stripe; change the logic in the method that builds $subscription_data so that
you only add the 'subscription_data' key when you actually populate it (e.g.,
when handling downgrade trials or trial periods in the blocks that currently set
that array), or remove the key from $subscription_data before the Checkout
creation if it is empty; update references to $subscription_data and its
'subscription_data' subkey so the Checkout payload omits that key entirely when
no fields were set.
In `@inc/helpers/validation-rules/class-exists.php`:
- Around line 56-59: The guard that currently uses empty($value) in the
class-exists validation allows false and arrays to bypass the existence check;
change it to only treat the explicit "no association" values as skip cases
(check $value === '' || $value === null || $value === 0 || $value === '0') so
malformed inputs like false or [] still run through the existence logic in the
class-exists validator.
In `@inc/loaders/class-table-loader.php`:
- Around line 198-199: The debug reset currently clears only wu_domain_mappings
and misses the new domain meta table introduced as Domains_Meta_Table
(referenced via $this->domainmeta_table); update the debug reset flow (the reset
methods in the Debug class such as reset_fake_data()/reset_all_data() or the
routine that truncates wu tables) to also clean the wu_domainmeta rows by
invoking the same deletion/truncate logic for $this->domainmeta_table or calling
its appropriate delete_all()/truncate method so orphaned wu_domainmeta entries
are removed during "reset fake/all data".
In `@inc/managers/class-event-manager.php`:
- Around line 504-510: Remove the hard-coded 'payment_gateway' => 'stripe' in
the payload closure and instead populate payment_gateway from the generated
payment data: derive the gateway value from the payment payload produced by
wu_generate_event_payload (e.g., the merged payload's payment/gateway field or
by calling wu_generate_event_payload('payment') and pulling its gateway
identifier). Update the payload closure (the fn() => array_merge(...) block) to
compute payment_gateway dynamically so sample data reflects the actual gateway
for non-Stripe payments.
In `@inc/managers/class-payment-manager.php`:
- Around line 110-115: The membership block is merging a customer payload
unconditionally which allows wu_generate_event_payload('customer',
$membership->get_customer()) to return mock data when get_customer() is falsy;
change the logic in the membership handling (the code around $membership and the
array_merge) to first fetch $customer = $membership->get_customer() and only
call/merge wu_generate_event_payload('customer', $customer) if $customer is
truthy (and optionally of the expected class/interface), otherwise skip merging
the customer payload so no mock customer data is injected into payment_received
events.
In `@patches/mpdf-psr-log-aware-trait-void-return.patch`:
- Around line 1-3: The patch header is malformed: it mixes a creation marker
(--- /dev/null) with a modification hunk and uses a relative path that escapes
the package root; fix patches/mpdf-psr-log-aware-trait-void-return.patch by
making the file operation consistent—either create the file (use both ---
/dev/null and +++ a/src/MpdfPsrLogAwareTrait.php or +++
b/src/MpdfPsrLogAwareTrait.php and an add hunk like @@ -0,0 +1,<n> @@) or modify
it (use --- a/src/MpdfPsrLogAwareTrait.php and +++
b/src/MpdfPsrLogAwareTrait.php with correct @@ ranges); ensure the path is
normalized to src/MpdfPsrLogAwareTrait.php (no ../) and update all other hunks
(lines 7-8 referenced) to match the chosen mode so Composer patching succeeds.
In `@readme.txt`:
- Line 4: Update the Tags line to use the correct trademark casing for
WordPress: replace the token "wordpress multisite" with "WordPress multisite"
(and ensure any other occurrences of the lowercase "wordpress" in the Tags value
such as "wordpress multisite" are capitalized as "WordPress"). This is a simple
text change in the Tags string on the readme.txt line containing "Tags:
multisite, domain mapping, wordpress multisite, multisite saas, waas".
- Around line 228-230: Update the unreleased changelog entry so the version is
bumped (e.g., change "[2.4.11]" to "[2.4.12]" or next version) and update the
"Stable tag" header to match that new version; also correct the typo in the
entry by changing "striped" to "stripped" and ensure the released entry
"[2.4.11] - Released on 2026-02-16" remains unchanged to avoid duplicate
versions.
In `@tests/WP_Ultimo/Update_Check_Test.php`:
- Line 1: The filename tests/WP_Ultimo/Update_Check_Test.php violates the
project's enforced lowercase-hyphenated naming convention and is blocking CI;
rename the file to a lowercase hyphenated name (e.g.,
class-update-check-test.php), update any references/imports/autoload mappings
that point to Update_Check_Test.php, and ensure the test class name or PHPUnit
bootstrap still matches the new filename (adjust require/include or autoloader
entries that reference Update_Check_Test.php to the new filename).
- Around line 125-133: Reformat the array passed to wp_json_encode so it follows
PHPCS array/function-call layout rules: break the outer array onto multiple
lines with the opening bracket on the same line as wp_json_encode(, put each
top-level key ('plugins' and 'active') on its own line, and ensure the nested
'plugins' array and its item keyed by $this->plugin_file are each on their own
lines and correctly indented; apply the same reformat to the similar block
around $this->plugin_file at lines 189-193 so the test no longer triggers PHPCS
formatting failures.
- Around line 201-205: The test mutates the global setting 'enable_beta_updates'
via wu_save_setting in Update_Check_Test and doesn't restore it; preserve the
original value before calling wu_save_setting('enable_beta_updates', false)
(e.g., $original = wu_get_setting('enable_beta_updates') or equivalent), then
restore it after the assertion (either in a finally block inside the test or in
the test class tearDown method) by calling
wu_save_setting('enable_beta_updates', $original) so the setting state is not
leaked to other tests.
In `@views/dashboard-widgets/thank-you.php`:
- Around line 253-258: The thumbnail in the thank-you widget uses
Site::get_featured_image('thumbnail') which returns a 150×150 image while other
widgets use the default wu-thumb-medium (400×300); either confirm the smaller
size is intentional or change the call in views/dashboard-widgets/thank-you.php
to use the medium size by removing the explicit 'thumbnail' argument (or pass
'wu-thumb-medium') so it matches my-sites/current-site widgets and grid layouts;
update the img src call to use Site::get_featured_image() (or
Site::get_featured_image('wu-thumb-medium')) accordingly and ensure any
CSS/layout still looks correct after the change.
- Line 9: The template views/dashboard-widgets/thank-you.php uses $class_name
but the element class (inc/ui/class-thank-you-element.php) passes the attribute
as className (extracted by wu_get_template()), causing an undefined variable;
fix by making the names consistent—either change the template to use $className
or change the key passed from className to class_name before calling
wu_get_template() (or map it inside wu_get_template()), ensuring the variable
referenced in the template matches the attribute provided.
---
Outside diff comments:
In `@assets/js/integration-test.js`:
- Around line 15-32: The AJAX request for action 'wu_test_hosting_integration'
can hang and leave that.loading true; add a timeout option to the $.ajax call
(e.g., timeout: 10000) and ensure the error callback sets that.loading = false
and handles timeout cases (textStatus === 'timeout') to set that.success = false
and an appropriate that.results message using
wu_integration_test_data.error_message or a timeout-specific message; update the
options object passed to $.ajax in integration-test.js where the AJAX call is
created.
In `@inc/class-cron.php`:
- Around line 255-275: The code builds and dispatches the
renewal_payment_created event regardless of whether the membership status update
succeeded because the return value $saved from $membership->save() is ignored;
modify the flow in the block that calls $membership->save() to check $saved (or
the truthiness of $membership->save()) and only construct $payload and call
wu_do_event('renewal_payment_created', $payload) when the save succeeded,
otherwise bail out or log an error; specifically, guard the payload construction
and the wu_do_event call behind a conditional that verifies $saved (or handles
the failure) so renewal_payment_created is not emitted on failed membership
saves.
In `@inc/functions/helper.php`:
- Around line 67-74: The function wu_get_isset currently type-hints $key as
string which breaks existing callers that pass numeric indices; update the
parameter to accept both strings and integers (e.g., change the type to
string|int or remove the strict string hint) so callers like
wu_get_isset($title, 0, ''), wu_get_isset($logo, 0, false), and
wu_get_isset($memberships, 0, false) no longer throw TypeError, keeping the rest
of wu_get_isset (including casting $array_or_obj to array and returning
$array_or_obj[$key] ?? $default_value) unchanged.
In `@inc/gateways/class-paypal-gateway.php`:
- Around line 1595-1605: The code echoes an error when $checkout_details is not
an array but then continues and dereferences
$checkout_details['pending_payment'], which can cause a crash; update the error
branch in the method in class-paypal-gateway.php to immediately stop further
processing by returning (or otherwise exiting the current method) after echoing
the error so the subsequent use of $checkout_details['pending_payment'] (and
$customer = $checkout_details['pending_payment']->get_customer()) only runs when
$checkout_details is a valid array/object.
In `@inc/sso/auth-functions.php`:
- Around line 192-202: The code currently passes raw $request_uri through
set_url_scheme() into wp_redirect() when it starts with 'http', enabling an
open-redirect; modify the https-upgrade branch in the function that uses
$request_uri, $host, set_url_scheme, wp_redirect and is_ssl to validate the
parsed URL host before redirecting: when str_starts_with($request_uri, 'http')
parse the URL (extract host) and only allow the redirect if the parsed host
equals the sanitized $host (or is in a configured allowlist); if the host does
not match, instead redirect to a safe local path (e.g. '/' or the path component
only) or refuse the redirect; always run the final redirect URL through
wp_sanitize_redirect() before calling wp_redirect().
In `@views/wizards/host-integrations/configuration.php`:
- Around line 35-51: The container using classes "wu-flex wu-justify-between"
drifts the Test Configuration button left when $back_url is falsy because the
left node is omitted; fix by rendering a placeholder element when $back_url is
empty (e.g. an empty <div> or <span> with the same sizing/visibility as the
link) or by conditionally switching the container class to "wu-flex
wu-justify-end" when $back_url is falsy; update the markup around the $back_url
conditional (the <a> block and the surrounding container) so the submit <button>
inside the <span> remains right-aligned in both cases.
---
Nitpick comments:
In `@assets/js/checkout.js`:
- Around line 789-794: The early return in init_password_strength when
jQuery('#pass-strength-result') is missing leaves this.password_strength_checker
unset, causing updated() to repeatedly re-query the DOM; fix by setting a
sentinel (e.g. this.password_strength_checker = 'missing' or
this._password_strength_checked = true) inside init_password_strength when the
element is absent and adjust the updated() guard (which currently tests
!this.password_strength_checker && jQuery('#field-password').length) to only
call init_password_strength when the sentinel is truly uninitialized (e.g. check
for undefined or null), so the DOM is not polled on every update.
In `@assets/js/integration-test.js`:
- Around line 27-31: Update the jQuery AJAX error callback in
assets/js/integration-test.js (the anonymous function that sets that.loading,
that.success, and that.results) to accept the standard parameters (jqXHR,
textStatus, errorThrown), log them (e.g., console.error(jqXHR, textStatus,
errorThrown)) and include a concise diagnostic in that.results when available
(for example append textStatus/errorThrown to the fallback message or use
jqXHR.responseText if present) while preserving the existing fallback to
wu_integration_test_data.error_message || 'Connection test failed. Please try
again.' so debuggability is improved without changing the current behavior.
In `@inc/admin-pages/class-top-admin-nav-menu.php`:
- Around line 211-223: Duplicate construction of the settings-tab node used in
calls to $wp_admin_bar->add_node (the array with keys 'id' =>
'wp-ultimo-settings-' . $tab, 'title' => $tab_info['title'], 'href' =>
network_admin_url(...), 'meta' => ['class' => 'wp-ultimo-top-menu','title' =>
__('Go to the settings page','ultimate-multisite')]) should be extracted into a
small helper or closure inside the class (e.g., a private method
build_settings_tab_node($tab, $tab_info, $parent) or $make_node =
function($tab,$tab_info,$parent){...}) that returns the node array, then replace
both $wp_admin_bar->add_node(...) calls to call
$wp_admin_bar->add_node($this->build_settings_tab_node($tab,$tab_info,'wp-ultimo-settings'))
and the other with the different parent; keep the same id/title/href/meta values
(including 'wp-ultimo-top-menu') so behavior is unchanged.
In `@inc/admin-pages/customer-panel/class-account-admin-page.php`:
- Around line 141-147: Normalize and whitelist the request-derived $update_type
before passing it into apply_filters('wu_account_update_message', ...) to avoid
untrusted values reaching filter callbacks: sanitize the incoming value (e.g.,
cast to string and use sanitize_key or similar), check it against an allowlist
of expected types (e.g., 'payment_method', 'password', 'email' — include a
sensible default like 'general' or '' when not matched), then use that
sanitized/whitelisted variable in the apply_filters call (referencing the
$update_type variable and the wu_account_update_message filter) so only known
contexts are ever provided to filter handlers.
In `@inc/debug/class-debug.php`:
- Line 61: The footer debug UI is being injected unconditionally via
add_action('wp_footer', [$this, 'render_checkout_autofill_button']); — restrict
this to privileged users by only registering the hook (or early-returning in
render_checkout_autofill_button) when the current user is a network admin/super
admin: wrap the add_action call (or add a capability check at the top of
render_checkout_autofill_button) with is_user_logged_in() &&
(current_user_can('manage_network') || is_super_admin()), so the button/script
is only output for privileged users such as network admins.
In `@inc/integrations/capabilities/interface-node-management-capability.php`:
- Line 138: The method name install_deps is inconsistent with the project's
full-word naming convention; rename the interface method to install_dependencies
and update all references and implementations (e.g., any classes implementing
the interface and any callers of install_deps) to use install_dependencies,
adjust method signatures (public function install_dependencies(string $app_id):
array) and run tests/grep to ensure no remaining install_deps usages remain.
- Around line 24-139: Add a public constant ID to the Node_Management_Capability
interface for consistency with sibling interfaces (e.g.,
Domain_Selling_Capability); declare public const ID = 'node-management' inside
the Node_Management_Capability interface (near the top, after the interface
declaration) so implementations can reference the capability by a stable name
while keeping get_capability_id() behavior unchanged.
In `@inc/models/class-checkout-form.php`:
- Around line 1262-1312: The payment lookup logic duplicated in
pay_invoice_form_fields() (lines where wu_get_payment_by_hash, wu_get_payment,
current_user_can and wu_mock_payment are used) should be extracted into a
private static helper, e.g., private static function
resolve_payment_from_request(): ?Payment (or appropriate return type); move the
three lookup steps into that helper and replace the duplicated block in both
pay_invoice_form_fields() and finish_checkout_form_fields() to call
self::resolve_payment_from_request(); ensure the helper returns null when no
payment found so existing early-return checks (if (! $payment) return []) remain
valid.
In `@inc/sso/class-sso.php`:
- Around line 385-389: The current replacement uses a dummy base and str_replace
which can accidentally remove the literal "https://a.com/" if it appears in the
query string; change the logic so you do not prepend a dummy URL and instead
extract only the path+query portion before calling remove_query_arg and
assigning back to $_SERVER['REQUEST_URI']—use wp_unslash($_SERVER['REQUEST_URI']
?? '') to get the raw request, parse or trim any leading scheme/host if present
(e.g. via wp_parse_url or parsing only path+query), call remove_query_arg('sso',
<path+query>) and then set $_SERVER['REQUEST_URI'] to that result (no outer
str_replace). Ensure the code references the same symbols:
$_SERVER['REQUEST_URI'], wp_unslash, remove_query_arg and remove the str_replace
dummy-base approach.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
assets/js/checkout.min.jsis excluded by!**/*.min.jsassets/js/integration-test.min.jsis excluded by!**/*.min.jscomposer.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
.wiki/how-can-i-cancel-my-subscription.mdassets/js/checkout.jsassets/js/integration-test.jscomposer.jsoninc/admin-pages/class-checkout-form-edit-admin-page.phpinc/admin-pages/class-customer-edit-admin-page.phpinc/admin-pages/class-discount-code-edit-admin-page.phpinc/admin-pages/class-domain-edit-admin-page.phpinc/admin-pages/class-email-edit-admin-page.phpinc/admin-pages/class-hosting-integration-wizard-admin-page.phpinc/admin-pages/class-payment-edit-admin-page.phpinc/admin-pages/class-payment-list-admin-page.phpinc/admin-pages/class-product-edit-admin-page.phpinc/admin-pages/class-top-admin-nav-menu.phpinc/admin-pages/customer-panel/class-account-admin-page.phpinc/checkout/class-cart.phpinc/class-cron.phpinc/class-wp-ultimo.phpinc/database/domains/class-domains-meta-table.phpinc/debug/class-debug.phpinc/functions/checkout-form.phpinc/functions/helper.phpinc/gateways/class-base-gateway.phpinc/gateways/class-base-stripe-gateway.phpinc/gateways/class-paypal-gateway.phpinc/gateways/class-stripe-checkout-gateway.phpinc/helpers/validation-rules/class-exists.phpinc/installers/class-multisite-network-installer.phpinc/integrations/capabilities/interface-domain-selling-capability.phpinc/integrations/capabilities/interface-node-management-capability.phpinc/integrations/providers/cloudflare/class-cloudflare-integration.phpinc/integrations/providers/cloudways/class-cloudways-integration.phpinc/integrations/providers/cpanel/class-cpanel-integration.phpinc/integrations/providers/hestia/class-hestia-integration.phpinc/integrations/providers/rocket/class-rocket-integration.phpinc/integrations/providers/runcloud/class-runcloud-integration.phpinc/integrations/providers/serverpilot/class-serverpilot-integration.phpinc/list-tables/class-domain-list-table.phpinc/loaders/class-table-loader.phpinc/managers/class-email-manager.phpinc/managers/class-event-manager.phpinc/managers/class-payment-manager.phpinc/models/class-checkout-form.phpinc/models/class-payment.phpinc/sso/auth-functions.phpinc/sso/class-sso.phpinc/ui/class-payment-methods-element.phpinc/ui/class-site-actions-element.phppatches/mpdf-psr-log-aware-trait-void-return.patchreadme.txttests/WP_Ultimo/Admin_Pages/Payment_List_Admin_Page_Test.phptests/WP_Ultimo/Update_Check_Test.phpviews/dashboard-widgets/domain-mapping.phpviews/dashboard-widgets/payment-methods.phpviews/dashboard-widgets/thank-you.phpviews/emails/admin/membership-expired.phpviews/emails/admin/payment-failed.phpviews/emails/customer/membership-expired.phpviews/emails/customer/payment-failed.phpviews/settings/fields/field-select2.phpviews/wizards/host-integrations/configuration.php
💤 Files with no reviewable changes (1)
- .wiki/how-can-i-cancel-my-subscription.md
Comment on lines
+358
to
+497
| public function handle_send_invoice_modal(): void { | ||
|
|
||
| $customer_id = absint(wu_request('customer_id')); | ||
| $customer = wu_get_customer($customer_id); | ||
|
|
||
| if ( ! $customer) { | ||
| wp_send_json_error(new \WP_Error('invalid-customer', __('Please select a valid customer.', 'ultimate-multisite'))); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| $membership_id = absint(wu_request('membership_id', 0)); | ||
| $products_raw = wu_request('products', ''); | ||
| $custom_items = wu_request('custom_items', ''); | ||
|
|
||
| /* | ||
| * Build line items from products. | ||
| */ | ||
| $line_items = []; | ||
|
|
||
| if ( ! empty($products_raw)) { | ||
| $product_ids = array_filter(array_map('absint', explode(',', (string) $products_raw))); | ||
|
|
||
| foreach ($product_ids as $product_id) { | ||
| $product = wu_get_product($product_id); | ||
|
|
||
| if ( ! $product) { | ||
| continue; | ||
| } | ||
|
|
||
| $line_items[] = new \WP_Ultimo\Checkout\Line_Item( | ||
| [ | ||
| 'product' => $product, | ||
| 'quantity' => 1, | ||
| 'unit_price' => $product->get_amount(), | ||
| 'title' => $product->get_name(), | ||
| ] | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * Build line items from custom entries. | ||
| */ | ||
| if ( ! empty($custom_items)) { | ||
| $lines = array_filter(array_map('trim', explode("\n", (string) $custom_items))); | ||
|
|
||
| foreach ($lines as $line) { | ||
| $parts = array_map('trim', explode('|', $line)); | ||
| $title = $parts[0] ?? ''; | ||
| $unit_price = isset($parts[1]) ? wu_to_float($parts[1]) : 0; | ||
| $quantity = isset($parts[2]) ? absint($parts[2]) : 1; | ||
|
|
||
| if (empty($title) || $unit_price <= 0) { | ||
| continue; | ||
| } | ||
|
|
||
| $line_items[] = new \WP_Ultimo\Checkout\Line_Item( | ||
| [ | ||
| 'type' => 'fee', | ||
| 'hash' => uniqid(), | ||
| 'title' => sanitize_text_field($title), | ||
| 'unit_price' => $unit_price, | ||
| 'quantity' => max(1, $quantity), | ||
| ] | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (empty($line_items)) { | ||
| wp_send_json_error(new \WP_Error('no-items', __('Please add at least one product or custom line item.', 'ultimate-multisite'))); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| /* | ||
| * Calculate totals from line items. | ||
| */ | ||
| $subtotal = 0; | ||
| $tax_total = 0; | ||
| $total = 0; | ||
|
|
||
| foreach ($line_items as $line_item) { | ||
| $line_item->recalculate_totals(); | ||
|
|
||
| $subtotal += $line_item->get_subtotal(); | ||
| $tax_total += $line_item->get_tax_total(); | ||
| $total += $line_item->get_total(); | ||
| } | ||
|
|
||
| /* | ||
| * Create the pending payment. | ||
| */ | ||
| $payment_data = [ | ||
| 'customer_id' => $customer->get_id(), | ||
| 'membership_id' => $membership_id, | ||
| 'status' => Payment_Status::PENDING, | ||
| 'subtotal' => $subtotal, | ||
| 'tax_total' => $tax_total, | ||
| 'total' => $total, | ||
| 'line_items' => $line_items, | ||
| ]; | ||
|
|
||
| $payment = wu_create_payment($payment_data); | ||
|
|
||
| if (is_wp_error($payment)) { | ||
| wp_send_json_error($payment); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| /* | ||
| * Fire the invoice_sent event. | ||
| */ | ||
| $send_notification = wu_request('send_notification'); | ||
|
|
||
| if ($send_notification) { | ||
| $payload = array_merge( | ||
| wu_generate_event_payload('payment', $payment), | ||
| wu_generate_event_payload('customer', $customer), | ||
| [ | ||
| 'payment_url' => $payment->get_payment_url(), | ||
| 'invoice_message' => sanitize_textarea_field(wu_request('invoice_message', '')), | ||
| ] | ||
| ); | ||
|
|
||
| wu_do_event('invoice_sent', $payload); | ||
| } | ||
|
|
||
| wp_send_json_success( | ||
| [ | ||
| 'redirect_url' => wu_network_admin_url( | ||
| 'wp-ultimo-edit-payment', | ||
| [ | ||
| 'id' => $payment->get_id(), | ||
| ] | ||
| ), | ||
| ] | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Consider sanitizing products_raw and validating membership_id when non-zero.
A couple of observations on the handler:
-
Line 369: When
membership_idis non-zero, there's no validation that the membership actually exists or belongs to the selected customer. This could create an orphaned payment–membership link if an invalid ID is submitted. -
Line 418:
uniqid()is fine for a non-security hash, but note it can produce duplicates under high concurrency. If that matters,wp_unique_id()oruniqid('', true)would be safer.
The overall flow — validate customer → build line items → calculate totals → create payment → optionally fire event — is solid.
Suggested membership validation when non-zero
$membership_id = absint(wu_request('membership_id', 0));
+
+ if ($membership_id) {
+ $membership = wu_get_membership($membership_id);
+
+ if ( ! $membership || $membership->get_customer_id() !== $customer->get_id()) {
+ wp_send_json_error(new \WP_Error('invalid-membership', __('The selected membership does not belong to this customer.', 'ultimate-multisite')));
+
+ return;
+ }
+ }
+
$products_raw = wu_request('products', '');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/admin-pages/class-payment-list-admin-page.php` around lines 358 - 497, In
handle_send_invoice_modal: sanitize the incoming products_raw (cast to string
and run through sanitize_text_field before explode) and validate membership_id
when non-zero by loading the membership (e.g. $membership =
wu_get_membership($membership_id)) and returning a wp_send_json_error if the
membership doesn't exist or does not belong to the selected $customer (compare
its owner/customer id to $customer->get_id()); also replace the uniqid() used
for fee line 'hash' with a less collision-prone call such as wp_unique_id() or
uniqid('', true). Ensure you reference $products_raw, membership_id,
handle_send_invoice_modal, and the Line_Item creation block when making these
changes.
Comment on lines
+975
to
+990
| var vue = form ? form.__vue__ : null; | ||
|
|
||
| if (!vue) { | ||
| return; | ||
| } | ||
|
|
||
| // Select paid plan via Vue method | ||
| vue.add_plan(2); | ||
|
|
||
| // Set all fields | ||
| vue.username = 'user' + r; | ||
| vue.email_address = 'user' + r + '@test.com'; | ||
| vue.site_title = 'Test Site ' + r; | ||
| vue.subdomain_url = 'site' + r; | ||
| vue.gateway = 'stripe'; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file and verify the code at the specified lines
head -n 995 inc/debug/class-debug.php | tail -n 30Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 941
🏁 Script executed:
# Search for __vue__ usage patterns
rg -n '__vue__' --type phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 151
🏁 Script executed:
# Search for add_plan function and plans array
rg -n 'add_plan|\.plans' --type php -C 3Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2849
🏁 Script executed:
# Search for form initialization and gateway configuration
rg -n 'form\.__vue__|gateway|stripe' inc/debug/class-debug.php -C 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2212
🏁 Script executed:
# Check if plan ID 2 is hardcoded elsewhere or if plans are defined
rg -n "add_plan\s*\(\s*2\s*\)|plan.*2|plans\[" --type php -C 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 6932
🏁 Script executed:
# Look for checkout form structure and plan definitions in the entire codebase
rg -n 'wu_form|form.*plans|gateway.*stripe' --type php --type js -C 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 50394
Address fragile Vue internals and hard-coded plan ID in debug autofill.
Line 975 accesses form.__vue__, which relies on Vue's private implementation details and can break across Vue versions. More importantly, line 982 hard-codes plan ID 2—this will fail if that plan doesn't exist in the installation, especially after migrations or customizations where plan IDs differ.
Implement the safer approach: check that add_plan is callable and available, then dynamically select the first available plan instead of assuming a fixed ID exists.
🛠️ Safer fallback approach
- var vue = form ? form.__vue__ : null;
+ var vue = form && form.__vue__ ? form.__vue__ : null;
if (!vue) {
return;
}
- // Select paid plan via Vue method
- vue.add_plan(2);
+ // Select first available plan if API exists
+ if (typeof vue.add_plan === 'function' && Array.isArray(vue.plans) && vue.plans.length) {
+ var firstPlanId = vue.plans[0].id || vue.plans[0].value || null;
+ if (firstPlanId) {
+ vue.add_plan(firstPlanId);
+ }
+ }📝 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.
Suggested change
| var vue = form ? form.__vue__ : null; | |
| if (!vue) { | |
| return; | |
| } | |
| // Select paid plan via Vue method | |
| vue.add_plan(2); | |
| // Set all fields | |
| vue.username = 'user' + r; | |
| vue.email_address = 'user' + r + '@test.com'; | |
| vue.site_title = 'Test Site ' + r; | |
| vue.subdomain_url = 'site' + r; | |
| vue.gateway = 'stripe'; | |
| var vue = form && form.__vue__ ? form.__vue__ : null; | |
| if (!vue) { | |
| return; | |
| } | |
| // Select first available plan if API exists | |
| if (typeof vue.add_plan === 'function' && Array.isArray(vue.plans) && vue.plans.length) { | |
| var firstPlanId = vue.plans[0].id || vue.plans[0].value || null; | |
| if (firstPlanId) { | |
| vue.add_plan(firstPlanId); | |
| } | |
| } | |
| // Set all fields | |
| vue.username = 'user' + r; | |
| vue.email_address = 'user' + r + '@test.com'; | |
| vue.site_title = 'Test Site ' + r; | |
| vue.subdomain_url = 'site' + r; | |
| vue.gateway = 'stripe'; | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/debug/class-debug.php` around lines 975 - 990, The code currently relies
on the private property form.__vue__ and hard-codes plan ID 2 when calling
vue.add_plan(2); update it to avoid using Vue internals by first verifying form
and that form.__vue__ (or a safer public ref if available) exists and that
vue.add_plan is a callable function, then determine a plan ID dynamically (e.g.,
inspect vue.plans or vue.availablePlans and pick the first entry's id, or fall
back to using a safe default only if none found) before calling
vue.add_plan(planId); also keep the subsequent assignments to vue.username,
vue.email_address, vue.site_title, vue.subdomain_url and vue.gateway unchanged
but guard them behind the same existence check for vue to avoid runtime errors.
Comment on lines
+829
to
836
| $stored_redirect = get_user_meta(get_current_user_id(), '_wu_change_payment_redirect', true); | ||
|
|
||
| $session = $this->get_stripe_client()->checkout->sessions->create($subscription_data); | ||
| $s_customer_id = $session->customer; | ||
| if ($stored_redirect) { | ||
| delete_user_meta(get_current_user_id(), '_wu_change_payment_redirect'); | ||
| $return_url = add_query_arg('updated', 'payment_method', $stored_redirect); | ||
| } else { | ||
| $return_url = remove_query_arg('wu-stripe-portal', wu_get_current_url()); | ||
| } |
Contributor
There was a problem hiding this comment.
Stale portal config ID can cause repeated failures.
Line 867-905: If the Stripe portal configuration is deleted remotely (e.g., via the Stripe dashboard), the cached wu_stripe_portal_config_id site option becomes stale. Every portal redirect attempt will fail at line 913 and show a wp_die error. Consider invalidating the cached ID on failure and retrying once:
Suggested retry on stale portal config
+ $retry = false;
+ create_portal:
try {
// ...existing code...
$portal_config_id = get_site_option('wu_stripe_portal_config_id');
if ( ! $portal_config_id) {
- $portal_config = $this->get_stripe_client()->billingPortal->configurations->create(
+ // ...create portal config...
+ }
$subscription_data = [
'return_url' => $return_url,
'customer' => $s_customer_id,
'configuration' => $portal_config_id,
];
$session = $this->get_stripe_client()->billingPortal->sessions->create($subscription_data);
wp_safe_redirect($session->url);
exit;
} catch (\Throwable $e) {
+ if ( ! $retry && $portal_config_id) {
+ delete_site_option('wu_stripe_portal_config_id');
+ $retry = true;
+ goto create_portal;
+ }
+
wp_die(
esc_html($e->getMessage()),A cleaner alternative is to wrap the session creation in its own try/catch and delete the cached option on config-specific errors.
Also applies to: 867-905
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/gateways/class-base-stripe-gateway.php` around lines 829 - 836, The
Stripe portal session creation can fail when the cached option
wu_stripe_portal_config_id is stale; modify the code around the portal session
creation (the logic that reads get_option('wu_stripe_portal_config_id') and
calls Stripe to create a portal session) to wrap the session creation in a
try/catch, and on catching a config-specific Stripe error (invalid or not_found
for the portal config) call delete_option('wu_stripe_portal_config_id') to
invalidate the cache and then retry the session creation one time; if the retry
still fails, surface the original error as before. Ensure you reference the
existing option key wu_stripe_portal_config_id and only retry once to avoid
loops.
| @@ -0,0 +1,261 @@ | |||
| <?php | |||
Contributor
There was a problem hiding this comment.
Pipeline blocker: filename violates enforced naming rules.
CI is failing on this filename; rename it to a lowercase, hyphenated class-file format (for example, class-update-check-test.php) to satisfy the checks.
🧰 Tools
🪛 GitHub Actions: Code Quality
[error] 1-1: Filenames should be all lowercase with hyphens as word separators. Expected update-check-test.php, but found Update_Check_Test.php.
🪛 GitHub Check: Code Quality Checks
[failure] 1-1:
Class file names should be based on the class name with "class-" prepended. Expected class-update-check-test.php, but found Update_Check_Test.php.
[failure] 1-1:
Filenames should be all lowercase with hyphens as word separators. Expected update-check-test.php, but found Update_Check_Test.php.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Update_Check_Test.php` at line 1, The filename
tests/WP_Ultimo/Update_Check_Test.php violates the project's enforced
lowercase-hyphenated naming convention and is blocking CI; rename the file to a
lowercase hyphenated name (e.g., class-update-check-test.php), update any
references/imports/autoload mappings that point to Update_Check_Test.php, and
ensure the test class name or PHPUnit bootstrap still matches the new filename
(adjust require/include or autoloader entries that reference
Update_Check_Test.php to the new filename).
Comment on lines
+201
to
+205
| // Ensure beta updates are disabled (the default) | ||
| wu_save_setting('enable_beta_updates', false); | ||
|
|
||
| $filtered = apply_filters('site_transient_update_plugins', $transient); | ||
|
|
Contributor
There was a problem hiding this comment.
Avoid leaking settings state across tests.
This test mutates enable_beta_updates and does not restore it, which can affect later tests.
💡 Suggested fix
- // Ensure beta updates are disabled (the default)
- wu_save_setting('enable_beta_updates', false);
-
- $filtered = apply_filters('site_transient_update_plugins', $transient);
+ $previous_beta_setting = wu_get_setting('enable_beta_updates', false);
+ wu_save_setting('enable_beta_updates', false);
+ try {
+ $filtered = apply_filters('site_transient_update_plugins', $transient);
+ } finally {
+ wu_save_setting('enable_beta_updates', $previous_beta_setting);
+ }📝 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.
Suggested change
| // Ensure beta updates are disabled (the default) | |
| wu_save_setting('enable_beta_updates', false); | |
| $filtered = apply_filters('site_transient_update_plugins', $transient); | |
| $previous_beta_setting = wu_get_setting('enable_beta_updates', false); | |
| wu_save_setting('enable_beta_updates', false); | |
| try { | |
| $filtered = apply_filters('site_transient_update_plugins', $transient); | |
| } finally { | |
| wu_save_setting('enable_beta_updates', $previous_beta_setting); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/WP_Ultimo/Update_Check_Test.php` around lines 201 - 205, The test
mutates the global setting 'enable_beta_updates' via wu_save_setting in
Update_Check_Test and doesn't restore it; preserve the original value before
calling wu_save_setting('enable_beta_updates', false) (e.g., $original =
wu_get_setting('enable_beta_updates') or equivalent), then restore it after the
assertion (either in a finally block inside the test or in the test class
tearDown method) by calling wu_save_setting('enable_beta_updates', $original) so
the setting state is not leaked to other tests.
Comment on lines
+253
to
258
| <div class="wu-flex-shrink sm:wu-mr-4"> | ||
|
|
||
| <img | ||
| class="wu-w-full sm:wu-w-16 sm:wu-h-16 wu-rounded wu-object-cover" | ||
| src="<?php echo esc_attr($site->get_featured_image('wu-thumb-medium')); ?>" | ||
| class="wu-mb-4 sm:wu-mb-0 wu-rounded" | ||
| src="<?php echo esc_attr($site->get_featured_image('thumbnail')); ?>" | ||
| alt="Thumbnail of Site" /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where wu-thumb-medium is registered
echo "=== Searching for wu-thumb-medium registration ==="
rg -n "wu-thumb-medium" --type=php -B3 -A3
echo ""
echo "=== Searching for get_featured_image method ==="
rg -n "get_featured_image" --type=php -B2 -A10 | head -100Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 10876
Confirm intentionality of thumbnail image size in thank-you widget or align with other widgets.
Site::get_featured_image() defaults to wu-thumb-medium (400×300 px), but this widget explicitly uses thumbnail (150×150 px), resulting in a 62.5% width reduction. Other similar dashboard widgets (my-sites, current-site) and grid layouts use wu-thumb-medium by default. Either confirm this smaller size is intentional for this specific widget, or align it with the standard wu-thumb-medium size used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@views/dashboard-widgets/thank-you.php` around lines 253 - 258, The thumbnail
in the thank-you widget uses Site::get_featured_image('thumbnail') which returns
a 150×150 image while other widgets use the default wu-thumb-medium (400×300);
either confirm the smaller size is intentional or change the call in
views/dashboard-widgets/thank-you.php to use the medium size by removing the
explicit 'thumbnail' argument (or pass 'wu-thumb-medium') so it matches
my-sites/current-site widgets and grid layouts; update the img src call to use
Site::get_featured_image() (or Site::get_featured_image('wu-thumb-medium'))
accordingly and ensure any CSS/layout still looks correct after the change.
- Add plugin symlink in CI workflow so get_plugins()/get_plugin_data() can find the plugin - Exclude tests/ from WordPress.Files.FileName PHPCS rule (PHPUnit uses PascalCase) - Fix PHPCS formatting issues in Update_Check_Test.php Co-Authored-By: Claude Opus 4.6 <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: |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
tests/WP_Ultimo/Update_Check_Test.php (2)
203-207:⚠️ Potential issue | 🟡 MinorRestore
enable_beta_updatesafter the assertion path.This test mutates global settings and does not restore them, which can leak state into subsequent tests.
💡 Proposed fix
- // Ensure beta updates are disabled (the default) - wu_save_setting('enable_beta_updates', false); - - $filtered = apply_filters('site_transient_update_plugins', $transient); + $previous_beta_setting = wu_get_setting('enable_beta_updates', false); + wu_save_setting('enable_beta_updates', false); + try { + $filtered = apply_filters('site_transient_update_plugins', $transient); + } finally { + wu_save_setting('enable_beta_updates', $previous_beta_setting); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Update_Check_Test.php` around lines 203 - 207, This test mutates the global setting via wu_save_setting('enable_beta_updates', false) and doesn't restore it; capture the original value before changing it, set enable_beta_updates to false for the assertion, then restore the original value after apply_filters('site_transient_update_plugins', $transient) (preferably in a finally block or in the test tearDown) so subsequent tests don't see leaked state.
1-1:⚠️ Potential issue | 🔴 CriticalRename this test file to match enforced filename conventions.
The current filename still violates the project naming rule and can keep code-quality checks failing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/WP_Ultimo/Update_Check_Test.php` at line 1, The test file name violates the project's filename conventions; rename the file so it matches the test class name and the project's naming rules (for example, change WP_Ultimo/Update_Check_Test.php to a convention-compliant name such as WP_Ultimo/UpdateCheckTest.php or rename the test class to match the existing filename), ensuring the filename and the test class (e.g., Update_Check_Test) follow the enforced convention so CI/code-quality checks pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/WP_Ultimo/Update_Check_Test.php`:
- Around line 203-207: This test mutates the global setting via
wu_save_setting('enable_beta_updates', false) and doesn't restore it; capture
the original value before changing it, set enable_beta_updates to false for the
assertion, then restore the original value after
apply_filters('site_transient_update_plugins', $transient) (preferably in a
finally block or in the test tearDown) so subsequent tests don't see leaked
state.
- Line 1: The test file name violates the project's filename conventions; rename
the file so it matches the test class name and the project's naming rules (for
example, change WP_Ultimo/Update_Check_Test.php to a convention-compliant name
such as WP_Ultimo/UpdateCheckTest.php or rename the test class to match the
existing filename), ensuring the filename and the test class (e.g.,
Update_Check_Test) follow the enforced convention so CI/code-quality checks
pass.
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: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/helpers/validation-rules/class-exists.php`:
- Around line 56-59: Change the equality checks in the early-return if-condition
to Yoda-style comparisons so static analysis passes: replace ($value === null ||
$value === '' || $value === 0 || $value === '0') with the constants on the left
(null === $value, '' === $value, 0 === $value, '0' === $value) inside the same
conditional in the class-exists validation rule where $value is checked, leaving
the return true behavior intact.
Comment on lines
+56
to
+59
| // Allow explicit "no association" sentinels for optional foreign keys. | ||
| if ($value === null || $value === '' || $value === 0 || $value === '0') { | ||
| return true; | ||
| } |
Contributor
There was a problem hiding this comment.
Use Yoda conditions to satisfy the coding standard.
The logic fix is correct—using strict equality checks instead of empty() properly handles the "no association" sentinels. However, the static analysis requires Yoda-style conditions (constant on the left side) to pass CI checks.
🔧 Proposed fix for Yoda conditions
// Allow explicit "no association" sentinels for optional foreign keys.
- if ($value === null || $value === '' || $value === 0 || $value === '0') {
+ if (null === $value || '' === $value || 0 === $value || '0' === $value) {
return true;
}🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 57-57:
Use Yoda Condition checks, you must.
[failure] 57-57:
Use Yoda Condition checks, you must.
[failure] 57-57:
Use Yoda Condition checks, you must.
[failure] 57-57:
Use Yoda Condition checks, you must.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/helpers/validation-rules/class-exists.php` around lines 56 - 59, Change
the equality checks in the early-return if-condition to Yoda-style comparisons
so static analysis passes: replace ($value === null || $value === '' || $value
=== 0 || $value === '0') with the constants on the left (null === $value, '' ===
$value, 0 === $value, '0' === $value) inside the same conditional in the
class-exists validation rule where $value is checked, leaving the return true
behavior intact.
Co-Authored-By: Claude Opus 4.6 <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: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
readme.txt (1)
4-4:⚠️ Potential issue | 🟡 MinorUse "WordPress" (capital P) in the Tags field.
The lowercase "wordpress multisite" should be "WordPress multisite" per WordPress.org trademark guidelines.
📝 Proposed fix
-Tags: multisite, domain mapping, wordpress multisite, multisite saas, waas +Tags: multisite, domain mapping, WordPress multisite, multisite saas, waasAs per coding guidelines: the static analysis tool (LanguageTool) flags this as a violation of the official WordPress name casing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@readme.txt` at line 4, Update the Tags value to use the correct WordPress casing by replacing the token "wordpress multisite" with "WordPress multisite" in the Tags field (the tags string containing "multisite, domain mapping, wordpress multisite, multisite saas, waas"); ensure the capitalization matches the official WordPress trademark style.
🧹 Nitpick comments (1)
readme.txt (1)
249-249: Consider using "Add-on" for consistency.The file predominantly uses the hyphenated form "add-on" (lines 47, 192, 222), but line 249 uses "Addon". For consistency, consider using "Add-on settings" here.
✏️ Proposed fix
-- Improved: Addon settings grouped under dedicated admin bar submenu. +- Improved: Add-on settings grouped under dedicated admin bar submenu.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@readme.txt` at line 249, Replace the inconsistent "Addon" token in the changelog entry "Improved: Addon settings grouped under dedicated admin bar submenu." with the hyphenated form "Add-on" to match the rest of the document (e.g., lines using "add-on"); update the string exactly so it reads "Improved: Add-on settings grouped under dedicated admin bar submenu." ensuring consistent capitalization and hyphenation across the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/admin-pages/class-top-admin-nav-menu.php`:
- Around line 214-217: The node ID and settings URL build use the raw
filter-derived $tab (in the array where 'id' => 'wp-ultimo-settings-' . $tab and
'href' => network_admin_url('admin.php?page=wp-ultimo-settings&tab=') . $tab),
which can produce invalid IDs/URLs; sanitize and normalize $tab before use
(e.g., cast to a slug-like string for IDs) and build the URL with add_query_arg
to ensure proper encoding (use the normalized tab when creating the fragment/ID
for the node and pass the original/sanitized value to add_query_arg for the
href). Apply the same normalization and add_query_arg pattern to the other
occurrence around lines 243-246.
In `@inc/gateways/class-stripe-checkout-gateway.php`:
- Around line 354-359: The formatting error is caused by a newline between a
closing brace and the subsequent elseif; in the method containing the
conditional that checks $this->order->has_trial() (where the current line shows
"elseif ($this->order->has_trial())"), move the elseif up to the same line as
the preceding closing brace or otherwise ensure there is exactly one space after
the closing brace before the elseif so the pattern becomes "} elseif (...)" to
satisfy the static analyzer.
---
Duplicate comments:
In `@readme.txt`:
- Line 4: Update the Tags value to use the correct WordPress casing by replacing
the token "wordpress multisite" with "WordPress multisite" in the Tags field
(the tags string containing "multisite, domain mapping, wordpress multisite,
multisite saas, waas"); ensure the capitalization matches the official WordPress
trademark style.
---
Nitpick comments:
In `@readme.txt`:
- Line 249: Replace the inconsistent "Addon" token in the changelog entry
"Improved: Addon settings grouped under dedicated admin bar submenu." with the
hyphenated form "Add-on" to match the rest of the document (e.g., lines using
"add-on"); update the string exactly so it reads "Improved: Add-on settings
grouped under dedicated admin bar submenu." ensuring consistent capitalization
and hyphenation across the file.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
inc/admin-pages/class-top-admin-nav-menu.phpinc/gateways/class-base-stripe-gateway.phpinc/gateways/class-stripe-checkout-gateway.phpinc/ui/class-tours.phpreadme.txtviews/dashboard-widgets/thank-you.php
🚧 Files skipped from review as they are similar to previous changes (1)
- views/dashboard-widgets/thank-you.php
Comment on lines
+214
to
+217
| 'id' => 'wp-ultimo-settings-' . $tab, | ||
| 'parent' => 'wp-ultimo-settings', | ||
| 'title' => $tab_info['title'], | ||
| 'href' => network_admin_url('admin.php?page=wp-ultimo-settings&tab=') . $tab, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists
fd "class-top-admin-nav-menu.php" --type fRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 125
🏁 Script executed:
# Read the relevant sections of the file to verify the lines
wc -l inc/admin-pages/class-top-admin-nav-menu.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 129
🏁 Script executed:
# Read lines 200-260 to see both mentioned locations
sed -n '200,260p' inc/admin-pages/class-top-admin-nav-menu.php | cat -nRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1924
🏁 Script executed:
# Find where Settings::get_instance()->get_sections() is defined
fd "class.*[Ss]ettings" --type f -e php | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 186
🏁 Script executed:
# Search for get_sections method
rg "function\s+get_sections" -A 10 --type phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 7671
🏁 Script executed:
# Check for apply_filters or do_action calls on sections
rg "apply_filters.*section" --type php -iRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1436
🏁 Script executed:
# Read the full file to see exact line numbers
head -n 260 inc/admin-pages/class-top-admin-nav-menu.php | tail -n +190 | cat -nRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2212
🏁 Script executed:
# Check the file beginning to see imports and class structure
head -n 50 inc/admin-pages/class-top-admin-nav-menu.php | cat -nRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1381
🏁 Script executed:
# Verify add_query_arg usage in the repository
rg "add_query_arg" --type php -c | head -5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 277
🏁 Script executed:
# Check if add_query_arg is used elsewhere in this file
rg "add_query_arg" inc/admin-pages/class-top-admin-nav-menu.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check sanitize_html_class usage in codebase
rg "sanitize_html_class" --type php -c | head -5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 125
🏁 Script executed:
# Check if $tab is sanitized or validated anywhere else before use
rg "\$tab" inc/admin-pages/class-top-admin-nav-menu.php -B 5 -A 5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Check if the Settings get_sections keys are validated/slugified
rg "register_settings_section|wu_register_settings_section" -A 10 --type php | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2682
Harden dynamic tab handling for node IDs and settings URLs.
$tab is filter-derived. Using it raw in node IDs and concatenated URLs can create malformed links/IDs for non-slug keys. Build URLs via add_query_arg and normalize the node ID fragment.
Suggested hardening diff
- $wp_admin_bar->add_node(
+ $tab_node_id = 'wp-ultimo-settings-' . sanitize_html_class((string) $tab);
+ $tab_href = add_query_arg(
+ [
+ 'page' => 'wp-ultimo-settings',
+ 'tab' => (string) $tab,
+ ],
+ network_admin_url('admin.php')
+ );
+
+ $wp_admin_bar->add_node(
[
- 'id' => 'wp-ultimo-settings-' . $tab,
+ 'id' => $tab_node_id,
'parent' => 'wp-ultimo-settings',
'title' => $tab_info['title'],
- 'href' => network_admin_url('admin.php?page=wp-ultimo-settings&tab=') . $tab,
+ 'href' => $tab_href,
'meta' => [
'class' => 'wp-ultimo-top-menu',
'title' => __('Go to the settings page', 'ultimate-multisite'),
@@
- $wp_admin_bar->add_node(
+ $tab_node_id = 'wp-ultimo-settings-' . sanitize_html_class((string) $tab);
+ $tab_href = add_query_arg(
+ [
+ 'page' => 'wp-ultimo-settings',
+ 'tab' => (string) $tab,
+ ],
+ network_admin_url('admin.php')
+ );
+
+ $wp_admin_bar->add_node(
[
- 'id' => 'wp-ultimo-settings-' . $tab,
+ 'id' => $tab_node_id,
'parent' => 'wp-ultimo-settings-addons',
'title' => $tab_info['title'],
- 'href' => network_admin_url('admin.php?page=wp-ultimo-settings&tab=') . $tab,
+ 'href' => $tab_href,
'meta' => [
'class' => 'wp-ultimo-top-menu',
'title' => __('Go to the settings page', 'ultimate-multisite'),Also applies to: 243-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/admin-pages/class-top-admin-nav-menu.php` around lines 214 - 217, The
node ID and settings URL build use the raw filter-derived $tab (in the array
where 'id' => 'wp-ultimo-settings-' . $tab and 'href' =>
network_admin_url('admin.php?page=wp-ultimo-settings&tab=') . $tab), which can
produce invalid IDs/URLs; sanitize and normalize $tab before use (e.g., cast to
a slug-like string for IDs) and build the URL with add_query_arg to ensure
proper encoding (use the normalized tab when creating the fragment/ID for the
node and pass the original/sanitized value to add_query_arg for the href). Apply
the same normalization and add_query_arg pattern to the other occurrence around
lines 243-246.
Comment on lines
+354
to
+359
| } | ||
|
|
||
| /* | ||
| * Adds recurring stuff. | ||
| * Handle trial periods. | ||
| */ | ||
| $subscription_data['subscription_data'] = [ | ||
| 'items' => array_values($stripe_cart), | ||
| ]; | ||
| elseif ($this->order->has_trial()) { |
Contributor
There was a problem hiding this comment.
Fix spacing after closing brace.
Static analysis flags a formatting issue: expected 1 space after the closing brace on line 354, but a newline was found. The elseif on line 359 should be on the same line as the closing brace.
Proposed fix
if ($current_time < $next_charge_date) {
// The `trial_end` date has to be at least 2 days in the future.
$next_charge = $next_charge_date->diff($current_time)->days > 2 ? $next_charge : strtotime('+2 days');
$subscription_data['subscription_data']['trial_end'] = $next_charge;
}
- }
-
- /*
- * Handle trial periods.
- */
- elseif ($this->order->has_trial()) {
+ } elseif ($this->order->has_trial()) {
+ /*
+ * Handle trial periods.
+ */
$subscription_data['subscription_data']['trial_end'] = $this->order->get_billing_start_date();
}📝 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.
Suggested change
| } | |
| /* | |
| * Adds recurring stuff. | |
| * Handle trial periods. | |
| */ | |
| $subscription_data['subscription_data'] = [ | |
| 'items' => array_values($stripe_cart), | |
| ]; | |
| elseif ($this->order->has_trial()) { | |
| if ($current_time < $next_charge_date) { | |
| // The `trial_end` date has to be at least 2 days in the future. | |
| $next_charge = $next_charge_date->diff($current_time)->days > 2 ? $next_charge : strtotime('+2 days'); | |
| $subscription_data['subscription_data']['trial_end'] = $next_charge; | |
| } | |
| } elseif ($this->order->has_trial()) { | |
| /* | |
| * Handle trial periods. | |
| */ | |
| $subscription_data['subscription_data']['trial_end'] = $this->order->get_billing_start_date(); | |
| } |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 354-354:
Expected 1 space after closing brace; newline found
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/gateways/class-stripe-checkout-gateway.php` around lines 354 - 359, The
formatting error is caused by a newline between a closing brace and the
subsequent elseif; in the method containing the conditional that checks
$this->order->has_trial() (where the current line shows "elseif
($this->order->has_trial())"), move the elseif up to the same line as the
preceding closing brace or otherwise ensure there is exactly one space after the
closing brace before the elseif so the pattern becomes "} elseif (...)" to
satisfy the static analyzer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests