Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the redundant network requests in capture_charge #6408

Merged
merged 12 commits into from
Sep 28, 2023
Merged

Remove the redundant network requests in capture_charge #6408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2c9b83c
Remove extra get intent call on capture
mordeth May 31, 2023
747b35a
Update unit tests
mordeth May 31, 2023
179f5d9
Rename the intent metadata param for readability
anu-rock Sep 14, 2023
4958980
Update the merged metadata during capture
anu-rock Sep 14, 2023
2fb4c9b
Merge branch 'develop' into fix/optimise-capture-charge-calls
anu-rock Sep 14, 2023
bfcbce8
Fix phpcs issue
anu-rock Sep 14, 2023
33b3955
Add changelog
anu-rock Sep 14, 2023
d7a8d3b
Add typehints
anu-rock Sep 21, 2023
bc2f2a2
Fix references to payment type constants
anu-rock Sep 21, 2023
53119d8
Merge branch 'develop' into fix/optimise-capture-charge-calls
anu-rock Sep 22, 2023
19184ee
Fix psalm error about Payment_Type
anu-rock Sep 27, 2023
77ca53d
Merge branch 'develop' into fix/optimise-capture-charge-calls
anu-rock Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/fix-optimise-capture-charge-calls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fix

Speed up capturing terminal and authorized payments.
4 changes: 2 additions & 2 deletions includes/admin/class-wc-rest-payments-orders-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
23 changes: 8 additions & 15 deletions includes/class-wc-payment-gateway-wcpay.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
anu-rock marked this conversation as resolved.
Show resolved Hide resolved
$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 ) );
}
Expand All @@ -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 );

Expand Down
9 changes: 9 additions & 0 deletions includes/core/server/request/class-capture-intention.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
135 changes: 36 additions & 99 deletions tests/unit/test-class-wc-payment-gateway-wcpay.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() );
Expand Down Expand Up @@ -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' ] ) );
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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 ) ) );
Expand Down Expand Up @@ -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 ) ) );
Expand Down Expand Up @@ -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 ) ) );
Expand Down Expand Up @@ -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(),
Expand All @@ -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() );
Expand All @@ -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(
[
Expand Down Expand Up @@ -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() );
Expand Down
Loading