Skip to content

Commit 1882aa1

Browse files
Update payment method type check for charge.succeeded (#4084)
* Update payment method type check for charge.succeeded * Add changelog and readme entries * Fix unit tests * Define sync methods array for better readability Co-authored-by: Malith Senaweera <[email protected]> * Include event ID in message log for missing charge object. Co-authored-by: Malith Senaweera <[email protected]> --------- Co-authored-by: Malith Senaweera <[email protected]>
1 parent ddfac13 commit 1882aa1

File tree

4 files changed

+109
-10
lines changed

4 files changed

+109
-10
lines changed

changelog.txt

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* Update - Update payment method type for Amazon Pay orders.
2020
* Fix - Compatibility with email preview in the Auth Requested email
2121
* Update - Update Alipay and bank debit icons.
22+
* Tweak - Update payment method type check for charge.succeeded webhook.
2223

2324
= 9.3.1 - 2025-03-14 =
2425
* Fix - Temporarily disables the subscriptions detached notice feature due to long loading times on stores with many subscriptions.

includes/class-wc-stripe-webhook-handler.php

+46-10
Original file line numberDiff line numberDiff line change
@@ -525,15 +525,30 @@ public function process_webhook_capture( $notification ) {
525525
* @param object $notification
526526
*/
527527
public function process_webhook_charge_succeeded( $notification ) {
528-
// The following payment methods are synchronous so does not need to be handle via webhook.
529-
if ( ( isset( $notification->data->object->source->type ) && WC_Stripe_Payment_Methods::CARD === $notification->data->object->source->type ) || ( isset( $notification->data->object->source->type ) && 'three_d_secure' === $notification->data->object->source->type ) ) {
528+
if ( empty( $notification->data->object ) ) {
529+
WC_Stripe_Logger::log( 'Missing charge object in charge.succeeded webhook, Event ID: %s', $notification->id ?? 'unknown' );
530530
return;
531531
}
532532

533-
$order = WC_Stripe_Helper::get_order_by_charge_id( $notification->data->object->id );
533+
// https://docs.stripe.com/api/events/types#event_types-charge.succeeded
534+
$charge = $notification->data->object;
535+
536+
// The following payment methods are synchronous so does not need to be handled via webhook.
537+
$payment_method_type = $this->get_payment_method_type_from_charge( $charge );
538+
$synchronous_methods = [
539+
WC_Stripe_Payment_Methods::CARD,
540+
WC_Stripe_Payment_Methods::AMAZON_PAY,
541+
'three_d_secure',
542+
];
543+
544+
if ( in_array( $payment_method_type, $synchronous_methods, true ) ) {
545+
return;
546+
}
547+
548+
$order = WC_Stripe_Helper::get_order_by_charge_id( $charge->id );
534549

535550
if ( ! $order ) {
536-
WC_Stripe_Logger::log( 'Could not find order via charge ID: ' . $notification->data->object->id );
551+
WC_Stripe_Logger::log( 'Could not find order via charge ID: ' . $charge->id );
537552
return;
538553
}
539554

@@ -545,15 +560,15 @@ public function process_webhook_charge_succeeded( $notification ) {
545560
// setting is enabled, Stripe API still sends a "charge.succeeded" webhook but
546561
// the payment has not been captured, yet. This ensures that the payment has been
547562
// captured, before completing the payment.
548-
if ( ! $notification->data->object->captured ) {
563+
if ( ! $charge->captured ) {
549564
return;
550565
}
551566

552567
// Store other data such as fees
553-
$order->set_transaction_id( $notification->data->object->id );
568+
$order->set_transaction_id( $charge->id );
554569

555-
if ( isset( $notification->data->object->balance_transaction ) ) {
556-
$this->update_fees( $order, $notification->data->object->balance_transaction );
570+
if ( isset( $charge->balance_transaction ) ) {
571+
$this->update_fees( $order, $charge->balance_transaction );
557572
}
558573

559574
/**
@@ -568,10 +583,10 @@ public function process_webhook_charge_succeeded( $notification ) {
568583
* to ensure the review.closed event handler will update the status to the proper status.
569584
*/
570585
if ( 'manual_review' !== $this->get_risk_outcome( $notification ) ) {
571-
$order->payment_complete( $notification->data->object->id );
586+
$order->payment_complete( $charge->id );
572587

573588
/* translators: transaction id */
574-
$order->add_order_note( sprintf( __( 'Stripe charge complete (Charge ID: %s)', 'woocommerce-gateway-stripe' ), $notification->data->object->id ) );
589+
$order->add_order_note( sprintf( __( 'Stripe charge complete (Charge ID: %s)', 'woocommerce-gateway-stripe' ), $charge->id ) );
575590
}
576591

577592
if ( is_callable( [ $order, 'save' ] ) ) {
@@ -1358,6 +1373,27 @@ private function get_order_from_intent( $intent ) {
13581373
// Fall back to finding the order via the intent ID.
13591374
return WC_Stripe_Helper::get_order_by_intent_id( $intent->id );
13601375
}
1376+
1377+
/**
1378+
* Get the payment method type from the charge object.
1379+
* https://docs.stripe.com/api/charges/object
1380+
*
1381+
* @param object $charge The charge object from Stripe
1382+
* @return string|null The payment method type, or null if not found
1383+
*/
1384+
private function get_payment_method_type_from_charge( $charge ) {
1385+
// We don't expect $charge->source to be set,
1386+
// but we keep it here to ensure backwards compatibility.
1387+
if ( isset( $charge->source->type ) ) {
1388+
return $charge->source->type;
1389+
}
1390+
1391+
if ( isset( $charge->payment_method_details->type ) ) {
1392+
return $charge->payment_method_details->type;
1393+
}
1394+
1395+
return null;
1396+
}
13611397
}
13621398

13631399
new WC_Stripe_Webhook_Handler();

readme.txt

+1
Original file line numberDiff line numberDiff line change
@@ -129,5 +129,6 @@ If you get stuck, you can ask for help in the [Plugin Forum](https://wordpress.o
129129
* Update - Update payment method type for Amazon Pay orders.
130130
* Fix - Compatibility with email preview in the Auth Requested email
131131
* Update - Update Alipay and bank debit icons.
132+
* Tweak - Update payment method type check for charge.succeeded webhook.
132133

133134
[See changelog for all versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt).

tests/phpunit/test-wc-stripe-webhook-handler.php

+61
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ private function mock_webhook_handler( $exclude_methods = [] ) {
5454
'get_intent_from_order',
5555
'get_latest_charge_from_intent',
5656
'process_response',
57+
'update_fees',
5758
];
5859

5960
$methods = array_diff( $methods, $exclude_methods );
@@ -624,4 +625,64 @@ public function provide_test_process_payment_intent() {
624625
],
625626
];
626627
}
628+
629+
/**
630+
* Test for `process_webhook_charge_succeeded`, that it is skipped for synchronous payment methods.
631+
*
632+
* @param string $payment_method_type The payment method type.
633+
* @return void
634+
* @dataProvider provide_test_process_webhook_charge_succeeded_skipped_for_synchronous_payment_methods
635+
*/
636+
public function test_process_webhook_charge_succeeded_skipped_for_synchronous_payment_methods( $payment_method_type ) {
637+
$charge_id = 'ch_mock9G5K2X1Q';
638+
$notification = (object) [
639+
'type' => 'charge.succeeded',
640+
'data' => (object) [
641+
'object' => (object) [
642+
'id' => $charge_id,
643+
'payment_method_details' => (object) [
644+
'type' => $payment_method_type,
645+
],
646+
'captured' => true,
647+
'balance_transaction' => (object) [
648+
'fee' => 100,
649+
],
650+
],
651+
],
652+
];
653+
654+
// We want to assert an early return by checking that we don't run the next line, i.e.
655+
// retrieving the order by charge ID. However, we are using WC_Stripe_Helper::get_order_by_charge_id()
656+
// which is a static method, and phpunit does not natively support mocking static methods.
657+
658+
// We will instead create the mock order for the charge ID, so we are able to retrieve an order,
659+
// and make sure the next few checks pass so that it reaches the line that calls update_fees()
660+
// which we can mock and check if it was called.
661+
$order = WC_Helper_Order::create_order();
662+
$order->set_status( 'on-hold' );
663+
$order->set_transaction_id( $charge_id );
664+
$order->save();
665+
666+
if ( WC_Stripe_Payment_Methods::SEPA_DEBIT === $payment_method_type ) {
667+
$this->mock_webhook_handler->expects( $this->once() )->method( 'update_fees' );
668+
} else {
669+
$this->mock_webhook_handler->expects( $this->never() )->method( 'update_fees' );
670+
}
671+
672+
$this->mock_webhook_handler->process_webhook_charge_succeeded( $notification );
673+
}
674+
675+
/**
676+
* Provider for `test_process_webhook_charge_succeeded_skipped_for_synchronous_payment_methods`.
677+
*
678+
* @return array
679+
*/
680+
public function provide_test_process_webhook_charge_succeeded_skipped_for_synchronous_payment_methods() {
681+
return [
682+
'card' => [ WC_Stripe_Payment_Methods::CARD ],
683+
'amazon_pay' => [ WC_Stripe_Payment_Methods::AMAZON_PAY ],
684+
'three_d_secure' => [ 'three_d_secure' ],
685+
'sepa_debit' => [ WC_Stripe_Payment_Methods::SEPA_DEBIT ],
686+
];
687+
}
627688
}

0 commit comments

Comments
 (0)