Add modern PayPal REST API gateway with subscription support#305
Add modern PayPal REST API gateway with subscription support#305superdav42 wants to merge 11 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a PayPal REST gateway (OAuth, token caching, API wrapper, orders/subscriptions, webhooks), a reusable Base_PayPal_Gateway, a PayPal OAuth onboarding handler, a webhook processor, and updates the legacy NVP PayPal gateway and Base_Gateway URL helper return types and gateway manager registration/hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Site as WP Site (PayPal_REST_Gateway)
participant PayPal as PayPal API
participant Webhook as PayPal_Webhook_Handler
participant DB as WP Ultimo Storage
User->>Site: Start checkout (create order/subscription)
Site->>PayPal: Create Order/Subscription -> returns approval URL
PayPal-->>User: Approval page (redirect)
User->>PayPal: Approve payment/subscription
PayPal-->>Site: Redirect callback (token / subscription_id)
Site->>PayPal: Confirm order/subscription (API)
PayPal-->>Site: Confirmation (status, transaction ids)
Site->>DB: Persist payment/membership updates
PayPal->>Webhook: Send webhook events (captures, refunds, subs updates)
Webhook->>Site: Verify & dispatch -> Site updates DB/state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 1
🧹 Nitpick comments (1)
inc/gateways/class-paypal-gateway.php (1)
848-852: Consider using integer for HTTP response code.The
responseparameter inwp_die()expects an integer. While the string'200'works (WordPress casts it), using an integer200would be more conventional and explicit.This pattern appears in multiple
wp_die()calls throughout the file (lines 848, 862, 885, 906, 927, 1351, 1381, 1477, 1558, 1580, 1658, 1687, 1705, 1724).🔎 Example fix (apply to all occurrences)
- 'response' => '200', + 'response' => 200,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
inc/gateways/class-paypal-gateway.php
🧰 Additional context used
🪛 PHPMD (2.15.0)
inc/gateways/class-paypal-gateway.php
1802-1802: Avoid unused local variables such as '$custom'. (undefined)
(UnusedLocalVariable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: PHP 8.4
- GitHub Check: Build Plugin for Testing
- GitHub Check: PHP 8.1
🔇 Additional comments (7)
inc/gateways/class-paypal-gateway.php (7)
833-866: LGTM!Good addition of WP_Error handling. The error detection for both
WP_Errorobjects and empty/invalid responses covers the main failure scenarios. Logging withLogLevel::ERRORand usingwp_die()withback_linkprovides clear user feedback and debugging capability.
878-931: LGTM!Consistent error handling for missing required objects (payment, cart, membership, customer). Each failure path now logs the error and provides a user-friendly message with a back link, addressing the original issue where errors were silently swallowed.
1333-1397: LGTM!Comprehensive error handling for
CreateRecurringPaymentsProfile. The changes properly:
- Log before initiating the request
- Handle
WP_Errorfromwp_remote_post- Check for both
failureandfailurewithwarningACK values- Extract and display PayPal error codes/messages
- Log success with transaction details for debugging
1469-1480: LGTM!Good addition of logging for unexpected HTTP responses before the
wp_die()call. This will help diagnose edge cases where PayPal returns non-200 status codes.
1532-1662: LGTM!The
complete_single_paymentmethod now has consistent error handling that mirrorscreate_recurring_profile. The addition of logging at entry, for failures, and on success provides good observability for debugging payment issues.
1668-1738: LGTM!The
confirmation_formmethod now properly validates all required objects before rendering the template:
- Validates checkout details response
- Checks for pending payment existence
- Verifies customer and membership data
The return type change to
voidis appropriate since the method now useswp_die()for errors instead of returning error strings.
1779-1790: LGTM - Core fix for the checkout loop issue.This is the critical fix addressing the root cause from Issue #193. The method now properly checks for
ACK=FailureandACK=FailureWithWarningresponses and returns aWP_Errorinstead of treating the response as valid. This prevents the silent loop back to/register.
| $body['pending_payment'] = $pending_payment; | ||
|
|
||
| $custom = explode('|', (string) $body['PAYMENTREQUEST_0_CUSTOM']); | ||
| $custom = explode('|', (string) wu_get_isset($body, 'PAYMENTREQUEST_0_CUSTOM', '')); |
There was a problem hiding this comment.
Remove unused variable.
The $custom variable is assigned but never used. This appears to be dead code.
🔎 Proposed fix
- $custom = explode('|', (string) wu_get_isset($body, 'PAYMENTREQUEST_0_CUSTOM', ''));
-
return $body;Based on static analysis hint from PHPMD.
📝 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.
| $custom = explode('|', (string) wu_get_isset($body, 'PAYMENTREQUEST_0_CUSTOM', '')); | |
| return $body; |
🧰 Tools
🪛 PHPMD (2.15.0)
1802-1802: Avoid unused local variables such as '$custom'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In inc/gateways/class-paypal-gateway.php around line 1802, the variable $custom
is assigned from explode(...) but never used; remove that unused assignment line
to eliminate dead code and satisfy PHPMD static analysis.
🔨 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
🤖 Fix all issues with AI agents
In @inc/gateways/class-base-paypal-gateway.php:
- Around line 164-177: Replace usages of the PHP 8-only function str_starts_with
(e.g., the check in class-base-paypal-gateway.php that tests
$gateway_subscription_id) with a PHP 7.4-compatible check: use
strpos($gateway_subscription_id, 'I-') === 0 (or an equivalent substr/strncmp)
to detect the prefix; apply the same replacement to the other ~28 occurrences
across the codebase or alternatively add a simple polyfill for str_starts_with
if you prefer to keep the original calls, but do not change composer PHP
requirement unless intentionally bumping minimum PHP to 8.0.
In @inc/gateways/class-paypal-oauth-handler.php:
- Around line 724-754: The delete_webhooks_on_disconnect method is calling
$gateway_manager->get_gateway('paypal-rest') which returns gateway metadata, not
an instance; replace that call with the Gateway_Manager method that returns an
actual gateway object (for example use get_gateway_instance('paypal-rest') or
the manager's equivalent instance factory), then verify the returned value is an
instance of PayPal_REST_Gateway before calling set_test_mode() and
delete_webhook(); update the variable name if needed (e.g., $gateway_instance)
and keep the existing error logging and try/catch logic unchanged.
- Around line 688-714: The instanceof check in install_webhook_after_oauth is
wrong because $gateway_manager->get_gateway('paypal-rest') returns metadata, so
replace that call with wu_get_gateway('paypal-rest') to obtain an actual
PayPal_REST_Gateway instance; update the code path that currently assigns
$gateway from Gateway_Manager::get_instance()->get_gateway(...) to call
wu_get_gateway('paypal-rest'), keep the subsequent set_test_mode/install_webhook
logic and the same logging branches, and ensure the instanceof
PayPal_REST_Gateway check now succeeds and the webhook installation is
attempted.
In @inc/gateways/class-paypal-webhook-handler.php:
- Around line 183-187: The current check in class PayPal_Webhook_Handler (when
$webhook_id is empty) returns $this->test_mode which accepts any webhook in test
mode; change this to never auto-accept missing webhook IDs. Replace the line
"return $this->test_mode" so the method logs a clear error and returns false (do
not accept the request), e.g., keep the warning log or escalate to error and
then return false; if you need an explicit opt-in for development, add a boolean
property like $allow_unverified_webhooks_in_test (default false) and only return
true when that property is explicitly true.
- Around line 172-176: The current unconditional bypass in the signature-check
block (when $this->test_mode and WP_DEBUG) is unsafe; change it to only skip
cryptographic verification while still requiring the webhook headers to exist
and be well-formed: keep the $this->test_mode and WP_DEBUG branch but replace
the immediate return true with a header presence check for the PayPal webhook
headers (e.g., 'Paypal-Transmission-Id', 'Paypal-Transmission-Sig',
'Paypal-Transmission-Time', 'Paypal-Cert-Url') and return true only if those
headers are present (otherwise return false and log an error); update the log
call ($this->log('Skipping signature verification in debug mode')) to include
which headers were missing so the verification path in the class (the method
using $this->test_mode and the log call) still defends against spoofed requests
in debug/test environments.
🧹 Nitpick comments (7)
inc/gateways/class-paypal-oauth-handler.php (1)
393-394: Remove unused variables or add TODO for future use.
$consent_statusand$risk_statusare captured from PayPal's OAuth callback but never used. If these are intended for future validation logic, add a TODO comment; otherwise, remove them to reduce dead code.♻️ Suggested fix
- $consent_status = isset($_GET['consentStatus']) && 'true' === $_GET['consentStatus']; - $risk_status = isset($_GET['isEmailConfirmed']) ? sanitize_text_field(wp_unslash($_GET['isEmailConfirmed'])) : ''; + // Note: consentStatus and isEmailConfirmed are available in the callback but not currently used + // They could be used for additional validation if needed in the futureinc/gateways/class-paypal-gateway.php (1)
1793-1793: Remove unused$customvariable.The
$customvariable is parsed from the response but never used.♻️ Suggested fix
- $custom = explode('|', (string) wu_get_isset($body, 'PAYMENTREQUEST_0_CUSTOM', ''));inc/gateways/class-paypal-webhook-handler.php (1)
419-481: Handle potential duplicate payment creation race condition.The check for existing payment on line 449-454 and subsequent payment creation could have a race condition if two webhook deliveries arrive simultaneously. Consider using a database lock or unique constraint.
💡 Consideration
The current flow:
- Check if payment exists by gateway_payment_id
- If not, create payment
If two webhooks arrive simultaneously, both could pass the existence check before either creates the payment. Consider wrapping this in a transaction or using a unique constraint on
gateway_payment_idin the database.inc/gateways/class-paypal-rest-gateway.php (4)
376-395: Remove unused$descriptionvariable.The
$descriptionvariable is created on line 379 but never used in the subscription creation flow.♻️ Suggested fix
protected function create_subscription($payment, $membership, $customer, $cart, $type): void { $currency = strtoupper($payment->get_currency()); - $description = $this->get_subscription_description($cart); // First, create or get the billing plan $plan_id = $this->get_or_create_plan($cart, $currency);
788-788: Unused$customer_idfrom destructuring.The
$customer_idis extracted but not used. If it's not needed, use an underscore or remove it from destructuring.♻️ Suggested fix
- [$payment_id, $membership_id, $customer_id] = $custom_parts; + [$payment_id, $membership_id] = $custom_parts;Apply the same fix on line 880.
Also applies to: 880-880
1262-1313: Inline JavaScript in settings should be extracted.The inline JavaScript for the connect/disconnect buttons works but could be extracted to a separate JS file for better maintainability and to enable proper linting.
993-1009:process_membership_updateis incomplete.The method logs the update request but doesn't actually implement the update logic. PayPal subscription updates are limited, but the method should either attempt what's possible or return an error explaining limitations.
💡 Suggested improvement
public function process_membership_update(&$membership, $customer) { $subscription_id = $membership->get_gateway_subscription_id(); if (empty($subscription_id)) { return new \WP_Error( 'wu_paypal_no_subscription', __('No subscription ID found for this membership.', 'ultimate-multisite') ); } - // Note: PayPal subscription updates are limited - // For significant changes, may need to cancel and recreate - $this->log(sprintf('Membership update requested for subscription: %s', $subscription_id)); + // PayPal REST API has limited subscription update capabilities + // For amount/duration changes, cancel current subscription and create new one + $this->log(sprintf('Membership update requested for subscription: %s - cancelling and requiring new subscription', $subscription_id)); - return true; + // Cancel the current subscription + $this->process_cancellation($membership, $customer); + + // Reset gateway info so a new subscription will be created + $membership->set_gateway_subscription_id(''); + $membership->set_auto_renew(false); + + return true; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
inc/gateways/class-base-gateway.phpinc/gateways/class-base-paypal-gateway.phpinc/gateways/class-paypal-gateway.phpinc/gateways/class-paypal-oauth-handler.phpinc/gateways/class-paypal-rest-gateway.phpinc/gateways/class-paypal-webhook-handler.phpinc/managers/class-gateway-manager.php
🧰 Additional context used
🧬 Code graph analysis (4)
inc/gateways/class-paypal-oauth-handler.php (5)
inc/gateways/class-paypal-rest-gateway.php (2)
init(85-90)is_configured(155-162)inc/gateways/class-paypal-webhook-handler.php (2)
init(45-51)get_api_base_url(59-62)inc/functions/settings.php (2)
wu_get_setting(37-40)wu_save_setting(51-54)inc/gateways/class-base-paypal-gateway.php (2)
get_api_base_url(98-101)is_configured(291-291)inc/functions/helper.php (1)
wu_log_add(208-211)
inc/gateways/class-base-gateway.php (2)
inc/gateways/class-base-paypal-gateway.php (2)
get_payment_url_on_gateway(130-143)get_subscription_url_on_gateway(155-177)inc/gateways/class-base-stripe-gateway.php (3)
get_payment_url_on_gateway(2939-2950)get_subscription_url_on_gateway(2962-2967)get_customer_url_on_gateway(2979-2984)
inc/gateways/class-paypal-gateway.php (2)
inc/gateways/class-base-paypal-gateway.php (2)
is_configured(291-291)get_connection_status(299-299)inc/functions/helper.php (1)
wu_log_add(208-211)
inc/gateways/class-paypal-rest-gateway.php (3)
inc/gateways/class-base-paypal-gateway.php (4)
get_api_base_url(98-101)log(221-224)add_partner_attribution_header(205-210)get_subscription_description(111-118)inc/gateways/class-paypal-oauth-handler.php (2)
init(79-91)get_api_base_url(119-122)inc/gateways/class-paypal-webhook-handler.php (3)
init(45-51)get_api_base_url(59-62)log(574-577)
🪛 GitHub Check: Code Quality Checks
inc/gateways/class-base-gateway.php
[warning] 536-536:
The method parameter $gateway_payment_id is never used
[warning] 551-551:
The method parameter $gateway_subscription_id is never used
[warning] 566-566:
The method parameter $gateway_customer_id is never used
inc/gateways/class-paypal-webhook-handler.php
[warning] 367-367:
It is recommended not to use reserved keyword "resource" as function parameter name. Found: $resource
[warning] 341-341:
It is recommended not to use reserved keyword "resource" as function parameter name. Found: $resource
[warning] 325-325:
It is recommended not to use reserved keyword "resource" as function parameter name. Found: $resource
[warning] 297-297:
It is recommended not to use reserved keyword "resource" as function parameter name. Found: $resource
[warning] 280-280:
It is recommended not to use reserved keyword "resource" as function parameter name. Found: $resource
[warning] 221-221:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
🪛 PHPMD (2.15.0)
inc/gateways/class-paypal-oauth-handler.php
393-393: Avoid unused local variables such as '$consent_status'. (undefined)
(UnusedLocalVariable)
394-394: Avoid unused local variables such as '$risk_status'. (undefined)
(UnusedLocalVariable)
inc/gateways/class-base-gateway.php
536-536: Avoid unused parameters such as '$gateway_payment_id'. (undefined)
(UnusedFormalParameter)
551-551: Avoid unused parameters such as '$gateway_subscription_id'. (undefined)
(UnusedFormalParameter)
566-566: Avoid unused parameters such as '$gateway_customer_id'. (undefined)
(UnusedFormalParameter)
inc/gateways/class-paypal-gateway.php
1793-1793: Avoid unused local variables such as '$custom'. (undefined)
(UnusedLocalVariable)
inc/gateways/class-base-paypal-gateway.php
253-253: Avoid unused parameters such as '$atts'. (undefined)
(UnusedFormalParameter)
253-253: Avoid unused parameters such as '$site'. (undefined)
(UnusedFormalParameter)
inc/gateways/class-paypal-rest-gateway.php
376-376: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
379-379: Avoid unused local variables such as '$description'. (undefined)
(UnusedLocalVariable)
610-610: Avoid unused parameters such as '$type'. (undefined)
(UnusedFormalParameter)
788-788: Avoid unused local variables such as '$customer_id'. (undefined)
(UnusedLocalVariable)
880-880: Avoid unused local variables such as '$customer_id'. (undefined)
(UnusedLocalVariable)
924-924: Avoid unused parameters such as '$customer'. (undefined)
(UnusedFormalParameter)
957-957: Avoid unused parameters such as '$membership'. (undefined)
(UnusedFormalParameter)
957-957: Avoid unused parameters such as '$customer'. (undefined)
(UnusedFormalParameter)
993-993: Avoid unused parameters such as '$customer'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: PHP 8.4
🔇 Additional comments (16)
inc/gateways/class-base-gateway.php (1)
534-569: LGTM - Return type declarations align with gateway pattern.These base class stub methods correctly return empty strings and declare explicit return types, matching the established pattern in
Base_Stripe_Gateway. The unused parameters are intentional as these methods are designed to be overridden by child classes.inc/managers/class-gateway-manager.php (3)
417-431: LGTM - PayPal gateway registration and webhook initialization.The new PayPal REST gateway registration with OAuth-based descriptions, the legacy PayPal gateway re-labeling, and the webhook handler initialization are well-structured.
543-547: Appropriate change from actions to filters for URL retrieval.Using
add_filterinstead ofadd_actionis semantically correct here since these hooks are meant to return URL data that can be used by other components. This aligns with the filter-based data retrieval pattern.
556-556: Direct method call is cleaner.Replacing
call_user_func([$gateway, 'fields'])with$gateway->fields()improves readability without changing behavior.inc/gateways/class-paypal-oauth-handler.php (1)
79-91: OAuth handler initialization looks correct.The handler properly loads test mode setting, partner credentials, registers AJAX handlers with nonce verification, and hooks into admin_init for OAuth return handling.
inc/gateways/class-base-paypal-gateway.php (2)
253-283: Filter callback signature is correct.The
$attsand$siteparameters are required by thewu_element_get_site_actionsfilter signature even though they're not used in this implementation. This is standard practice for WordPress filter callbacks.
23-79: Well-structured base class for PayPal gateways.The abstract class provides good shared functionality including:
- Cross-gateway hook compatibility via
$other_ids- Partner attribution tracking via
$bn_code- Common capability declarations for recurring payments
inc/gateways/class-paypal-gateway.php (4)
841-874: Excellent error handling for PayPal API failures.This change directly addresses the PR objective. When
get_checkout_details()returns aWP_Error, the code now properly displays the error viawp_die()instead of silently failing and looping back. The logging and contextual error messages will help with debugging.
1770-1781: Key fix:get_checkout_detailsnow returnsWP_Erroron API failures.This is the core fix for issue #193. Previously, ACK=Failure responses were not properly detected, causing the checkout to loop. Now the method:
- Checks for ACK=Failure or FailureWithWarning
- Logs the error with code and message
- Returns a
WP_Errorthat callers can handle
1378-1392: Good addition ofFailureWithWarninghandling.PayPal can return
FailureWithWarningin addition toFailure. Handling both ensures edge cases are properly caught and displayed to users with the PayPal error code and message.Also applies to: 1560-1574
1661-1729:confirmation_form()now properly validates and handles errors.The method now:
- Returns void (breaking change noted in AI summary)
- Validates pending_payment, customer, and membership exist
- Uses
wp_die()to display errors with back links- Logs errors for debugging
This prevents silent failures and provides clear user feedback.
inc/gateways/class-paypal-webhook-handler.php (2)
280-287: Parameter name$resourceis acceptable.The static analysis warning about
$resourcebeing a reserved keyword is a false positive. Whileresourceis a type in PHP, it's not a reserved keyword and can be used as a parameter name.Also applies to: 297-315, 325-331, 341-357, 367-382, 392-407
70-147: Well-structured webhook processor with comprehensive event handling.The webhook handler properly:
- Validates incoming JSON
- Verifies webhook signatures via PayPal API
- Routes events to appropriate handlers
- Returns appropriate HTTP status codes
inc/gateways/class-paypal-rest-gateway.php (3)
218-273: Good token caching implementation.The access token caching with a 5-minute safety buffer before expiration is a solid pattern. This prevents token expiration mid-request while minimizing unnecessary token refreshes.
506-596: Billing plan caching is efficient.Caching plan IDs via
update_site_optionwith an MD5 hash key based on plan parameters avoids recreating identical plans on PayPal. The verification check for existing plans handles the case where a plan was deleted on PayPal's side.
85-90: Gateway initialization and hooks are well-structured.The init method properly loads test mode and credentials. The hooks method initializes the OAuth handler, registers webhook installation handlers, and sets up admin notices - providing a complete gateway lifecycle.
Also applies to: 132-147
| if (str_starts_with($gateway_subscription_id, 'I-')) { | ||
| // REST API subscription | ||
| return sprintf( | ||
| 'https://www.%spaypal.com/billing/subscriptions/%s', | ||
| $sandbox_prefix, | ||
| $gateway_subscription_id | ||
| ); | ||
| } | ||
|
|
||
| // Legacy NVP recurring payment profile | ||
| $base_url = 'https://www.%spaypal.com/us/cgi-bin/webscr?cmd=_profile-recurring-payments&encrypted_profile_id=%s'; | ||
|
|
||
| return sprintf($base_url, $sandbox_prefix, $gateway_subscription_id); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check composer.json for PHP version requirement
echo "=== composer.json PHP requirement ==="
if [ -f composer.json ]; then
cat composer.json | jq '.require.php // .config."platform"."php" // empty' 2>/dev/null || grep -i '"php"' composer.json
else
echo "No composer.json found"
fi
echo ""
echo "=== Checking README or plugin header for version info ==="
head -50 README.md 2>/dev/null | grep -i "php\|version" || echo "No README found"
echo ""
echo "=== Checking main plugin file for version requirements ==="
fd "*.php" -x grep -l "Requires PHP\|requires_php" {} \; 2>/dev/null | head -5
echo ""
echo "=== Checking if str_starts_with polyfill exists ==="
rg "function str_starts_with|if.*str_starts_with" -A 2 -B 2
echo ""
echo "=== Checking all uses of str_starts_with in codebase ==="
rg "str_starts_with" --type php -nRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 22048
str_starts_with() is incompatible with PHP 7.4.1 (the minimum supported version per composer.json).
The function str_starts_with() (used in lines 164 and 191) requires PHP 8.0+. Since the plugin supports PHP 7.4.1 and no polyfill exists in the codebase, this will cause a fatal error on any PHP 7.4 installation. This affects not only the PayPal gateway but also 28 other files throughout the codebase using the same function.
Either update the minimum PHP version requirement to 8.0+, add a polyfill for str_starts_with(), or replace the function call with PHP 7.4-compatible alternatives (e.g., strpos() === 0).
🤖 Prompt for AI Agents
In @inc/gateways/class-base-paypal-gateway.php around lines 164 - 177, Replace
usages of the PHP 8-only function str_starts_with (e.g., the check in
class-base-paypal-gateway.php that tests $gateway_subscription_id) with a PHP
7.4-compatible check: use strpos($gateway_subscription_id, 'I-') === 0 (or an
equivalent substr/strncmp) to detect the prefix; apply the same replacement to
the other ~28 occurrences across the codebase or alternatively add a simple
polyfill for str_starts_with if you prefer to keep the original calls, but do
not change composer PHP requirement unless intentionally bumping minimum PHP to
8.0.
There was a problem hiding this comment.
We have a polyfill.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for str_starts_with polyfill definition
echo "=== Searching for str_starts_with polyfill ==="
rg "function str_starts_with" --type php -n -C 5
echo ""
echo "=== Checking for polyfill files ==="
fd -e php polyfill -x cat {}
echo ""
echo "=== Searching for function_exists check for str_starts_with ==="
rg "function_exists.*str_starts_with" --type php -n -C 3
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Check composer.json for polyfill dependencies
echo "=== Checking composer.json for polyfill packages ==="
if [ -f composer.json ]; then
cat composer.json | jq '.require, .["require-dev"]' 2>/dev/null | grep -i polyfill || echo "No polyfill packages found in composer.json"
else
echo "No composer.json found"
fi
echo ""
echo "=== Checking for vendor/symfony polyfills ==="
if [ -d vendor/symfony ]; then
ls -la vendor/symfony/ | grep polyfill
else
echo "No vendor/symfony directory found"
fi
echo ""
echo "=== Checking for autoload/bootstrap files ==="
fd -e php "bootstrap|autoload|init|loader" -x echo "File: {}" \; -x head -30 {} \; | head -100
echo ""
echo "=== Checking main plugin file for polyfill loading ==="
rg "require.*polyfill|include.*polyfill|polyfill" --type php -n -C 2 | head -50
Length of output: 5546
@superdav42, you're absolutely right—I apologize for the incorrect flag! 🙏
I can confirm that your composer.json includes symfony/polyfill-php80 (v1.31.0), which provides str_starts_with() and other PHP 8.0 functions for PHP 7.4 environments. The polyfill is loaded via Composer's autoloader, so the usage of str_starts_with() throughout the codebase is completely safe and compatible with PHP 7.4.1+.
Thank you for the correction—the code is good to go as-is!
(\_/)
(o.o) learned something!
(> <)
| protected function install_webhook_after_oauth(string $mode_prefix): void { | ||
|
|
||
| try { | ||
| // Get the PayPal REST gateway instance | ||
| $gateway_manager = \WP_Ultimo\Managers\Gateway_Manager::get_instance(); | ||
| $gateway = $gateway_manager->get_gateway('paypal-rest'); | ||
|
|
||
| if (! $gateway instanceof PayPal_REST_Gateway) { | ||
| wu_log_add('paypal', 'Could not get PayPal REST gateway instance for webhook installation', LogLevel::WARNING); | ||
| return; | ||
| } | ||
|
|
||
| // Ensure the gateway is in the correct mode | ||
| $gateway->set_test_mode('sandbox' === $mode_prefix); | ||
|
|
||
| // Install the webhook | ||
| $result = $gateway->install_webhook(); | ||
|
|
||
| if (true === $result) { | ||
| wu_log_add('paypal', sprintf('Webhook installed successfully for %s mode after OAuth', $mode_prefix)); | ||
| } elseif (is_wp_error($result)) { | ||
| wu_log_add('paypal', sprintf('Failed to install webhook after OAuth: %s', $result->get_error_message()), LogLevel::ERROR); | ||
| } | ||
| } catch (\Exception $e) { | ||
| wu_log_add('paypal', sprintf('Exception installing webhook after OAuth: %s', $e->getMessage()), LogLevel::ERROR); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "function get_gateway|function wu_get_gateway" --type=php -A 15Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 9592
🏁 Script executed:
rg -n "registered_gateways\[" --type=php -B 2 -A 2 | head -50Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 1583
🏁 Script executed:
rg -n "PayPal_REST_Gateway|register_gateway.*paypal" --type=php -A 5Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 3367
Type check should use wu_get_gateway() to get a gateway instance instead of using get_gateway() which returns metadata array.
The instanceof check on line 695 will always fail because $gateway_manager->get_gateway() returns an associative array with gateway metadata, not a PayPal_REST_Gateway instance. Use wu_get_gateway('paypal-rest') instead, which properly instantiates and returns the gateway object. This prevents webhook installation from being silently skipped.
🐛 Proposed fix
protected function install_webhook_after_oauth(string $mode_prefix): void {
try {
- // Get the PayPal REST gateway instance
- $gateway_manager = \WP_Ultimo\Managers\Gateway_Manager::get_instance();
- $gateway = $gateway_manager->get_gateway('paypal-rest');
+ // Get a fresh PayPal REST gateway instance
+ $gateway = wu_get_gateway('paypal-rest');
- if (! $gateway instanceof PayPal_REST_Gateway) {
+ if (! $gateway) {
wu_log_add('paypal', 'Could not get PayPal REST gateway instance for webhook installation', LogLevel::WARNING);
return;
}🤖 Prompt for AI Agents
In @inc/gateways/class-paypal-oauth-handler.php around lines 688 - 714, The
instanceof check in install_webhook_after_oauth is wrong because
$gateway_manager->get_gateway('paypal-rest') returns metadata, so replace that
call with wu_get_gateway('paypal-rest') to obtain an actual PayPal_REST_Gateway
instance; update the code path that currently assigns $gateway from
Gateway_Manager::get_instance()->get_gateway(...) to call
wu_get_gateway('paypal-rest'), keep the subsequent set_test_mode/install_webhook
logic and the same logging branches, and ensure the instanceof
PayPal_REST_Gateway check now succeeds and the webhook installation is
attempted.
| if ($this->test_mode && defined('WP_DEBUG') && WP_DEBUG) { | ||
| $this->log('Skipping signature verification in debug mode'); | ||
| return true; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
Security concern: Debug mode bypasses signature verification entirely.
Skipping signature verification when WP_DEBUG is true and in test mode could allow spoofed webhooks in development environments that might still have real data. Consider requiring at least the webhook headers to be present, even if verification is skipped.
🔒 Suggested improvement
// If headers are missing, we can't verify
if (empty($auth_algo) || empty($cert_url) || empty($transmission_id) || empty($transmission_sig) || empty($transmission_time)) {
$this->log('Missing webhook signature headers', LogLevel::WARNING);
- // In development/testing, you might want to skip verification
- if ($this->test_mode && defined('WP_DEBUG') && WP_DEBUG) {
- $this->log('Skipping signature verification in debug mode');
- return true;
- }
+ // Even in debug mode, require headers to be present to prevent accidental acceptance of malformed requests
return false;
}🤖 Prompt for AI Agents
In @inc/gateways/class-paypal-webhook-handler.php around lines 172 - 176, The
current unconditional bypass in the signature-check block (when $this->test_mode
and WP_DEBUG) is unsafe; change it to only skip cryptographic verification while
still requiring the webhook headers to exist and be well-formed: keep the
$this->test_mode and WP_DEBUG branch but replace the immediate return true with
a header presence check for the PayPal webhook headers (e.g.,
'Paypal-Transmission-Id', 'Paypal-Transmission-Sig', 'Paypal-Transmission-Time',
'Paypal-Cert-Url') and return true only if those headers are present (otherwise
return false and log an error); update the log call ($this->log('Skipping
signature verification in debug mode')) to include which headers were missing so
the verification path in the class (the method using $this->test_mode and the
log call) still defends against spoofed requests in debug/test environments.
| if (empty($webhook_id)) { | ||
| $this->log('Webhook ID not configured, skipping verification', LogLevel::WARNING); | ||
| // Allow in test mode without webhook ID | ||
| return $this->test_mode; | ||
| } |
There was a problem hiding this comment.
Allowing webhooks without webhook ID in test mode is risky.
Returning true when webhook ID is not configured in test mode means any request to the webhook endpoint will be accepted. This could mask configuration issues and process illegitimate requests.
🔒 Suggested improvement
if (empty($webhook_id)) {
$this->log('Webhook ID not configured, skipping verification', LogLevel::WARNING);
- // Allow in test mode without webhook ID
- return $this->test_mode;
+ // Require webhook ID to be configured for security
+ return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (empty($webhook_id)) { | |
| $this->log('Webhook ID not configured, skipping verification', LogLevel::WARNING); | |
| // Allow in test mode without webhook ID | |
| return $this->test_mode; | |
| } | |
| if (empty($webhook_id)) { | |
| $this->log('Webhook ID not configured, skipping verification', LogLevel::WARNING); | |
| // Require webhook ID to be configured for security | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In @inc/gateways/class-paypal-webhook-handler.php around lines 183 - 187, The
current check in class PayPal_Webhook_Handler (when $webhook_id is empty)
returns $this->test_mode which accepts any webhook in test mode; change this to
never auto-accept missing webhook IDs. Replace the line "return
$this->test_mode" so the method logs a clear error and returns false (do not
accept the request), e.g., keep the warning log or escalate to error and then
return false; if you need an explicit opt-in for development, add a boolean
property like $allow_unverified_webhooks_in_test (default false) and only return
true when that property is explicitly true.
- Add Base_PayPal_Gateway abstract class for shared PayPal functionality - Add PayPal_REST_Gateway with modern PayPal REST API implementation - Add PayPal_OAuth_Handler for OAuth 2.0 authentication flow - Add PayPal_Webhook_Handler for webhook event processing - Update Gateway_Manager to register new PayPal REST gateway and use filters - Fix Base_Gateway methods to properly return strings for filter callbacks - Maintain backwards compatibility with legacy PayPal gateway Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove WP_DEBUG-based webhook signature bypass, use wu_paypal_skip_webhook_verification filter - Fix empty webhook_id and client_credentials fallbacks to return false instead of test_mode - Add WU_PAYPAL_PARTNER_* constants and wu_paypal_partner_credentials filter support - Hide OAuth Connect button when partner credentials not configured - Add phpcs:ignore for base64_encode (required for PayPal Basic auth) - Rename $resource parameter to $event_data to avoid reserved keyword warning - Fix PHPCS formatting errors in oauth handler - Add 20 unit tests for PayPal REST Gateway (config, URLs, credentials) - Add 14 unit tests for PayPal Webhook Handler (signature, subscriptions, refunds) - Add paypal-rest and paypal assertions to Gateway Manager tests Co-Authored-By: Claude Opus 4.6 <[email protected]>
superdav42
force-pushed
the
fix/paypal-checkout-loop-193
branch
from
9ad43be to
bc0bed6
Compare
superdav42
changed the title
🔨 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
♻️ Duplicate comments (2)
inc/gateways/class-paypal-oauth-handler.php (1)
733-737:⚠️ Potential issue | 🟠 MajorUse
wu_get_gateway('paypal-rest')for instance retrieval.Line 734 and Line 769 call
Gateway_Manager::get_gateway(), which returns metadata arrays, so theinstanceof PayPal_REST_Gatewaychecks always fail and webhook install/delete is skipped.🐛 Proposed fix
protected function install_webhook_after_oauth(string $mode_prefix): void { try { - // Get the PayPal REST gateway instance - $gateway_manager = \WP_Ultimo\Managers\Gateway_Manager::get_instance(); - $gateway = $gateway_manager->get_gateway('paypal-rest'); + $gateway = wu_get_gateway('paypal-rest'); if (! $gateway instanceof PayPal_REST_Gateway) { wu_log_add('paypal', 'Could not get PayPal REST gateway instance for webhook installation', LogLevel::WARNING); return; } @@ protected function delete_webhooks_on_disconnect(): void { try { - $gateway_manager = \WP_Ultimo\Managers\Gateway_Manager::get_instance(); - $gateway = $gateway_manager->get_gateway('paypal-rest'); + $gateway = wu_get_gateway('paypal-rest'); if (! $gateway instanceof PayPal_REST_Gateway) { return; }Also applies to: 768-772
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/gateways/class-paypal-oauth-handler.php` around lines 733 - 737, The code is using \WP_Ultimo\Managers\Gateway_Manager::get_gateway(...) which returns gateway metadata arrays so the instanceof PayPal_REST_Gateway checks fail; replace calls to Gateway_Manager::get_gateway('paypal-rest') with the helper wu_get_gateway('paypal-rest') wherever used (the blocks performing the instanceof PayPal_REST_Gateway check for webhook install/delete), so you get the actual gateway object and the instanceof check will work; update both occurrences that currently call get_gateway(...) to use wu_get_gateway(...) before proceeding with webhook installation/deletion.inc/gateways/class-base-paypal-gateway.php (1)
164-165:⚠️ Potential issue | 🔴 CriticalReplace
str_starts_with()or require PHP 8+ explicitly.Line 164 and Line 191 use a PHP 8-only function. On PHP 7.4 environments this is a fatal runtime error.
🐛 PHP 7.4-compatible fix
- if (str_starts_with($gateway_subscription_id, 'I-')) { + if (0 === strpos((string) $gateway_subscription_id, 'I-')) { @@ - return str_starts_with($subscription_id, 'I-'); + return 0 === strpos($subscription_id, 'I-');#!/bin/bash # Verify declared PHP support and locate PHP 8-only prefix checks. # Expected: either PHP >=8.0 declared, or no unsupported usages remain. if [ -f composer.json ]; then rg -n '"php"\s*:' composer.json fi rg -n --type=php '\bstr_starts_with\s*\('Also applies to: 191-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/gateways/class-base-paypal-gateway.php` around lines 164 - 165, The code uses the PHP 8-only function str_starts_with on $gateway_subscription_id (and again later near the other check) which breaks on PHP 7.4; replace those calls with a PHP 7-compatible check (e.g. use strpos($gateway_subscription_id, 'I-') === 0) or add a small polyfill that defines str_starts_with when not already present, and ensure both occurrences (the check around $gateway_subscription_id and the duplicate at the later check) are updated consistently; alternatively, if you intend to require PHP 8+, update composer.json's "php" platform requirement to >=8.0 and run tests to verify no remaining str_starts_with usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/gateways/class-paypal-rest-gateway.php`:
- Around line 155-162: is_configured() currently returns true for OAuth-only
setups (checks merchant_id) but the token acquisition path requires client_id +
client_secret, causing a mismatch; update the logic so configured status matches
what the gateway actually needs: either (A) change is_configured() to require
both client_id and client_secret (remove merchant_id-only success) or (B)
implement the OAuth token path used by token acquisition (e.g., in the method
that retrieves tokens) so merchant_id-only setups can obtain tokens. Refer to
is_configured, merchant_id, client_id, client_secret and the token acquisition
logic (the method that requests/returns access tokens) and make the chosen
approach consistent across both places.
---
Duplicate comments:
In `@inc/gateways/class-base-paypal-gateway.php`:
- Around line 164-165: The code uses the PHP 8-only function str_starts_with on
$gateway_subscription_id (and again later near the other check) which breaks on
PHP 7.4; replace those calls with a PHP 7-compatible check (e.g. use
strpos($gateway_subscription_id, 'I-') === 0) or add a small polyfill that
defines str_starts_with when not already present, and ensure both occurrences
(the check around $gateway_subscription_id and the duplicate at the later check)
are updated consistently; alternatively, if you intend to require PHP 8+, update
composer.json's "php" platform requirement to >=8.0 and run tests to verify no
remaining str_starts_with usages.
In `@inc/gateways/class-paypal-oauth-handler.php`:
- Around line 733-737: The code is using
\WP_Ultimo\Managers\Gateway_Manager::get_gateway(...) which returns gateway
metadata arrays so the instanceof PayPal_REST_Gateway checks fail; replace calls
to Gateway_Manager::get_gateway('paypal-rest') with the helper
wu_get_gateway('paypal-rest') wherever used (the blocks performing the
instanceof PayPal_REST_Gateway check for webhook install/delete), so you get the
actual gateway object and the instanceof check will work; update both
occurrences that currently call get_gateway(...) to use wu_get_gateway(...)
before proceeding with webhook installation/deletion.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
inc/gateways/class-base-gateway.phpinc/gateways/class-base-paypal-gateway.phpinc/gateways/class-paypal-gateway.phpinc/gateways/class-paypal-oauth-handler.phpinc/gateways/class-paypal-rest-gateway.phpinc/gateways/class-paypal-webhook-handler.phpinc/managers/class-gateway-manager.php
| public function is_configured(): bool { | ||
|
|
||
| // Either OAuth connected OR manual credentials | ||
| $has_oauth = ! empty($this->merchant_id); | ||
| $has_manual = ! empty($this->client_id) && ! empty($this->client_secret); | ||
|
|
||
| return $has_oauth || $has_manual; | ||
| } |
There was a problem hiding this comment.
is_configured() and token acquisition criteria are inconsistent.
Line 158–161 marks OAuth-only setups as configured, but Line 233 requires client_id + client_secret for every API call. This can present “connected” status while checkout/webhook API operations fail.
💡 Minimal safety fix (until OAuth token path is implemented here)
public function is_configured(): bool {
- // Either OAuth connected OR manual credentials
- $has_oauth = ! empty($this->merchant_id);
- $has_manual = ! empty($this->client_id) && ! empty($this->client_secret);
-
- return $has_oauth || $has_manual;
+ return ! empty($this->client_id) && ! empty($this->client_secret);
}Also applies to: 233-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/gateways/class-paypal-rest-gateway.php` around lines 155 - 162,
is_configured() currently returns true for OAuth-only setups (checks
merchant_id) but the token acquisition path requires client_id + client_secret,
causing a mismatch; update the logic so configured status matches what the
gateway actually needs: either (A) change is_configured() to require both
client_id and client_secret (remove merchant_id-only success) or (B) implement
the OAuth token path used by token acquisition (e.g., in the method that
retrieves tokens) so merchant_id-only setups can obtain tokens. Refer to
is_configured, merchant_id, client_id, client_secret and the token acquisition
logic (the method that requests/returns access tokens) and make the chosen
approach consistent across both places.
Move partner credential handling to a proxy server at ultimatemultisite.com/wp-json/paypal-connect/v1, matching the existing Stripe Connect proxy pattern. This keeps partner credentials out of the open source codebase. The OAuth handler now: - Calls POST /oauth/init on the proxy to create referral URLs - Calls POST /oauth/verify to verify merchant status - Calls POST /deauthorize on disconnect (non-blocking) - No longer loads or stores partner credentials locally Co-Authored-By: Claude Opus 4.6 <[email protected]>
🔨 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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/gateways/class-paypal-oauth-handler.php`:
- Around line 219-220: Remove the unused local variables $consent_status and
$risk_status from the method in class-paypal-oauth-handler.php: delete their
assignments (the lines that set $consent_status = ... and $risk_status = ...)
since they are never read; if these values are intended for future use, replace
the assignments with a clear TODO comment or add a short doc comment explaining
planned usage instead of leaving unused variables in the method.
| $consent_status = isset($_GET['consentStatus']) && 'true' === $_GET['consentStatus']; | ||
| $risk_status = isset($_GET['isEmailConfirmed']) ? sanitize_text_field(wp_unslash($_GET['isEmailConfirmed'])) : ''; |
There was a problem hiding this comment.
Remove unused variables $consent_status and $risk_status.
These variables are assigned but never used in the method. If they're intended for future use, consider documenting that; otherwise, remove them to avoid confusion.
🧹 Proposed fix
$permissions_granted = isset($_GET['permissionsGranted']) && 'true' === $_GET['permissionsGranted'];
- $consent_status = isset($_GET['consentStatus']) && 'true' === $_GET['consentStatus'];
- $risk_status = isset($_GET['isEmailConfirmed']) ? sanitize_text_field(wp_unslash($_GET['isEmailConfirmed'])) : '';
$tracking_id = isset($_GET['tracking_id']) ? sanitize_text_field(wp_unslash($_GET['tracking_id'])) : '';📝 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.
| $consent_status = isset($_GET['consentStatus']) && 'true' === $_GET['consentStatus']; | |
| $risk_status = isset($_GET['isEmailConfirmed']) ? sanitize_text_field(wp_unslash($_GET['isEmailConfirmed'])) : ''; | |
| $permissions_granted = isset($_GET['permissionsGranted']) && 'true' === $_GET['permissionsGranted']; | |
| $tracking_id = isset($_GET['tracking_id']) ? sanitize_text_field(wp_unslash($_GET['tracking_id'])) : ''; |
🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 219-219: Avoid unused local variables such as '$consent_status'. (undefined)
(UnusedLocalVariable)
[warning] 220-220: Avoid unused local variables such as '$risk_status'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/gateways/class-paypal-oauth-handler.php` around lines 219 - 220, Remove
the unused local variables $consent_status and $risk_status from the method in
class-paypal-oauth-handler.php: delete their assignments (the lines that set
$consent_status = ... and $risk_status = ...) since they are never read; if
these values are intended for future use, replace the assignments with a clear
TODO comment or add a short doc comment explaining planned usage instead of
leaving unused variables in the method.
The settings renderer was escaping <script> tags in note-type field
desc callbacks, causing JavaScript to appear as visible text instead
of executing. Move connect button and webhook button handlers to
enqueue_connect_scripts() and enqueue_webhook_scripts() methods that
use add_action('admin_footer') to output JS safely.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
🔨 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
♻️ Duplicate comments (1)
inc/gateways/class-paypal-rest-gateway.php (1)
155-162:⚠️ Potential issue | 🟠 MajorAlign configured-state with token requirements.
Line [157]-[161] marks OAuth-only setups as configured, but Line [233] still hard-fails without
client_id+client_secret. This can show “connected” while checkout/webhook calls fail.💡 Minimal consistency fix
public function is_configured(): bool { - // Either OAuth connected OR manual credentials - $has_oauth = ! empty($this->merchant_id); - $has_manual = ! empty($this->client_id) && ! empty($this->client_secret); - - return $has_oauth || $has_manual; + return ! empty($this->client_id) && ! empty($this->client_secret); }Also applies to: 233-238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/gateways/class-paypal-rest-gateway.php` around lines 155 - 162, The configured-state check in is_configured currently treats having only merchant_id (OAuth) as configured while other code still requires client_id and client_secret and will hard-fail; update is_configured (the method is_configured) to reflect the actual requirements used elsewhere by returning true only when the gateway has either a full OAuth token pair that the code accepts or when client_id and client_secret are both present (i.e., require client_id + client_secret for non-OAuth flows), or alternatively make the later credential-checking logic accept OAuth-only by branching on merchant_id vs client_id/client_secret so both sides agree (reference merchant_id, client_id, client_secret and the method is_configured).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/gateways/class-paypal-rest-gateway.php`:
- Around line 673-679: The loop that extracts the approval URL currently only
checks for link rel 'approve' and should also accept 'payer-action' as a
defensive edge case; update the foreach over $result['links'] in
class-paypal-rest-gateway.php (the block using $approval_url and $link['rel'])
to treat either 'approve' or 'payer-action' as valid (e.g., check if
$link['rel'] is in ['approve','payer-action']) and set $approval_url =
$link['href'] when matched, keeping the existing break/fallback logic intact.
- Around line 868-873: The capture response parsing fails because the PayPal
capture call that sets $capture returns a minimal representation lacking
purchase_units and captures; update the code that makes the PayPal capture
request (the call that assigns $capture) to include the HTTP header "Prefer:
return=representation" so PayPal returns the full order payload, or
alternatively, after a successful capture call perform a GET
/v2/checkout/orders/{token} to fetch the full order and use that response for
parsing; then update the logic that reads $purchase_unit and uses
$purchase_unit['payments']['captures'][0]['custom_id'] and id to use the
full-order response (or the GET result) and remove/invalidate the incomplete
fallback to purchase_units[0].custom_id when the minimal response is returned.
---
Duplicate comments:
In `@inc/gateways/class-paypal-rest-gateway.php`:
- Around line 155-162: The configured-state check in is_configured currently
treats having only merchant_id (OAuth) as configured while other code still
requires client_id and client_secret and will hard-fail; update is_configured
(the method is_configured) to reflect the actual requirements used elsewhere by
returning true only when the gateway has either a full OAuth token pair that the
code accepts or when client_id and client_secret are both present (i.e., require
client_id + client_secret for non-OAuth flows), or alternatively make the later
credential-checking logic accept OAuth-only by branching on merchant_id vs
client_id/client_secret so both sides agree (reference merchant_id, client_id,
client_secret and the method is_configured).
| // Find approval URL | ||
| $approval_url = ''; | ||
| foreach ($result['links'] ?? [] as $link) { | ||
| if ('approve' === $link['rel']) { | ||
| $approval_url = $link['href']; | ||
| break; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PayPal Orders v2 API create-order response link rel values approve payer-action
💡 Result:
In the Orders v2 create-order response, the links[].rel values tell you the next buyer/merchant step (HATEOAS):
-
rel: "approve"— Redirect the buyer to this URL to approve the order in the PayPal checkout flow. You typically do this right after creating an order (status likeCREATED) and before you authorize/capture. [1][2] -
rel: "payer-action"— Redirect the buyer to this URL when the order status isPAYER_ACTION_REQUIRED, meaning the payer must take additional action in the PayPal flow (commonly: finish/confirm the checkout experience, re-approve, etc.) before you can authorize/capture successfully. [3][4]
(For completeness, you’ll also commonly see rel: "self" as the API URL to fetch the order.) [4]
Sources: [1] (docs.paypal.ai) [2] (developer.paypal.com) [3] (docs.paypal.ai) [4] (developer.paypal.com)
Citations:
- 1: https://docs.paypal.ai/apis/orders/orders?utm_source=openai
- 2: https://developer.paypal.com/api/rest/reference/orders/v2/errors/?utm_source=openai
- 3: https://docs.paypal.ai/payments/methods/paypal/api/one-time/orders-api-integration?utm_source=openai
- 4: https://developer.paypal.com/docs/api/orders/sdk/v2/?utm_source=openai
🏁 Script executed:
# Check if 'payer-action' is mentioned anywhere in the codebase
rg "payer-action" --type php
# Also check for any references to Orders v2 or approval links in the same file
rg "payer-action|approval_url" inc/gateways/class-paypal-rest-gateway.php -B 2 -A 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1039
Handle Orders v2 payer-action approval link relation as defensive edge case handling.
Line 676 only checks rel === 'approve', which correctly handles the normal Orders v2 flow where a newly created order returns the approve link. However, when an order status transitions to PAYER_ACTION_REQUIRED (requiring additional buyer action in PayPal), the response contains a payer-action link instead. While less common in typical checkout flows, adding support for this relation provides defensive coverage for edge cases.
🔧 Suggested fix
$approval_url = '';
foreach ($result['links'] ?? [] as $link) {
- if ('approve' === $link['rel']) {
+ $rel = $link['rel'] ?? '';
+ if (in_array($rel, ['approve', 'payer-action'], true)) {
$approval_url = $link['href'];
break;
}
}📝 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.
| // Find approval URL | |
| $approval_url = ''; | |
| foreach ($result['links'] ?? [] as $link) { | |
| if ('approve' === $link['rel']) { | |
| $approval_url = $link['href']; | |
| break; | |
| } | |
| // Find approval URL | |
| $approval_url = ''; | |
| foreach ($result['links'] ?? [] as $link) { | |
| $rel = $link['rel'] ?? ''; | |
| if (in_array($rel, ['approve', 'payer-action'], true)) { | |
| $approval_url = $link['href']; | |
| break; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/gateways/class-paypal-rest-gateway.php` around lines 673 - 679, The loop
that extracts the approval URL currently only checks for link rel 'approve' and
should also accept 'payer-action' as a defensive edge case; update the foreach
over $result['links'] in class-paypal-rest-gateway.php (the block using
$approval_url and $link['rel']) to treat either 'approve' or 'payer-action' as
valid (e.g., check if $link['rel'] is in ['approve','payer-action']) and set
$approval_url = $link['href'] when matched, keeping the existing break/fallback
logic intact.
| // Parse custom_id | ||
| $purchase_unit = $capture['purchase_units'][0] ?? []; | ||
| $custom_parts = explode('|', $purchase_unit['payments']['captures'][0]['custom_id'] ?? ''); | ||
|
|
||
| if (count($custom_parts) !== 3) { | ||
| wp_die( |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PayPal Orders v2 API create order capture response custom_id location
💡 Result:
Where custom_id shows up (JSON “location”)
1) You set it on Create Order
- Request field:
purchase_units[].custom_id(Orders v2 “Create order”). (developer.paypal.com)
2) You get it back on Capture (if you ask for the full representation)
- Call:
POST /v2/checkout/orders/{id}/capturewith headerPrefer: return=representation(otherwise the response can be minimal: justid,status,links). (developer.paypal.com) - In the full capture response, the capture object includes
custom_idat:purchase_units[].payments.captures[].custom_id(developer.paypal.com)
3) You can also read it later from the capture resource
- Use the capture id from
purchase_units[].payments.captures[].id, thenGET /v2/payments/captures/{capture_id}and read:custom_id(developer.paypal.com)
“Location” of the created/captured resource (URL)
PayPal returns HATEOAS links; the resource URL is in the link with rel: "self" (e.g., .../v2/checkout/orders/{id}) in create/capture responses. (developer.paypal.com)
Note
custom_id is for your reconciliation/metadata; you can’t query Orders/Captures by custom_id (you must query by order id or capture id). (stackoverflow.com)
Citations:
- 1: https://developer.paypal.com/docs/api/orders/sdk/v2/?utm_source=openai
- 2: https://developer.paypal.com/docs/api/orders/sdk/v2/?utm_source=openai
- 3: https://developer.paypal.com/docs/api/payments/v2/
- 4: https://developer.paypal.com/docs/api/payments/v2/
- 5: https://developer.paypal.com/docs/api/orders/sdk/v2/?utm_source=openai
- 6: https://stackoverflow.com/questions/75511315/paypal-api-how-to-get-order-payment-details-by-custom-parameter?utm_source=openai
🏁 Script executed:
find . -type f -name "class-paypal-rest-gateway.php" | head -5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 125
🏁 Script executed:
cd inc/gateways && wc -l class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 115
🏁 Script executed:
cd inc/gateways && sed -n '850,890p' class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1291
🏁 Script executed:
cd inc/gateways && sed -n '1,100p' class-paypal-rest-gateway.php | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1103
🏁 Script executed:
cd inc/gateways && rg -n "capture\['purchase_units" class-paypal-rest-gateway.php -B 5 -A 5Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 441
🏁 Script executed:
cd inc/gateways && rg -n "Prefer.*return" class-paypal-rest-gateway.php -B 3 -A 3Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
cd inc/gateways && rg -n "/capture" class-paypal-rest-gateway.php -B 2 -A 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 396
🏁 Script executed:
cd inc/gateways && sed -n '800,868p' class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2119
🏁 Script executed:
cd inc/gateways && rg -n "api_request" class-paypal-rest-gateway.php | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 1272
🏁 Script executed:
cd inc/gateways && sed -n '1,50p' class-paypal-rest-gateway.php && rg -n "function api_request" class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1202
🏁 Script executed:
cd inc/gateways && rg -n "function api_request" class-paypal-rest-gateway.php -A 30Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 962
🏁 Script executed:
cd inc/gateways && rg -n "Prefer" class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
cd inc && find . -name "*.php" -exec rg -l "Prefer.*representation" {} \;Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🏁 Script executed:
cd inc/gateways && sed -n '285,330p' class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1411
🏁 Script executed:
cd inc/gateways && rg -n "purchase_units" class-paypal-rest-gateway.php -B 2 -A 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 435
🏁 Script executed:
cd inc/gateways && rg -n "custom_id" class-paypal-rest-gateway.php -B 2 -A 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 995
🏁 Script executed:
cd inc/gateways && sed -n '660,680p' class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 547
🏁 Script executed:
cd inc/gateways && sed -n '615,640p' class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1076
🏁 Script executed:
cd inc/gateways && grep -n "Prefer" class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🌐 Web query:
PayPal v2 capture endpoint minimal response without Prefer header JSON structure
💡 Result:
If you omit the Prefer header on POST /v2/checkout/orders/{id}/capture, PayPal’s default is Prefer: return=minimal, meaning the response body is a minimal JSON containing only:
idstatuslinks(HATEOAS) (developer.paypal.com)
Minimal response structure:
{
"id": "ORDER_ID",
"status": "COMPLETED",
"links": [
{
"href": "https://api-m.paypal.com/v2/checkout/orders/ORDER_ID",
"rel": "self",
"method": "GET"
}
]
}Citations:
🏁 Script executed:
cd inc/gateways && rg -n "'/v2/checkout/orders/'" class-paypal-rest-gateway.php -A 2 -B 2Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 239
🏁 Script executed:
cd inc/gateways && sed -n '845,920p' class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 2295
🏁 Script executed:
cd inc/gateways && sed -n '869,910p' class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 1434
🏁 Script executed:
cd inc/gateways && rg -n "payments\['captures" class-paypal-rest-gateway.phpRepository: Ultimate-Multisite/ultimate-multisite
Length of output: 63
🌐 Web query:
PayPal Orders v2 API how to get custom_id after capture complete purchase_units
💡 Result:
To read back a purchase_units[].custom_id after you capture an order, you must request (or fetch) an order representation that includes purchase_units.
Option A (recommended): Capture with full representation
POST /v2/checkout/orders/{id}/capture returns minimal fields by default unless you send Prefer: return=representation (then it returns the full order, including purchase_units). (developer.paypal.com)
POST /v2/checkout/orders/{order_id}/capture
Prefer: return=representationThen read:
resp.purchase_units[i].custom_idOption B: GET the order after capture
You can call Show order details at any stage (including after it’s COMPLETED) and read the same field from the returned purchase_units. (developer.paypal.com)
GET /v2/checkout/orders/{order_id}Then:
order.purchase_units[i].custom_idIf you only have a capture_id
From the capture response (or order details) you can store purchase_units[].payments.captures[].id (developer.paypal.com) and then GET /v2/payments/captures/{capture_id} to retrieve capture details (the capture resource includes “custom ID” as part of its capture identification fields). (developer.paypal.com)
Citations:
- 1: https://developer.paypal.com/docs/api/orders/sdk/v2/?utm_source=openai
- 2: https://developer.paypal.com/api/rest/integration/orders-api/?utm_source=openai
- 3: https://developer.paypal.com/docs/tracking/orders-api/integrate/
- 4: https://developer.paypal.com/docs/api/payments/v2/?utm_source=openai
Request Prefer header for full representation in capture response.
Line 849 calls the capture endpoint without the Prefer: return=representation header, causing PayPal to return a minimal response containing only id, status, and links. The code then attempts to read purchase_units[0].payments.captures[0].custom_id and purchases_units[0].payments.captures[0].id at lines 870 and 903, which do not exist in the minimal response. This causes all order confirmations to fail with "Invalid order data".
The suggested fallback to purchase_units[0].custom_id is incomplete—the minimal response does not include purchase_units at all. To fix this properly, either:
- Add
Prefer: return=representationheader to the capture request to receive the full order representation, or - Call
GET /v2/checkout/orders/{token}after capture to retrieve the full order details includingpurchase_units.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/gateways/class-paypal-rest-gateway.php` around lines 868 - 873, The
capture response parsing fails because the PayPal capture call that sets
$capture returns a minimal representation lacking purchase_units and captures;
update the code that makes the PayPal capture request (the call that assigns
$capture) to include the HTTP header "Prefer: return=representation" so PayPal
returns the full order payload, or alternatively, after a successful capture
call perform a GET /v2/checkout/orders/{token} to fetch the full order and use
that response for parsing; then update the logic that reads $purchase_unit and
uses $purchase_unit['payments']['captures'][0]['custom_id'] and id to use the
full-order response (or the GET result) and remove/invalidate the incomplete
fallback to purchase_units[0].custom_id when the minimal response is returned.
The settings framework runs description HTML through wp_kses() which strips data-nonce and data-sandbox attributes from buttons. Move nonce and sandbox values into the admin_footer script as JS variables instead. Co-Authored-By: Claude Opus 4.6 <[email protected]>
🔨 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: |
Platform fees apply when merchants connect via OAuth (Partner Referral) and have not purchased an addon. Manual credential users pay no fee. Implementation: - Proxy: new /partner-token endpoint returns short-lived partner token - Plugin: creates orders with partner token + PayPal-Auth-Assertion header and payment_instruction.platform_fees - Falls back gracefully to standard order if proxy is unavailable - Fee notice displayed in settings (matching Stripe's notice) Co-Authored-By: Claude Opus 4.6 <[email protected]>
🔨 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: |
- Accept both 'approve' and 'payer-action' link rel in approval URL loops - Add Prefer: return=representation header to capture API for full response - Remove unused $consent_status and $risk_status variables from OAuth handler - Suppress unused parameter warnings in Base_Gateway URL methods Co-Authored-By: Claude Opus 4.6 <[email protected]>
🔨 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: |
- Combine connection status and connect button into single OAuth widget - Use same dashicons-admin-links icon as Stripe Connect button - Hide manual API key fields behind "Use Direct API Keys (Advanced)" toggle - Remove separate webhook configure button (auto-created on connect/save) - Add blue info box explaining automatic webhook configuration - Match Stripe's green connected/blue disconnected visual styling Co-Authored-By: Claude Opus 4.6 <[email protected]>
🔨 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: |
Only register the PayPal (Legacy) NVP gateway if it's already active or has credentials configured. New installs will only see the modern PayPal REST gateway. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- PayPal_REST_Gateway_Test: 9 new tests for settings registration, render_oauth_connection output, webhook listener URL, and maybe_install_webhook skip logic - PayPal_OAuth_Handler_Test: 10 tests for singleton, is_configured, is_merchant_connected (sandbox/live), get_merchant_details, and init hook registration - Gateway_Manager_Test: 4 new tests for legacy PayPal conditional registration (hidden without config, shown with credentials, shown when active, paypal-rest always registered) - E2E Cypress spec: 035-paypal-checkout-flow.spec.js tests PayPal gateway setup, checkout form rendering, and form submission with PayPal selected - PHP fixtures for gateway setup and result verification Co-Authored-By: Claude Opus 4.6 <[email protected]>
🔨 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: |
OAuth Connect button is now hidden by default and only shown when the PayPal proxy plugin at ultimatemultisite.com reports that partner credentials are configured (GET /status endpoint). When OAuth is disabled (current state): - Manual API key fields shown directly without advanced toggle - No Connect with PayPal button visible When OAuth is enabled (after partnership setup): - Full Connect UI with connect/disconnect buttons - Manual keys behind "Use Direct API Keys (Advanced)" toggle Feature flag checks (in priority order): 1. WU_PAYPAL_OAUTH_ENABLED constant 2. wu_paypal_oauth_enabled filter 3. Cached proxy /status response (12h transient) Co-Authored-By: Claude Opus 4.6 <[email protected]>
🔨 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
Summary
Adds a modern PayPal REST API gateway alongside the existing legacy NVP gateway. The new gateway uses PayPal's v2 Orders and Subscriptions APIs with proper webhook handling.
New Files
class-paypal-rest-gateway.php— Full gateway implementation (one-time + recurring payments via REST API v2)class-paypal-webhook-handler.php— Webhook processing for subscription events, payment completions, refundsclass-paypal-oauth-handler.php— OAuth Partner Referrals flow for merchant onboarding ("Connect with PayPal")class-base-paypal-gateway.php— Shared base class for both PayPal gateways (URLs, logging, BN code)Key Features
gateway_payment_idbefore creating)WU_PAYPAL_PARTNER_*), settings, orwu_paypal_partner_credentialsfilterSecurity Fixes
WP_DEBUG-based webhook signature bypass — replaced with explicitwu_paypal_skip_webhook_verificationfilterTests
PayPal_REST_Gateway(configuration, URLs, connection status, mode switching)PayPal_Webhook_Handler(signature verification, event handling, duplicate prevention, refunds)Gateway_Manager_Testto verify bothpaypal-restandpaypalregistrationTest plan
Fixes #193
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores