From 13c5eb78de0ee6b56c4eba179141f280c3c67732 Mon Sep 17 00:00:00 2001 From: Timur Karimov Date: Thu, 20 Jun 2024 17:01:57 +0200 Subject: [PATCH 1/6] Ensure only relevant tokens are retrieved for the gateway --- includes/class-wc-payments-token-service.php | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/includes/class-wc-payments-token-service.php b/includes/class-wc-payments-token-service.php index 6a8f9e21d0a..144e29f9e51 100644 --- a/includes/class-wc-payments-token-service.php +++ b/includes/class-wc-payments-token-service.php @@ -163,14 +163,18 @@ public function woocommerce_get_customer_payment_tokens( $tokens, $user_id, $gat } } - $retrievable_payment_method_types = [ Payment_Method::CARD ]; - - if ( in_array( Payment_Method::SEPA, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { - $retrievable_payment_method_types[] = Payment_Method::SEPA; - } - - if ( in_array( Payment_Method::LINK, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { - $retrievable_payment_method_types[] = Payment_Method::LINK; + $retrievable_payment_method_types = []; + + foreach ( self::REUSABLE_GATEWAYS_BY_PAYMENT_METHOD as $payment_method => $gateway ) { + if ( $gateway === $gateway_id ) { + if ( Payment_Method::LINK === $payment_method ) { + if ( in_array( Payment_Method::LINK, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { + $retrievable_payment_method_types[] = Payment_Method::LINK; + } + } else { + $retrievable_payment_method_types[] = $payment_method; + } + } } $payment_methods = []; From a5aa44b1ab6d94e9c36013d6909072644d4d454a Mon Sep 17 00:00:00 2001 From: Timur Karimov Date: Fri, 21 Jun 2024 13:59:04 +0200 Subject: [PATCH 2/6] Handle empty gateway & fix unit tests --- includes/class-wc-payments-token-service.php | 25 ++++-- .../test-class-wc-payments-token-service.php | 89 ++++++------------- 2 files changed, 46 insertions(+), 68 deletions(-) diff --git a/includes/class-wc-payments-token-service.php b/includes/class-wc-payments-token-service.php index 144e29f9e51..0951c3f2f5a 100644 --- a/includes/class-wc-payments-token-service.php +++ b/includes/class-wc-payments-token-service.php @@ -164,15 +164,26 @@ public function woocommerce_get_customer_payment_tokens( $tokens, $user_id, $gat } $retrievable_payment_method_types = []; + if ( empty( $gateway_id ) ) { + $retrievable_payment_method_types[] = Payment_Method::CARD; - foreach ( self::REUSABLE_GATEWAYS_BY_PAYMENT_METHOD as $payment_method => $gateway ) { - if ( $gateway === $gateway_id ) { - if ( Payment_Method::LINK === $payment_method ) { - if ( in_array( Payment_Method::LINK, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { - $retrievable_payment_method_types[] = Payment_Method::LINK; + if ( in_array( Payment_Method::SEPA, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { + $retrievable_payment_method_types[] = Payment_Method::SEPA; + } + + if ( in_array( Payment_Method::LINK, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { + $retrievable_payment_method_types[] = Payment_Method::LINK; + } + } else { + foreach ( self::REUSABLE_GATEWAYS_BY_PAYMENT_METHOD as $payment_method => $gateway ) { + if ( $gateway === $gateway_id ) { + if ( Payment_Method::LINK === $payment_method ) { + if ( in_array( Payment_Method::LINK, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { + $retrievable_payment_method_types[] = Payment_Method::LINK; + } + } else { + $retrievable_payment_method_types[] = $payment_method; } - } else { - $retrievable_payment_method_types[] = $payment_method; } } } diff --git a/tests/unit/test-class-wc-payments-token-service.php b/tests/unit/test-class-wc-payments-token-service.php index f9465769f89..4835cf32b09 100644 --- a/tests/unit/test-class-wc-payments-token-service.php +++ b/tests/unit/test-class-wc-payments-token-service.php @@ -489,17 +489,23 @@ public function test_woocommerce_get_customer_payment_tokens_multiple_tokens_mul $gateway->settings['upe_enabled_payment_method_ids'] = $payment_methods; // Array keys should match the database ID of the token. - $tokens = [ + $card_tokens = [ 1 => $this->generate_card_token( 'pm_111', 1 ), 2 => $this->generate_card_token( 'pm_222', 2 ), + ]; + $sepa_tokens = [ 3 => $this->generate_sepa_token( 'pm_333', 3 ), 4 => $this->generate_sepa_token( 'pm_444', 4 ), + ]; + $stripe_link_tokens = [ 5 => $this->generate_link_token( 'pm_555', 5 ), 6 => $this->generate_link_token( 'pm_666', 6 ), ]; + $all_saved_tokens = $card_tokens + $sepa_tokens + $stripe_link_tokens; + $this->mock_customer_service - ->expects( $this->once() ) + ->expects( $this->exactly( 2 ) ) ->method( 'get_customer_id_by_user_id' ) ->willReturn( $customer_id ); @@ -509,7 +515,6 @@ public function test_woocommerce_get_customer_payment_tokens_multiple_tokens_mul ->method( 'get_payment_methods_for_customer' ) ->withConsecutive( [ $customer_id, Payment_Method::CARD ], - [ $customer_id, Payment_Method::SEPA ], [ $customer_id, Payment_Method::LINK ] ) ->willReturnOnConsecutiveCalls( @@ -517,20 +522,27 @@ public function test_woocommerce_get_customer_payment_tokens_multiple_tokens_mul $this->generate_card_pm_response( 'pm_111' ), $this->generate_card_pm_response( 'pm_222' ), ], - [ - $this->generate_sepa_pm_response( 'pm_333' ), - $this->generate_sepa_pm_response( 'pm_444' ), - ], [ $this->generate_link_pm_response( 'pm_555' ), $this->generate_link_pm_response( 'pm_666' ), + ], + [ + $this->generate_sepa_pm_response( 'pm_333' ), + $this->generate_sepa_pm_response( 'pm_444' ), ] ); - $result = $this->token_service->woocommerce_get_customer_payment_tokens( $tokens, 1, 'woocommerce_payments' ); + $card_and_link_result = $this->token_service->woocommerce_get_customer_payment_tokens( $all_saved_tokens, 1, WC_Payment_Gateway_WCPay::GATEWAY_ID ); + $sepa_result = $this->token_service->woocommerce_get_customer_payment_tokens( $all_saved_tokens, 1, WC_Payment_Gateway_WCPay::GATEWAY_ID . '_' . Payment_Method::SEPA ); + $this->assertSame( - array_keys( $tokens ), - array_keys( $result ) + array_keys( $card_tokens + $stripe_link_tokens ), + array_keys( $card_and_link_result ) + ); + + $this->assertSame( + array_keys( $sepa_tokens ), + array_keys( $sepa_result ) ); } @@ -656,51 +668,6 @@ public function test_woocommerce_get_customer_payment_tokens_not_added_twice_for $this->assertEquals( 'pm_444', $result_tokens[3]->get_token() ); } - public function test_woocommerce_get_customer_payment_tokens_not_added_from_different_gateway() { - $this->mock_cache->method( 'get' )->willReturn( [ 'is_deferred_intent_creation_upe_enabled' => true ] ); - $gateway_id = WC_Payment_Gateway_WCPay::GATEWAY_ID; - $tokens = []; - $payment_methods = [ Payment_Method::CARD, Payment_Method::SEPA ]; - - $gateway = WC_Payments::get_gateway(); - $gateway->settings['upe_enabled_payment_method_ids'] = $payment_methods; - - $this->mock_customer_service - ->expects( $this->any() ) - ->method( 'get_customer_id_by_user_id' ) - ->willReturn( 'cus_12345' ); - - $this->mock_customer_service - ->expects( $this->exactly( 2 ) ) - ->method( 'get_payment_methods_for_customer' ) - ->withConsecutive( - [ 'cus_12345', Payment_Method::CARD ], - [ 'cus_12345', Payment_Method::SEPA ] - ) - ->willReturnOnConsecutiveCalls( - [ - $this->generate_card_pm_response( 'pm_mock0' ), - $this->generate_card_pm_response( 'pm_222' ), - ], - [ - $this->generate_sepa_pm_response( 'other_gateway_pm_111' ), - $this->generate_sepa_pm_response( 'other_gateway_pm_222' ), - $this->generate_sepa_pm_response( 'other_gateway_pm_333' ), - $this->generate_sepa_pm_response( 'other_gateway_pm_444' ), - $this->generate_sepa_pm_response( 'other_gateway_pm_555' ), - ] - ); - - $result = $this->token_service->woocommerce_get_customer_payment_tokens( $tokens, 1, $gateway_id ); - $result_tokens = array_values( $result ); - - $this->assertEquals( 2, count( $result_tokens ) ); - $this->assertEquals( $gateway_id, $result_tokens[0]->get_gateway_id() ); - $this->assertEquals( $gateway_id, $result_tokens[1]->get_gateway_id() ); - $this->assertEquals( 'pm_mock0', $result_tokens[0]->get_token() ); - $this->assertEquals( 'pm_222', $result_tokens[1]->get_token() ); - } - public function test_woocommerce_get_customer_payment_tokens_payment_methods_only_for_retrievable_types() { $enabled_upe_payment_methods = [ Payment_Method::CARD, @@ -716,7 +683,6 @@ public function test_woocommerce_get_customer_payment_tokens_payment_methods_onl $gateway = WC_Payments::get_gateway(); $gateway->settings['upe_enabled_payment_method_ids'] = $enabled_upe_payment_methods; $tokens = []; - $gateway_id = 'woocommerce_payments'; $customer_id = 'cus_12345'; $this->mock_customer_service @@ -729,22 +695,23 @@ public function test_woocommerce_get_customer_payment_tokens_payment_methods_onl ->method( 'get_payment_methods_for_customer' ) ->withConsecutive( [ $customer_id, Payment_Method::CARD ], + [ $customer_id, Payment_Method::LINK ], [ $customer_id, Payment_Method::SEPA ], - [ $customer_id, Payment_Method::LINK ] ) ->willReturnOnConsecutiveCalls( [ $this->generate_card_pm_response( 'pm_mock0' ), ], [ - $this->generate_sepa_pm_response( 'pm_mock_2' ), + $this->generate_link_pm_response( 'pm_mock_3' ), ], [ - $this->generate_link_pm_response( 'pm_mock_3' ), - ] + $this->generate_sepa_pm_response( 'pm_mock_2' ), + ], ); - $this->token_service->woocommerce_get_customer_payment_tokens( $tokens, 1, $gateway_id ); + $this->token_service->woocommerce_get_customer_payment_tokens( $tokens, 1, WC_Payment_Gateway_WCPay::GATEWAY_ID ); + $this->token_service->woocommerce_get_customer_payment_tokens( $tokens, 1, WC_Payment_Gateway_WCPay::GATEWAY_ID . '_' . Payment_Method::SEPA ); } /** From c99fd159b3ad679bb367697425527be98360118d Mon Sep 17 00:00:00 2001 From: Timur Karimov Date: Fri, 21 Jun 2024 14:45:42 +0200 Subject: [PATCH 3/6] refactor the code --- includes/class-wc-payments-token-service.php | 95 +++++++++++++++----- 1 file changed, 71 insertions(+), 24 deletions(-) diff --git a/includes/class-wc-payments-token-service.php b/includes/class-wc-payments-token-service.php index 0951c3f2f5a..46cbe3b5f65 100644 --- a/includes/class-wc-payments-token-service.php +++ b/includes/class-wc-payments-token-service.php @@ -163,30 +163,7 @@ public function woocommerce_get_customer_payment_tokens( $tokens, $user_id, $gat } } - $retrievable_payment_method_types = []; - if ( empty( $gateway_id ) ) { - $retrievable_payment_method_types[] = Payment_Method::CARD; - - if ( in_array( Payment_Method::SEPA, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { - $retrievable_payment_method_types[] = Payment_Method::SEPA; - } - - if ( in_array( Payment_Method::LINK, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { - $retrievable_payment_method_types[] = Payment_Method::LINK; - } - } else { - foreach ( self::REUSABLE_GATEWAYS_BY_PAYMENT_METHOD as $payment_method => $gateway ) { - if ( $gateway === $gateway_id ) { - if ( Payment_Method::LINK === $payment_method ) { - if ( in_array( Payment_Method::LINK, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ) ) { - $retrievable_payment_method_types[] = Payment_Method::LINK; - } - } else { - $retrievable_payment_method_types[] = $payment_method; - } - } - } - } + $retrievable_payment_method_types = $this->get_retrievable_payment_method_types( $gateway_id ); $payment_methods = []; @@ -228,6 +205,76 @@ public function woocommerce_get_customer_payment_tokens( $tokens, $user_id, $gat return $tokens; } + /** + * Retrieves the payment method types for which tokens should be retrieved. + * + * This function determines the appropriate payment method types based on the provided gateway ID. + * - If a gateway ID is provided, it retrieves the payment methods specific to that gateway to prevent duplication of saved tokens under incorrect payment methods during checkout. + * - If no gateway ID is provided, it retrieves the default payment methods to fetch all saved tokens, e.g., for the My Account page. + * + * @param string|null $gateway_id The optional ID of the gateway. + * @return array The list of retrievable payment method types. + */ + private function get_retrievable_payment_method_types( $gateway_id = null ) { + if ( empty( $gateway_id ) ) { + return $this->get_all_retrievable_payment_methods(); + } else { + return $this->get_gateway_specific_retrievable_payment_methods( $gateway_id ); + } + } + + /** + * Returns all the enabled retrievable payment method types. + * + * @return array Enabled retrievable payment method types. + */ + private function get_all_retrievable_payment_methods() { + $methods = [ Payment_Method::CARD ]; + + if ( $this->is_payment_method_enabled( Payment_Method::SEPA ) ) { + $methods[] = Payment_Method::SEPA; + } + + if ( $this->is_payment_method_enabled( Payment_Method::LINK ) ) { + $methods[] = Payment_Method::LINK; + } + + return $methods; + } + /** + * Returns retrievable payment method types for a given gateway. + * + * @param string $gateway_id The ID of the gateway. + * @return array Retrievable payment method types for the specified gateway. + */ + private function get_gateway_specific_retrievable_payment_methods( $gateway_id ) { + $methods = []; + + foreach ( self::REUSABLE_GATEWAYS_BY_PAYMENT_METHOD as $payment_method => $gateway ) { + if ( $gateway === $gateway_id ) { + if ( Payment_Method::LINK ) { + if ( $this->is_payment_method_enabled( Payment_Method::LINK ) ) { + $methods[] = Payment_Method::LINK; + } + } else { + $methods[] = $payment_method; + } + } + } + + return $methods; + } + + /** + * Checks if a payment method is enabled. + * + * @param string $payment_method The payment method to check. + * @return bool True if the payment method is enabled, false otherwise. + */ + private function is_payment_method_enabled( $payment_method ) { + return in_array( $payment_method, WC_Payments::get_gateway()->get_upe_enabled_payment_method_ids(), true ); + } + /** * Delete token from Stripe. * From bd57bc7d4f7b183ce233604314c351b4937d9d9b Mon Sep 17 00:00:00 2001 From: Timur Karimov Date: Fri, 21 Jun 2024 14:46:40 +0200 Subject: [PATCH 4/6] add changelog entry --- changelog/fix-token-retrieval-per-payment-method | 4 ++++ includes/class-wc-payments-token-service.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changelog/fix-token-retrieval-per-payment-method diff --git a/changelog/fix-token-retrieval-per-payment-method b/changelog/fix-token-retrieval-per-payment-method new file mode 100644 index 00000000000..1ec9a65c246 --- /dev/null +++ b/changelog/fix-token-retrieval-per-payment-method @@ -0,0 +1,4 @@ +Significance: minor +Type: fix + +Retrieve saved tokens only relevant for the specific payment gateway. diff --git a/includes/class-wc-payments-token-service.php b/includes/class-wc-payments-token-service.php index 46cbe3b5f65..ecd4670e266 100644 --- a/includes/class-wc-payments-token-service.php +++ b/includes/class-wc-payments-token-service.php @@ -252,7 +252,7 @@ private function get_gateway_specific_retrievable_payment_methods( $gateway_id ) foreach ( self::REUSABLE_GATEWAYS_BY_PAYMENT_METHOD as $payment_method => $gateway ) { if ( $gateway === $gateway_id ) { - if ( Payment_Method::LINK ) { + if ( Payment_Method::LINK === $payment_method ) { if ( $this->is_payment_method_enabled( Payment_Method::LINK ) ) { $methods[] = Payment_Method::LINK; } From c3a000e5ecc3395bb2394356a18f8a6ace0ba14c Mon Sep 17 00:00:00 2001 From: Timur Karimov Date: Fri, 21 Jun 2024 15:27:50 +0200 Subject: [PATCH 5/6] improve readability --- includes/class-wc-payments-token-service.php | 27 ++++++++++---------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/includes/class-wc-payments-token-service.php b/includes/class-wc-payments-token-service.php index ecd4670e266..a23a30d053f 100644 --- a/includes/class-wc-payments-token-service.php +++ b/includes/class-wc-payments-token-service.php @@ -210,16 +210,16 @@ public function woocommerce_get_customer_payment_tokens( $tokens, $user_id, $gat * * This function determines the appropriate payment method types based on the provided gateway ID. * - If a gateway ID is provided, it retrieves the payment methods specific to that gateway to prevent duplication of saved tokens under incorrect payment methods during checkout. - * - If no gateway ID is provided, it retrieves the default payment methods to fetch all saved tokens, e.g., for the My Account page. + * - If no gateway ID is provided, it retrieves the default payment methods to fetch all saved tokens, e.g., for the Blocks checkout or My Account page. * * @param string|null $gateway_id The optional ID of the gateway. * @return array The list of retrievable payment method types. */ private function get_retrievable_payment_method_types( $gateway_id = null ) { if ( empty( $gateway_id ) ) { - return $this->get_all_retrievable_payment_methods(); + return $this->get_all_retrievable_payment_types(); } else { - return $this->get_gateway_specific_retrievable_payment_methods( $gateway_id ); + return $this->get_gateway_specific_retrievable_payment_types( $gateway_id ); } } @@ -228,18 +228,18 @@ private function get_retrievable_payment_method_types( $gateway_id = null ) { * * @return array Enabled retrievable payment method types. */ - private function get_all_retrievable_payment_methods() { - $methods = [ Payment_Method::CARD ]; + private function get_all_retrievable_payment_types() { + $types = [ Payment_Method::CARD ]; if ( $this->is_payment_method_enabled( Payment_Method::SEPA ) ) { - $methods[] = Payment_Method::SEPA; + $types[] = Payment_Method::SEPA; } if ( $this->is_payment_method_enabled( Payment_Method::LINK ) ) { - $methods[] = Payment_Method::LINK; + $types[] = Payment_Method::LINK; } - return $methods; + return $types; } /** * Returns retrievable payment method types for a given gateway. @@ -247,22 +247,23 @@ private function get_all_retrievable_payment_methods() { * @param string $gateway_id The ID of the gateway. * @return array Retrievable payment method types for the specified gateway. */ - private function get_gateway_specific_retrievable_payment_methods( $gateway_id ) { - $methods = []; + private function get_gateway_specific_retrievable_payment_types( $gateway_id ) { + $types = []; foreach ( self::REUSABLE_GATEWAYS_BY_PAYMENT_METHOD as $payment_method => $gateway ) { if ( $gateway === $gateway_id ) { + // Stripe Link is part of the card gateway, so we need to check separately if Link is enabled. if ( Payment_Method::LINK === $payment_method ) { if ( $this->is_payment_method_enabled( Payment_Method::LINK ) ) { - $methods[] = Payment_Method::LINK; + $types[] = Payment_Method::LINK; } } else { - $methods[] = $payment_method; + $types[] = $payment_method; } } } - return $methods; + return $types; } /** From ece0e83715a7e40cdc4d58b2bec198ba2e626d32 Mon Sep 17 00:00:00 2001 From: Timur Karimov Date: Tue, 25 Jun 2024 09:47:28 +0200 Subject: [PATCH 6/6] simplify branching logic --- includes/class-wc-payments-token-service.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/includes/class-wc-payments-token-service.php b/includes/class-wc-payments-token-service.php index a23a30d053f..0eb810bce75 100644 --- a/includes/class-wc-payments-token-service.php +++ b/includes/class-wc-payments-token-service.php @@ -251,16 +251,16 @@ private function get_gateway_specific_retrievable_payment_types( $gateway_id ) { $types = []; foreach ( self::REUSABLE_GATEWAYS_BY_PAYMENT_METHOD as $payment_method => $gateway ) { - if ( $gateway === $gateway_id ) { - // Stripe Link is part of the card gateway, so we need to check separately if Link is enabled. - if ( Payment_Method::LINK === $payment_method ) { - if ( $this->is_payment_method_enabled( Payment_Method::LINK ) ) { - $types[] = Payment_Method::LINK; - } - } else { - $types[] = $payment_method; - } + if ( $gateway !== $gateway_id ) { + continue; + } + + // Stripe Link is part of the card gateway, so we need to check separately if Link is enabled. + if ( Payment_Method::LINK === $payment_method && ! $this->is_payment_method_enabled( Payment_Method::LINK ) ) { + continue; } + + $types[] = $payment_method; } return $types;