Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a Settings REST API endpoint; implements Pay-What-You-Want pricing and per-product custom amounts/recurring; introduces billing-period restrictions for discount codes; conditions checkout validation/payment collection on whether payment is required; large docs/hook-generator and E2E test/fixture adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Vue
participant API as Settings Endpoint
participant Auth as API Auth
participant Store as Settings Store
participant Logger as Maybe Logger
Client->>API: Request (GET/POST/PUT/PATCH /settings...)
API->>Auth: verify permissions
Auth-->>API: allowed / denied
alt allowed
API->>Store: read or update settings
Store-->>API: payload / result
API->>API: filter_sensitive_settings & validate
API->>Logger: maybe_log_api_call(request)
API-->>Client: 200 JSON or summary
else denied
API-->>Client: 403 WP_Error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/functions/helper.php (1)
498-625: Fix array union operator with numeric keys to include all intended tags.The array union operator (
+) preserves the left-hand array's values for overlapping numeric keys. Sincearray_keys($allowed_html)returns numeric keys[0, 1, ...]and the extra tags array also has numeric keys[0, 1, 2, ...], the+operator will drop extra tags that occupy indices already present inarray_keys()(specifically 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img', 'input'). Usearray_merge()instead to concatenate the arrays and ensure all tags are included.🔧 Suggested fix
- ] + array_merge_recursive($allowed_html, array_fill_keys(array_keys($allowed_html) + ['div', 'span', 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img', 'input', 'textarea'], $vue_and_data_attributes)); + ] + array_merge_recursive( + $allowed_html, + array_fill_keys( + array_unique(array_merge(array_keys($allowed_html), ['div', 'span', 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'strong', 'em', 'b', 'i', 'ul', 'ol', 'li', 'a', 'img', 'input', 'textarea'])), + $vue_and_data_attributes + ) + );
🤖 Fix all issues with AI agents
In `@inc/apis/class-settings-endpoint.php`:
- Around line 258-270: The loop that calls wu_save_setting() with raw $value
should sanitize/validate via the Field API first: implement a helper (e.g.
sanitize_setting_value(string $key, $value)) that iterates
Settings::get_instance()->get_sections(), finds the field definition for $key,
constructs a \WP_Ultimo\UI\Field($key, $field_def), calls
$field->set_value($value) and returns $field->get_value(); then replace the
direct calls to wu_save_setting($key, $value) in the foreach over
$filtered_settings (and the similar persistence block later) to call $sanitized
= sanitize_setting_value($key, $value) and either pass $sanitized to
wu_save_setting or skip/reject unknown keys (returning null) as appropriate.
- Around line 426-436: The maybe_log_api_call method currently logs raw request
bodies via $request->get_body() which can leak sensitive data; before building
the $payload (or before calling wp_json_encode) sanitize the body by removing or
replacing sensitive keys (e.g., "password", "token", "authorization", "api_key",
"credit_card", "ssn") or omit the body entirely, e.g., call a helper like
redact_request_body($request->get_body()) and use that redacted result for the
'body_params' value so wp_json_encode(wu_log_add(...)) never receives plaintext
secrets; update maybe_log_api_call to perform this redaction and ensure
get_body()/body_params references the sanitized output.
In `@inc/class-settings.php`:
- Around line 711-720: The privacy-policy link in the description passed to
add_field for 'enable_error_reporting' lacks rel="noopener noreferrer" and the
URL isn't escaped; update the sprintf call building the description so the <a
href="%s" ...> uses esc_url( 'https://ultimatemultisite.com/privacy-policy/' )
and add rel="noopener noreferrer" to the anchor (i.e., modify the description
string for add_field('general','enable_error_reporting', ...) to include
rel="noopener noreferrer" and wrap the URL with esc_url()).
In `@readme.txt`:
- Around line 243-249: The changelog contains duplicate "Version [2.4.10]"
entries (the headers "Version [2.4.10]" and the block dated "2026-XX-XX"); merge
the two blocks into a single 2.4.10 entry combining both bullet points, and
replace the placeholder date "2026-XX-XX" with a concrete release date (e.g.,
"2026-01-25") or label the entry "Unreleased" if it isn't finalized; update the
single "Version [2.4.10]" header and remove the redundant header/block so only
one consolidated 2.4.10 section remains.
🧹 Nitpick comments (1)
inc/apis/class-settings-endpoint.php (1)
409-412: Drop unused$valueto satisfy PHPMDLine 409 declares
$valuebut never uses it. Usingforeach (array_keys(...))or$key => $_avoids the PHPMD warning.🧹 Suggested tidy‑up
- foreach ($settings as $key => $value) { + foreach ($settings as $key => $_) { if ($this->is_sensitive_setting($key)) { unset($settings[ $key ]); } }
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/models/class-discount-code.php (1)
500-569: Billing-period restrictions can be bypassed when discount codes are first applied to cart.The
set_discount_code()method (line 1374) validates without duration/unit parameters, allowing a discount code withlimit_billing_periodsenabled to pass validation. The complete validation only occurs later inapply_discounts_to_item()(line 2415) when product-specific data is available. If callers expect billing period restrictions to be enforced immediately when adding a coupon, this creates a bypass. Consider either deferring coupon acceptance until line-item application or enforcing a hard failure inis_valid()whenlimit_billing_periodsis enabled but no duration params are provided.
🤖 Fix all issues with AI agents
In `@inc/admin-pages/class-discount-code-edit-admin-page.php`:
- Around line 532-588: get_available_billing_periods currently skips products
based solely on $product->is_recurring(), which misses PWYW products with
pwyw_recurring_mode == 'force_recurring'; update the recurrence check in
get_available_billing_periods to treat a product as recurring if
$product->is_recurring() OR ($product->get_pwyw_recurring_mode() ===
'force_recurring') (or the equivalent accessor/name used on the Product class),
then continue using $product->get_duration(), $product->get_duration_unit(),
get_price_variations(), and
Discount_Code::get_billing_period_key/parse_billing_period_key as before so PWYW
forced-recurring products are included in the $periods list and sorting.
In `@inc/admin-pages/class-membership-list-admin-page.php`:
- Around line 233-237: The billing_period parsing accepts untrusted input:
ensure $duration is clamped to >=1 and $duration_unit is validated against a
whitelist before using it to build the cart; specifically, after parsing
$billing_period (the code that sets $billing_parts, $duration and
$duration_unit) replace the loose values with a sanitized $duration = max(1,
absint($billing_parts[0] ?? 1)) and $duration_unit only set if in an allowed
array (e.g. ['day','week','month','year']) otherwise default to 'month' so
invalid units or zero durations cannot create bad variations in the cart.
In `@inc/checkout/class-checkout.php`:
- Around line 1621-1633: process_order() builds the cart params without
including the PWYW fields so submitted custom amounts are lost; update the cart
construction inside process_order() to read the same keys as create_order()—use
$this->request_or_session('custom_amounts', []) and
$this->request_or_session('pwyw_recurring', []) (and propagate them into the
'custom_amounts' and 'pwyw_recurring' keys of the cart array) so the cart passed
to order creation retains the PWYW values.
In `@inc/sso/class-magic-link.php`:
- Around line 167-190: Before generating a token in
generate_cross_network_magic_link, validate that the user actually has access to
the target site and that the redirect URL is safe: call
verify_user_site_access($user_id, $site_id) (or equivalent membership check) and
return false if it fails, then if $redirect_to is provided validate it with
filter_var($redirect_to, FILTER_VALIDATE_URL) and ensure its host matches the
parsed host of $site_url (using parse_url) to prevent mismatched/remote
redirects; only after these checks succeed proceed to generate_token() and
persist $token_data so you do not consume tokens for invalid requests.
In `@views/legacy/signup/steps/step-domain-url-preview.php`:
- Around line 38-40: The subdirectory branch uses a hardcoded 'yoursite'
fallback in the sprintf for the element with id "wu-your-site" (v-html="site_url
? site_url : 'yoursite'"); replace that literal with the translatable
$dynamic_part so both branches use the same localized fallback: update the
sprintf format and the v-html fallback to reference $dynamic_part (escaped via
esc_html) rather than the hardcoded string, keeping the same element ids
"wu-your-site" and "wu-site-domain" and preserving proper escaping and quoting.
🧹 Nitpick comments (1)
inc/sso/class-magic-link.php (1)
201-205: Keep URL filter parity withgenerate_magic_link().This method skips the
wu_magic_link_urlfilter, so downstream customizations won’t apply to cross-network links. Consider applying the same filter for consistency and extensibility.🔧 Proposed change
- return add_query_arg( + $magic_link = add_query_arg( [self::TOKEN_QUERY_ARG => $token], $site_url ); + + return apply_filters('wu_magic_link_url', $magic_link, $user_id, $site_id, $redirect_to);
| protected function get_available_billing_periods() { | ||
|
|
||
| $periods = []; | ||
|
|
||
| foreach (wu_get_products() as $product) { | ||
| if ( ! $product->is_recurring()) { | ||
| continue; | ||
| } | ||
|
|
||
| $duration = $product->get_duration(); | ||
| $duration_unit = $product->get_duration_unit(); | ||
| $period_key = Discount_Code::get_billing_period_key($duration, $duration_unit); | ||
|
|
||
| if ( ! isset($periods[ $period_key ])) { | ||
| $periods[ $period_key ] = $this->format_billing_period_label($duration, $duration_unit); | ||
| } | ||
|
|
||
| // Also check for price variations | ||
| $price_variations = $product->get_price_variations(); | ||
|
|
||
| if ( ! empty($price_variations)) { | ||
| foreach ($price_variations as $variation) { | ||
| $var_duration = isset($variation['duration']) ? (int) $variation['duration'] : 0; | ||
| $var_duration_unit = isset($variation['duration_unit']) ? $variation['duration_unit'] : ''; | ||
|
|
||
| if ($var_duration > 0 && ! empty($var_duration_unit)) { | ||
| $var_period_key = Discount_Code::get_billing_period_key($var_duration, $var_duration_unit); | ||
|
|
||
| if ( ! isset($periods[ $var_period_key ])) { | ||
| $periods[ $var_period_key ] = $this->format_billing_period_label($var_duration, $var_duration_unit); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Sort by duration for consistent display | ||
| uksort( | ||
| $periods, | ||
| function ($a, $b) { | ||
| $a_parts = Discount_Code::parse_billing_period_key($a); | ||
| $b_parts = Discount_Code::parse_billing_period_key($b); | ||
|
|
||
| if ( ! $a_parts || ! $b_parts) { | ||
| return 0; | ||
| } | ||
|
|
||
| // Convert to days for comparison | ||
| $a_days = $this->get_period_in_days($a_parts['duration'], $a_parts['duration_unit']); | ||
| $b_days = $this->get_period_in_days($b_parts['duration'], $b_parts['duration_unit']); | ||
|
|
||
| return $a_days <=> $b_days; | ||
| } | ||
| ); | ||
|
|
||
| return $periods; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for is_recurring, pay_what_you_want, and pwyw patterns
rg -n "is_recurring\(|pay_what_you_want|pwyw_recurring_mode" -A 2 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 33826
🏁 Script executed:
# Also search for the Product class definition and is_recurring method
rg -n "class.*Product|function is_recurring|def is_recurring" -A 5 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 14004
🏁 Script executed:
# Check how PWYW products are represented and handled
rg -n "is_pay_what_you_want|pwyw|pay.what.you.want" -i -A 3 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 30256
🏁 Script executed:
# Search for how get_available_billing_periods is used
rg -n "get_available_billing_periods" -B 2 -A 5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1616
🏁 Script executed:
# Check if discount codes have special handling for PWYW products
rg -n "class.*Discount.*Code|is_pay_what_you_want.*discount" -i -A 3 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 50394
🏁 Script executed:
# Look at the discount code model to understand what billing periods are used for
fd "class-discount" --type f | head -5 | xargs catRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 36757
PWYW recurring products with force_recurring mode won't appear in available billing periods.
The method relies on is_recurring() which checks the recurring flag and amount > 0, but doesn't account for pwyw_recurring_mode. A PWYW product configured with force_recurring may have recurring=false, causing is_recurring() to return false and the product to be skipped entirely. This means its billing period information never reaches the discount code admin form, preventing admins from restricting codes to periods that only exist on PWYW products.
🤖 Prompt for AI Agents
In `@inc/admin-pages/class-discount-code-edit-admin-page.php` around lines 532 -
588, get_available_billing_periods currently skips products based solely on
$product->is_recurring(), which misses PWYW products with pwyw_recurring_mode ==
'force_recurring'; update the recurrence check in get_available_billing_periods
to treat a product as recurring if $product->is_recurring() OR
($product->get_pwyw_recurring_mode() === 'force_recurring') (or the equivalent
accessor/name used on the Product class), then continue using
$product->get_duration(), $product->get_duration_unit(), get_price_variations(),
and Discount_Code::get_billing_period_key/parse_billing_period_key as before so
PWYW forced-recurring products are included in the $periods list and sorting.
| // Parse the billing period into duration and duration_unit. | ||
| $billing_period = wu_request('billing_period', '1-month'); | ||
| $billing_parts = explode('-', $billing_period, 2); | ||
| $duration = isset($billing_parts[0]) ? absint($billing_parts[0]) : 1; | ||
| $duration_unit = isset($billing_parts[1]) ? $billing_parts[1] : 'month'; |
There was a problem hiding this comment.
Validate and clamp the billing period inputs before building the cart.
billing_period comes from the request; absint() can yield 0, and duration_unit isn’t validated. This can lead to invalid variations or avoidable cart errors. Consider whitelisting units and enforcing duration >= 1.
✅ Suggested hardening
- $billing_period = wu_request('billing_period', '1-month');
- $billing_parts = explode('-', $billing_period, 2);
- $duration = isset($billing_parts[0]) ? absint($billing_parts[0]) : 1;
- $duration_unit = isset($billing_parts[1]) ? $billing_parts[1] : 'month';
+ $billing_period = wu_request('billing_period', '1-month');
+ $billing_parts = explode('-', $billing_period, 2);
+ $duration = max(1, absint($billing_parts[0] ?? 1));
+ $duration_unit = $billing_parts[1] ?? 'month';
+ $allowed_units = ['day', 'week', 'month', 'year'];
+ if ( ! in_array($duration_unit, $allowed_units, true)) {
+ $duration_unit = 'month';
+ }📝 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.
| // Parse the billing period into duration and duration_unit. | |
| $billing_period = wu_request('billing_period', '1-month'); | |
| $billing_parts = explode('-', $billing_period, 2); | |
| $duration = isset($billing_parts[0]) ? absint($billing_parts[0]) : 1; | |
| $duration_unit = isset($billing_parts[1]) ? $billing_parts[1] : 'month'; | |
| // Parse the billing period into duration and duration_unit. | |
| $billing_period = wu_request('billing_period', '1-month'); | |
| $billing_parts = explode('-', $billing_period, 2); | |
| $duration = max(1, absint($billing_parts[0] ?? 1)); | |
| $duration_unit = $billing_parts[1] ?? 'month'; | |
| $allowed_units = ['day', 'week', 'month', 'year']; | |
| if ( ! in_array($duration_unit, $allowed_units, true)) { | |
| $duration_unit = 'month'; | |
| } |
🤖 Prompt for AI Agents
In `@inc/admin-pages/class-membership-list-admin-page.php` around lines 233 - 237,
The billing_period parsing accepts untrusted input: ensure $duration is clamped
to >=1 and $duration_unit is validated against a whitelist before using it to
build the cart; specifically, after parsing $billing_period (the code that sets
$billing_parts, $duration and $duration_unit) replace the loose values with a
sanitized $duration = max(1, absint($billing_parts[0] ?? 1)) and $duration_unit
only set if in an allowed array (e.g. ['day','week','month','year']) otherwise
default to 'month' so invalid units or zero durations cannot create bad
variations in the cart.
| public function generate_cross_network_magic_link(int $user_id, int $site_id, string $site_url, string $redirect_to = '') { | ||
|
|
||
| if ( ! $this->is_enabled()) { | ||
| return false; | ||
| } | ||
|
|
||
| $user = get_userdata($user_id); | ||
|
|
||
| if ( ! $user) { | ||
| return false; | ||
| } | ||
|
|
||
| $token = $this->generate_token(); | ||
| $user_agent = $this->get_user_agent(); | ||
| $ip_address = $this->get_client_ip(); | ||
|
|
||
| $token_data = [ | ||
| 'user_id' => $user_id, | ||
| 'site_id' => $site_id, | ||
| 'redirect_to' => $redirect_to, | ||
| 'created_at' => time(), | ||
| 'user_agent' => $user_agent, | ||
| 'ip_address' => $ip_address, | ||
| ]; |
There was a problem hiding this comment.
Validate user/site access and the target URL before issuing a cross-network link.
Right now the method can return a link that will always fail later (user not a member or URL mismatched/invalid) while still consuming a token. Add verify_user_site_access() and basic URL validation (and ideally a host match) before generating the token.
🛠️ Proposed fix
public function generate_cross_network_magic_link(int $user_id, int $site_id, string $site_url, string $redirect_to = '') {
- if ( ! $this->is_enabled()) {
+ if ( ! $this->is_enabled() ) {
return false;
}
$user = get_userdata($user_id);
- if ( ! $user) {
+ if ( ! $user ) {
return false;
}
+
+ if ( ! $this->verify_user_site_access($user_id, $site_id) ) {
+ return false;
+ }
+
+ $site_url = wp_http_validate_url($site_url);
+ if ( ! $site_url ) {
+ return false;
+ }🤖 Prompt for AI Agents
In `@inc/sso/class-magic-link.php` around lines 167 - 190, Before generating a
token in generate_cross_network_magic_link, validate that the user actually has
access to the target site and that the redirect URL is safe: call
verify_user_site_access($user_id, $site_id) (or equivalent membership check) and
return false if it fails, then if $redirect_to is provided validate it with
filter_var($redirect_to, FILTER_VALIDATE_URL) and ensure its host matches the
parsed host of $site_url (using parse_url) to prevent mismatched/remote
redirects; only after these checks succeed proceed to generate_token() and
persist $token_data so you do not consume tokens for invalid requests.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@inc/checkout/class-checkout.php`:
- Around line 1958-1991: The temporary Cart created in should_collect_payment()
only passed 'products' and 'country', which can miscompute payment needs because
Cart->should_collect_payment() depends on pricing-related inputs; update the
Cart construction to pass the same pricing/session fields used elsewhere in
checkout/cart (pull via $this->request_or_session) — include discounts/coupons,
duration/interval, trial parameters, PWYW amount, recurring/price overrides and
any other pricing-related keys your checkout normally sends — so that new
Cart(...) mirrors the real checkout/cart data before calling
should_collect_payment() on it.
In `@inc/documentation/generate-berlindb-hooks.php`:
- Around line 351-545: The script currently calls
file_put_contents($output_file, $content) but always prints a success message;
modify the block around file_put_contents so you capture its return value (e.g.
$bytes = file_put_contents($output_file, $content)), check for === false, and on
failure write a clear error to STDERR (including $output_file) and exit with a
non-zero status (exit(1)); only print the "Written to ..." message to STDERR
when the write succeeded. Use the existing symbols $output_file, $content,
fwrite and STDERR to implement the check and error path.
- Around line 52-79: The find_line_number function should guard against
unreadable files and cache files per filename: change the single static $lines
to a static associative cache (e.g. $cache) keyed by $file, return 0 immediately
if !is_readable($file) before calling file(), and load file contents into
$cache[$file] = file($file) when first accessed; keep the loop and the rest of
the logic intact so existing callers (e.g. the $hook_lines pre-computation)
continue to work.
- Around line 141-194: The greedy regex in extract_schema_columns that finds the
$columns block ('/\$columns\s*=\s*\[(.+)\];/s') can over-capture; change it to a
non-greedy match so it stops at the first closing bracket by using
'/\$columns\s*=\s*\[(.+?)\];/s' (keep the s flag), leaving the rest of the
function unchanged so $m[1] still contains the intended block for parsing.
In `@lang/ultimate-multisite.pot`:
- Around line 4193-4199: Update the source string in
class-product-edit-admin-page.php that currently reads "This products will be
offered inside upgrade/downgrade forms as order bumps." to the correct plural
"These products will be offered inside upgrade/downgrade forms as order bumps."
(refer to the msgid "This products will be offered inside upgrade/downgrade
forms as order bumps." in the POT diff), then regenerate the POT so the
corrected string replaces the old msgid in lang/ultimate-multisite.pot.
- Around line 7043-7046: Update the source string in the checkout code that
generates the discount-code error: replace "The code %s do not exist or is no
longer valid." with the grammatically correct "The code %s does not exist or is
no longer valid." (the string originates from the message in
inc/checkout/class-cart.php referenced by the msgid in
lang/ultimate-multisite.pot); after changing the source string, regenerate the
POT so the updated msgid is propagated to translations.
- Around line 9019-9024: Remove the trailing space from the "Force Synchronous
Site Publication " label in the source (inc/class-settings.php where that msgid
is defined, e.g., in the Settings class or the method that builds the label) so
it becomes "Force Synchronous Site Publication" exactly, then regenerate the
POT/translation files so the corrected msgid replaces the one with the trailing
space; ensure the change is made at the original msgid location referenced in
inc/class-settings.php to avoid breaking exact msgid matching.
| function extract_schema_columns(string $file): array { | ||
|
|
||
| $source = file_get_contents($file); | ||
|
|
||
| // Extract the $columns array block. | ||
| if (! preg_match('/\$columns\s*=\s*\[(.+)\];/s', $source, $m)) { | ||
| return []; | ||
| } | ||
|
|
||
| $columns_block = $m[1]; | ||
| $all_columns = []; | ||
| $transition_columns = []; | ||
| $seen_names = []; | ||
|
|
||
| // Split into individual column arrays. | ||
| // Each column is delimited by [ ... ], | ||
| preg_match_all('/\[\s*(.*?)\s*\]/s', $columns_block, $entries); | ||
|
|
||
| foreach ($entries[1] as $entry) { | ||
| $col = []; | ||
|
|
||
| // Extract name | ||
| if (preg_match("/'name'\s*=>\s*'([^']+)'/", $entry, $nm)) { | ||
| $col['name'] = $nm[1]; | ||
| } else { | ||
| continue; | ||
| } | ||
|
|
||
| // Extract type | ||
| if (preg_match("/'type'\s*=>\s*'([^']+)'/", $entry, $tm)) { | ||
| $col['type'] = $tm[1]; | ||
| } else { | ||
| $col['type'] = 'mixed'; | ||
| } | ||
|
|
||
| // Deduplicate columns (some schemas define the same column twice). | ||
| if (isset($seen_names[ $col['name'] ])) { | ||
| continue; | ||
| } | ||
|
|
||
| $seen_names[ $col['name'] ] = true; | ||
| $all_columns[] = $col; | ||
|
|
||
| // Check for transition => true | ||
| if (preg_match("/'transition'\s*=>\s*true/", $entry)) { | ||
| $transition_columns[] = $col; | ||
| } | ||
| } | ||
|
|
||
| return [ | ||
| 'all' => $all_columns, | ||
| 'transition' => $transition_columns, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l inc/documentation/generate-berlindb-hooks.php
sed -n '1,200p' inc/documentation/generate-berlindb-hooks.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 5940
Use a non-greedy regex for the $columns block.
The current greedy match can over-capture if other arrays follow.
Patch
-if (! preg_match('/\$columns\s*=\s*\[(.+)\];/s', $source, $m)) {
+if (! preg_match('/\$columns\s*=\s*\[(.+?)\];/s', $source, $m)) {
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.
| function extract_schema_columns(string $file): array { | |
| $source = file_get_contents($file); | |
| // Extract the $columns array block. | |
| if (! preg_match('/\$columns\s*=\s*\[(.+)\];/s', $source, $m)) { | |
| return []; | |
| } | |
| $columns_block = $m[1]; | |
| $all_columns = []; | |
| $transition_columns = []; | |
| $seen_names = []; | |
| // Split into individual column arrays. | |
| // Each column is delimited by [ ... ], | |
| preg_match_all('/\[\s*(.*?)\s*\]/s', $columns_block, $entries); | |
| foreach ($entries[1] as $entry) { | |
| $col = []; | |
| // Extract name | |
| if (preg_match("/'name'\s*=>\s*'([^']+)'/", $entry, $nm)) { | |
| $col['name'] = $nm[1]; | |
| } else { | |
| continue; | |
| } | |
| // Extract type | |
| if (preg_match("/'type'\s*=>\s*'([^']+)'/", $entry, $tm)) { | |
| $col['type'] = $tm[1]; | |
| } else { | |
| $col['type'] = 'mixed'; | |
| } | |
| // Deduplicate columns (some schemas define the same column twice). | |
| if (isset($seen_names[ $col['name'] ])) { | |
| continue; | |
| } | |
| $seen_names[ $col['name'] ] = true; | |
| $all_columns[] = $col; | |
| // Check for transition => true | |
| if (preg_match("/'transition'\s*=>\s*true/", $entry)) { | |
| $transition_columns[] = $col; | |
| } | |
| } | |
| return [ | |
| 'all' => $all_columns, | |
| 'transition' => $transition_columns, | |
| ]; | |
| } | |
| function extract_schema_columns(string $file): array { | |
| $source = file_get_contents($file); | |
| // Extract the $columns array block. | |
| if (! preg_match('/\$columns\s*=\s*\[(.+?)\];/s', $source, $m)) { | |
| return []; | |
| } | |
| $columns_block = $m[1]; | |
| // …rest of function… | |
| } |
🤖 Prompt for AI Agents
In `@inc/documentation/generate-berlindb-hooks.php` around lines 141 - 194, The
greedy regex in extract_schema_columns that finds the $columns block
('/\$columns\s*=\s*\[(.+)\];/s') can over-capture; change it to a non-greedy
match so it stops at the first closing bracket by using
'/\$columns\s*=\s*\[(.+?)\];/s' (keep the s flag), leaving the rest of the
function unchanged so $m[1] still contains the intended block for parsing.
| $output_file = __DIR__ . '/berlindb-dynamic-hooks.php'; | ||
| $prefix = 'wu'; | ||
| $src = $query_php_relative; | ||
|
|
||
| ob_start(); | ||
|
|
||
| echo "<?php\n"; | ||
|
|
||
| // Build the @see line for transition hooks. | ||
| $transition_see = $hook_lines['transition'] ? "see {$src}:{$hook_lines['transition']}" : "see {$src}"; | ||
|
|
||
| echo <<<'HEADER' | ||
| /** | ||
| * BerlinDB Dynamic Hooks Reference | ||
| * | ||
| * AUTO-GENERATED — do not edit manually. | ||
| * Regenerate with: php inc/documentation/generate-berlindb-hooks.php | ||
| * | ||
| * This file exists solely so that @10up/wp-hooks-documentor can discover | ||
| * the dynamic hooks fired by BerlinDB's Query class for every registered | ||
| * Ultimate Multisite model. It is never loaded at runtime. | ||
| * | ||
| * @package WP_Ultimo | ||
| * @subpackage Documentation | ||
| * @since 2.3.0 | ||
| * @generated | ||
| */ | ||
|
|
||
| defined('ABSPATH') || exit; | ||
|
|
||
| // phpcs:disable -- This file is never executed; it only carries docblocks. | ||
|
|
||
| // Variable declarations to keep static analysers and editors happy. | ||
| $old_value = null; | ||
| $new_value = null; | ||
| $item_id = 0; | ||
| $query = null; | ||
| $clauses = []; | ||
| $search_columns = []; | ||
| $search = ''; | ||
| $items = []; | ||
| $item = []; | ||
| $sql = ''; | ||
|
|
||
| HEADER; | ||
|
|
||
| foreach ($models as $model) { | ||
| $item = $model['item_name']; | ||
| $items = $model['item_name_plural']; | ||
| $label = humanize($item); | ||
| $labels = humanize($items); | ||
| $section = ucwords($label); | ||
|
|
||
| echo "\n// ─── {$section} " . str_repeat('─', max(1, 72 - strlen($section))) . "\n"; | ||
|
|
||
| // ── Transition hooks ──────────────────────────────────────────────── | ||
| foreach ($model['transition_cols'] as $col) { | ||
| $col_name = $col['name']; | ||
| $phpdoc = mysql_type_to_phpdoc($col['type']); | ||
| $hook_name = "{$prefix}_transition_{$item}_{$col_name}"; | ||
| $col_human = str_replace('_', ' ', $col_name); | ||
| $see_line = $transition_see; | ||
|
|
||
| echo <<<HOOK | ||
|
|
||
| /** | ||
| * Fires when the {$col_human} of a {$label} transitions from one value to another. | ||
| * | ||
| * This hook is fired by BerlinDB when a database UPDATE changes the `{$col_name}` | ||
| * column for a {$label} row. | ||
| * | ||
| * @since 2.0.0 | ||
| * @{$see_line} | ||
| * | ||
| * @param {$phpdoc} \$old_value The previous {$col_human} value. | ||
| * @param {$phpdoc} \$new_value The new {$col_human} value. | ||
| * @param int \$item_id The {$label} ID. | ||
| */ | ||
| do_action('{$hook_name}', \$old_value, \$new_value, \$item_id); | ||
|
|
||
| HOOK; | ||
| } | ||
|
|
||
| // ── Query hooks ───────────────────────────────────────────────────── | ||
|
|
||
| $see_pre_get = $hook_lines['pre_get'] ? "see {$src}:{$hook_lines['pre_get']}" : "see {$src}"; | ||
| $see_parse_query = $hook_lines['parse_query'] ? "see {$src}:{$hook_lines['parse_query']}" : "see {$src}"; | ||
| $see_query_clauses = $hook_lines['query_clauses'] ? "see {$src}:{$hook_lines['query_clauses']}" : "see {$src}"; | ||
| $see_search_cols = $hook_lines['search_columns'] ? "see {$src}:{$hook_lines['search_columns']}" : "see {$src}"; | ||
| $see_the_items = $hook_lines['the_items'] ? "see {$src}:{$hook_lines['the_items']}" : "see {$src}"; | ||
| $see_filter_item = $hook_lines['filter_item'] ? "see {$src}:{$hook_lines['filter_item']}" : "see {$src}"; | ||
| $see_found_query = $hook_lines['found_query'] ? "see {$src}:{$hook_lines['found_query']}" : "see {$src}"; | ||
|
|
||
| echo <<<HOOK | ||
|
|
||
| /** | ||
| * Fires before {$labels} are fetched from the database. | ||
| * | ||
| * @since 2.0.0 | ||
| * @{$see_pre_get} | ||
| * | ||
| * @param \\BerlinDB\\Database\\Query \$query The query instance (passed by reference). | ||
| */ | ||
| do_action_ref_array('{$prefix}_pre_get_{$items}', [&\$query]); | ||
|
|
||
| /** | ||
| * Fires after {$labels} query vars have been parsed. | ||
| * | ||
| * @since 2.0.0 | ||
| * @{$see_parse_query} | ||
| * | ||
| * @param \\BerlinDB\\Database\\Query \$query The query instance (passed by reference). | ||
| */ | ||
| do_action_ref_array('{$prefix}_parse_{$items}_query', [&\$query]); | ||
|
|
||
| /** | ||
| * Filters the SQL clauses for a {$labels} query. | ||
| * | ||
| * @since 2.0.0 | ||
| * @{$see_query_clauses} | ||
| * | ||
| * @param array \$clauses { | ||
| * Associative array of SQL clause strings. | ||
| * | ||
| * @type string \$fields The SELECT fields. | ||
| * @type string \$join The JOIN clause. | ||
| * @type string \$where The WHERE clause. | ||
| * @type string \$orderby The ORDER BY clause. | ||
| * @type string \$limits The LIMIT clause. | ||
| * @type string \$groupby The GROUP BY clause. | ||
| * } | ||
| * @param \\BerlinDB\\Database\\Query \$query The query instance (passed by reference). | ||
| * @return array | ||
| */ | ||
| \$clauses = apply_filters_ref_array('{$prefix}_{$items}_query_clauses', [\$clauses, &\$query]); | ||
|
|
||
| /** | ||
| * Filters the columns to search when performing a {$labels} search. | ||
| * | ||
| * @since 2.0.0 | ||
| * @{$see_search_cols} | ||
| * | ||
| * @param string[] \$search_columns Array of column names to search. | ||
| * @param string \$search The search term. | ||
| * @param \\BerlinDB\\Database\\Query \$query The query instance. | ||
| * @return string[] | ||
| */ | ||
| \$search_columns = apply_filters('{$prefix}_{$items}_search_columns', \$search_columns, \$search, \$query); | ||
|
|
||
| /** | ||
| * Filters the found {$labels} after a query. | ||
| * | ||
| * @since 2.0.0 | ||
| * @{$see_the_items} | ||
| * | ||
| * @param object[] \$items The array of found {$label} objects. | ||
| * @param \\BerlinDB\\Database\\Query \$query The query instance (passed by reference). | ||
| * @return object[] | ||
| */ | ||
| \$items = apply_filters_ref_array('{$prefix}_the_{$items}', [\$items, &\$query]); | ||
|
|
||
| /** | ||
| * Filters a single {$label} item before it is inserted or updated in the database. | ||
| * | ||
| * @since 2.0.0 | ||
| * @{$see_filter_item} | ||
| * | ||
| * @param array \$item The item data as an associative array. | ||
| * @param \\BerlinDB\\Database\\Query \$query The query instance (passed by reference). | ||
| * @return array | ||
| */ | ||
| \$item = apply_filters_ref_array('{$prefix}_filter_{$item}_item', [\$item, &\$query]); | ||
|
|
||
| /** | ||
| * Filters the FOUND_ROWS() query for {$labels}. | ||
| * | ||
| * @since 2.0.0 | ||
| * @{$see_found_query} | ||
| * | ||
| * @param string \$sql The SQL query to count found rows. | ||
| * @param \\BerlinDB\\Database\\Query \$query The query instance (passed by reference). | ||
| * @return string | ||
| */ | ||
| \$sql = apply_filters_ref_array('{$prefix}_found_{$items}_query', [\$sql, &\$query]); | ||
|
|
||
| HOOK; | ||
| } | ||
|
|
||
| echo "\n// phpcs:enable\n"; | ||
|
|
||
| $content = ob_get_clean(); | ||
|
|
||
| file_put_contents($output_file, $content); | ||
|
|
||
| fwrite(STDERR, "Written to {$output_file}\n"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg "file_put_contents" -n inc/documentation/generate-berlindb-hooks.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 127
Handle file_put_contents return value
file_put_contents() can return false, but the script always reports success. Check its return value, emit an error on failure, and exit with a non-zero status.
Suggested fix
$content = ob_get_clean();
-file_put_contents($output_file, $content);
+$_bytes = file_put_contents($output_file, $content);
+if ($_bytes === false) {
+ fwrite(STDERR, "Failed to write {$output_file}\n");
+ exit(1);
+}
fwrite(STDERR, "Written to {$output_file}\n");🤖 Prompt for AI Agents
In `@inc/documentation/generate-berlindb-hooks.php` around lines 351 - 545, The
script currently calls file_put_contents($output_file, $content) but always
prints a success message; modify the block around file_put_contents so you
capture its return value (e.g. $bytes = file_put_contents($output_file,
$content)), check for === false, and on failure write a clear error to STDERR
(including $output_file) and exit with a non-zero status (exit(1)); only print
the "Written to ..." message to STDERR when the write succeeded. Use the
existing symbols $output_file, $content, fwrite and STDERR to implement the
check and error path.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
PWYW fixes: - Add custom_amounts and pwyw_recurring to Cart in process_order() - Add missing pricing inputs to should_collect_payment() temporary cart - Remove v-bind:name suppression for PWYW duration fields in product edit - Update Product::is_recurring() to check pwyw_recurring_mode - Add wu_product_is_recurring filter for extensibility Settings API refactor: - Create save_setting() helper for DRY code - Add sanitize_setting_value() using Field API for proper sanitization - Add get_field_definition() to look up field definitions from Settings - Remove body_params from API logging to avoid storing sensitive data Grammar and text fixes: - "This products" → "These products" - "do not exist" → "does not exist" - Remove trailing space from "Force Synchronous Site Publication" Security improvements: - Add rel="noopener noreferrer" and esc_url() to privacy policy link Cleanup: - Delete stale inc/berlindb-dynamic-hooks.php duplicate - Fix changelog: separate 2.4.10 (released) from 2.4.11 (unreleased) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/checkout/class-cart.php (1)
1698-1785:⚠️ Potential issue | 🟡 MinorEnsure minimum price is enforced even when no custom amount is provided.
If the suggested amount is below the minimum, the cart currently allows under‑minimum pricing. Clamp to the minimum (or error) to keep the rule consistent.🛠️ Proposed fix
- } else { - // Use suggested amount as default - $amount = $product->get_pwyw_suggested_amount(); - } + } else { + // Use suggested amount as default, but never below minimum + $amount = max( + $product->get_pwyw_suggested_amount(), + $product->get_pwyw_minimum_amount() + ); + }
🧹 Nitpick comments (2)
inc/admin-pages/class-product-edit-admin-page.php (1)
333-356: Consider adding validation that suggested price ≥ minimum price.The PWYW amount fields look good, but there's no client-side validation to ensure
pwyw_suggested_amountis greater than or equal topwyw_minimum_amount. An admin could accidentally set a suggested price below the minimum, which would confuse customers at checkout.You could add a Vue-based warning or validation in the wrapper attributes, or handle this server-side in
handle_save().inc/apis/class-settings-endpoint.php (1)
483-499: Avoid unused loop variable to satisfy PHPMD.
Usearray_keys()(or a named variable) to remove the unused$_.🛠️ Proposed fix
- foreach ($settings as $key => $_) { + foreach (array_keys($settings) as $key) { if ($this->is_sensitive_setting($key)) { unset($settings[ $key ]); } }
🔨 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: |
…-multisite into tweask-again # Conflicts: # assets/js/checkout.js
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@assets/js/checkout.js`:
- Around line 460-515: In create_order(), the try/catch wrapping a commented-out
history.pushState is unreachable; either re-enable the pushState call so the
catch can meaningfully handle failures or remove the entire try/catch block (and
the commented pushState) to satisfy the linter; update the wu_on_create_order
flow accordingly and ensure no leftover commented history.pushState remains in
the create_order method.
In `@inc/checkout/class-checkout.php`:
- Around line 1961-1999: The temporary Cart built in should_collect_payment()
omits state, city, membership_id, payment_id, auto_renew and cart_type which can
change tax, proration and recovery logic; update the Cart instantiation in
should_collect_payment() to pass these values from request_or_session (e.g.
'state' => $this->request_or_session('billing_state'), 'city' =>
$this->request_or_session('billing_city'), 'membership_id' =>
$this->request_or_session('membership_id'), 'payment_id' =>
$this->request_or_session('payment_id'), 'auto_renew' =>
$this->request_or_session('auto_renew', true), 'cart_type' =>
$this->request_or_session('cart_type', 'new')) so Cart->get_total() and
Cart->should_collect_payment() reflect real checkout state; alternatively expose
a filter in should_collect_payment() to allow callers to inject/modify the cart
payload before creating the Cart.
| create_order() { | ||
|
|
||
| /* | ||
| * Bail if there is no order summary to update. | ||
| */ | ||
| if (!jQuery('#wu-order-summary-content').length) { | ||
| if (! jQuery('#wu-order-summary-content').length) { | ||
|
|
||
| return; | ||
| return; | ||
|
|
||
| } // end if; | ||
| } // end if; | ||
|
|
||
| this.block(); | ||
| this.block(); | ||
|
|
||
| this.order = false; | ||
| this.order = false; | ||
|
|
||
| const that = this; | ||
| const that = this; | ||
|
|
||
| const _request = this.debounce(this.request); | ||
| const _request = this.debounce(this.request); | ||
|
|
||
| const data = { ...this.$data }; | ||
| const data = { ...this.$data }; | ||
|
|
||
| delete data.stored_templates; | ||
| delete data.state_list; | ||
| delete data.city_list; | ||
| delete data.labels; | ||
| delete data.stored_templates; | ||
| delete data.state_list; | ||
| delete data.city_list; | ||
| delete data.labels; | ||
|
|
||
| _request('wu_create_order', this.filter_for_request(data, 'wu_create_order'), function (results) { | ||
| _request('wu_create_order', this.filter_for_request(data, 'wu_create_order'), function (results) { | ||
|
|
||
| that.order = results.data.order; | ||
| that.order = results.data.order; | ||
|
|
||
| that.state_list = results.data.states; | ||
| that.state_list = results.data.states; | ||
|
|
||
| that.city_list = results.data.cities; | ||
| that.city_list = results.data.cities; | ||
|
|
||
| that.labels = results.data.labels; | ||
| that.labels = results.data.labels; | ||
|
|
||
| that.cart_type = results.data.order.type; | ||
| that.cart_type = results.data.order.type; | ||
|
|
||
| that.errors = results.data.order.errors; | ||
| that.errors = results.data.order.errors; | ||
|
|
||
| that.hooks.doAction('wu_on_create_order', that, results.data); | ||
| that.hooks.doAction('wu_on_create_order', that, results.data); | ||
|
|
||
| if (results.data.order.url) { | ||
| if (results.data.order.url) { | ||
|
|
||
| try { | ||
| try { | ||
|
|
||
| // history.pushState({}, null, wu_checkout.baseurl + results.data.order.url); | ||
| // history.pushState({}, null, wu_checkout.baseurl + results.data.order.url); | ||
|
|
||
| } catch (err) { | ||
| } catch (err) { | ||
|
|
||
| // eslint-disable-next-line no-console | ||
| console.warn('Browser does not support pushState.', err); | ||
| // eslint-disable-next-line no-console | ||
| console.warn('Browser does not support pushState.', err); | ||
|
|
||
| } // end try; | ||
| } // end try; | ||
|
|
||
| } // ed if; | ||
| } // ed if; |
There was a problem hiding this comment.
Remove unreachable try/catch block.
The try block is empty (pushState is commented), so the catch is unreachable per lint. Either re-enable pushState or remove the block to clear the error.
🧹 Minimal cleanup
- if (results.data.order.url) {
-
- try {
-
- // history.pushState({}, null, wu_checkout.baseurl + results.data.order.url);
-
- } catch (err) {
-
- // eslint-disable-next-line no-console
- console.warn('Browser does not support pushState.', err);
-
- } // end try;
-
- } // ed if;📝 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.
| create_order() { | |
| /* | |
| * Bail if there is no order summary to update. | |
| */ | |
| if (!jQuery('#wu-order-summary-content').length) { | |
| if (! jQuery('#wu-order-summary-content').length) { | |
| return; | |
| return; | |
| } // end if; | |
| } // end if; | |
| this.block(); | |
| this.block(); | |
| this.order = false; | |
| this.order = false; | |
| const that = this; | |
| const that = this; | |
| const _request = this.debounce(this.request); | |
| const _request = this.debounce(this.request); | |
| const data = { ...this.$data }; | |
| const data = { ...this.$data }; | |
| delete data.stored_templates; | |
| delete data.state_list; | |
| delete data.city_list; | |
| delete data.labels; | |
| delete data.stored_templates; | |
| delete data.state_list; | |
| delete data.city_list; | |
| delete data.labels; | |
| _request('wu_create_order', this.filter_for_request(data, 'wu_create_order'), function (results) { | |
| _request('wu_create_order', this.filter_for_request(data, 'wu_create_order'), function (results) { | |
| that.order = results.data.order; | |
| that.order = results.data.order; | |
| that.state_list = results.data.states; | |
| that.state_list = results.data.states; | |
| that.city_list = results.data.cities; | |
| that.city_list = results.data.cities; | |
| that.labels = results.data.labels; | |
| that.labels = results.data.labels; | |
| that.cart_type = results.data.order.type; | |
| that.cart_type = results.data.order.type; | |
| that.errors = results.data.order.errors; | |
| that.errors = results.data.order.errors; | |
| that.hooks.doAction('wu_on_create_order', that, results.data); | |
| that.hooks.doAction('wu_on_create_order', that, results.data); | |
| if (results.data.order.url) { | |
| if (results.data.order.url) { | |
| try { | |
| try { | |
| // history.pushState({}, null, wu_checkout.baseurl + results.data.order.url); | |
| // history.pushState({}, null, wu_checkout.baseurl + results.data.order.url); | |
| } catch (err) { | |
| } catch (err) { | |
| // eslint-disable-next-line no-console | |
| console.warn('Browser does not support pushState.', err); | |
| // eslint-disable-next-line no-console | |
| console.warn('Browser does not support pushState.', err); | |
| } // end try; | |
| } // end try; | |
| } // ed if; | |
| } // ed if; | |
| create_order() { | |
| /* | |
| * Bail if there is no order summary to update. | |
| */ | |
| if (! jQuery('#wu-order-summary-content').length) { | |
| return; | |
| } // end if; | |
| this.block(); | |
| this.order = false; | |
| const that = this; | |
| const _request = this.debounce(this.request); | |
| const data = { ...this.$data }; | |
| delete data.stored_templates; | |
| delete data.state_list; | |
| delete data.city_list; | |
| delete data.labels; | |
| _request('wu_create_order', this.filter_for_request(data, 'wu_create_order'), function (results) { | |
| that.order = results.data.order; | |
| that.state_list = results.data.states; | |
| that.city_list = results.data.cities; | |
| that.labels = results.data.labels; | |
| that.cart_type = results.data.order.type; | |
| that.errors = results.data.order.errors; | |
| that.hooks.doAction('wu_on_create_order', that, results.data); |
🧰 Tools
🪛 Biome (2.3.13)
[error] 508-513: This code is unreachable
(lint/correctness/noUnreachable)
🤖 Prompt for AI Agents
In `@assets/js/checkout.js` around lines 460 - 515, In create_order(), the
try/catch wrapping a commented-out history.pushState is unreachable; either
re-enable the pushState call so the catch can meaningfully handle failures or
remove the entire try/catch block (and the commented pushState) to satisfy the
linter; update the wu_on_create_order flow accordingly and ensure no leftover
commented history.pushState remains in the create_order method.
| /** | ||
| * Determines whether payment should be collected for the current checkout. | ||
| * | ||
| * Uses $this->order if available, otherwise builds a temporary Cart | ||
| * from the request/session data to check. | ||
| * | ||
| * @since 2.0.20 | ||
| * @return bool | ||
| */ | ||
| public function should_collect_payment(): bool { | ||
|
|
||
| if ($this->order) { | ||
| return $this->order->should_collect_payment(); | ||
| } | ||
|
|
||
| $products = $this->request_or_session('products', []); | ||
|
|
||
| if (empty($products)) { | ||
| return true; | ||
| } | ||
|
|
||
| try { | ||
| $cart = new Cart( | ||
| [ | ||
| 'products' => (array) $products, | ||
| 'country' => $this->request_or_session('billing_country'), | ||
| 'discount_code' => $this->request_or_session('discount_code'), | ||
| 'duration' => $this->request_or_session('duration'), | ||
| 'duration_unit' => $this->request_or_session('duration_unit'), | ||
| 'custom_amounts' => $this->request_or_session('custom_amounts', []), | ||
| 'pwyw_recurring' => $this->request_or_session('pwyw_recurring', []), | ||
| ] | ||
| ); | ||
|
|
||
| return $cart->should_collect_payment(); | ||
| } catch (\Throwable $e) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the Cart class
fd -t f "class-cart.php|Cart.php" inc/
# Check Cart's constructor signature
rg -n "public function __construct" inc/checkout/class-cart.php -A 20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 669
🏁 Script executed:
#!/bin/bash
# Read the full Cart constructor to see all parameters
sed -n '263,400p' inc/checkout/class-cart.php | cat -nRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 4457
🏁 Script executed:
#!/bin/bash
# Search for tax logic using state/city
rg -n -C5 "state|city" inc/checkout/class-cart.php | head -100Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2932
🏁 Script executed:
#!/bin/bash
# Find the should_collect_payment method in Cart
rg -n "should_collect_payment" inc/checkout/class-cart.php -A 30Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2173
🏁 Script executed:
#!/bin/bash
# Check if is_free() and get_recurring_total() depend on taxes
rg -n "function is_free|function get_recurring_total" inc/checkout/class-cart.php -A 15Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 958
🏁 Script executed:
#!/bin/bash
# Check if get_total() includes taxes
rg -n "function get_total\(" inc/checkout/class-cart.php -A 20 | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 678
🏁 Script executed:
#!/bin/bash
# Check if line_item get_total includes taxes
rg -n "class.*LineItem" inc/checkout/ -A 3Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for LineItem class
fd -t f "line" inc/checkout/ -type f
rg -n "get_total\(\)" inc/checkout/class-cart.php | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 813
🏁 Script executed:
#!/bin/bash
# Find where line_items are created and check their structure
rg -n "line_item.*get_total|->get_total" inc/checkout/class-cart.php -B 3 | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1469
🏁 Script executed:
#!/bin/bash
# Search for where line_items array is built to understand the object type
rg -n "\$this->line_items\s*=|->line_items\[\]" inc/checkout/class-cart.php -B 2 -A 2 | head -80Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Look for line_items initialization and use
rg -n "line_items" inc/checkout/class-cart.php | head -30Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1561
🏁 Script executed:
#!/bin/bash
# Search for where taxes are added to line items
rg -n "tax|->get_total" inc/checkout/class-cart.php | grep -i "tax\|recalculate"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2588
🏁 Script executed:
#!/bin/bash
# Check if membership_id, payment_id, auto_renew, cart_type affect the build_cart or should_collect_payment logic
rg -n "membership_id|payment_id|auto_renew|cart_type" inc/checkout/class-cart.php | head -40Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1970
🏁 Script executed:
#!/bin/bash
# Check what wu_get_applicable_tax_rates returns with empty state/city
rg -n "wu_get_applicable_tax_rates" --type-list 2>/dev/null || rg -rn "function wu_get_applicable_tax_rates" .Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 172
🏁 Script executed:
#!/bin/bash
# Get the full implementation of wu_get_applicable_tax_rates
rg -n "function wu_get_applicable_tax_rates" inc/functions/tax.php -A 30Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 886
🏁 Script executed:
#!/bin/bash
# Verify what happens when state/city are empty strings in wu_get_applicable_tax_rates
sed -n '130,180p' inc/functions/tax.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1170
State/city omission will miscalculate payment collection.
The temporary Cart constructed in should_collect_payment() omits state, city, membership_id, payment_id, auto_renew, and cart_type. These parameters directly affect cart calculations:
- Tax rates are determined by
wu_get_applicable_tax_rates($country, $state, $city)(Cart line 2490). Omitting state/city causes them to default to empty strings, bypassing state/city-specific tax rules. - Cart type determines setup fee and proration logic (Cart lines 1275, 2306, 2347). Defaulting to 'new' skips upgrade/downgrade calculations.
- membership_id/payment_id trigger recovery and upgrade workflows. Omitting these builds a fresh cart instead.
- auto_renew affects cart setup. Defaulting to
trueignores the actual customer preference.
Since should_collect_payment() depends on get_total() which includes calculated taxes, the decision to collect payment will be incorrect when taxes or prorations vary by location or cart type.
Pass the actual values from the request/session to align with real checkout state, or apply a filter to allow customization.
🤖 Prompt for AI Agents
In `@inc/checkout/class-checkout.php` around lines 1961 - 1999, The temporary Cart
built in should_collect_payment() omits state, city, membership_id, payment_id,
auto_renew and cart_type which can change tax, proration and recovery logic;
update the Cart instantiation in should_collect_payment() to pass these values
from request_or_session (e.g. 'state' =>
$this->request_or_session('billing_state'), 'city' =>
$this->request_or_session('billing_city'), 'membership_id' =>
$this->request_or_session('membership_id'), 'payment_id' =>
$this->request_or_session('payment_id'), 'auto_renew' =>
$this->request_or_session('auto_renew', true), 'cart_type' =>
$this->request_or_session('cart_type', 'new')) so Cart->get_total() and
Cart->should_collect_payment() reflect real checkout state; alternatively expose
a filter in should_collect_payment() to allow callers to inject/modify the cart
payload before creating the Cart.
Replace fragile UI-based wizard setup with WP-CLI/PHP fixture approach: - Add PHP fixtures for DB tables, products, checkout form, trial products, and verification scripts - Create 000-setup, 010-manual-checkout-flow, 020-free-trial-flow specs - Fix wpCli command (npx wp-env instead of npm run env) and add wpCliFile - Remove broken checkout commands and old specs that used vague selectors - Remove domain-mapping-roles and plugin specs (incompatible with wp-env) - Fix wizard.spec.js: handle variable button text on installer steps, add missing recommended-plugins step, reset custom login page on setup - Remove WooCommerce addon from core .wp-env.json test plugins - Add env:fix-perms script for Docker permission issues All 15 tests across 7 specs pass reliably on consecutive runs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@tests/e2e/cypress/fixtures/setup-checkout-form.php`:
- Line 6: Replace the loose text search using
WP_Ultimo\Models\Checkout_Form::query([ 'search' => 'main-form', ... ]) with the
exact-slug helper wu_get_checkout_form_by_slug('main-form') to fetch the form by
slug, and when creating a form check the return of wp_insert_post for WP_Error
(and falsy values) before proceeding—if wp_insert_post returns a WP_Error or
false, handle/log/return the error similar to the existing form creation check
so failures are detected and handled.
In `@tests/e2e/cypress/fixtures/setup-tables.php`:
- Line 13: The call to update_network_option currently has a prohibited space
before the opening parenthesis; remove the space so the function name is
immediately followed by the opening parenthesis (i.e., change
update_network_option( null, WP_Ultimo::NETWORK_OPTION_SETUP_FINISHED, time() )
to use no space between update_network_option and the '('), and ensure arguments
remain the same (null, WP_Ultimo::NETWORK_OPTION_SETUP_FINISHED, time()) so
static analysis passes.
In `@tests/e2e/cypress/fixtures/setup-trial-product.php`:
- Around line 7-10: The calls to the product setter methods (product->set_name,
product->set_slug, product->set_amount, product->set_duration) contain
prohibited spaces inside the parentheses; remove the extra space after the
opening parenthesis and before the closing parenthesis for each call so they
read like set_name('Trial Plan'), set_slug('trial-plan'), set_amount(19.99),
set_duration(1) to satisfy the code style/analysis rules.
In `@tests/e2e/cypress/fixtures/verify-manual-checkout-results.php`:
- Around line 22-29: Replace the call to json_encode(...) with
wp_json_encode(...) in the echo statement that outputs the array of
$um_payment_status, $um_payment_gateway, $um_payment_total,
$um_membership_status, $um_site_count, and $um_site_type; ensure the array
structure is unchanged and only the encoder function name (json_encode ->
wp_json_encode) is updated so the fixture uses WordPress' JSON encoder.
In `@tests/e2e/cypress/fixtures/verify-trial-results.php`:
- Around line 20-25: Replace the direct use of json_encode() with WordPress'
wp_json_encode() in the code that outputs the trial verification array (the echo
json_encode([...]) call); update the echo statement to call wp_json_encode() so
it uses WP's JSON encoding and error handling while keeping the same array keys
('um_payment_status', 'um_membership_status', 'um_membership_trial_end',
'um_site_type').
In `@tests/e2e/cypress/integration/000-setup.spec.js`:
- Around line 54-64: Add a readback assertion for the 'force_publish_sites_sync'
setting in the test so both settings changed by the cy.wpCli call are verified:
after calling cy.wpCli with wu_save_setting('enable_email_verification', ...)
and wu_save_setting('force_publish_sites_sync', true) add another cy.wpCli
invocation that runs wu_get_setting('force_publish_sites_sync') and assert the
result stdout contains "1" or "true" (whichever the app returns) to confirm the
change; reference the existing cy.wpCli helper and the
wu_get_setting/wu_save_setting symbols to locate where to add the check in the
"Should disable email verification and enable sync site publish" test.
- Around line 24-31: The test currently only logs the wpCliFile output and
doesn't assert success; update the "Should create a test product/plan" test to
assert the command succeeded by checking result.code (or result.stdout) contains
a valid product ID and that productId is non-empty; locate the cy.wpCliFile(...)
call in the it("Should create a test product/plan", ...) block and after
extracting productId (const productId = result.stdout.trim()) add an assertion
(e.g., expect(result.code).to.equal(0) or expect(productId).to.match(/\d+/) /
expect(productId).to.not.be.empty) so failures fail the spec instead of only
being logged.
In `@tests/e2e/cypress/integration/020-free-trial-flow.spec.js`:
- Around line 13-31: The test currently calls
cy.wpCliFile("tests/e2e/cypress/fixtures/setup-trial-product.php", {
failOnNonZeroExit: false }) and ignores the result, which can hide failures;
update the then handler for cy.wpCliFile to assert the command succeeded and
returned a non-empty numeric productId (check result.code === 0 or result.stderr
is empty and result.stdout.trim() is a positive integer) and fail the test early
if not—use the existing productId variable name in the assertion and log to
preserve behavior.
In `@tests/e2e/cypress/support/commands/index.js`:
- Around line 16-22: The wpCliFile helper currently interpolates filePath into
the shell command unsafely; sanitize and properly quote it before calling
cy.exec: in wpCliFile, validate filePath (reject empty, absolute paths starting
with '/' or '\\', any segment of '..', or any characters like
backticks/newlines/quotes), normalize it (e.g., use path.posix.normalize and
ensure the normalized path does not climb above the plugin directory), then
construct containerPath from the sanitized relative path and pass a safely
quoted containerPath to cy.exec (escape any single quotes if you use
single-quote wrapping) so the shell invocation in the cy.exec call cannot be
broken or used for path traversal or command injection.
🧹 Nitpick comments (9)
tests/e2e/cypress/support/commands/index.js (2)
5-10: Return thecy.execchainable fromwpCli.
Line 5: without a return, callers can’t reliably chain or assert on the exec result.Proposed fix
Cypress.Commands.add("wpCli", (command, options = {}) => { - cy.exec(`npx wp-env run tests-cli wp ${command}`, { + return cy.exec(`npx wp-env run tests-cli wp ${command}`, { ...options, timeout: options.timeout || 60000, }); });
25-37: Return thecy.requestchainable fromloginByApi.
Line 25: returning enables chaining and explicit waiting/inspection of the response.Proposed fix
Cypress.Commands.add("loginByApi", (username, password) => { - cy.request({ + return cy.request({ method: "POST", url: "/wp-login.php", form: true, body: { log: username, pwd: password, "wp-submit": "Log In", redirect_to: "/wp-admin/", testcookie: 1, }, }); });package.json (1)
77-78: Pin the Alpine image tag used in env:fix-perms.Line 77 uses
alpine(latest), which is non-deterministic and can change behavior over time. Pin to a specific stable release branch such asalpine:3.22or better yet a patch tag likealpine:3.22.3(or image digest) to keepenv:start(Line 78) reproducible.tests/e2e/cypress/fixtures/setup-product.php (1)
13-15: Consider adding error handling for save() failure.The
save()method can return aWP_Erroron failure (per the docstring in the relevant code snippets). If save fails,get_id()may return null/0, potentially causing silent test failures.Proposed fix
-$product->save(); +$result = $product->save(); + +if ( is_wp_error( $result ) ) { + echo 'error:' . $result->get_error_message(); + return; +} echo $product->get_id();tests/e2e/cypress/fixtures/setup-trial-product.php (1)
16-18: Consider adding error handling for save() failure.Similar to
setup-product.php, ifsave()fails,get_id()may return null, leading to an invalid product ID being appended to the pricing table.Proposed fix
-$product->save(); - -$product_id = $product->get_id(); +$result = $product->save(); + +if ( is_wp_error( $result ) ) { + echo 'error:' . $result->get_error_message(); + return; +} + +$product_id = $product->get_id();tests/e2e/cypress/fixtures/setup-checkout-form.php (1)
31-40: Consider checkingwp_insert_postreturn value for errors.
wp_insert_postcan return aWP_Erroron failure. Without checking, a failed page insertion would save an error object as thedefault_registration_pagesetting.Proposed fix
$page_id = wp_insert_post( [ 'post_name' => 'register', 'post_title' => 'Register', 'post_content' => '[wu_checkout slug="main-form"]', 'post_status' => 'publish', 'post_type' => 'page', 'post_author' => 1, ] ); +if ( is_wp_error( $page_id ) ) { + echo 'error:' . $page_id->get_error_message(); + return; +} + wu_save_setting( 'default_registration_page', $page_id );tests/e2e/cypress/integration/wizard.spec.js (1)
77-80: Consider adding an explicit URL assertion before clicking.The
cy.url({ timeout: 120000 })on line 77 validates navigation, but for consistency with other steps, consider addingcy.assertPageUrl()to verify the full page context before interacting with the button.Proposed enhancement
/** * Steps: Recommended Plugins * May download plugins from wordpress.org via AJAX; allow extra time. */ cy.url({ timeout: 120000 }).should("include", "step=recommended-plugins"); + cy.assertPageUrl({ + pathname: "/wp-admin/network/admin.php", + page: page_name, + step: "recommended-plugins", + }); cy.get('button[data-testid="button-primary"]') .should("not.be.disabled") .click();tests/e2e/cypress/integration/010-manual-checkout-flow.spec.js (2)
1-11: Optional: share test-data helpers to reduce duplication.This same timestamp-based customer/site data pattern appears in the free-trial spec; a small helper would centralize future edits.
13-83: Make this spec self-sufficient instead of relying on earlier setup specs.This flow implicitly depends on the checkout form and manual gateway being preconfigured (likely by 000-setup). If this spec runs alone or in parallel, it can fail unexpectedly. Consider adding a local
before()that ensures the checkout form exists and the manual gateway is enabled (or at least verifies those preconditions).
| * Create a checkout form and registration page for e2e testing. | ||
| * Idempotent: skips creation if the form already exists. | ||
| */ | ||
| $existing = WP_Ultimo\Models\Checkout_Form::query( [ 'search' => 'main-form', 'number' => 1 ] ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure
find . -type f -name "*.php" | grep -i checkout_form | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 128
🏁 Script executed:
# Look for the Checkout_Form model
find . -type f -path "*Models*" -name "*.php" | grep -i checkoutRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 128
🏁 Script executed:
# Check the structure of inc/database directory
ls -la inc/database/ 2>/dev/null || echo "Directory not found, searching..."
find . -type d -name "*checkout*" | head -10Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1142
🏁 Script executed:
# Find the setup-checkout-form.php file mentioned in the review
find . -type f -name "setup-checkout-form.php"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 133
🏁 Script executed:
# Let's examine the setup-checkout-form.php file
cat -n ./tests/e2e/cypress/fixtures/setup-checkout-form.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1438
🏁 Script executed:
# Now let's look at the checkout-forms database directory
ls -la ./inc/database/checkout-forms/Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 523
🏁 Script executed:
# Search for the Checkout_Form model class
find . -type f -name "*.php" -path "*Models*" | xargs grep -l "class Checkout_Form"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 128
🏁 Script executed:
# Find the Checkout_Form model class
find . -type f -name "*.php" | xargs grep -l "class Checkout_Form" | grep -v TestRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 582
🏁 Script executed:
# Examine the Checkout_Form query class
cat -n ./inc/database/checkout-forms/class-checkout-form-query.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2551
🏁 Script executed:
# Let's also check the schema to understand the structure
cat -n ./inc/database/checkout-forms/class-checkout-forms-schema.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2856
🏁 Script executed:
# Now let's look at the Checkout_Form model
cat -n ./inc/models/class-checkout-form.php | head -100Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2310
🏁 Script executed:
# Let's find the Query base class to understand what parameters are supported
find . -type f -path "*Database/Engine*" -name "*.php" | head -10Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Search for the Query class definition
find . -type f -name "*.php" -path "*Engine*" | xargs grep -l "class Query"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 65
🏁 Script executed:
# Search for the Query engine class
find . -type f -name "*.php" | xargs grep -l "class Query" | grep -i engineRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 118
🏁 Script executed:
# Let's check the engine directory structure
ls -la ./inc/database/engine/Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 646
🏁 Script executed:
# Look for query parameter handling - search for how 'search' parameter is handled
rg "search" ./inc/database/engine/ -A 3 -B 3Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Let's examine the Query base class
cat -n ./inc/database/engine/class-query.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 3525
🏁 Script executed:
# Search for how 'search' parameter is handled in the entire database engine
rg "search" ./inc/database/engine/*.php -A 2 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Let's search for query parameter handling more broadly
rg "'search'" . --type php -A 2 -B 2 | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2542
🏁 Script executed:
# Let's search for how other models query by slug
rg "slug" ./inc/models/ --type php -B 2 -A 2 | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2774
🏁 Script executed:
# Search for other usage of query() method with different parameters
rg "::query\(" . --type php -A 1 | grep -E "(slug|search)" | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 230
🏁 Script executed:
# Let's check if wp_insert_post error handling is addressed elsewhere in the codebase
rg "wp_insert_post" . --type php -A 3 | head -60Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 4579
🏁 Script executed:
# Let's check if we can query by slug directly - look for other query examples in tests
rg "::query\(" ./tests --type php -A 2 | head -40Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2937
🏁 Script executed:
# Let's search for BerlinDB documentation or see how direct field queries work
rg "->query" ./inc --type php -A 1 | grep -E "slug|name|id" -A 1 | head -30Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 485
🏁 Script executed:
# Check the specific test file for Checkout_Form to see how queries are tested
cat -n ./tests/WP_Ultimo/Models/Checkout_Form_Test.php | head -100Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 3660
🏁 Script executed:
# Let's check how BerlinDB or the Query class handles direct field parameters
# Search for examples of direct field queries in the codebase
rg "query\(\s*\[\s*'" ./inc --type php -A 2 | head -60Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Let's look for wp_error checking patterns in the codebase for wp_insert_post
rg "is_wp_error.*wp_insert_post\|wp_insert_post.*is_wp_error" . --type php -B 2 -A 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Let's verify if direct field queries are used elsewhere
rg "query\(\s*\[\s*'(name|slug|id)'" ./inc --type php -A 1Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Let's check the parent class BerlinDB more carefully - look for how query parameters are handled
rg "search.*searchable\|searchable.*search" ./inc/database/engine/ -A 2 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Look at the schema to understand which columns are searchable
rg "searchable" ./inc/database/checkout-forms/ -A 2 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 964
🏁 Script executed:
# Let's check if BerlinDB documentation or comments mention parameter handling
rg "search\|slug" ./inc/database/checkout-forms/class-checkout-form-query.php -B 3 -A 3Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Search for how query methods handle field-specific parameters in other models
rg "query.*slug\|slug.*query" ./inc/functions/ --type php -B 2 -A 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Let's look at how wp_insert_post results are used in the file
cat -n ./tests/e2e/cypress/fixtures/setup-checkout-form.php | tail -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 636
🏁 Script executed:
# Search for other E2E fixtures that use wp_insert_post to see if they check for errors
rg "wp_insert_post" ./tests/e2e/cypress/fixtures/ -A 4 -B 1 | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 547
🏁 Script executed:
# Let's check if there's any documentation or comments in BerlinDB integration
find . -type f -name "*.php" | xargs grep -l "BerlinDB" | head -5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 270
🏁 Script executed:
# Let's check if direct field matching with slug works by looking at BerlinDB usage
rg "column_defaults\|get_column" ./inc/database/engine/ -A 5 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
# Let's understand how BerlinDB supports field-specific queries
# Check if there are examples of __in or direct field parameters
rg "'[a-z_]+__in'" ./inc --type php -A 1 | head -30Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1493
🏁 Script executed:
# Let's also check if the concern about search matching multiple fields is valid
# by testing a real scenario - see if there are multiple checkoutforms with similar names
rg "Checkout_Form" ./inc/functions/ --type php -A 5 -B 2 | head -40Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2210
🏁 Script executed:
# Let's look at how BerlinDB query actually works by checking documentation in code comments
cat -n ./inc/database/engine/class-query.php | grep -A 20 "public function\|@param\|@return"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1744
🏁 Script executed:
# I found a reference to wu_get_checkout_form_by_slug! Let's see the implementation
cat -n ./inc/functions/checkout-form.php | grep -A 10 "wu_get_checkout_form_by_slug"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 779
🏁 Script executed:
# Let's see the complete checkout-form.php file
cat -n ./inc/functions/checkout-form.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 6703
🏁 Script executed:
# Check if is_wp_error is used anywhere in E2E fixtures
rg "is_wp_error" ./tests/e2e/cypress/fixtures/ -A 2 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 477
Use dedicated slug lookup function and add error handling for wp_insert_post.
The 'search' => 'main-form' performs a text search which may match unintended forms. Use the existing wu_get_checkout_form_by_slug('main-form') function for exact slug matching. Additionally, wp_insert_post() can return a WP_Error on failure but the code doesn't check for this; add error handling similar to the form creation check above.
Proposed fix
-$existing = WP_Ultimo\Models\Checkout_Form::query( [ 'search' => 'main-form', 'number' => 1 ] );
+$existing = wu_get_checkout_form_by_slug( 'main-form' );
-if ( $existing ) {
- $form = $existing[0];
+if ( $existing ) {
+ $form = $existing;
$page_id = wu_get_setting( 'default_registration_page', 0 );
echo 'form:' . $form->get_id() . ',page:' . $page_id;
return;
}
$page_id = wp_insert_post( [...] );
+
+if ( is_wp_error( $page_id ) ) {
+ echo 'error:' . $page_id->get_error_message();
+ return;
+}🤖 Prompt for AI Agents
In `@tests/e2e/cypress/fixtures/setup-checkout-form.php` at line 6, Replace the
loose text search using WP_Ultimo\Models\Checkout_Form::query([ 'search' =>
'main-form', ... ]) with the exact-slug helper
wu_get_checkout_form_by_slug('main-form') to fetch the form by slug, and when
creating a form check the return of wp_insert_post for WP_Error (and falsy
values) before proceeding—if wp_insert_post returns a WP_Error or false,
handle/log/return the error similar to the existing form creation check so
failures are detected and handled.
| it("Should create a test product/plan", () => { | ||
| cy.wpCliFile("tests/e2e/cypress/fixtures/setup-product.php", { | ||
| failOnNonZeroExit: false, | ||
| }).then((result) => { | ||
| const productId = result.stdout.trim(); | ||
| cy.log(`Created test product with ID: ${productId}`); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Assert product creation success.
The fixture result is only logged; add a quick check to catch failures early.
✅ Suggested guard
cy.wpCliFile("tests/e2e/cypress/fixtures/setup-product.php", {
failOnNonZeroExit: false,
}).then((result) => {
const productId = result.stdout.trim();
+ expect(productId, "test product id").to.not.equal("");
cy.log(`Created test product with ID: ${productId}`);
});🤖 Prompt for AI Agents
In `@tests/e2e/cypress/integration/000-setup.spec.js` around lines 24 - 31, The
test currently only logs the wpCliFile output and doesn't assert success; update
the "Should create a test product/plan" test to assert the command succeeded by
checking result.code (or result.stdout) contains a valid product ID and that
productId is non-empty; locate the cy.wpCliFile(...) call in the it("Should
create a test product/plan", ...) block and after extracting productId (const
productId = result.stdout.trim()) add an assertion (e.g.,
expect(result.code).to.equal(0) or expect(productId).to.match(/\d+/) /
expect(productId).to.not.be.empty) so failures fail the spec instead of only
being logged.
| it("Should disable email verification and enable sync site publish", () => { | ||
| cy.wpCli( | ||
| "eval \"wu_save_setting('enable_email_verification', 'never'); wu_save_setting('force_publish_sites_sync', true);\"" | ||
| ); | ||
|
|
||
| cy.wpCli( | ||
| "eval \"echo wu_get_setting('enable_email_verification', 'always');\"" | ||
| ).then((result) => { | ||
| expect(result.stdout).to.contain("never"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Also verify force_publish_sites_sync is actually set.
You set two settings but only assert one; add a quick readback for force_publish_sites_sync to avoid silent regressions.
✅ Suggested check
cy.wpCli(
"eval \"wu_save_setting('enable_email_verification', 'never'); wu_save_setting('force_publish_sites_sync', true);\""
);
cy.wpCli(
"eval \"echo wu_get_setting('enable_email_verification', 'always');\""
).then((result) => {
expect(result.stdout).to.contain("never");
});
+
+ cy.wpCli(
+ "eval \"echo wu_get_setting('force_publish_sites_sync', false) ? 'true' : 'false';\""
+ ).then((result) => {
+ expect(result.stdout).to.contain("true");
+ });🤖 Prompt for AI Agents
In `@tests/e2e/cypress/integration/000-setup.spec.js` around lines 54 - 64, Add a
readback assertion for the 'force_publish_sites_sync' setting in the test so
both settings changed by the cy.wpCli call are verified: after calling cy.wpCli
with wu_save_setting('enable_email_verification', ...) and
wu_save_setting('force_publish_sites_sync', true) add another cy.wpCli
invocation that runs wu_get_setting('force_publish_sites_sync') and assert the
result stdout contains "1" or "true" (whichever the app returns) to confirm the
change; reference the existing cy.wpCli helper and the
wu_get_setting/wu_save_setting symbols to locate where to add the check in the
"Should disable email verification and enable sync site publish" test.
| before(() => { | ||
| cy.loginByForm( | ||
| Cypress.env("admin").username, | ||
| Cypress.env("admin").password | ||
| ); | ||
|
|
||
| // Enable trial without payment method | ||
| cy.wpCli( | ||
| "eval \"wu_save_setting('allow_trial_without_payment_method', true);\"" | ||
| ); | ||
|
|
||
| // Create the trial product | ||
| cy.wpCliFile("tests/e2e/cypress/fixtures/setup-trial-product.php", { | ||
| failOnNonZeroExit: false, | ||
| }).then((result) => { | ||
| const productId = result.stdout.trim(); | ||
| cy.log(`Created trial product with ID: ${productId}`); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Fail fast if trial product creation fails.
With failOnNonZeroExit: false and no stdout check, a fixture failure can be silently ignored, leading to flaky downstream errors. Add a simple assertion on the returned ID (or stderr) to surface failures early.
✅ Suggested guard
cy.wpCliFile("tests/e2e/cypress/fixtures/setup-trial-product.php", {
failOnNonZeroExit: false,
}).then((result) => {
const productId = result.stdout.trim();
+ expect(productId, "trial product id").to.not.equal("");
cy.log(`Created trial product with ID: ${productId}`);
});📝 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.
| before(() => { | |
| cy.loginByForm( | |
| Cypress.env("admin").username, | |
| Cypress.env("admin").password | |
| ); | |
| // Enable trial without payment method | |
| cy.wpCli( | |
| "eval \"wu_save_setting('allow_trial_without_payment_method', true);\"" | |
| ); | |
| // Create the trial product | |
| cy.wpCliFile("tests/e2e/cypress/fixtures/setup-trial-product.php", { | |
| failOnNonZeroExit: false, | |
| }).then((result) => { | |
| const productId = result.stdout.trim(); | |
| cy.log(`Created trial product with ID: ${productId}`); | |
| }); | |
| }); | |
| before(() => { | |
| cy.loginByForm( | |
| Cypress.env("admin").username, | |
| Cypress.env("admin").password | |
| ); | |
| // Enable trial without payment method | |
| cy.wpCli( | |
| "eval \"wu_save_setting('allow_trial_without_payment_method', true);\"" | |
| ); | |
| // Create the trial product | |
| cy.wpCliFile("tests/e2e/cypress/fixtures/setup-trial-product.php", { | |
| failOnNonZeroExit: false, | |
| }).then((result) => { | |
| const productId = result.stdout.trim(); | |
| expect(productId, "trial product id").to.not.equal(""); | |
| cy.log(`Created trial product with ID: ${productId}`); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In `@tests/e2e/cypress/integration/020-free-trial-flow.spec.js` around lines 13 -
31, The test currently calls
cy.wpCliFile("tests/e2e/cypress/fixtures/setup-trial-product.php", {
failOnNonZeroExit: false }) and ignores the result, which can hide failures;
update the then handler for cy.wpCliFile to assert the command succeeded and
returned a non-empty numeric productId (check result.code === 0 or result.stderr
is empty and result.stdout.trim() is a positive integer) and fail the test early
if not—use the existing productId variable name in the assertion and log to
preserve behavior.
| Cypress.Commands.add("wpCliFile", (filePath, options = {}) => { | ||
| const containerPath = `/var/www/html/wp-content/plugins/ultimate-multisite/${filePath}`; | ||
|
|
||
| cy.exec(`npx wp-env run tests-cli wp eval-file ${containerPath}`, { | ||
| ...options, | ||
| timeout: options.timeout || 60000, | ||
| }); |
There was a problem hiding this comment.
Validate and quote filePath before shell execution.
Line 16-20: filePath is interpolated into a shell command and could include .. or quotes that escape the plugin root or break the command.
Proposed fix
Cypress.Commands.add("wpCliFile", (filePath, options = {}) => {
- const containerPath = `/var/www/html/wp-content/plugins/ultimate-multisite/${filePath}`;
+ if (filePath.includes("..")) {
+ throw new Error("filePath must stay within the plugin root");
+ }
+ const containerPath =
+ `/var/www/html/wp-content/plugins/ultimate-multisite/${filePath}`.replace(/"/g, '\\"');
- cy.exec(`npx wp-env run tests-cli wp eval-file ${containerPath}`, {
+ return cy.exec(`npx wp-env run tests-cli wp eval-file "${containerPath}"`, {
...options,
timeout: options.timeout || 60000,
});
});📝 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.
| Cypress.Commands.add("wpCliFile", (filePath, options = {}) => { | |
| const containerPath = `/var/www/html/wp-content/plugins/ultimate-multisite/${filePath}`; | |
| cy.exec(`npx wp-env run tests-cli wp eval-file ${containerPath}`, { | |
| ...options, | |
| timeout: options.timeout || 60000, | |
| }); | |
| Cypress.Commands.add("wpCliFile", (filePath, options = {}) => { | |
| if (filePath.includes("..")) { | |
| throw new Error("filePath must stay within the plugin root"); | |
| } | |
| const containerPath = | |
| `/var/www/html/wp-content/plugins/ultimate-multisite/${filePath}`.replace(/"/g, '\\"'); | |
| return cy.exec(`npx wp-env run tests-cli wp eval-file "${containerPath}"`, { | |
| ...options, | |
| timeout: options.timeout || 60000, | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In `@tests/e2e/cypress/support/commands/index.js` around lines 16 - 22, The
wpCliFile helper currently interpolates filePath into the shell command
unsafely; sanitize and properly quote it before calling cy.exec: in wpCliFile,
validate filePath (reject empty, absolute paths starting with '/' or '\\', any
segment of '..', or any characters like backticks/newlines/quotes), normalize it
(e.g., use path.posix.normalize and ensure the normalized path does not climb
above the plugin directory), then construct containerPath from the sanitized
relative path and pass a safely quoted containerPath to cy.exec (escape any
single quotes if you use single-quote wrapping) so the shell invocation in the
cy.exec call cannot be broken or used for path traversal or command injection.
Apply WordPress coding standards: escape echo output with esc_html(), use wp_json_encode() instead of json_encode(), fix spacing in function calls and arrays. 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: |
…rror handling - Reference actual spec files (000-setup, 010-manual-checkout-flow, 020-free-trial-flow) instead of non-existent ones (checkout-registration, setup-wizard-complete, etc.) - Remove invalid Cypress CLI flags (--video, --screenshot-on-failure, --record, --reporter-options) that caused 'unknown option' errors - Remove continue-on-error from test steps so failures properly fail the job - Remove redundant 'Fail job if tests failed' step 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: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@inc/class-ajax.php`:
- Around line 112-115: The current condition overwrites a user-provided
$args['query']['number']; change the logic so the user-provided value is
preserved by copying $args['query']['number'] into $args['number'] when it's
present: check ! empty($args['query']['number']) and set $args['number'] =
$args['query']['number']; alternatively, if the intent was to set a default only
when query.number is missing, invert the condition to if (
empty($args['query']['number']) ) and set $args['query']['number'] =
$args['number'] — update the code around the $args['query']['number'] /
$args['number'] handling accordingly.
🧹 Nitpick comments (1)
inc/documentation/berlindb-dynamic-hooks.php (1)
143-157: Minor grammar issues in generated docblocks — fix in the generator script.Several auto-generated descriptions have awkward phrasing that would improve documentation readability:
- Line 144: "the has trialed of a customer" → "the has_trialed flag of a customer"
- Line 159: "the vip of a customer" → "the VIP status of a customer"
- Line 349: "The previous value value" → "The previous value" (duplicate word)
- Lines 386, 538, 568, 842: "the active of a..." → "the active status of a..."
- Line 523: "the domain of a domain" → "the domain name of a domain record"
- Lines 690, 705: "a event" → "an event"
Since this file should not be edited manually, consider updating
inc/documentation/generate-berlindb-hooks.phpto improve the humanization logic for these edge cases.
| // Number can be in the query array or it's own. Code uses both. | ||
| if (! empty($args['query']['number'])) { | ||
| $args['query']['number'] = $args['number']; | ||
| } |
There was a problem hiding this comment.
Logic appears inverted: user-provided query.number is overwritten instead of preserved.
The condition ! empty($args['query']['number']) triggers when the user has provided a value, then overwrites it with $args['number']. This discards user input rather than synchronizing it.
If the intent is for query.number to override args.number:
-// Number can be in the query array or it's own. Code uses both.
-if (! empty($args['query']['number'])) {
- $args['query']['number'] = $args['number'];
-}
+// Allow query.number to override the top-level number if provided.
+if (! empty($args['query']['number'])) {
+ $args['number'] = $args['query']['number'];
+}Alternatively, if the intent is to set a default when not provided:
-if (! empty($args['query']['number'])) {
+if (empty($args['query']['number'])) {
$args['query']['number'] = $args['number'];
},
🤖 Prompt for AI Agents
In `@inc/class-ajax.php` around lines 112 - 115, The current condition overwrites
a user-provided $args['query']['number']; change the logic so the user-provided
value is preserved by copying $args['query']['number'] into $args['number'] when
it's present: check ! empty($args['query']['number']) and set $args['number'] =
$args['query']['number']; alternatively, if the intent was to set a default only
when query.number is missing, invert the condition to if (
empty($args['query']['number']) ) and set $args['query']['number'] =
$args['number'] — update the code around the $args['query']['number'] /
$args['number'] handling accordingly.
🔨 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: |
Remove PHP 7.4, 8.0, and 8.1 from the CI test matrix. 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: |
🔨 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: |
1 participant
and a few other tweaks
closes #330
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Documentation
Tests