Patch: Fix subscription renewals exceptions (#8683)

Co-authored-by: Timur Karimov <[email protected]> Co-authored-by: Timur Karimov <[email protected]> Co-authored-by: Daniel Mallory <[email protected]> Co-authored-by: Daniel Mallory <[email protected]>
Automattic · Apr 22, 2024 · 174ea29 · 174ea29
1 parent da0a15c
commit 174ea29
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 58 deletions.
diff --git a/changelog/fix-8678-use-legacy-approach-off-session b/changelog/fix-8678-use-legacy-approach-off-session
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Bugfix for failing subscription renewal payments.
diff --git a/changelog/remove-deprecated-param-from-asset-data-registry b/changelog/remove-deprecated-param-from-asset-data-registry
@@ -0,0 +1,4 @@
+Significance: minor
+Type: dev
+
+Remove deprecated param from asset data registry interface.
diff --git a/includes/class-wc-payment-gateway-wcpay.php b/includes/class-wc-payment-gateway-wcpay.php
@@ -1512,7 +1512,7 @@ public function process_payment_for_order( $cart, $payment_information, $schedul
 				$request->set_cvc_confirmation( $payment_information->get_cvc_confirmation() );
 				$request->set_hook_args( $payment_information );
 				if ( $payment_information->is_using_saved_payment_method() ) {
-					$billing_details = WC_Payments_Utils::get_billing_details_from_order( $order );
+					$billing_details = WC_Payments_Utils::get_billing_details_from_order( $order, $payment_information->is_merchant_initiated() );
 
 					$is_legacy_card_object = strpos( $payment_information->get_payment_method() ?? '', 'card_' ) === 0;
 

diff --git a/includes/class-wc-payments-utils.php b/includes/class-wc-payments-utils.php
@@ -342,10 +342,31 @@ public static function map_search_orders_to_charge_ids( $search ) {
 	 * It only returns the fields that are present in the billing section of the checkout.
 	 *
 	 * @param WC_Order $order Order to extract the billing details from.
+	 * @param bool     $legacy Whether to use the legacy way of loading straight from the order.
+	 * @todo The $legacy flag is just a patch for the current approach, fixing the linked issue.
+	 * @see https://github.com/Automattic/woocommerce-payments/issues/8678
 	 *
 	 * @return array
 	 */
-	public static function get_billing_details_from_order( $order ) {
+	public static function get_billing_details_from_order( $order, $legacy = true ) {
+		if ( $legacy ) {
+			$billing_details = [
+				'address' => [
+					'city'        => $order->get_billing_city(),
+					'country'     => $order->get_billing_country(),
+					'line1'       => $order->get_billing_address_1(),
+					'line2'       => $order->get_billing_address_2(),
+					'postal_code' => $order->get_billing_postcode(),
+					'state'       => $order->get_billing_state(),
+				],
+				'email'   => $order->get_billing_email(),
+				'name'    => trim( $order->get_formatted_billing_full_name() ),
+				'phone'   => $order->get_billing_phone(),
+			];
+
+			return array_filter( $billing_details );
+		}
+
 		$billing_fields       = array_keys( WC()->checkout()->get_checkout_fields( 'billing' ) );
 		$address_field_to_key = [
 			'billing_city'      => 'city',

diff --git a/includes/multi-currency/Analytics.php b/includes/multi-currency/Analytics.php
@@ -114,6 +114,11 @@ public function register_admin_scripts() {
 	 * @return void
 	 */
 	public function register_customer_currencies() {
+		$data_registry = Package::container()->get( AssetDataRegistry::class );
+		if ( $data_registry->exists( 'customerCurrencies' ) ) {
+			return;
+		}
+
 		$currencies           = $this->multi_currency->get_all_customer_currencies();
 		$available_currencies = $this->multi_currency->get_available_currencies();
 		$currency_options     = [];
@@ -137,8 +142,7 @@ public function register_customer_currencies() {
 			];
 		}
 
-		$data_registry = Package::container()->get( AssetDataRegistry::class );
-		$data_registry->add( 'customerCurrencies', $currency_options, true );
+		$data_registry->add( 'customerCurrencies', $currency_options );
 	}
 
 	/**

diff --git a/tests/unit/core/service/test-class-wc-payments-customer-service-api.php b/tests/unit/core/service/test-class-wc-payments-customer-service-api.php
@@ -339,16 +339,16 @@ function ( $data ): bool {
 						'test_mode'       => false,
 						'billing_details' => [
 							'address' => [
+								'city'        => $order->get_billing_city(),
 								'country'     => $order->get_billing_country(),
 								'line1'       => $order->get_billing_address_1(),
 								'line2'       => $order->get_billing_address_2(),
-								'city'        => $order->get_billing_city(),
-								'state'       => $order->get_billing_state(),
 								'postal_code' => $order->get_billing_postcode(),
+								'state'       => $order->get_billing_state(),
 							],
-							'phone'   => $order->get_billing_phone(),
 							'email'   => $order->get_billing_email(),
 							'name'    => $order->get_billing_first_name() . ' ' . $order->get_billing_last_name(),
+							'phone'   => $order->get_billing_phone(),
 						],
 					]
 				),

diff --git a/tests/unit/multi-currency/test-class-analytics.php b/tests/unit/multi-currency/test-class-analytics.php
@@ -109,28 +109,18 @@ public function woocommerce_filter_provider() {
 		];
 	}
 
+	/**
+	 * Test for the register_customer_currencies method. Note that this function is called in the constructor,
+	 * and the customerCurrencies data key cannot be re-registered, so this test is only to ensure that it exists.
+	 */
 	public function test_register_customer_currencies() {
-		$this->mock_multi_currency->expects( $this->once() )
-			->method( 'get_all_customer_currencies' )
-			->willReturn( $this->mock_customer_currencies );
-
-		$this->mock_multi_currency->expects( $this->once() )
-			->method( 'get_available_currencies' )
-			->willReturn( $this->get_mock_available_currencies() );
-
-		$this->mock_multi_currency->expects( $this->once() )
-			->method( 'get_default_currency' )
-			->willReturn( new Currency( 'USD', 1.0 ) );
-
-		$this->analytics->register_customer_currencies();
-
 		$data_registry = Package::container()->get(
 			AssetDataRegistry::class
 		);
-
 		$this->assertTrue( $data_registry->exists( 'customerCurrencies' ) );
 	}
 
+
 	public function test_has_multi_currency_orders() {
 
 		// Use reflection to make the private method has_multi_currency_orders accessible.
@@ -143,30 +133,6 @@ public function test_has_multi_currency_orders() {
 		$this->assertTrue( $result );
 	}
 
-	public function test_register_customer_currencies_for_empty_customer_currencies() {
-		delete_option( MultiCurrency::CUSTOMER_CURRENCIES_KEY );
-
-		$this->mock_multi_currency->expects( $this->once() )
-			->method( 'get_all_customer_currencies' )
-			->willReturn( [] );
-
-		$this->mock_multi_currency->expects( $this->once() )
-			->method( 'get_available_currencies' )
-			->willReturn( $this->get_mock_available_currencies() );
-
-		$this->mock_multi_currency->expects( $this->once() )
-			->method( 'get_default_currency' )
-			->willReturn( new Currency( 'USD', 1.0 ) );
-
-		$this->analytics->register_customer_currencies();
-
-		$data_registry = Package::container()->get(
-			AssetDataRegistry::class
-		);
-
-		$this->assertTrue( $data_registry->exists( 'customerCurrencies' ) );
-	}
-
 	public function test_update_order_stats_data_with_non_multi_currency_order() {
 		$args  = $this->order_args_provider( 123, 0, 1, 15.50, 1.50, 0, 14.00 );
 		$order = wc_create_order();

diff --git a/tests/unit/test-class-wc-payments-customer-service.php b/tests/unit/test-class-wc-payments-customer-service.php
@@ -484,16 +484,16 @@ public function test_update_payment_method_with_billing_details_from_order() {
 				[
 					'billing_details' => [
 						'address' => [
-							'city'        => 'WooCity',
 							'country'     => Country_Code::UNITED_STATES,
 							'line1'       => 'WooAddress',
 							'line2'       => '',
-							'postal_code' => '12345',
+							'city'        => 'WooCity',
 							'state'       => 'NY',
+							'postal_code' => '12345',
 						],
+						'phone'   => '555-32123',
 						'email'   => '[email protected]',
 						'name'    => 'Jeroen Sormani',
-						'phone'   => '555-32123',
 					],
 				]
 			);
@@ -504,15 +504,6 @@ public function test_update_payment_method_with_billing_details_from_order() {
 	}
 
 	public function test_update_payment_method_with_billing_details_from_checkout_fields() {
-		$fields = wc()->checkout()->checkout_fields;
-		unset( $fields['billing']['billing_company'] );
-		unset( $fields['billing']['billing_country'] );
-		unset( $fields['billing']['billing_address_1'] );
-		unset( $fields['billing']['billing_address_2'] );
-		unset( $fields['billing']['billing_city'] );
-		unset( $fields['billing']['billing_state'] );
-		unset( $fields['billing']['billing_phone'] );
-		wc()->checkout()->checkout_fields = $fields;
 		$this->mock_api_client
 			->expects( $this->once() )
 			->method( 'update_payment_method' )
@@ -522,9 +513,15 @@ public function test_update_payment_method_with_billing_details_from_checkout_fi
 					'billing_details' => [
 						'address' => [
 							'postal_code' => '12345',
+							'city'        => 'WooCity',
+							'country'     => 'US',
+							'line1'       => 'WooAddress',
+							'line2'       => '',
+							'state'       => 'NY',
 						],
 						'email'   => '[email protected]',
 						'name'    => 'Jeroen Sormani',
+						'phone'   => '555-32123',
 					],
 				]
 			);