fix: wechat pay payment method title on payment processing #10806
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
frosso
commented
May 27, 2025
Comment on lines
+121
to
+223
|
|
||
| // For all European countries in the supported list, return EUR. | ||
| if ( in_array( | ||
| $account_country, | ||
| [ | ||
| Country_Code::AUSTRIA, | ||
| Country_Code::BELGIUM, | ||
| Country_Code::FINLAND, | ||
| Country_Code::FRANCE, | ||
| Country_Code::GERMANY, | ||
| Country_Code::IRELAND, | ||
| Country_Code::ITALY, | ||
| Country_Code::LUXEMBOURG, | ||
| Country_Code::NETHERLANDS, | ||
| Country_Code::PORTUGAL, | ||
| Country_Code::SPAIN, | ||
| ], | ||
| true | ||
| ) ) { | ||
| return [ Currency_Code::EURO ]; | ||
| } | ||
|
|
||
| if ( Country_Code::AUSTRALIA === $account_country ) { | ||
| return [ Currency_Code::AUSTRALIAN_DOLLAR ]; | ||
| } | ||
|
|
||
| if ( Country_Code::CANADA === $account_country ) { | ||
| return [ Currency_Code::CANADIAN_DOLLAR ]; | ||
| } | ||
|
|
||
| if ( Country_Code::DENMARK === $account_country ) { | ||
| return [ Currency_Code::DANISH_KRONE ]; | ||
| } | ||
|
|
||
| if ( Country_Code::HONG_KONG === $account_country ) { | ||
| return [ Currency_Code::HONG_KONG_DOLLAR ]; | ||
| } | ||
|
|
||
| if ( Country_Code::JAPAN === $account_country ) { | ||
| return [ Currency_Code::JAPANESE_YEN ]; | ||
| } | ||
|
|
||
| if ( Country_Code::NORWAY === $account_country ) { | ||
| return [ Currency_Code::NORWEGIAN_KRONE ]; | ||
| } | ||
|
|
||
| if ( Country_Code::SINGAPORE === $account_country ) { | ||
| return [ Currency_Code::SINGAPORE_DOLLAR ]; | ||
| } | ||
|
|
||
| if ( Country_Code::SWEDEN === $account_country ) { | ||
| return [ Currency_Code::SWEDISH_KRONA ]; | ||
| } | ||
|
|
||
| if ( Country_Code::SWITZERLAND === $account_country ) { | ||
| return [ Currency_Code::SWISS_FRANC ]; | ||
| } | ||
|
|
||
| if ( Country_Code::UNITED_KINGDOM === $account_country ) { | ||
| return [ Currency_Code::POUND_STERLING ]; | ||
| } | ||
|
|
||
| if ( Country_Code::UNITED_STATES === $account_country ) { | ||
| return [ Currency_Code::UNITED_STATES_DOLLAR ]; | ||
| } | ||
|
|
||
| return [ 'NONE_SUPPORTED' ]; | ||
| } | ||
|
|
||
| /** | ||
| * Get the list of supported countries | ||
| * | ||
| * @return string[] Array of country codes | ||
| */ | ||
| public static function get_supported_countries(): array { | ||
| return [ | ||
| Country_Code::UNITED_STATES, | ||
| Country_Code::AUSTRALIA, | ||
| Country_Code::CANADA, | ||
| Country_Code::AUSTRIA, | ||
| Country_Code::BELGIUM, | ||
| Country_Code::DENMARK, | ||
| Country_Code::FINLAND, | ||
| Country_Code::FRANCE, | ||
| Country_Code::GERMANY, | ||
| Country_Code::IRELAND, | ||
| Country_Code::ITALY, | ||
| Country_Code::LUXEMBOURG, | ||
| Country_Code::NETHERLANDS, | ||
| Country_Code::NORWAY, | ||
| Country_Code::PORTUGAL, | ||
| Country_Code::SPAIN, | ||
| Country_Code::SWEDEN, | ||
| Country_Code::SWITZERLAND, | ||
| Country_Code::UNITED_KINGDOM, | ||
| Country_Code::HONG_KONG, | ||
| Country_Code::JAPAN, | ||
| Country_Code::SINGAPORE, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Ported from the old Wechatpay_Payment_Method
frosso
commented
May 27, 2025
Comment on lines
+153
to
+155
| if ( ! $webhook ) { | ||
| return $payload; | ||
| } |
There was a problem hiding this comment.
A quick cheeky fix: wc_get_webhook could also return null, so $webhook->get_delivery_url() could fail.
frosso
commented
May 27, 2025
| @@ -831,6 +828,127 @@ public function test_correct_payment_method_title_for_order() { | |||
| } | |||
| } | |||
|
|
|||
| public function test_process_payment_sets_card_payment_method_and_title() { | |||
There was a problem hiding this comment.
I added these tests to check my new changes in the process_payment method, ensuring the payment method title is correctly set.
|
Size Change: 0 B Total Size: 1.24 MB ℹ️ View Unchanged
|
frosso
commented
May 27, 2025
| @@ -1777,6 +1776,33 @@ public function process_payment_for_order( $cart, $payment_information, $schedul | |||
|
|
|||
| $this->maybe_add_customer_notification_note( $order, $processing ); | |||
|
|
|||
| if ( empty( $_POST['payment_request_type'] ) && empty( $_POST['express_payment_type'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification | |||
| // Extract payment method details for setting the payment method title. | |||
| if ( $payment_needed ) { | |||
There was a problem hiding this comment.
Moving this logic from below - this is one of the main fixes.
frosso
commented
May 28, 2025
| @@ -169,7 +169,7 @@ public function test_woopay_order_payment_status_changed_with_verified_user_stor | |||
| $order = \WC_Helper_Order::create_order( $verified_user->ID ); | |||
| $order->set_billing_email( $verified_user->user_email ); | |||
| $order->save(); | |||
| WooPay_Session::woopay_order_payment_status_changed( $order->get_Id() ); | |||
There was a problem hiding this comment.
There was a little typo in the method's name
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the WeChat Pay integration to use the new config-driven WechatPayDefinition and ensures the payment method title is set before early exits during payment processing. Tests and legacy classes are updated or removed accordingly.
- Converts legacy
Wechatpay_Payment_MethodtoWechatPayDefinitionand updates registries - Moves title-setting logic before early returns in
process_payment_for_order - Adds/updates unit tests and fixes a method name casing issue in an existing test
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| includes/payment-methods/Configs/Definitions/WechatPayDefinition.php | Adds the new WeChat Pay definition class |
| includes/class-wc-payment-gateway-wcpay.php | Moves title-setting earlier in process_payment_for_order |
| includes/class-wc-payments.php | Removes legacy WeChat Pay class includes |
| includes/payment-methods/Configs/Registry/PaymentMethodDefinitionRegistry.php | Registers the new WechatPayDefinition |
| includes/woopay/class-woopay-order-status-sync.php | Guards against missing webhook object in payload creation |
| tests/unit/woopay/test-class-woopay-session.php | Fixes get_Id() to get_id() call |
| tests/unit/payment-methods/test-class-upe-split-payment-gateway.php | Adds tests for setting titles on card and SEPA payments |
| tests/WCPAY_UnitTestCase.php | Cleans up test products and transients before each test |
| changelog/fix-payment-method-title-for-wechat-pay | Adds changelog entry for this patch |
Comments suppressed due to low confidence (1)
tests/unit/woopay/test-class-woopay-session.php:172
- The method name was corrected from
get_Id()toget_id(). Verify that all other calls to this method in tests use the correct lowercaseidcasing to avoid failures.
WooPay_Session::woopay_order_payment_status_changed( $order->get_id() );
frosso
commented
May 28, 2025
| @@ -38,6 +38,34 @@ class WooPay_Session_Test extends WCPAY_UnitTestCase { | |||
| public function set_up() { | |||
| parent::set_up(); | |||
|
|
|||
| // Clear any existing test products and their lookup table entries. | |||
There was a problem hiding this comment.
I needed to add this change to the base unit test case. I'm still not sure why, but the WC_Helper_Order::create_order() call started to become problematic. I started seeing these kinds of errors in the WooPay_Session_Test unit tests:
WordPress database error Duplicate entry '327' for key 'wptests_wc_product_meta_lookup.PRIMARY' for query INSERT INTO wptests_wc_product_meta_lookup [...]
It seems that there is a problem with duplicate keys. I ended up clearing up any dummy sku products that have been added by the helper, which seems to fix the issue 🤷
marcinbot
reviewed
Jun 2, 2025
marcinbot
approved these changes
Jun 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes WOOPMNT-5015
Fixes #9981
Changes proposed in this Pull Request
There were a few issues with the main ticket.
First off, WeChat Pay didn't have a
titleattribute in its definition.I adjusted the definitions and converted the
Wechatpay_Payment_Methodclass toWeChatPayDefinitionWe also needed to set the payment method title before the SI or PI early return logic.
Before these changes, the WeChat Pay payment method title was not correctly set, because this block was executed after an early return:
Testing instructions
wechat_pay_enabledaccount meta)npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge