WPS Migration: Fix capture failure #60136
Conversation
Testing GuidelinesHi @annemirasol , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a race condition in the PayPal gateway where duplicate capture requests were being made when changing order status to "processing". The issue occurred because the PayPal status was not immediately available for the second request due to asynchronous webhook processing.
- Updates PayPal status handling to use lowercase values consistently
- Sets the PayPal status immediately after successful capture requests
- Adds explicit order save operations to ensure status is persisted
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| class-wc-gateway-paypal-webhook-handler.php | Normalizes PayPal status to lowercase and adds order save after approval |
| class-wc-gateway-paypal-request.php | Updates status check to lowercase, sets status after capture response |
plugins/woocommerce/includes/gateways/paypal/includes/class-wc-gateway-paypal-request.php
Show resolved
Hide resolved
plugins/woocommerce/includes/gateways/paypal/includes/class-wc-gateway-paypal-request.php
Outdated
Show resolved
Hide resolved
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
|
Caution Review failedFailed to post review comments. Configuration used: .coderabbit.yml 📒 Files selected for processing (10)
🧰 Additional context used📓 Path-based instructions (2)**/*.{php,js,jsx,ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/code-quality.mdc)
Files:
**/*.{php,js,ts,jsx,tsx}⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: in the woocommerce woopayments rest controller, it's acceptable to let apiexception bubble up from o...Applied to files:
📚 Learning: in woocommerce payment gateway provider classes, prefer using `is_callable()` over `method_exists()`...Applied to files:
📚 Learning: in plugins/woocommerce/src/internal/admin/settings/payments.php, the process_payment_provider_states...Applied to files:
📚 Learning: in php payment gateway provider classes, when using constants as option keys in $payment_gateway->ge...Applied to files:
📚 Learning: in woocommerce codebase, prefer using `wc_string_to_bool()` over `filter_var($value, filter_validate...Applied to files:
📚 Learning: in woocommerce payments settings, when processing payment provider states, if a gateway appears in t...Applied to files:
📚 Learning: in the woocommerce payments settings provider state tracking system (plugins/woocommerce/src/interna...Applied to files:
📚 Learning: in plugins/woocommerce/client/admin/client/settings-payments/components/other-payment-gateways/other...Applied to files:
📚 Learning: the `kp_settings_page::get_setting_status('credentials')` method from the klarna plugin returns a bo...Applied to files:
🧬 Code Graph Analysis (2)plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-paypal-webhooks-controller.php (1)
plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-paypal-webhooks-proxy-controller.php (2)
🪛 PHPMD (2.15.0)plugins/woocommerce/includes/rest-api/Controllers/Version3/class-wc-rest-paypal-webhooks-controller.php81-81: Avoid unused local variables such as '$data'. (Unused Code Rules) (UnusedLocalVariable) plugins/woocommerce/tests/php/includes/gateways/paypal/class-wc-gateway-paypal-request-test.php70-70: Avoid unused local variables such as '$result'. (Unused Code Rules) (UnusedLocalVariable) 124-124: Avoid unused parameters such as '$value'. (Unused Code Rules) (UnusedFormalParameter) 124-124: Avoid unused parameters such as '$parsed_url'. (Unused Code Rules) (UnusedFormalParameter) 152-152: Avoid unused parameters such as '$value'. (Unused Code Rules) (UnusedFormalParameter) 152-152: Avoid unused parameters such as '$parsed_url'. (Unused Code Rules) (UnusedFormalParameter) plugins/woocommerce/includes/gateways/paypal/includes/class-wc-gateway-paypal-request.php460-460: Avoid unused local variables such as '$include_tax'. (Unused Code Rules) (UnusedLocalVariable) 460-460: Avoid unused local variables such as '$rounding_enabled'. (Unused Code Rules) (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). (24)
🔇 Additional comments (19)
📝 WalkthroughWalkthroughThis change introduces comprehensive support for the PayPal Orders v2 API in WooCommerce, including new helper and webhook handler classes, REST API proxy and webhook controllers, and conditional logic to switch between legacy and Orders v2 flows. It updates the PayPal gateway, request handling, and settings, and adds unit tests for the new API integration. Changes
Sequence Diagram(s)PayPal Orders v2 Payment Flow (High-Level)sequenceDiagram
participant Customer
participant WooCommerce (WC_Gateway_Paypal)
participant PayPal Proxy API (WC_REST_Paypal_Proxy_Controller)
participant PayPal API
participant Webhook Handler
Customer->>WooCommerce (WC_Gateway_Paypal): Place Order
WooCommerce (WC_Gateway_Paypal)->>PayPal Proxy API: Create PayPal Order
PayPal Proxy API->>PayPal API: POST /v2/checkout/orders
PayPal API-->>PayPal Proxy API: Order ID + Approval URL
PayPal Proxy API-->>WooCommerce (WC_Gateway_Paypal): Order ID + Approval URL
WooCommerce (WC_Gateway_Paypal)-->>Customer: Redirect to Approval URL
Customer->>PayPal API: Approve Payment
PayPal API->>Webhook Handler: Send Webhook (e.g., ORDER.APPROVED)
Webhook Handler->>WooCommerce (WC_Gateway_Paypal): Update Order, Capture/Authorize Payment
WooCommerce (WC_Gateway_Paypal)->>PayPal Proxy API: Capture/Authorize Payment
PayPal Proxy API->>PayPal API: POST /capture or /authorize
PayPal API-->>PayPal Proxy API: Payment Status
PayPal Proxy API-->>WooCommerce (WC_Gateway_Paypal): Payment Status
WooCommerce (WC_Gateway_Paypal)->>Order: Update Status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Hi @Mayisha! Your PR contains REST API changes. Please consider updating the REST API documentation if your changes affect the public API. Changed API files: |
Submission Review Guidelines:
Changes proposed in this Pull Request:
This PR fixes the issue reported in #60044 (review)
If you change the order status to
processing, there are two API requests to capture the payment. We have a status check before sending the request. As we were setting the status during webhook processing, the status was not set by the time the 2nd request was made. In this PR, I have set the status when the order capture request is successful.How to test the changes in this Pull Request:
on-hold.Authorized.processing.Authorized.Authorization has already been captured.error in the logs.Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment