From f33af705ab222ec0e08ce4ecc3410df425716899 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Wed, 27 Dec 2023 16:26:10 +0530 Subject: [PATCH 01/15] Streamline the params in attach_intent_info_to_order The method has too many params whose values can easily be derived from a single intent object. This commit replaces all derivable params with an intent param. This will make it easier to add new intent-based data values to order meta. --- ...ass-wc-rest-payments-orders-controller.php | 14 +-------- includes/class-wc-payment-gateway-wcpay.php | 4 +-- includes/class-wc-payments-order-service.php | 30 ++++++++++++++++++- .../class-upe-payment-gateway.php | 15 ++++------ .../class-wc-payments-invoice-service.php | 10 +------ src/Internal/Service/OrderService.php | 4 +-- ...ass-wc-rest-payments-orders-controller.php | 21 ++----------- .../test-class-upe-split-payment-gateway.php | 2 +- .../src/Internal/Service/OrderServiceTest.php | 6 ++-- ...-payment-gateway-wcpay-process-payment.php | 8 ++--- .../test-class-wc-payments-order-service.php | 10 ++----- 11 files changed, 54 insertions(+), 70 deletions(-) diff --git a/includes/admin/class-wc-rest-payments-orders-controller.php b/includes/admin/class-wc-rest-payments-orders-controller.php index a833fc9092f..e23f1ebfcff 100644 --- a/includes/admin/class-wc-rest-payments-orders-controller.php +++ b/includes/admin/class-wc-rest-payments-orders-controller.php @@ -189,19 +189,7 @@ public function capture_terminal_payment( WP_REST_Request $request ) { // Update the order: set the payment method and attach intent attributes. $order->set_payment_method( WC_Payment_Gateway_WCPay::GATEWAY_ID ); $order->set_payment_method_title( __( 'WooCommerce In-Person Payments', 'woocommerce-payments' ) ); - $intent_id = $intent->get_id(); - $intent_status = $intent->get_status(); - $charge = $intent->get_charge(); - $charge_id = $charge ? $charge->get_id() : null; - $this->order_service->attach_intent_info_to_order( - $order, - $intent_id, - $intent_status, - $intent->get_payment_method_id(), - $intent->get_customer_id(), - $charge_id, - $intent->get_currency() - ); + $this->order_service->attach_intent_info_to_order( $order, $intent ); $this->order_service->update_order_status_from_intent( $order, $intent ); // Certain payments (eg. Interac) are captured on the client-side (mobile app). diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index 415d9e0a411..02030f3aa19 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -1485,7 +1485,7 @@ public function process_payment_for_order( $cart, $payment_information, $schedul } } - $this->order_service->attach_intent_info_to_order( $order, $intent_id, $status, $payment_method, $customer_id, $charge_id, $currency ); + $this->order_service->attach_intent_info_to_order( $order, $intent ); $this->attach_exchange_info_to_order( $order, $charge_id ); if ( Intent_Status::SUCCEEDED === $status ) { $this->duplicate_payment_prevention_service->remove_session_processing_order( $order->get_id() ); @@ -3001,7 +3001,7 @@ public function update_order_status() { $charge_id = ! empty( $charge ) ? $charge->get_id() : null; $this->attach_exchange_info_to_order( $order, $charge_id ); - $this->order_service->attach_intent_info_to_order( $order, $intent_id, $status, $intent->get_payment_method_id(), $intent->get_customer_id(), $charge_id, $intent->get_currency() ); + $this->order_service->attach_intent_info_to_order( $order, $intent ); $this->order_service->attach_transaction_fee_to_order( $order, $charge ); } else { // For $0 orders, fetch the Setup Intent instead. diff --git a/includes/class-wc-payments-order-service.php b/includes/class-wc-payments-order-service.php index eab529f6d92..f47462f7594 100644 --- a/includes/class-wc-payments-order-service.php +++ b/includes/class-wc-payments-order-service.php @@ -752,6 +752,34 @@ public function get_fraud_meta_box_type_for_order( $order ) : string { /** * Given the payment intent data, adds it to the given order as metadata and parses any notes that need to be added * + * @param WC_Order $order The order. + * @param WC_Payments_API_Abstract_Intention $intent The payment or setup intention object. + * + * @throws Order_Not_Found_Exception + */ + public function attach_intent_info_to_order( WC_Order $order, WC_Payments_API_Abstract_Intention $intent ) { + // first, let's prepare all the metadata needed for refunds, required for status change etc. + $intent_id = $intent->get_id(); + $intent_status = $intent->get_status(); + $payment_method = $intent->get_payment_method_id(); + $customer_id = $intent->get_customer_id(); + $currency = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_currency() : $order->get_currency(); + $charge = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_charge() : null; + $charge_id = $charge ? $charge->get_id() : null; + // next, save it in order meta. + $order->set_transaction_id( $intent_id ); + $this->set_intent_id_for_order( $order, $intent_id ); + $this->set_payment_method_id_for_order( $order, $payment_method ); + $this->set_charge_id_for_order( $order, $charge_id ); + $this->set_intention_status_for_order( $order, $intent_status ); + $this->set_customer_id_for_order( $order, $customer_id ); + $this->set_wcpay_intent_currency_for_order( $order, $currency ); + $order->save(); + } + + /** + * Legacy version of the attach_intent_info_to_order method. + * * @param WC_Order $order The order. * @param string $intent_id The intent ID. * @param string $intent_status Intent status. @@ -762,7 +790,7 @@ public function get_fraud_meta_box_type_for_order( $order ) : string { * * @throws Order_Not_Found_Exception */ - public function attach_intent_info_to_order( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency ) { + public function attach_intent_info_to_order__legacy( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency ) { // first, let's save all the metadata that needed for refunds, required for status change etc. $order->set_transaction_id( $intent_id ); $this->set_intent_id_for_order( $order, $intent_id ); diff --git a/includes/payment-methods/class-upe-payment-gateway.php b/includes/payment-methods/class-upe-payment-gateway.php index b1ba28589b6..a7c66def28b 100644 --- a/includes/payment-methods/class-upe-payment-gateway.php +++ b/includes/payment-methods/class-upe-payment-gateway.php @@ -245,13 +245,10 @@ public function update_payment_intent( $payment_intent_id = '', $order_id = null // Attach the intent and exchange info to the order before doing the redirect, // so that when processing redirect, the up-to-date intent information is available. - $intent_id = $updated_payment_intent->get_id(); - $intent_status = $updated_payment_intent->get_status(); - $payment_method = $updated_payment_intent->get_payment_method_id(); - $charge = $updated_payment_intent->get_charge(); - $charge_id = $charge ? $charge->get_id() : null; + $charge = $updated_payment_intent->get_charge(); + $charge_id = $charge ? $charge->get_id() : null; - $this->order_service->attach_intent_info_to_order( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency ); + $this->order_service->attach_intent_info_to_order( $order, $updated_payment_intent ); $this->attach_exchange_info_to_order( $order, $charge_id ); } @@ -519,9 +516,7 @@ public function process_payment( $order_id ) { throw $e; } - $intent_id = $updated_payment_intent->get_id(); $intent_status = $updated_payment_intent->get_status(); - $payment_method = $updated_payment_intent->get_payment_method_id(); $charge = $updated_payment_intent->get_charge(); $payment_method_details = $charge ? $charge->get_payment_method_details() : []; $payment_method_type = $this->get_payment_method_type_from_payment_details( $payment_method_details ); @@ -532,7 +527,7 @@ public function process_payment( $order_id ) { * either does not complete properly, or the Stripe webhook which processes a successful order hits before * the redirect completes. */ - $this->order_service->attach_intent_info_to_order( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency ); + $this->order_service->attach_intent_info_to_order( $order, $updated_payment_intent ); $this->attach_exchange_info_to_order( $order, $charge_id ); $this->set_payment_method_title_for_order( $order, $payment_method_type, $payment_method_details ); if ( Intent_Status::SUCCEEDED === $intent_status ) { @@ -760,7 +755,7 @@ public function process_redirect_payment( $order, $intent_id, $save_payment_meth } } - $this->order_service->attach_intent_info_to_order( $order, $intent_id, $status, $payment_method_id, $customer_id, $charge_id, $currency ); + $this->order_service->attach_intent_info_to_order( $order, $intent ); $this->attach_exchange_info_to_order( $order, $charge_id ); if ( Intent_Status::SUCCEEDED === $status ) { $this->duplicate_payment_prevention_service->remove_session_processing_order( $order->get_id() ); diff --git a/includes/subscriptions/class-wc-payments-invoice-service.php b/includes/subscriptions/class-wc-payments-invoice-service.php index c4d0cd38fb5..2e2b23663d2 100644 --- a/includes/subscriptions/class-wc-payments-invoice-service.php +++ b/includes/subscriptions/class-wc-payments-invoice-service.php @@ -299,15 +299,7 @@ public function get_and_attach_intent_info_to_order( $order, $intent_id ) { $charge = $intent_object->get_charge(); - $this->order_service->attach_intent_info_to_order( - $order, - $intent_id, - $intent_object->get_status(), - $intent_object->get_payment_method_id(), - $intent_object->get_customer_id(), - $charge ? $charge->get_id() : null, - $intent_object->get_currency() - ); + $this->order_service->attach_intent_info_to_order( $order, $intent_object ); } /** diff --git a/src/Internal/Service/OrderService.php b/src/Internal/Service/OrderService.php index 25e962cdadd..7d301b27250 100644 --- a/src/Internal/Service/OrderService.php +++ b/src/Internal/Service/OrderService.php @@ -193,7 +193,7 @@ public function update_order_from_successful_intent( $charge_id = $intent->get_charge()->get_id(); } - $this->legacy_service->attach_intent_info_to_order( + $this->legacy_service->attach_intent_info_to_order__legacy( $order, $intent->get_id(), $intent->get_status(), @@ -253,7 +253,7 @@ public function update_order_from_intent_that_requires_action( ) { $order = $this->get_order( $order_id ); - $this->legacy_service->attach_intent_info_to_order( + $this->legacy_service->attach_intent_info_to_order__legacy( $order, $intent->get_id(), $intent->get_status(), diff --git a/tests/unit/admin/test-class-wc-rest-payments-orders-controller.php b/tests/unit/admin/test-class-wc-rest-payments-orders-controller.php index 3a76bcba3ea..3d44fc9bfa5 100644 --- a/tests/unit/admin/test-class-wc-rest-payments-orders-controller.php +++ b/tests/unit/admin/test-class-wc-rest-payments-orders-controller.php @@ -114,12 +114,7 @@ public function test_capture_terminal_payment_success() { ->method( 'attach_intent_info_to_order' ) ->with( $this->isInstanceOf( WC_Order::class ), - $this->mock_intent_id, - Intent_Status::REQUIRES_CAPTURE, - 'pm_mock', - 'cus_mock', - $this->mock_charge_id, - 'USD' + $mock_intent, ); $request = new WP_REST_Request( 'POST' ); @@ -171,12 +166,7 @@ public function test_capture_terminal_payment_succeeded_intent() { ->method( 'attach_intent_info_to_order' ) ->with( $this->isInstanceOf( WC_Order::class ), - $this->mock_intent_id, - Intent_Status::SUCCEEDED, - 'pm_mock', - 'cus_mock', - $this->mock_charge_id, - 'USD' + $mock_intent, ); $this->mock_gateway @@ -236,12 +226,7 @@ public function test_capture_terminal_payment_completed_order() { ->method( 'attach_intent_info_to_order' ) ->with( $this->isInstanceOf( WC_Order::class ), - $this->mock_intent_id, - Intent_Status::SUCCEEDED, - 'pm_mock', - 'cus_mock', - $this->mock_charge_id, - 'USD' + $mock_intent, ); $this->mock_gateway diff --git a/tests/unit/payment-methods/test-class-upe-split-payment-gateway.php b/tests/unit/payment-methods/test-class-upe-split-payment-gateway.php index 40cc78eda7d..b478c7d6715 100644 --- a/tests/unit/payment-methods/test-class-upe-split-payment-gateway.php +++ b/tests/unit/payment-methods/test-class-upe-split-payment-gateway.php @@ -1153,7 +1153,7 @@ public function test_process_redirect_setup_intent_succeded() { $setup_intent = WC_Helper_Intention::create_setup_intention( [ - 'id' => 'pi_mock', + 'id' => $intent_id, 'client_secret' => $client_secret, 'status' => $intent_status, 'payment_method' => $payment_method_id, diff --git a/tests/unit/src/Internal/Service/OrderServiceTest.php b/tests/unit/src/Internal/Service/OrderServiceTest.php index 63f40cf074f..3faae6b6ccd 100644 --- a/tests/unit/src/Internal/Service/OrderServiceTest.php +++ b/tests/unit/src/Internal/Service/OrderServiceTest.php @@ -332,7 +332,7 @@ public function test_update_order_from_successful_intent( $intent ) { ->willReturn( $mock_charge ); } - // Prepare all parameters for `attach_intent_info_to_order`. + // Prepare all parameters for `attach_intent_info_to_order__legacy`. $intent->expects( $this->once() ) ->method( 'get_id' ) ->willReturn( $intent_id ); @@ -355,7 +355,7 @@ public function test_update_order_from_successful_intent( $intent ) { ->willReturn( 'prod' ); $this->mock_legacy_service->expects( $this->once() ) - ->method( 'attach_intent_info_to_order' ) + ->method( 'attach_intent_info_to_order__legacy' ) ->with( $mock_order, $intent_id, @@ -412,7 +412,7 @@ public function test_update_order_from_intent_that_requires_action() { $mock_intent->expects( $this->once() )->method( 'get_status' )->willReturn( $intent_status ); $this->mock_legacy_service->expects( $this->once() ) - ->method( 'attach_intent_info_to_order' ) + ->method( 'attach_intent_info_to_order__legacy' ) ->with( $mock_order, $intent_id, diff --git a/tests/unit/test-class-wc-payment-gateway-wcpay-process-payment.php b/tests/unit/test-class-wc-payment-gateway-wcpay-process-payment.php index da7ed2798db..29a9d4129e6 100644 --- a/tests/unit/test-class-wc-payment-gateway-wcpay-process-payment.php +++ b/tests/unit/test-class-wc-payment-gateway-wcpay-process-payment.php @@ -303,7 +303,7 @@ public function test_intent_status_success() { $this->mock_order_service ->expects( $this->once() ) ->method( 'attach_intent_info_to_order' ) - ->with( $mock_order, $intent_id, $status, 'pm_mock', $customer_id, $charge_id, 'USD' ); + ->with( $mock_order, $intent ); $this->mock_order_service ->expects( $this->once() ) @@ -470,7 +470,7 @@ public function test_intent_status_requires_capture() { $this->mock_order_service ->expects( $this->once() ) ->method( 'attach_intent_info_to_order' ) - ->with( $mock_order, $intent_id, $status, 'pm_mock', $customer_id, $charge_id, 'USD' ); + ->with( $mock_order, $intent ); // Assert: The Order_Service is called correctly. $this->mock_order_service @@ -918,7 +918,7 @@ public function test_intent_status_requires_action() { $this->mock_order_service ->expects( $this->once() ) ->method( 'attach_intent_info_to_order' ) - ->with( $mock_order, $intent_id, $status, 'pm_mock', $customer_id, $charge_id, 'USD' ); + ->with( $mock_order, $intent ); $this->mock_order_service ->expects( $this->once() ) @@ -1034,7 +1034,7 @@ public function test_setup_intent_status_requires_action() { $this->mock_order_service ->expects( $this->once() ) ->method( 'attach_intent_info_to_order' ) - ->with( $mock_order, $intent_id, $status, 'pm_mock', $customer_id, '', 'USD' ); + ->with( $mock_order, $intent ); // Assert: Order status was not updated. $mock_order diff --git a/tests/unit/test-class-wc-payments-order-service.php b/tests/unit/test-class-wc-payments-order-service.php index 1107c94fcd5..77208c8296c 100644 --- a/tests/unit/test-class-wc-payments-order-service.php +++ b/tests/unit/test-class-wc-payments-order-service.php @@ -1206,13 +1206,9 @@ public function test_get_fraud_meta_box_type() { } public function test_attach_intent_info_to_order() { - $intent_id = 'pi_mock'; - $intent_status = 'succeeded'; - $payment_method = 'woocommerce_payments'; - $customer_id = 'cus_12345'; - $charge_id = 'ch_mock'; - $currency = 'USD'; - $this->order_service->attach_intent_info_to_order( $this->order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency ); + $intent_id = 'pi_mock'; + $intent = WC_Helper_Intention::create_intention( [ 'id' => $intent_id ] ); + $this->order_service->attach_intent_info_to_order( $this->order, $intent ); $this->assertEquals( $intent_id, $this->order->get_meta( '_intent_id', true ) ); } From fbc8664dfb36ab954c5961dea6941868716ffe18 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Wed, 27 Dec 2023 16:42:00 +0530 Subject: [PATCH 02/15] Save balance transaction id to order meta while processing a payment --- includes/class-wc-payments-order-service.php | 38 ++++++++++++++++---- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/includes/class-wc-payments-order-service.php b/includes/class-wc-payments-order-service.php index f47462f7594..adb1f433c37 100644 --- a/includes/class-wc-payments-order-service.php +++ b/includes/class-wc-payments-order-service.php @@ -41,6 +41,13 @@ class WC_Payments_Order_Service { */ const CHARGE_ID_META_KEY = '_charge_id'; + /** + * Meta key used to store payment transaction Id. + * + * @const string + */ + const PAYMENT_TRANSACTION_ID_META_KEY = '_payment_transaction_id'; + /** * Meta key used to store intention status. * @@ -524,6 +531,20 @@ public function set_charge_id_for_order( $order, $charge_id ) { $order->save_meta_data(); } + /** + * Set the payment metadata for payment transaction id. + * + * @param mixed $order The order. + * @param string $payment_transaction_id The value to be set. + * + * @throws Order_Not_Found_Exception + */ + public function set_payment_transaction_id_for_order( $order, $payment_transaction_id ) { + $order = $this->get_order( $order ); + $order->update_meta_data( self::PAYMENT_TRANSACTION_ID_META_KEY, $payment_transaction_id ); + $order->save_meta_data(); + } + /** * Get the payment metadata for charge id. * @@ -759,18 +780,21 @@ public function get_fraud_meta_box_type_for_order( $order ) : string { */ public function attach_intent_info_to_order( WC_Order $order, WC_Payments_API_Abstract_Intention $intent ) { // first, let's prepare all the metadata needed for refunds, required for status change etc. - $intent_id = $intent->get_id(); - $intent_status = $intent->get_status(); - $payment_method = $intent->get_payment_method_id(); - $customer_id = $intent->get_customer_id(); - $currency = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_currency() : $order->get_currency(); - $charge = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_charge() : null; - $charge_id = $charge ? $charge->get_id() : null; + $intent_id = $intent->get_id(); + $intent_status = $intent->get_status(); + $payment_method = $intent->get_payment_method_id(); + $customer_id = $intent->get_customer_id(); + $currency = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_currency() : $order->get_currency(); + $charge = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_charge() : null; + $charge_id = $charge ? $charge->get_id() : null; + $payment_transaction = $charge ? $charge->get_balance_transaction() : null; + $payment_transaction_id = $payment_transaction ? $payment_transaction['id'] : ''; // next, save it in order meta. $order->set_transaction_id( $intent_id ); $this->set_intent_id_for_order( $order, $intent_id ); $this->set_payment_method_id_for_order( $order, $payment_method ); $this->set_charge_id_for_order( $order, $charge_id ); + $this->set_payment_transaction_id_for_order( $order, $payment_transaction_id ); $this->set_intention_status_for_order( $order, $intent_status ); $this->set_customer_id_for_order( $order, $customer_id ); $this->set_wcpay_intent_currency_for_order( $order, $currency ); From 7f9fc281c5fc862b430d7b3b926806df5b973b47 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Wed, 27 Dec 2023 17:06:19 +0530 Subject: [PATCH 03/15] Add changelog --- changelog/update-7856-add-charge-txn-id-to-order-meta | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/update-7856-add-charge-txn-id-to-order-meta diff --git a/changelog/update-7856-add-charge-txn-id-to-order-meta b/changelog/update-7856-add-charge-txn-id-to-order-meta new file mode 100644 index 00000000000..38037b5a6d1 --- /dev/null +++ b/changelog/update-7856-add-charge-txn-id-to-order-meta @@ -0,0 +1,4 @@ +Significance: minor +Type: update + +Store balance transaction ID in order metadata. From 83d5af89335bd77c2cdcb9fa58d3418c21d998e7 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Wed, 27 Dec 2023 18:57:35 +0530 Subject: [PATCH 04/15] Replace the value of balance transaction in create charge helper with a more accurate mock object --- tests/unit/helpers/class-wc-helper-intention.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/unit/helpers/class-wc-helper-intention.php b/tests/unit/helpers/class-wc-helper-intention.php index 06130bb3b7e..3879db1f32d 100644 --- a/tests/unit/helpers/class-wc-helper-intention.php +++ b/tests/unit/helpers/class-wc-helper-intention.php @@ -38,7 +38,15 @@ public static function create_charge( $data = [] ) { 'amount_captured' => 5000, 'amount_refunded' => 0, 'application_fee_amount' => 0, - 'balance_transaction' => 'txn_mock', + 'balance_transaction' => [ + 'id' => 'txn_mock', + 'amount' => 5000, + 'available_on' => 1703808000, + 'created' => new DateTime( '2022-05-20 19:05:38' ), + 'currency' => 'usd', + 'exchange_rate' => null, + 'fee' => 82, + ], 'billing_details' => [], 'currency' => 'usd', 'dispute' => [], From d73ab093e01d7f32d464ac74242c3fb1c505ee1b Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Tue, 2 Jan 2024 16:47:49 +0530 Subject: [PATCH 05/15] Fix psalm error UndefinedMethod --- includes/class-wc-payments-order-service.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/class-wc-payments-order-service.php b/includes/class-wc-payments-order-service.php index adb1f433c37..f3d4e6bb728 100644 --- a/includes/class-wc-payments-order-service.php +++ b/includes/class-wc-payments-order-service.php @@ -773,12 +773,12 @@ public function get_fraud_meta_box_type_for_order( $order ) : string { /** * Given the payment intent data, adds it to the given order as metadata and parses any notes that need to be added * - * @param WC_Order $order The order. - * @param WC_Payments_API_Abstract_Intention $intent The payment or setup intention object. + * @param WC_Order $order The order. + * @param WC_Payments_API_Payment_Intention|WC_Payments_API_Setup_Intention $intent The payment or setup intention object. * * @throws Order_Not_Found_Exception */ - public function attach_intent_info_to_order( WC_Order $order, WC_Payments_API_Abstract_Intention $intent ) { + public function attach_intent_info_to_order( WC_Order $order, $intent ) { // first, let's prepare all the metadata needed for refunds, required for status change etc. $intent_id = $intent->get_id(); $intent_status = $intent->get_status(); From 8c2f80ffe414e040c88c32e5e73e7e3f5bf1df27 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Tue, 2 Jan 2024 17:02:05 +0530 Subject: [PATCH 06/15] Update method call in the main gateway class Looks like new reference of the method that came with the latest code from develop. --- includes/class-wc-payment-gateway-wcpay.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index 1e356ef6e17..4a12828256e 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -1879,7 +1879,7 @@ public function process_redirect_payment( $order, $intent_id, $save_payment_meth } } - $this->order_service->attach_intent_info_to_order( $order, $intent_id, $status, $payment_method_id, $customer_id, $charge_id, $currency ); + $this->order_service->attach_intent_info_to_order( $order, $intent ); $this->attach_exchange_info_to_order( $order, $charge_id ); if ( Intent_Status::SUCCEEDED === $status ) { $this->duplicate_payment_prevention_service->remove_session_processing_order( $order->get_id() ); From a7bef878a2b8799e82d98fefb3b0be541a124ed3 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Thu, 11 Jan 2024 14:20:39 +0530 Subject: [PATCH 07/15] Remove code duplication between the two attach intent methods --- includes/class-wc-payments-order-service.php | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/includes/class-wc-payments-order-service.php b/includes/class-wc-payments-order-service.php index 52ae371884d..d4caaeb3550 100644 --- a/includes/class-wc-payments-order-service.php +++ b/includes/class-wc-payments-order-service.php @@ -547,6 +547,9 @@ public function set_charge_id_for_order( $order, $charge_id ) { * @throws Order_Not_Found_Exception */ public function set_payment_transaction_id_for_order( $order, $payment_transaction_id ) { + if ( ! isset( $payment_transaction_id ) || null === $payment_transaction_id ) { + return; + } $order = $this->get_order( $order ); $order->update_meta_data( self::PAYMENT_TRANSACTION_ID_META_KEY, $payment_transaction_id ); $order->save_meta_data(); @@ -811,20 +814,14 @@ public function attach_intent_info_to_order( WC_Order $order, $intent ) { $payment_transaction = $charge ? $charge->get_balance_transaction() : null; $payment_transaction_id = $payment_transaction ? $payment_transaction['id'] : ''; // next, save it in order meta. - $order->set_transaction_id( $intent_id ); - $this->set_intent_id_for_order( $order, $intent_id ); - $this->set_payment_method_id_for_order( $order, $payment_method ); - $this->set_charge_id_for_order( $order, $charge_id ); - $this->set_payment_transaction_id_for_order( $order, $payment_transaction_id ); - $this->set_intention_status_for_order( $order, $intent_status ); - $this->set_customer_id_for_order( $order, $customer_id ); - $this->set_wcpay_intent_currency_for_order( $order, $currency ); - $order->save(); + $this->attach_intent_info_to_order__legacy( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency, $payment_transaction_id ); } /** * Legacy version of the attach_intent_info_to_order method. * + * TODO: This method should ultimately be merged with `attach_intent_info_to_order` and then removed. + * * @param WC_Order $order The order. * @param string $intent_id The intent ID. * @param string $intent_status Intent status. @@ -832,10 +829,11 @@ public function attach_intent_info_to_order( WC_Order $order, $intent ) { * @param string $customer_id Customer ID. * @param string $charge_id Charge ID. * @param string $currency Currency code. + * @param string $payment_transaction_id The transaction ID of the linked charge. * * @throws Order_Not_Found_Exception */ - public function attach_intent_info_to_order__legacy( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency ) { + public function attach_intent_info_to_order__legacy( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency, $payment_transaction_id = null ) { // first, let's save all the metadata that needed for refunds, required for status change etc. $order->set_transaction_id( $intent_id ); $this->set_intent_id_for_order( $order, $intent_id ); @@ -844,6 +842,7 @@ public function attach_intent_info_to_order__legacy( $order, $intent_id, $intent $this->set_intention_status_for_order( $order, $intent_status ); $this->set_customer_id_for_order( $order, $customer_id ); $this->set_wcpay_intent_currency_for_order( $order, $currency ); + $this->set_payment_transaction_id_for_order( $order, $payment_transaction_id ); $order->save(); } From adc9a67919afb8df6bea59510f30566b49383c5b Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Thu, 11 Jan 2024 14:24:42 +0530 Subject: [PATCH 08/15] Prepend wcpay to the meta key --- includes/class-wc-payments-order-service.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/includes/class-wc-payments-order-service.php b/includes/class-wc-payments-order-service.php index d4caaeb3550..e5b79e1001c 100644 --- a/includes/class-wc-payments-order-service.php +++ b/includes/class-wc-payments-order-service.php @@ -41,13 +41,6 @@ class WC_Payments_Order_Service { */ const CHARGE_ID_META_KEY = '_charge_id'; - /** - * Meta key used to store payment transaction Id. - * - * @const string - */ - const PAYMENT_TRANSACTION_ID_META_KEY = '_payment_transaction_id'; - /** * Meta key used to store intention status. * @@ -120,6 +113,13 @@ class WC_Payments_Order_Service { */ const WCPAY_MODE_META_KEY = '_wcpay_mode'; + /** + * Meta key used to store payment transaction Id. + * + * @const string + */ + const WCPAY_PAYMENT_TRANSACTION_ID_META_KEY = '_wcpay_payment_transaction_id'; + /** * Client for making requests to the WooCommerce Payments API * @@ -551,7 +551,7 @@ public function set_payment_transaction_id_for_order( $order, $payment_transacti return; } $order = $this->get_order( $order ); - $order->update_meta_data( self::PAYMENT_TRANSACTION_ID_META_KEY, $payment_transaction_id ); + $order->update_meta_data( self::WCPAY_PAYMENT_TRANSACTION_ID_META_KEY, $payment_transaction_id ); $order->save_meta_data(); } From 1f4681ac38ffcf9e6eca44eef15927205cc5a9d4 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Thu, 11 Jan 2024 17:34:58 +0530 Subject: [PATCH 09/15] Remove the unnecessary variable in test --- tests/unit/test-class-wc-payments-order-service.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test-class-wc-payments-order-service.php b/tests/unit/test-class-wc-payments-order-service.php index 8f24d7dbdf0..1f9b3f819a7 100644 --- a/tests/unit/test-class-wc-payments-order-service.php +++ b/tests/unit/test-class-wc-payments-order-service.php @@ -1212,8 +1212,7 @@ public function test_get_fraud_meta_box_type() { } public function test_attach_intent_info_to_order() { - $intent_id = 'pi_mock'; - $intent = WC_Helper_Intention::create_intention( [ 'id' => $intent_id ] ); + $intent = WC_Helper_Intention::create_intention( [ 'id' => 'pi_mock' ] ); $this->order_service->attach_intent_info_to_order( $this->order, $intent ); $this->assertEquals( $intent_id, $this->order->get_meta( '_intent_id', true ) ); From 69c82bd3f13980ab69e84e03bb93cf960b259424 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Thu, 11 Jan 2024 12:07:35 +0000 Subject: [PATCH 10/15] Use null coalescing instead of ternary operator Co-authored-by: Naman Malhotra --- includes/class-wc-payments-order-service.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/class-wc-payments-order-service.php b/includes/class-wc-payments-order-service.php index e5b79e1001c..f5b3efdcf24 100644 --- a/includes/class-wc-payments-order-service.php +++ b/includes/class-wc-payments-order-service.php @@ -812,7 +812,7 @@ public function attach_intent_info_to_order( WC_Order $order, $intent ) { $charge = $intent instanceof WC_Payments_API_Payment_Intention ? $intent->get_charge() : null; $charge_id = $charge ? $charge->get_id() : null; $payment_transaction = $charge ? $charge->get_balance_transaction() : null; - $payment_transaction_id = $payment_transaction ? $payment_transaction['id'] : ''; + $payment_transaction_id = $payment_transaction['id'] ?? ''; // next, save it in order meta. $this->attach_intent_info_to_order__legacy( $order, $intent_id, $intent_status, $payment_method, $customer_id, $charge_id, $currency, $payment_transaction_id ); } From bb873661258043e843dfcbe459134bc18c13e260 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Thu, 11 Jan 2024 17:55:39 +0530 Subject: [PATCH 11/15] Revert "Remove the unnecessary variable in test" This reverts commit 1f4681ac38ffcf9e6eca44eef15927205cc5a9d4. --- tests/unit/test-class-wc-payments-order-service.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test-class-wc-payments-order-service.php b/tests/unit/test-class-wc-payments-order-service.php index 1f9b3f819a7..8f24d7dbdf0 100644 --- a/tests/unit/test-class-wc-payments-order-service.php +++ b/tests/unit/test-class-wc-payments-order-service.php @@ -1212,7 +1212,8 @@ public function test_get_fraud_meta_box_type() { } public function test_attach_intent_info_to_order() { - $intent = WC_Helper_Intention::create_intention( [ 'id' => 'pi_mock' ] ); + $intent_id = 'pi_mock'; + $intent = WC_Helper_Intention::create_intention( [ 'id' => $intent_id ] ); $this->order_service->attach_intent_info_to_order( $this->order, $intent ); $this->assertEquals( $intent_id, $this->order->get_meta( '_intent_id', true ) ); From e745e712cea0ae7b7edd1e46971197bee2153729 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Tue, 16 Jan 2024 15:30:58 +0530 Subject: [PATCH 12/15] Prevent overwriting order meta after successful payment --- includes/class-wc-payments-order-service.php | 4 ++++ .../test-class-wc-payments-order-service.php | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/includes/class-wc-payments-order-service.php b/includes/class-wc-payments-order-service.php index f5b3efdcf24..21b549768a7 100644 --- a/includes/class-wc-payments-order-service.php +++ b/includes/class-wc-payments-order-service.php @@ -803,6 +803,10 @@ public function get_fraud_meta_box_type_for_order( $order ) : string { * @throws Order_Not_Found_Exception */ public function attach_intent_info_to_order( WC_Order $order, $intent ) { + // We don't want to allow metadata for a successful payment to be disrupted. + if ( Intent_Status::SUCCEEDED === $this->get_intention_status_for_order( $order ) ) { + return; + } // first, let's prepare all the metadata needed for refunds, required for status change etc. $intent_id = $intent->get_id(); $intent_status = $intent->get_status(); diff --git a/tests/unit/test-class-wc-payments-order-service.php b/tests/unit/test-class-wc-payments-order-service.php index 8f24d7dbdf0..f5ac340757e 100644 --- a/tests/unit/test-class-wc-payments-order-service.php +++ b/tests/unit/test-class-wc-payments-order-service.php @@ -1219,6 +1219,26 @@ public function test_attach_intent_info_to_order() { $this->assertEquals( $intent_id, $this->order->get_meta( '_intent_id', true ) ); } + public function test_attach_intent_info_to_order_after_successful_payment() { + $intent = WC_Helper_Intention::create_intention( + [ + 'id' => 'pi_mock', + 'status' => Intent_Status::SUCCEEDED, + ] + ); + $this->order_service->attach_intent_info_to_order( $this->order, $intent ); + + $another_intent = WC_Helper_Intention::create_intention( + [ + 'id' => 'pi_mock_2', + 'status' => Intent_Status::CANCELED, + ] + ); + $this->order_service->attach_intent_info_to_order( $this->order, $another_intent ); + + $this->assertEquals( Intent_Status::SUCCEEDED, $this->order->get_meta( '_intention_status', true ) ); + } + /** * Several methods use the private method get_order to get the order being worked on. If an order is not found * then an exception is thrown. This test attempt to confirm that exception gets thrown. From 2af5d524fa391dbb84e2e59ccabcd861f4840663 Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Tue, 16 Jan 2024 15:34:43 +0530 Subject: [PATCH 13/15] Add test for the new setter method --- tests/unit/test-class-wc-payments-order-service.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/unit/test-class-wc-payments-order-service.php b/tests/unit/test-class-wc-payments-order-service.php index f5ac340757e..b1d1f030ff0 100644 --- a/tests/unit/test-class-wc-payments-order-service.php +++ b/tests/unit/test-class-wc-payments-order-service.php @@ -1211,6 +1211,12 @@ public function test_get_fraud_meta_box_type() { $this->assertEquals( $fraud_meta_box_type_from_service, $fraud_meta_box_type ); } + public function test_set_payment_transaction_id_for_order() { + $transaction_id = 'txn_mock'; + $this->order_service->set_payment_transaction_id_for_order( $this->order, $transaction_id ); + $this->assertSame( $this->order->get_meta( '_wcpay_payment_transaction_id', true ), $transaction_id ); + } + public function test_attach_intent_info_to_order() { $intent_id = 'pi_mock'; $intent = WC_Helper_Intention::create_intention( [ 'id' => $intent_id ] ); From 8022a4a1d27192f29ce57d5f2b34de1f845291ab Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Tue, 16 Jan 2024 15:50:31 +0530 Subject: [PATCH 14/15] Save payment transaction id in the new order service --- src/Internal/Service/OrderService.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Internal/Service/OrderService.php b/src/Internal/Service/OrderService.php index 7d301b27250..55886ce5ca8 100644 --- a/src/Internal/Service/OrderService.php +++ b/src/Internal/Service/OrderService.php @@ -189,8 +189,10 @@ public function update_order_from_successful_intent( $charge = null; $charge_id = null; if ( $intent instanceof WC_Payments_API_Payment_Intention ) { - $charge = $intent->get_charge(); - $charge_id = $intent->get_charge()->get_id(); + $charge = $intent->get_charge(); + $charge_id = $intent->get_charge()->get_id(); + $payment_transaction = $charge ? $charge->get_balance_transaction() : null; + $payment_transaction_id = $payment_transaction['id'] ?? ''; } $this->legacy_service->attach_intent_info_to_order__legacy( @@ -200,7 +202,8 @@ public function update_order_from_successful_intent( $context->get_payment_method()->get_id(), $context->get_customer_id(), $charge_id, - $context->get_currency() + $context->get_currency(), + $payment_transaction_id, ); $this->legacy_service->attach_transaction_fee_to_order( $order, $charge ); From edd6872674bdd636bc4400d9705fbbe50a8b173d Mon Sep 17 00:00:00 2001 From: Anurag Bhandari Date: Tue, 16 Jan 2024 15:56:57 +0530 Subject: [PATCH 15/15] Fix the undefined variable error --- src/Internal/Service/OrderService.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Internal/Service/OrderService.php b/src/Internal/Service/OrderService.php index 55886ce5ca8..57265fcecc9 100644 --- a/src/Internal/Service/OrderService.php +++ b/src/Internal/Service/OrderService.php @@ -186,8 +186,9 @@ public function update_order_from_successful_intent( ) { $order = $this->get_order( $order_id ); - $charge = null; - $charge_id = null; + $charge = null; + $charge_id = null; + $payment_transaction_id = null; if ( $intent instanceof WC_Payments_API_Payment_Intention ) { $charge = $intent->get_charge(); $charge_id = $intent->get_charge()->get_id();