Remove the redundant network requests in capture_charge (#6408)

Co-authored-by: Anurag Bhandari <[email protected]>
Automattic · Sep 28, 2023 · 32e8bd0 · 32e8bd0
1 parent 07dc42b
commit 32e8bd0
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 116 deletions.
diff --git a/changelog/fix-optimise-capture-charge-calls b/changelog/fix-optimise-capture-charge-calls
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Speed up capturing terminal and authorized payments.
diff --git a/includes/admin/class-wc-rest-payments-orders-controller.php b/includes/admin/class-wc-rest-payments-orders-controller.php
@@ -212,7 +212,7 @@ public function capture_terminal_payment( WP_REST_Request $request ) {
 				'id'     => $intent->get_id(),
 			];
 
-			$result = $is_intent_captured ? $result_for_captured_intent : $this->gateway->capture_charge( $order, false );
+			$result = $is_intent_captured ? $result_for_captured_intent : $this->gateway->capture_charge( $order, false, $intent_metadata );
 
 			if ( Intent_Status::SUCCEEDED !== $result['status'] ) {
 				$http_code = $result['http_code'] ?? 502;
@@ -287,7 +287,7 @@ public function capture_authorization( WP_REST_Request $request ) {
 
 			$this->add_fraud_outcome_manual_entry( $order, 'approve' );
 
-			$result = $this->gateway->capture_charge( $order, false );
+			$result = $this->gateway->capture_charge( $order, false, $intent_metadata );
 
 			if ( Intent_Status::SUCCEEDED !== $result['status'] ) {
 				return new WP_Error(

diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php
@@ -2661,10 +2661,11 @@ public function add_order_actions( $actions ) {
 	 *
 	 * @param WC_Order $order - Order to capture charge on.
 	 * @param bool     $include_level3 - Whether to include level 3 data in payment intent.
+	 * @param array    $intent_metadata - Intent metadata retrieved earlier in the calling method.
 	 *
 	 * @return array An array containing the status (succeeded/failed), id (intent ID), message (error message if any), and http code
 	 */
-	public function capture_charge( $order, $include_level3 = true ) {
+	public function capture_charge( $order, $include_level3 = true, $intent_metadata = [] ) {
 		$amount                   = $order->get_total();
 		$is_authorization_expired = false;
 		$intent                   = null;
@@ -2673,23 +2674,14 @@ public function capture_charge( $order, $include_level3 = true ) {
 		$http_code                = null;
 
 		try {
-			$intent_id = $order->get_transaction_id();
-
-			$request = Get_Intention::create( $intent_id );
-			$intent  = $request->send( 'wcpay_get_intent_request', $order );
-
-			$payment_type = $this->is_payment_recurring( $order->get_id() ) ? Payment_Type::RECURRING() : Payment_Type::SINGLE();
-
-			$metadata_from_intent = $intent->get_metadata(); // mobile app may have set metadata.
-			$metadata_from_order  = $this->get_metadata_from_order( $order, $payment_type );
-			$merged_metadata      = array_merge( (array) $metadata_from_order, (array) $metadata_from_intent ); // prioritize metadata from mobile app.
-
-			$wcpay_request = Update_Intention::create( $intent_id );
-			$wcpay_request->set_metadata( $merged_metadata );
-			$wcpay_request->send( 'wcpay_prepare_intention_for_capture', $order );
+			$intent_id           = $order->get_transaction_id();
+			$payment_type        = $this->is_payment_recurring( $order->get_id() ) ? Payment_Type::RECURRING() : Payment_Type::SINGLE();
+			$metadata_from_order = $this->get_metadata_from_order( $order, $payment_type );
+			$merged_metadata     = array_merge( (array) $metadata_from_order, (array) $intent_metadata ); // prioritize metadata from mobile app.
 
 			$capture_intention_request = Capture_Intention::create( $intent_id );
 			$capture_intention_request->set_amount_to_capture( WC_Payments_Utils::prepare_amount( $amount, $order->get_currency() ) );
+			$capture_intention_request->set_metadata( $merged_metadata );
 			if ( $include_level3 ) {
 				$capture_intention_request->set_level3( $this->get_level3_data_from_order( $order ) );
 			}
@@ -2702,6 +2694,7 @@ public function capture_charge( $order, $include_level3 = true ) {
 				$error_message = $e->getMessage();
 				$http_code     = $e->get_http_code();
 
+				$request = Get_Intention::create( $intent_id );
 				// Fetch the Intent to check if it's already expired and the site missed the "charge.expired" webhook.
 				$intent = $request->send( 'wcpay_get_intent_request', $order );
 

diff --git a/includes/core/server/request/class-capture-intention.php b/includes/core/server/request/class-capture-intention.php
@@ -76,6 +76,15 @@ public function set_level3( $level3 ) {
 		$this->set_param( 'level3', $this->fix_level3_data( $level3 ) );
 	}
 
+	/**
+	 * Setter for intent metadata.
+	 *
+	 * @param array $metadata Intent metadata that includes stuff like order details, card reader specifics, etc..
+	 */
+	public function set_metadata( array $metadata ): void {
+		$this->set_param( 'metadata', $metadata );
+	}
+
 	/**
 	 * Formats the response from the server.
 	 *

diff --git a/tests/unit/test-class-wc-payment-gateway-wcpay.php b/tests/unit/test-class-wc-payment-gateway-wcpay.php
@@ -851,27 +851,13 @@ public function test_capture_charge_success() {
 
 		$mock_intent = WC_Helper_Intention::create_intention( [ 'status' => Intent_Status::REQUIRES_CAPTURE ] );
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
-
-		$request->expects( $this->once() )
-			->method( 'format_response' )
-			->willReturn( $mock_intent );
-
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' )
-			->with(
-				$this->callback(
-					function( $argument ) {
-						return is_array( $argument ) && ! empty( $argument );
-					}
-				)
-			);
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $this->anything() );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->willReturn( WC_Helper_Intention::create_intention() );
@@ -925,25 +911,13 @@ public function test_capture_charge_success_non_usd() {
 			]
 		);
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
-
-		$request->expects( $this->once() )
-			->method( 'format_response' )
-			->willReturn( $mock_intent );
-
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' );
-
-		$update_intent_request->expects( $this->once() )
-			->method( 'format_response' )
-			->willReturn( $mock_intent );
-
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $this->anything() );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->willReturn( WC_Helper_Intention::create_intention( [ 'currency' => 'eur' ] ) );
@@ -994,20 +968,13 @@ public function test_capture_charge_failure() {
 
 		$mock_intent = WC_Helper_Intention::create_intention( [ 'status' => Intent_Status::REQUIRES_CAPTURE ] );
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
-
-		$request->expects( $this->once() )
-			->method( 'format_response' )
-			->willReturn( $mock_intent );
-
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' );
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $this->anything() );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->willReturn( $mock_intent );
@@ -1061,20 +1028,13 @@ public function test_capture_charge_failure_non_usd() {
 			]
 		);
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
-
-		$request->expects( $this->once() )
-			->method( 'format_response' )
-			->willReturn( $mock_intent );
-
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' );
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $this->anything() );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->willReturn( $mock_intent );
@@ -1124,20 +1084,19 @@ public function test_capture_charge_api_failure() {
 
 		$mock_intent = WC_Helper_Intention::create_intention( [ 'status' => Intent_Status::REQUIRES_CAPTURE ] );
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 2, $intent_id );
+		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
 
-		$request->expects( $this->exactly( 2 ) )
+		$request->expects( $this->exactly( 1 ) )
 			->method( 'format_response' )
 			->willReturn( $mock_intent );
 
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' );
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $this->anything() );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->will( $this->throwException( new API_Exception( 'test exception', 'server_error', 500 ) ) );
@@ -1192,25 +1151,19 @@ public function test_capture_charge_api_failure_non_usd() {
 			]
 		);
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 2, $intent_id );
-
-		$request->expects( $this->exactly( 2 ) )
-			->method( 'format_response' )
-			->willReturn( $mock_intent );
-
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' );
+		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
 
-		$update_intent_request->expects( $this->once() )
+		$request->expects( $this->exactly( 1 ) )
 			->method( 'format_response' )
 			->willReturn( $mock_intent );
 
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $this->anything() );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->will( $this->throwException( new API_Exception( 'test exception', 'server_error', 500 ) ) );
@@ -1261,20 +1214,19 @@ public function test_capture_charge_expired() {
 
 		$mock_intent = WC_Helper_Intention::create_intention( [ 'status' => Intent_Status::CANCELED ] );
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 2, $intent_id );
+		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
 
-		$request->expects( $this->exactly( 2 ) )
+		$request->expects( $this->exactly( 1 ) )
 			->method( 'format_response' )
 			->willReturn( $mock_intent );
 
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' );
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $this->anything() );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->will( $this->throwException( new API_Exception( 'test exception', 'server_error', 500 ) ) );
@@ -1326,12 +1278,14 @@ public function test_capture_charge_metadata() {
 				'status'   => Intent_Status::REQUIRES_CAPTURE,
 				'metadata' => [
 					'customer_name' => 'Test',
+					'reader_ID'     => 'wisepad',
 				],
 			]
 		);
 
 		$merged_metadata = [
 			'customer_name'        => 'Test',
+			'reader_ID'            => 'wisepad',
 			'customer_email'       => $order->get_billing_email(),
 			'site_url'             => esc_url( get_site_url() ),
 			'order_id'             => $order->get_id(),
@@ -1344,22 +1298,13 @@ public function test_capture_charge_metadata() {
 			'subscription_payment' => 'no',
 		];
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
-
-		$request->expects( $this->once() )
-			->method( 'format_response' )
-			->willReturn( $mock_intent );
-
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' )
-			->with( $merged_metadata );
-
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $merged_metadata );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->willReturn( WC_Helper_Intention::create_intention() );
@@ -1369,7 +1314,7 @@ public function test_capture_charge_metadata() {
 			->method( 'get_account_country' )
 			->willReturn( 'US' );
 
-		$result = $this->wcpay_gateway->capture_charge( $order );
+		$result = $this->wcpay_gateway->capture_charge( $order, true, $merged_metadata );
 
 		$note = wc_get_order_notes(
 			[
@@ -1407,21 +1352,13 @@ public function test_capture_charge_without_level3() {
 
 		$mock_intent = WC_Helper_Intention::create_intention( [ 'status' => Intent_Status::REQUIRES_CAPTURE ] );
 
-		$request = $this->mock_wcpay_request( Get_Intention::class, 1, $intent_id );
-
-		$request->expects( $this->once() )
-			->method( 'format_response' )
-			->willReturn( $mock_intent );
-
-		$update_intent_request = $this->mock_wcpay_request( Update_Intention::class, 1, $intent_id );
-		$update_intent_request->expects( $this->once() )
-			->method( 'set_metadata' );
-
 		$capture_intent_request = $this->mock_wcpay_request( Capture_Intention::class, 1, $intent_id );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'set_amount_to_capture' )
 			->with( $mock_intent->get_amount() );
-
+		$capture_intent_request->expects( $this->once() )
+			->method( 'set_metadata' )
+			->with( $this->anything() );
 		$capture_intent_request->expects( $this->once() )
 			->method( 'format_response' )
 			->willReturn( WC_Helper_Intention::create_intention() );