From ed6d60969994b31582bc9049ccc30fffee2c9734 Mon Sep 17 00:00:00 2001 From: Daniel Guerra Date: Wed, 10 Jul 2024 11:43:30 -0600 Subject: [PATCH 1/7] Allow Afterpay gateway to process payments when the state/county is optional for GB and NZ addresses --- ...empty-state-when-optional-afterpay-gateway | 4 + includes/class-wc-payment-gateway-wcpay.php | 7 +- tests/WCPAY_UnitTestCase.php | 5 +- .../test-class-wc-payment-gateway-wcpay.php | 98 +++++++++++++++++++ 4 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 changelog/fix-9046-allow-empty-state-when-optional-afterpay-gateway diff --git a/changelog/fix-9046-allow-empty-state-when-optional-afterpay-gateway b/changelog/fix-9046-allow-empty-state-when-optional-afterpay-gateway new file mode 100644 index 00000000000..34b8d13c9d4 --- /dev/null +++ b/changelog/fix-9046-allow-empty-state-when-optional-afterpay-gateway @@ -0,0 +1,4 @@ +Significance: minor +Type: fix + +Allow Afterpay gateway to process payments when the state/county is optional for GB and NZ addresses. diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index c5126c398e0..d41cc44f0fe 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -4496,7 +4496,12 @@ private function upe_needs_redirection( $payment_methods ) { */ private function handle_afterpay_shipping_requirement( WC_Order $order, Create_And_Confirm_Intention $request ): void { $check_if_usable = function ( array $address ): bool { - return $address['country'] && $address['state'] && $address['city'] && $address['postal_code'] && $address['line1']; + if ( in_array( $address['country'], [ 'GB', 'NZ' ], true ) ) { + $is_state_usable = true; + } else { + $is_state_usable = ! empty( $address['state'] ); + } + return $address['country'] && $is_state_usable && $address['city'] && $address['postal_code'] && $address['line1']; }; $shipping_data = $this->order_service->get_shipping_data_from_order( $order ); diff --git a/tests/WCPAY_UnitTestCase.php b/tests/WCPAY_UnitTestCase.php index 4470710b519..d7f38e3b5d0 100644 --- a/tests/WCPAY_UnitTestCase.php +++ b/tests/WCPAY_UnitTestCase.php @@ -114,14 +114,15 @@ public function createMock( string $original_class_name ): MockObject { // phpcs * @param mixed $response The expected response. * @param WC_Payments_API_Client $api_client_mock Specific API client mock if necessary. * @param WC_Payments_Http $http_mock Specific HTTP mock if necessary. + * @param bool $force_request_mock When true, a request will be mocked even if $total_api_calls is 0. * * @return Request|MockObject The mocked request. */ - protected function mock_wcpay_request( string $request_class, int $total_api_calls = 1, $request_class_constructor_id = null, $response = null, $api_client_mock = null, $http_mock = null ) { + protected function mock_wcpay_request( string $request_class, int $total_api_calls = 1, $request_class_constructor_id = null, $response = null, $api_client_mock = null, $http_mock = null, $force_request_mock = false ) { $http_mock = $http_mock ? $http_mock : $this->createMock( WC_Payments_Http::class ); $api_client_mock = $api_client_mock ? $api_client_mock : $this->createMock( WC_Payments_API_Client::class ); - if ( 1 > $total_api_calls ) { + if ( 1 > $total_api_calls && ! $force_request_mock ) { $api_client_mock->expects( $this->never() )->method( 'send_request' ); // No expectation for calls, return here. diff --git a/tests/unit/test-class-wc-payment-gateway-wcpay.php b/tests/unit/test-class-wc-payment-gateway-wcpay.php index 6879500a7b2..2c7be79941e 100644 --- a/tests/unit/test-class-wc-payment-gateway-wcpay.php +++ b/tests/unit/test-class-wc-payment-gateway-wcpay.php @@ -13,6 +13,7 @@ use WCPay\Core\Server\Request\Get_Charge; use WCPay\Core\Server\Request\Get_Intention; use WCPay\Core\Server\Request\Get_Setup_Intention; +use WCPay\Constants\Country_Code; use WCPay\Constants\Order_Status; use WCPay\Constants\Intent_Status; use WCPay\Constants\Payment_Method; @@ -21,6 +22,7 @@ use WCPay\Exceptions\Amount_Too_Small_Exception; use WCPay\Exceptions\API_Exception; use WCPay\Exceptions\Fraud_Prevention_Enabled_Exception; +use WCPay\Exceptions\Invalid_Address_Exception; use WCPay\Exceptions\Process_Payment_Exception; use WCPay\Exceptions\Order_ID_Mismatch_Exception; use WCPay\Fraud_Prevention\Fraud_Prevention_Service; @@ -2825,6 +2827,102 @@ public function test_process_payment_for_order_upe_payment_method() { $this->card_gateway->process_payment_for_order( WC()->cart, $pi ); } + /** + * @dataProvider process_payment_for_order_afterpay_clearpay_provider + */ + public function test_process_payment_for_order_afterpay_clearpay( array $address, ?string $expected_exception ) { + $payment_method = 'woocommerce_payments_afterpay_clearpay'; + $expected_upe_payment_method_for_pi_creation = 'afterpay_clearpay'; + $order = WC_Helper_Order::create_order(); + $order->set_currency( 'USD' ); + $order->set_total( 100 ); + $order->set_billing_city( $address['city'] ); + $order->set_billing_state( $address['state'] ); + $order->set_billing_postcode( $address['postcode'] ); + $order->set_billing_country( $address['country'] ); + $order->save(); + + $_POST['wcpay-fraud-prevention-token'] = 'correct-token'; + $_POST['payment_method'] = $payment_method; + $pi = new Payment_Information( 'pm_test', $order, null, null, null, null, null, '', 'afterpay_clearpay' ); + + if ( $expected_exception ) { + $this->mock_wcpay_request( Create_And_Confirm_Intention::class, 0, null, null, null, null, true ); + $this->expectException( $expected_exception ); + } else { + $request = $this->mock_wcpay_request( Create_And_Confirm_Intention::class ); + $request->expects( $this->once() ) + ->method( 'set_payment_methods' ) + ->with( [ $expected_upe_payment_method_for_pi_creation ] ); + $request->expects( $this->once() ) + ->method( 'format_response' ) + ->willReturn( WC_Helper_Intention::create_intention( [ 'status' => 'success' ] ) ); + } + + $afterpay_gateway = current( + array_filter( + $this->gateways, + function ( $gateway ) { + return $gateway->get_payment_method()->get_id() === 'afterpay_clearpay'; + } + ) + ); + + $afterpay_gateway->process_payment_for_order( WC()->cart, $pi ); + } + + public function process_payment_for_order_afterpay_clearpay_provider() { + return [ + 'with valid full address' => [ + 'address' => [ + 'city' => 'WooCity', + 'state' => 'NY', + 'postcode' => '12345', + 'country' => Country_Code::UNITED_STATES, + ], + 'expected_exception' => null, + ], + 'with incomplete address' => [ + 'address' => [ + 'city' => 'WooCity', + 'state' => '', + 'postcode' => '12345', + 'country' => Country_Code::UNITED_STATES, + ], + 'expected_exception' => Invalid_Address_Exception::class, + ], + 'without state, GB' => [ + 'address' => [ + 'city' => 'London', + 'state' => '', + 'postcode' => 'HA9 9LY', + 'country' => Country_Code::UNITED_KINGDOM, + ], + 'expected_exception' => null, + + ], + 'without city, GB' => [ + 'address' => [ + 'city' => '', + 'state' => 'London', + 'postcode' => 'HA9 9LY', + 'country' => Country_Code::UNITED_KINGDOM, + ], + 'expected_exception' => Invalid_Address_Exception::class, + + ], + 'without state, NZ' => [ + 'address' => [ + 'city' => 'Wellington', + 'state' => '', + 'postcode' => '6011', + 'country' => Country_Code::NEW_ZEALAND, + ], + 'expected_exception' => null, + ], + ]; + } + public function test_process_payment_caches_mimimum_amount_and_displays_error_upon_exception() { delete_transient( 'wcpay_minimum_amount_usd' ); From a7d59fe8cd82492137bb6c13f6cbee9a2a810a0b Mon Sep 17 00:00:00 2001 From: Daniel Guerra Date: Wed, 10 Jul 2024 12:00:12 -0600 Subject: [PATCH 2/7] Use Country_Code constants instead of hard coded strings --- includes/class-wc-payment-gateway-wcpay.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index d41cc44f0fe..e6a9aa7fa00 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -4496,7 +4496,7 @@ private function upe_needs_redirection( $payment_methods ) { */ private function handle_afterpay_shipping_requirement( WC_Order $order, Create_And_Confirm_Intention $request ): void { $check_if_usable = function ( array $address ): bool { - if ( in_array( $address['country'], [ 'GB', 'NZ' ], true ) ) { + if ( in_array( $address['country'], [ Country_Code::UNITED_KINGDOM, Country_Code::NEW_ZEALAND ], true ) ) { $is_state_usable = true; } else { $is_state_usable = ! empty( $address['state'] ); From e2456a1eace7cc3fda2aa5c87e97d59bfa9e91a4 Mon Sep 17 00:00:00 2001 From: Daniel Guerra Date: Wed, 10 Jul 2024 12:33:22 -0600 Subject: [PATCH 3/7] Make the logic work based on WC locale definition instead of using a fixed array of countries --- includes/class-wc-payment-gateway-wcpay.php | 23 ++++++++---- .../test-class-wc-payment-gateway-wcpay.php | 35 +++++++++++++------ 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index e6a9aa7fa00..7167eab3364 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -4495,13 +4495,24 @@ private function upe_needs_redirection( $payment_methods ) { * @return void */ private function handle_afterpay_shipping_requirement( WC_Order $order, Create_And_Confirm_Intention $request ): void { - $check_if_usable = function ( array $address ): bool { - if ( in_array( $address['country'], [ Country_Code::UNITED_KINGDOM, Country_Code::NEW_ZEALAND ], true ) ) { - $is_state_usable = true; - } else { - $is_state_usable = ! empty( $address['state'] ); + $wc_locale_data = WC()->countries->get_country_locale(); + + $check_if_usable = function ( array $address ) use ( $wc_locale_data ): bool { + if ( $address['country'] ) { + $country_locale_data = $wc_locale_data[ $address['country'] ] ?? null; + + $is_state_not_required = ( + is_array( $country_locale_data ) && + isset( $country_locale_data['state'] ) && + isset( $country_locale_data['state']['required'] ) && + false === $country_locale_data['state']['required'] + ); + + if ( $is_state_not_required ) { + return $address['country'] && $address['city'] && $address['postal_code'] && $address['line1']; + } } - return $address['country'] && $is_state_usable && $address['city'] && $address['postal_code'] && $address['line1']; + return $address['country'] && $address['state'] && $address['city'] && $address['postal_code'] && $address['line1']; }; $shipping_data = $this->order_service->get_shipping_data_from_order( $order ); diff --git a/tests/unit/test-class-wc-payment-gateway-wcpay.php b/tests/unit/test-class-wc-payment-gateway-wcpay.php index 2c7be79941e..b1fa32c24d5 100644 --- a/tests/unit/test-class-wc-payment-gateway-wcpay.php +++ b/tests/unit/test-class-wc-payment-gateway-wcpay.php @@ -190,6 +190,13 @@ class WC_Payment_Gateway_WCPay_Test extends WCPAY_UnitTestCase { */ private $mock_duplicates_detection_service; + /** + * Backup of WC locale data + * + * @var array + */ + private $locale_backup; + /** * Pre-test setup */ @@ -271,6 +278,8 @@ public function set_up() { ->method( 'get_payment_metadata' ) ->willReturn( [] ); wcpay_get_test_container()->replace( OrderService::class, $mock_order_service ); + + $this->locale_backup = WC()->countries->get_country_locale(); } /** @@ -307,6 +316,7 @@ public function tear_down() { wcpay_get_test_container()->reset_all_replacements(); WC()->session->set( 'wc_notices', [] ); + WC()->countries->locale = $this->locale_backup; } public function test_process_redirect_payment_intent_processing() { @@ -2830,7 +2840,7 @@ public function test_process_payment_for_order_upe_payment_method() { /** * @dataProvider process_payment_for_order_afterpay_clearpay_provider */ - public function test_process_payment_for_order_afterpay_clearpay( array $address, ?string $expected_exception ) { + public function test_process_payment_for_order_afterpay_clearpay( array $address, array $locale_data, ?string $expected_exception ) { $payment_method = 'woocommerce_payments_afterpay_clearpay'; $expected_upe_payment_method_for_pi_creation = 'afterpay_clearpay'; $order = WC_Helper_Order::create_order(); @@ -2859,6 +2869,8 @@ public function test_process_payment_for_order_afterpay_clearpay( array $address ->willReturn( WC_Helper_Intention::create_intention( [ 'status' => 'success' ] ) ); } + WC()->countries->locale = $locale_data; + $afterpay_gateway = current( array_filter( $this->gateways, @@ -2880,6 +2892,8 @@ public function process_payment_for_order_afterpay_clearpay_provider() { 'postcode' => '12345', 'country' => Country_Code::UNITED_STATES, ], + // An empty locale data means all fields should be required. + 'locale_data' => [], 'expected_exception' => null, ], 'with incomplete address' => [ @@ -2889,6 +2903,10 @@ public function process_payment_for_order_afterpay_clearpay_provider() { 'postcode' => '12345', 'country' => Country_Code::UNITED_STATES, ], + 'locale_data' => [ + // A missing `required` attribute means that the field will be required. + 'US' => [ 'state' => [ 'label' => 'State' ] ], + ], 'expected_exception' => Invalid_Address_Exception::class, ], 'without state, GB' => [ @@ -2898,6 +2916,9 @@ public function process_payment_for_order_afterpay_clearpay_provider() { 'postcode' => 'HA9 9LY', 'country' => Country_Code::UNITED_KINGDOM, ], + 'locale_data' => [ + 'GB' => [ 'state' => [ 'required' => false ] ], + ], 'expected_exception' => null, ], @@ -2908,18 +2929,12 @@ public function process_payment_for_order_afterpay_clearpay_provider() { 'postcode' => 'HA9 9LY', 'country' => Country_Code::UNITED_KINGDOM, ], + 'locale_data' => [ + 'GB' => [ 'state' => [ 'required' => false ] ], + ], 'expected_exception' => Invalid_Address_Exception::class, ], - 'without state, NZ' => [ - 'address' => [ - 'city' => 'Wellington', - 'state' => '', - 'postcode' => '6011', - 'country' => Country_Code::NEW_ZEALAND, - ], - 'expected_exception' => null, - ], ]; } From 9039375ff87d3b036e0eb9a034ed71eafcb949e6 Mon Sep 17 00:00:00 2001 From: Daniel Guerra Date: Wed, 10 Jul 2024 12:34:49 -0600 Subject: [PATCH 4/7] Use the constants when able --- tests/unit/test-class-wc-payment-gateway-wcpay.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/unit/test-class-wc-payment-gateway-wcpay.php b/tests/unit/test-class-wc-payment-gateway-wcpay.php index b1fa32c24d5..a27ea5b6669 100644 --- a/tests/unit/test-class-wc-payment-gateway-wcpay.php +++ b/tests/unit/test-class-wc-payment-gateway-wcpay.php @@ -2905,7 +2905,7 @@ public function process_payment_for_order_afterpay_clearpay_provider() { ], 'locale_data' => [ // A missing `required` attribute means that the field will be required. - 'US' => [ 'state' => [ 'label' => 'State' ] ], + Country_Code::UNITED_STATES => [ 'state' => [ 'label' => 'State' ] ], ], 'expected_exception' => Invalid_Address_Exception::class, ], @@ -2917,10 +2917,9 @@ public function process_payment_for_order_afterpay_clearpay_provider() { 'country' => Country_Code::UNITED_KINGDOM, ], 'locale_data' => [ - 'GB' => [ 'state' => [ 'required' => false ] ], + Country_Code::UNITED_KINGDOM => [ 'state' => [ 'required' => false ] ], ], 'expected_exception' => null, - ], 'without city, GB' => [ 'address' => [ @@ -2930,10 +2929,9 @@ public function process_payment_for_order_afterpay_clearpay_provider() { 'country' => Country_Code::UNITED_KINGDOM, ], 'locale_data' => [ - 'GB' => [ 'state' => [ 'required' => false ] ], + Country_Code::UNITED_KINGDOM => [ 'state' => [ 'required' => false ] ], ], 'expected_exception' => Invalid_Address_Exception::class, - ], ]; } From ffc1fdb75f717b7b283019471f9472ff0cb7e974 Mon Sep 17 00:00:00 2001 From: Daniel Guerra Date: Wed, 10 Jul 2024 13:05:32 -0600 Subject: [PATCH 5/7] Rename variable --- includes/class-wc-payment-gateway-wcpay.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index 7167eab3364..914f0b41ac9 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -4501,14 +4501,14 @@ private function handle_afterpay_shipping_requirement( WC_Order $order, Create_A if ( $address['country'] ) { $country_locale_data = $wc_locale_data[ $address['country'] ] ?? null; - $is_state_not_required = ( + $is_state_optional = ( is_array( $country_locale_data ) && isset( $country_locale_data['state'] ) && isset( $country_locale_data['state']['required'] ) && false === $country_locale_data['state']['required'] ); - if ( $is_state_not_required ) { + if ( $is_state_optional ) { return $address['country'] && $address['city'] && $address['postal_code'] && $address['line1']; } } From b1dc09994dec4fd2c29a562fd88103bee4ea5030 Mon Sep 17 00:00:00 2001 From: Daniel Guerra Date: Wed, 10 Jul 2024 13:26:12 -0600 Subject: [PATCH 6/7] Support other fields being optional --- includes/class-wc-payment-gateway-wcpay.php | 32 ++++++++++++------- .../test-class-wc-payment-gateway-wcpay.php | 23 ++++++++++--- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index 914f0b41ac9..18fd5a128e9 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -4498,20 +4498,30 @@ private function handle_afterpay_shipping_requirement( WC_Order $order, Create_A $wc_locale_data = WC()->countries->get_country_locale(); $check_if_usable = function ( array $address ) use ( $wc_locale_data ): bool { - if ( $address['country'] ) { - $country_locale_data = $wc_locale_data[ $address['country'] ] ?? null; - - $is_state_optional = ( - is_array( $country_locale_data ) && - isset( $country_locale_data['state'] ) && - isset( $country_locale_data['state']['required'] ) && - false === $country_locale_data['state']['required'] - ); + if ( $address['country'] && isset( $wc_locale_data[ $address['country'] ] ) ) { + $country_locale_data = $wc_locale_data[ $address['country'] ]; + $fields_to_check = [ + 'state' => 'state', + 'city' => 'city', + 'postcode' => 'postal_code', + 'address_1' => 'line1', + ]; + + foreach ( $fields_to_check as $locale_field => $address_field ) { + $is_field_required = ( + ! isset( $country_locale_data[ $locale_field ] ) || + ! isset( $country_locale_data[ $locale_field ]['required'] ) || + false !== $country_locale_data[ $locale_field ]['required'] + ); - if ( $is_state_optional ) { - return $address['country'] && $address['city'] && $address['postal_code'] && $address['line1']; + if ( $is_field_required && ! $address[ $address_field ] ) { + return false; + } } + + return true; } + return $address['country'] && $address['state'] && $address['city'] && $address['postal_code'] && $address['line1']; }; diff --git a/tests/unit/test-class-wc-payment-gateway-wcpay.php b/tests/unit/test-class-wc-payment-gateway-wcpay.php index a27ea5b6669..dbc0e81876b 100644 --- a/tests/unit/test-class-wc-payment-gateway-wcpay.php +++ b/tests/unit/test-class-wc-payment-gateway-wcpay.php @@ -2885,7 +2885,7 @@ function ( $gateway ) { public function process_payment_for_order_afterpay_clearpay_provider() { return [ - 'with valid full address' => [ + 'with valid full address' => [ 'address' => [ 'city' => 'WooCity', 'state' => 'NY', @@ -2896,7 +2896,7 @@ public function process_payment_for_order_afterpay_clearpay_provider() { 'locale_data' => [], 'expected_exception' => null, ], - 'with incomplete address' => [ + 'with incomplete address' => [ 'address' => [ 'city' => 'WooCity', 'state' => '', @@ -2909,7 +2909,7 @@ public function process_payment_for_order_afterpay_clearpay_provider() { ], 'expected_exception' => Invalid_Address_Exception::class, ], - 'without state, GB' => [ + 'optional state' => [ 'address' => [ 'city' => 'London', 'state' => '', @@ -2921,7 +2921,22 @@ public function process_payment_for_order_afterpay_clearpay_provider() { ], 'expected_exception' => null, ], - 'without city, GB' => [ + 'optional state and postcode' => [ + 'address' => [ + 'city' => 'London', + 'state' => '', + 'postcode' => '', + 'country' => Country_Code::UNITED_KINGDOM, + ], + 'locale_data' => [ + Country_Code::UNITED_KINGDOM => [ + 'state' => [ 'required' => false ], + 'postcode' => [ 'required' => false ], + ], + ], + 'expected_exception' => null, + ], + 'optional state, invalid address' => [ 'address' => [ 'city' => '', 'state' => 'London', From 6071b460621cb308fbc2174b987a4ff9ac9903d1 Mon Sep 17 00:00:00 2001 From: Daniel Guerra <15204776+danielmx-dev@users.noreply.github.com> Date: Tue, 16 Jul 2024 19:28:42 +0300 Subject: [PATCH 7/7] Simplify required check Co-authored-by: Samir Merchant --- includes/class-wc-payment-gateway-wcpay.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php index 18fd5a128e9..34fe18e0040 100644 --- a/includes/class-wc-payment-gateway-wcpay.php +++ b/includes/class-wc-payment-gateway-wcpay.php @@ -4511,7 +4511,7 @@ private function handle_afterpay_shipping_requirement( WC_Order $order, Create_A $is_field_required = ( ! isset( $country_locale_data[ $locale_field ] ) || ! isset( $country_locale_data[ $locale_field ]['required'] ) || - false !== $country_locale_data[ $locale_field ]['required'] + $country_locale_data[ $locale_field ]['required'] ); if ( $is_field_required && ! $address[ $address_field ] ) {