From b324b2cef5c4ada1c6e8f4292e891c5c7a08bf00 Mon Sep 17 00:00:00 2001 From: Dat Hoang Date: Thu, 12 Dec 2024 17:33:33 +0700 Subject: [PATCH 1/5] Try adding seller_message to failed order notes --- includes/class-wc-payment-gateway-wcpay.php | 1 + ...wc-payment-gateway-wcpay-subscriptions.php | 7 +++-- includes/exceptions/class-api-exception.php | 30 +++++++++++++++---- .../class-wc-payments-api-client.php | 7 ++++- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index 84d30c82821..277cb825a40 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -1294,6 +1294,7 @@ public function process_payment( $order_id ) { ); $error_details = esc_html( rtrim( $e->getMessage(), '.' ) ); + $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); if ( $e instanceof API_Exception && 'card_error' === $e->get_error_type() ) { // If the payment failed with a 'card_error' API exception, initialize the fraud meta box diff --git a/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php b/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php index 31ec70bedf8..8ecca3bccfb 100644 --- a/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php +++ b/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php @@ -342,12 +342,15 @@ public function scheduled_subscription_payment( $amount, $renewal_order ) { $renewal_order->update_status( 'failed' ); if ( ! empty( $payment_information ) ) { + $error_details = esc_html( rtrim( $e->getMessage(), '.' ) ); + $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); + $note = sprintf( WC_Payments_Utils::esc_interpolated_html( /* translators: %1: the failed payment amount, %2: error message */ __( 'A payment of %1$s failed to complete with the following message: %2$s.', - 'woocommerce-payments' + 'woocommerce-payments' // @todo ), [ 'strong' => '', @@ -358,7 +361,7 @@ public function scheduled_subscription_payment( $amount, $renewal_order ) { wc_price( $amount, [ 'currency' => WC_Payments_Utils::get_order_intent_currency( $renewal_order ) ] ), $renewal_order ), - esc_html( rtrim( $e->getMessage(), '.' ) ) + $error_details ); $renewal_order->add_order_note( $note ); } diff --git a/includes/exceptions/class-api-exception.php b/includes/exceptions/class-api-exception.php index e13764ddcc5..95bc1862c0b 100644 --- a/includes/exceptions/class-api-exception.php +++ b/includes/exceptions/class-api-exception.php @@ -23,17 +23,24 @@ class API_Exception extends Base_Exception { /** * Error type attribute from the server. * - * @var string + * @var string|null */ private $error_type = null; /** * Decline code if it is a card error. * - * @var string + * @var string|null */ private $decline_code = null; + /** + * Merchant message. This message should not be shown to shoppers. + * + * @var string|null + */ + private $merchant_message = null; + /** * Constructor * @@ -42,13 +49,15 @@ class API_Exception extends Base_Exception { * @param int $http_code HTTP response code. * @param string $error_type Error type attribute. * @param string $decline_code The decline code if it is a card error. + * @param string $merchant_message The merchant message. This message should not be shown to shoppers. * @param int $code The Exception code. * @param \Throwable $previous The previous exception used for the exception chaining. */ - public function __construct( $message, $error_code, $http_code, $error_type = null, $decline_code = null, $code = 0, $previous = null ) { - $this->http_code = $http_code; - $this->error_type = $error_type; - $this->decline_code = $decline_code; + public function __construct( $message, $error_code, $http_code, $error_type = null, $decline_code = null, $merchant_message = null, $code = 0, $previous = null ) { + $this->http_code = $http_code; + $this->error_type = $error_type; + $this->decline_code = $decline_code; + $this->merchant_message = $merchant_message; parent::__construct( $message, $error_code, $code, $previous ); } @@ -79,4 +88,13 @@ public function get_error_type() { public function get_decline_code() { return $this->decline_code; } + + /** + * Returns the merchant message. + * + * @return string|null Merchant message. + */ + public function get_merchant_message(): ?string { + return $this->merchant_message; + } } diff --git a/includes/wc-payment-api/class-wc-payments-api-client.php b/includes/wc-payment-api/class-wc-payments-api-client.php index 13b25e07dd6..c59ed711992 100644 --- a/includes/wc-payment-api/class-wc-payments-api-client.php +++ b/includes/wc-payment-api/class-wc-payments-api-client.php @@ -2358,8 +2358,13 @@ protected function check_response_for_errors( $response ) { $error_message ); + $merchant_message = null; + if ( 'card_declined' === $error_code && isset( $response_body['error']['payment_intent']['charges']['data'][0]['outcome']['seller_message'] ) ) { + $merchant_message = $response_body['error']['payment_intent']['charges']['data'][0]['outcome']['seller_message']; + } + Logger::error( "$error_message ($error_code)" ); - throw new API_Exception( $message, $error_code, $response_code, $error_type, $decline_code ); + throw new API_Exception( $message, $error_code, $response_code, $error_type, $decline_code, $merchant_message ); } } From b0760655dc75c690237cbb42fb750f41e58934f0 Mon Sep 17 00:00:00 2001 From: Dat Hoang Date: Wed, 18 Dec 2024 15:17:06 +0700 Subject: [PATCH 2/5] Simplify $error_details and merchant_message --- includes/class-wc-payment-gateway-wcpay.php | 4 +++- .../trait-wc-payment-gateway-wcpay-subscriptions.php | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index d10003580e6..239dbc8cf29 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -1270,7 +1270,9 @@ public function process_payment( $order_id ) { ); $error_details = esc_html( rtrim( $e->getMessage(), '.' ) ); - $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); + if ( null !== $e->get_merchant_message() ) { + $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); + } if ( $e instanceof API_Exception && 'card_error' === $e->get_error_type() ) { // If the payment failed with a 'card_error' API exception, initialize the fraud meta box diff --git a/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php b/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php index 8ecca3bccfb..cdf68e1333c 100644 --- a/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php +++ b/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php @@ -343,14 +343,16 @@ public function scheduled_subscription_payment( $amount, $renewal_order ) { if ( ! empty( $payment_information ) ) { $error_details = esc_html( rtrim( $e->getMessage(), '.' ) ); - $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); + if ( null !== $e->get_merchant_message() ) { + $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); + } $note = sprintf( WC_Payments_Utils::esc_interpolated_html( /* translators: %1: the failed payment amount, %2: error message */ __( 'A payment of %1$s failed to complete with the following message: %2$s.', - 'woocommerce-payments' // @todo + 'woocommerce-payments' ), [ 'strong' => '', From d7cb63f140a41a94811fdfcbb6b8c7533804587e Mon Sep 17 00:00:00 2001 From: Dat Hoang Date: Wed, 18 Dec 2024 15:17:56 +0700 Subject: [PATCH 3/5] Add log and Fix errors detected by Psalm --- changelog/feat-9810-add-seller-message | 4 ++++ includes/class-wc-payment-gateway-wcpay.php | 2 +- includes/exceptions/class-amount-too-small-exception.php | 2 +- .../exceptions/class-cannot-combine-currencies-exception.php | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 changelog/feat-9810-add-seller-message diff --git a/changelog/feat-9810-add-seller-message b/changelog/feat-9810-add-seller-message new file mode 100644 index 00000000000..2669c24015b --- /dev/null +++ b/changelog/feat-9810-add-seller-message @@ -0,0 +1,4 @@ +Significance: minor +Type: add + +Add seller_message to failed order notes diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index 239dbc8cf29..46a071ed962 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -1270,7 +1270,7 @@ public function process_payment( $order_id ) { ); $error_details = esc_html( rtrim( $e->getMessage(), '.' ) ); - if ( null !== $e->get_merchant_message() ) { + if ( $e instanceof API_Exception && null !== $e->get_merchant_message() ) { $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); } diff --git a/includes/exceptions/class-amount-too-small-exception.php b/includes/exceptions/class-amount-too-small-exception.php index c7e51cd6f36..1ee4b034f62 100644 --- a/includes/exceptions/class-amount-too-small-exception.php +++ b/includes/exceptions/class-amount-too-small-exception.php @@ -41,7 +41,7 @@ public function __construct( $message, $minimum_amount, $currency, $http_code, $ $this->amount = $minimum_amount; $this->currency = $currency; - parent::__construct( $message, 'amount_too_small', $http_code, null, null, $code, $previous ); + parent::__construct( $message, 'amount_too_small', $http_code, null, null, null, $code, $previous ); } /** diff --git a/includes/exceptions/class-cannot-combine-currencies-exception.php b/includes/exceptions/class-cannot-combine-currencies-exception.php index 84fcc12797f..78a9a6af506 100644 --- a/includes/exceptions/class-cannot-combine-currencies-exception.php +++ b/includes/exceptions/class-cannot-combine-currencies-exception.php @@ -32,7 +32,7 @@ class Cannot_Combine_Currencies_Exception extends API_Exception { public function __construct( $message, $currency, $http_code, $code = 0, $previous = null ) { $this->currency = $currency; - parent::__construct( $message, 'cannot_combine_currencies', $http_code, null, null, $code, $previous ); + parent::__construct( $message, 'cannot_combine_currencies', $http_code, null, null, null, $code, $previous ); } /** From d5b199989b60281a4d2715c2f3a65c19edd194bd Mon Sep 17 00:00:00 2001 From: Dat Hoang Date: Thu, 19 Dec 2024 11:12:31 +0700 Subject: [PATCH 4/5] Introduce API_Merchant_Exception rather than add more prop to API_Exception --- includes/class-wc-payment-gateway-wcpay.php | 16 +++++- includes/class-wc-payments.php | 1 + ...wc-payment-gateway-wcpay-subscriptions.php | 3 +- .../class-amount-too-small-exception.php | 2 +- includes/exceptions/class-api-exception.php | 30 +++--------- .../class-api-merchant-exception.php | 49 +++++++++++++++++++ ...ss-cannot-combine-currencies-exception.php | 2 +- .../class-wc-payments-api-client.php | 9 ++-- 8 files changed, 80 insertions(+), 32 deletions(-) create mode 100644 includes/exceptions/class-api-merchant-exception.php diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index d304c99f83f..d1be21241b9 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -20,7 +20,19 @@ use WCPay\Constants\Intent_Status; use WCPay\Constants\Payment_Type; use WCPay\Constants\Payment_Method; -use WCPay\Exceptions\{ Add_Payment_Method_Exception, Amount_Too_Small_Exception, Process_Payment_Exception, Intent_Authentication_Exception, API_Exception, Invalid_Address_Exception, Fraud_Prevention_Enabled_Exception, Invalid_Phone_Number_Exception, Rate_Limiter_Enabled_Exception, Order_ID_Mismatch_Exception, Order_Not_Found_Exception, New_Process_Payment_Exception }; +use WCPay\Exceptions\{Add_Payment_Method_Exception, + Amount_Too_Small_Exception, + API_Merchant_Exception, + Process_Payment_Exception, + Intent_Authentication_Exception, + API_Exception, + Invalid_Address_Exception, + Fraud_Prevention_Enabled_Exception, + Invalid_Phone_Number_Exception, + Rate_Limiter_Enabled_Exception, + Order_ID_Mismatch_Exception, + Order_Not_Found_Exception, + New_Process_Payment_Exception}; use WCPay\Core\Server\Request\Cancel_Intention; use WCPay\Core\Server\Request\Capture_Intention; use WCPay\Core\Server\Request\Create_And_Confirm_Intention; @@ -1270,7 +1282,7 @@ public function process_payment( $order_id ) { ); $error_details = esc_html( rtrim( $e->getMessage(), '.' ) ); - if ( $e instanceof API_Exception && null !== $e->get_merchant_message() ) { + if ( $e instanceof API_Merchant_Exception ) { $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); } diff --git a/includes/class-wc-payments.php b/includes/class-wc-payments.php index 17300478794..fff5b9bb591 100644 --- a/includes/class-wc-payments.php +++ b/includes/class-wc-payments.php @@ -354,6 +354,7 @@ public static function init() { include_once __DIR__ . '/exceptions/class-base-exception.php'; include_once __DIR__ . '/exceptions/class-api-exception.php'; + include_once __DIR__ . '/exceptions/class-api-merchant-exception.php'; include_once __DIR__ . '/exceptions/class-connection-exception.php'; include_once __DIR__ . '/core/class-mode.php'; diff --git a/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php b/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php index cdf68e1333c..d2584f9b824 100644 --- a/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php +++ b/includes/compat/subscriptions/trait-wc-payment-gateway-wcpay-subscriptions.php @@ -11,6 +11,7 @@ use WCPay\Core\Server\Request\Get_Intention; use WCPay\Exceptions\API_Exception; +use WCPay\Exceptions\API_Merchant_Exception; use WCPay\Exceptions\Invalid_Payment_Method_Exception; use WCPay\Exceptions\Add_Payment_Method_Exception; use WCPay\Exceptions\Order_Not_Found_Exception; @@ -343,7 +344,7 @@ public function scheduled_subscription_payment( $amount, $renewal_order ) { if ( ! empty( $payment_information ) ) { $error_details = esc_html( rtrim( $e->getMessage(), '.' ) ); - if ( null !== $e->get_merchant_message() ) { + if ( $e instanceof API_Merchant_Exception ) { $error_details = $error_details . '. ' . esc_html( rtrim( $e->get_merchant_message(), '.' ) ); } diff --git a/includes/exceptions/class-amount-too-small-exception.php b/includes/exceptions/class-amount-too-small-exception.php index 1ee4b034f62..c7e51cd6f36 100644 --- a/includes/exceptions/class-amount-too-small-exception.php +++ b/includes/exceptions/class-amount-too-small-exception.php @@ -41,7 +41,7 @@ public function __construct( $message, $minimum_amount, $currency, $http_code, $ $this->amount = $minimum_amount; $this->currency = $currency; - parent::__construct( $message, 'amount_too_small', $http_code, null, null, null, $code, $previous ); + parent::__construct( $message, 'amount_too_small', $http_code, null, null, $code, $previous ); } /** diff --git a/includes/exceptions/class-api-exception.php b/includes/exceptions/class-api-exception.php index 95bc1862c0b..e13764ddcc5 100644 --- a/includes/exceptions/class-api-exception.php +++ b/includes/exceptions/class-api-exception.php @@ -23,24 +23,17 @@ class API_Exception extends Base_Exception { /** * Error type attribute from the server. * - * @var string|null + * @var string */ private $error_type = null; /** * Decline code if it is a card error. * - * @var string|null + * @var string */ private $decline_code = null; - /** - * Merchant message. This message should not be shown to shoppers. - * - * @var string|null - */ - private $merchant_message = null; - /** * Constructor * @@ -49,15 +42,13 @@ class API_Exception extends Base_Exception { * @param int $http_code HTTP response code. * @param string $error_type Error type attribute. * @param string $decline_code The decline code if it is a card error. - * @param string $merchant_message The merchant message. This message should not be shown to shoppers. * @param int $code The Exception code. * @param \Throwable $previous The previous exception used for the exception chaining. */ - public function __construct( $message, $error_code, $http_code, $error_type = null, $decline_code = null, $merchant_message = null, $code = 0, $previous = null ) { - $this->http_code = $http_code; - $this->error_type = $error_type; - $this->decline_code = $decline_code; - $this->merchant_message = $merchant_message; + public function __construct( $message, $error_code, $http_code, $error_type = null, $decline_code = null, $code = 0, $previous = null ) { + $this->http_code = $http_code; + $this->error_type = $error_type; + $this->decline_code = $decline_code; parent::__construct( $message, $error_code, $code, $previous ); } @@ -88,13 +79,4 @@ public function get_error_type() { public function get_decline_code() { return $this->decline_code; } - - /** - * Returns the merchant message. - * - * @return string|null Merchant message. - */ - public function get_merchant_message(): ?string { - return $this->merchant_message; - } } diff --git a/includes/exceptions/class-api-merchant-exception.php b/includes/exceptions/class-api-merchant-exception.php new file mode 100644 index 00000000000..ac10bd271bc --- /dev/null +++ b/includes/exceptions/class-api-merchant-exception.php @@ -0,0 +1,49 @@ +merchant_message = $merchant_message; + + parent::__construct( $message, $error_code, $http_code, $error_type, $decline_code, $code, $previous ); + } + + /** + * Returns the merchant message. + * + * @return string Merchant message. + */ + public function get_merchant_message(): string { + return $this->merchant_message; + } +} diff --git a/includes/exceptions/class-cannot-combine-currencies-exception.php b/includes/exceptions/class-cannot-combine-currencies-exception.php index 78a9a6af506..84fcc12797f 100644 --- a/includes/exceptions/class-cannot-combine-currencies-exception.php +++ b/includes/exceptions/class-cannot-combine-currencies-exception.php @@ -32,7 +32,7 @@ class Cannot_Combine_Currencies_Exception extends API_Exception { public function __construct( $message, $currency, $http_code, $code = 0, $previous = null ) { $this->currency = $currency; - parent::__construct( $message, 'cannot_combine_currencies', $http_code, null, null, null, $code, $previous ); + parent::__construct( $message, 'cannot_combine_currencies', $http_code, null, null, $code, $previous ); } /** diff --git a/includes/wc-payment-api/class-wc-payments-api-client.php b/includes/wc-payment-api/class-wc-payments-api-client.php index 7c2a32b22e0..e90094d57de 100644 --- a/includes/wc-payment-api/class-wc-payments-api-client.php +++ b/includes/wc-payment-api/class-wc-payments-api-client.php @@ -9,6 +9,7 @@ use WCPay\Constants\Intent_Status; use WCPay\Exceptions\API_Exception; +use WCPay\Exceptions\API_Merchant_Exception; use WCPay\Exceptions\Amount_Too_Small_Exception; use WCPay\Exceptions\Amount_Too_Large_Exception; use WCPay\Exceptions\Connection_Exception; @@ -2418,13 +2419,15 @@ protected function check_response_for_errors( $response ) { $error_message ); - $merchant_message = null; + Logger::error( "$error_message ($error_code)" ); + if ( 'card_declined' === $error_code && isset( $response_body['error']['payment_intent']['charges']['data'][0]['outcome']['seller_message'] ) ) { $merchant_message = $response_body['error']['payment_intent']['charges']['data'][0]['outcome']['seller_message']; + + throw new API_Merchant_Exception( $message, $error_code, $response_code, $merchant_message, $error_type, $decline_code ); } - Logger::error( "$error_message ($error_code)" ); - throw new API_Exception( $message, $error_code, $response_code, $error_type, $decline_code, $merchant_message ); + throw new API_Exception( $message, $error_code, $response_code, $error_type, $decline_code ); } } From ea96b63ec377ded68c8eaf334edadf68177133f6 Mon Sep 17 00:00:00 2001 From: Dat Hoang Date: Thu, 19 Dec 2024 13:37:57 +0700 Subject: [PATCH 5/5] Add tests --- .../test-class-wc-payments-api-client.php | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/unit/wc-payment-api/test-class-wc-payments-api-client.php b/tests/unit/wc-payment-api/test-class-wc-payments-api-client.php index 3fc4a56c8f6..fb95bcf1591 100644 --- a/tests/unit/wc-payment-api/test-class-wc-payments-api-client.php +++ b/tests/unit/wc-payment-api/test-class-wc-payments-api-client.php @@ -7,7 +7,9 @@ use WCPay\Constants\Country_Code; use WCPay\Constants\Intent_Status; +use WCPay\Core\Server\Request\Create_And_Confirm_Intention; use WCPay\Exceptions\API_Exception; +use WCPay\Exceptions\API_Merchant_Exception; use WCPay\Internal\Logger; use WCPay\Exceptions\Connection_Exception; use WCPay\Fraud_Prevention\Fraud_Prevention_Service; @@ -1195,6 +1197,24 @@ public function test_get_tracking_info() { $this->assertEquals( $expect, $result ); } + public function test_throws_api_merchant_exception() { + $mock_response = []; + $mock_response['error']['code'] = 'card_declined'; + $mock_response['error']['payment_intent']['charges']['data'][0]['outcome']['seller_message'] = 'Bank declined'; + $this->set_http_mock_response( + 401, + $mock_response + ); + + try { + // This is a dummy call to trigger the response so that our test can validate the exception. + $this->payments_api_client->create_subscription(); + } catch ( API_Merchant_Exception $e ) { + $this->assertSame( 'card_declined', $e->get_error_code() ); + $this->assertSame( 'Bank declined', $e->get_merchant_message() ); + } + } + /** * Set up http mock response. *