From e154bb3e27cc79328b9771222695bb68807b87f5 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 5 May 2022 11:19:56 +1200 Subject: [PATCH 01/16] use recommended `WC_Order::get_currency()` for getting renewal currency: - previously used raw `get_post_meta()` - this is no longer recommended, and could break on stores using (forthcoming) custom order tables --- .../Compatibility/WooCommerceSubscriptions.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index 52e63da6038..1c388a128ff 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -148,7 +148,7 @@ public function override_selected_currency( $return ) { $subscription_renewal = $this->cart_contains_renewal(); if ( $subscription_renewal ) { - return get_post_meta( $subscription_renewal['subscription_renewal']['renewal_order_id'], '_order_currency', true ); + return $subscription_renewal['subscription_renewal']['renewal_order_id']->get_currency(); } $switch_id = $this->get_subscription_switch_id_from_superglobal(); @@ -159,12 +159,12 @@ public function override_selected_currency( $return ) { $switch_cart_items = $this->get_subscription_switch_cart_items(); if ( 0 < count( $switch_cart_items ) ) { $switch_cart_item = array_shift( $switch_cart_items ); - return get_post_meta( $switch_cart_item['subscription_switch']['subscription_id'], '_order_currency', true ); + return $switch_cart_item['subscription_switch']['subscription_id']->get_currency(); } $subscription_resubscribe = $this->cart_contains_resubscribe(); if ( $subscription_resubscribe ) { - return get_post_meta( $subscription_resubscribe['subscription_resubscribe']['subscription_id'], '_order_currency', true ); + return $subscription_resubscribe['subscription_resubscribe']['subscription_id']->get_currency(); } return $return; From 14eb4e33219262a4e4af1f31d167511156903e64 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 5 May 2022 11:46:23 +1200 Subject: [PATCH 02/16] add changelog --- .../fix-4152-subscribe-renew-multicurrency-order-meta-api | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/fix-4152-subscribe-renew-multicurrency-order-meta-api diff --git a/changelog/fix-4152-subscribe-renew-multicurrency-order-meta-api b/changelog/fix-4152-subscribe-renew-multicurrency-order-meta-api new file mode 100644 index 00000000000..7011e831fb0 --- /dev/null +++ b/changelog/fix-4152-subscribe-renew-multicurrency-order-meta-api @@ -0,0 +1,4 @@ +Significance: minor +Type: fix + +Use high-level order currency API for multicurrency subscription renewal orders (get_post_meta is not recommended for orders). From 20ea6b0796fb535adac713e287c63caad100055e Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Mon, 9 May 2022 16:26:53 +1200 Subject: [PATCH 03/16] convert id to order/subscription object before calling get_currency: - fix logic issue in previous commit --- .../Compatibility/WooCommerceSubscriptions.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index 1c388a128ff..26504485afe 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -148,7 +148,8 @@ public function override_selected_currency( $return ) { $subscription_renewal = $this->cart_contains_renewal(); if ( $subscription_renewal ) { - return $subscription_renewal['subscription_renewal']['renewal_order_id']->get_currency(); + $order = wc_get_order( $subscription_renewal['subscription_renewal']['renewal_order_id'] ); + return $order ? $order->get_currency() : $return; } $switch_id = $this->get_subscription_switch_id_from_superglobal(); @@ -159,12 +160,14 @@ public function override_selected_currency( $return ) { $switch_cart_items = $this->get_subscription_switch_cart_items(); if ( 0 < count( $switch_cart_items ) ) { $switch_cart_item = array_shift( $switch_cart_items ); - return $switch_cart_item['subscription_switch']['subscription_id']->get_currency(); + $switch_subscription = wcs_get_subscription( $switch_cart_item['subscription_switch']['subscription_id'] ); + return $switch_subscription ? $switch_subscription->get_currency() : $return; } $subscription_resubscribe = $this->cart_contains_resubscribe(); if ( $subscription_resubscribe ) { - return $subscription_resubscribe['subscription_resubscribe']['subscription_id']->get_currency(); + $switch_subscription = $subscription_resubscribe['subscription_resubscribe']['subscription_id']; + return $switch_subscription ? $switch_subscription->get_currency() : $return; } return $return; From c433f4e926c260eca9d166ad75f28f20a07d310c Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Tue, 14 Jun 2022 13:32:01 +1200 Subject: [PATCH 04/16] =?UTF-8?q?fix=20fatal=20error=20on=20resubscribe=20?= =?UTF-8?q?=E2=80=93=20get=20subscription=20from=20ID?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../multi-currency/Compatibility/WooCommerceSubscriptions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index 26504485afe..0910a073fa9 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -166,7 +166,7 @@ public function override_selected_currency( $return ) { $subscription_resubscribe = $this->cart_contains_resubscribe(); if ( $subscription_resubscribe ) { - $switch_subscription = $subscription_resubscribe['subscription_resubscribe']['subscription_id']; + $switch_subscription = wcs_get_subscription( $subscription_resubscribe['subscription_resubscribe']['subscription_id'] ); return $switch_subscription ? $switch_subscription->get_currency() : $return; } From 4b733df3e00cf08c6dff8d2d627211742b75df90 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 17 Jun 2022 12:19:58 +1200 Subject: [PATCH 05/16] fix override_selected_currency test for renewal case: - Make a real order with non-default currency - integration test style. - Enhance mock_wcs_cart_contains_renewal so test can customise product and renewal order ids. - Mock the real order instead of hacking post meta (legacy implementation detail). - Fix up all uses of mock_wcs_cart_contains_renewal() for new signature. --- .../test-class-woocommerce-subscriptions.php | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php index 1762c258f14..345ade8a06f 100644 --- a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php +++ b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php @@ -98,7 +98,7 @@ public function woocommerce_filter_provider() { // Test should not convert the product price due to all checks return true. public function test_get_subscription_product_price_does_not_convert_price() { $this->mock_utils->method( 'is_call_in_backtrace' )->willReturn( true ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->mock_wcs_cart_contains_resubscribe( true ); $this->assertSame( 10.0, $this->woocommerce_subscriptions->get_subscription_product_price( 10.0, $this->mock_product ) ); } @@ -129,7 +129,7 @@ public function test_get_subscription_product_price_converts_price_if_only_backt // Test should convert product price due to the backtrace check returns false after the cart contains renewal check returns true. public function test_get_subscription_product_price_converts_price_if_only_renewal_in_cart() { $this->mock_utils->method( 'is_call_in_backtrace' )->willReturn( false ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->mock_wcs_cart_contains_resubscribe( false ); $this->mock_multi_currency->method( 'get_price' )->with( 10.0, 'product' )->willReturn( 25.0 ); $this->assertSame( 25.0, $this->woocommerce_subscriptions->get_subscription_product_price( 10.0, $this->mock_product ) ); @@ -249,7 +249,7 @@ public function test_maybe_disable_mixed_cart_return_yes() { // Returns code due to code was passed. public function test_override_selected_currency_return_currency_code_when_code_passed() { // Conditions added to return EUR, but CAD should be returned at the beginning of the method. - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->mock_wcs_cart_contains_resubscribe( false ); update_post_meta( 42, '_order_currency', 'EUR', true ); $this->mock_wcs_get_order_type_cart_items( false ); @@ -267,15 +267,19 @@ public function test_override_selected_currency_return_false() { // Returns code due to cart contains a subscription renewal. public function test_override_selected_currency_return_currency_code_when_renewal_in_cart() { - $this->mock_wcs_cart_contains_renewal( true ); - $this->mock_wcs_cart_contains_resubscribe( false ); - update_post_meta( 42, '_order_currency', 'CAD', true ); - $this->mock_wcs_get_order_type_cart_items( false ); - $this->assertSame( 'CAD', $this->woocommerce_subscriptions->override_selected_currency( false ) ); + // Set up an order with a non-default currency. + $order = WC_Helper_Order::create_order(); + $order->set_currency( 'JPY' ); + $order->save(); + + // Mock that order as the renewal in the cart. + $this->mock_wcs_cart_contains_renewal( 42, $order->get_id() ); + + $this->assertSame( 'JPY', $this->woocommerce_subscriptions->override_selected_currency( false ) ); } // Returns code due to GET states there is a subscription switch like on the product page after clicking upgrade/downgrade button. - public function test_override_selected_currency_return_currency_code_when_switch_initiated() { + public function SKIP_test_override_selected_currency_return_currency_code_when_switch_initiated() { $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_cart_contains_resubscribe( false ); $_GET['switch-subscription'] = 42; @@ -286,7 +290,7 @@ public function test_override_selected_currency_return_currency_code_when_switch } // Returns code due to cart contains a subscription switch. - public function test_override_selected_currency_return_currency_code_when_switch_in_cart() { + public function SKIP_test_override_selected_currency_return_currency_code_when_switch_in_cart() { $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_cart_contains_resubscribe( false ); update_post_meta( 42, '_order_currency', 'CAD', true ); @@ -295,7 +299,7 @@ public function test_override_selected_currency_return_currency_code_when_switch } // Returns code due to cart contains a subscription resubscribe. - public function test_override_selected_currency_return_currency_code_when_resubscribe_in_cart() { + public function SKIP_test_override_selected_currency_return_currency_code_when_resubscribe_in_cart() { $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_cart_contains_resubscribe( true ); update_post_meta( 42, '_order_currency', 'CAD', true ); @@ -306,7 +310,7 @@ public function test_override_selected_currency_return_currency_code_when_resubs public function test_should_convert_product_price_return_false_when_false_passed() { // Conditions added to return true, but it should return false if passed. $this->mock_utils->method( 'is_call_in_backtrace' )->willReturn( false ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->mock_wcs_cart_contains_resubscribe( true ); $this->assertFalse( $this->woocommerce_subscriptions->should_convert_product_price( false, $this->mock_product ) ); @@ -324,7 +328,7 @@ public function test_should_convert_product_price_return_false_when_renewal_in_c ] ) ->willReturn( true ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->mock_wcs_cart_contains_resubscribe( false ); $this->assertFalse( $this->woocommerce_subscriptions->should_convert_product_price( true, $this->mock_product ) ); } @@ -348,7 +352,7 @@ public function test_should_convert_product_price_return_false_when_resubscribe_ public function test_should_convert_product_price_return_true_when_backtrace_does_not_match() { $this->mock_utils->method( 'is_call_in_backtrace' )->willReturn( false ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->mock_wcs_cart_contains_resubscribe( true ); $this->assertTrue( $this->woocommerce_subscriptions->should_convert_product_price( true, $this->mock_product ) ); } @@ -405,7 +409,7 @@ public function test_should_convert_coupon_amount_return_false_when_renewal_in_c ->method( 'get_discount_type' ) ->willReturn( 'recurring_fee' ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->assertFalse( $this->woocommerce_subscriptions->should_convert_coupon_amount( true, $this->mock_coupon ) ); } @@ -429,7 +433,7 @@ public function test_should_convert_coupon_amount_return_true_with_early_renewal ->method( 'get_discount_type' ) ->willReturn( 'recurring_fee' ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->assertTrue( $this->woocommerce_subscriptions->should_convert_coupon_amount( true, $this->mock_coupon ) ); } @@ -447,7 +451,7 @@ public function test_should_convert_coupon_amount_return_true_when_apply_coupon_ ->method( 'get_discount_type' ) ->willReturn( 'recurring_fee' ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->assertTrue( $this->woocommerce_subscriptions->should_convert_coupon_amount( true, $this->mock_coupon ) ); } @@ -465,7 +469,7 @@ public function test_should_convert_coupon_amount_return_true_when_coupon_type_d ->method( 'get_discount_type' ) ->willReturn( 'failing_fee' ); - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->assertTrue( $this->woocommerce_subscriptions->should_convert_coupon_amount( true, $this->mock_coupon ) ); } @@ -500,7 +504,7 @@ public function test_should_hide_widgets_return_false() { } public function test_should_hide_widgets_return_true_when_renewal_in_cart() { - $this->mock_wcs_cart_contains_renewal( true ); + $this->mock_wcs_cart_contains_renewal( 42, 43 ); $this->mock_wcs_cart_contains_resubscribe( false ); $this->assertTrue( $this->woocommerce_subscriptions->should_hide_widgets( false ) ); } @@ -528,14 +532,16 @@ public function test_should_hide_widgets_return_true_when_switch_found_in_cart() $this->assertTrue( $this->woocommerce_subscriptions->should_hide_widgets( false ) ); } - private function mock_wcs_cart_contains_renewal( $value ) { + // Simulate (mock) a renewal in the cart. + // Pass 0 / no args to unmock. + private function mock_wcs_cart_contains_renewal( $product_id = 0, $renewal_order_id = 0 ) { WC_Subscriptions::wcs_cart_contains_renewal( - function () use ( $value ) { - if ( $value ) { + function () use ( $product_id, $renewal_order_id ) { + if ( $product_id && $renewal_order_id ) { return [ - 'product_id' => 42, + 'product_id' => $product_id, 'subscription_renewal' => [ - 'renewal_order_id' => 42, + 'renewal_order_id' => $renewal_order_id, ], ]; } From 37bb933114a16933f8bb993b6f0df9187e0387aa Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 17 Jun 2022 12:36:01 +1200 Subject: [PATCH 06/16] Repair multicurrency sub switch unit test: - Rename / update comment to clarify test scope. - Mock up a real order (aka subscription) with custom currency. - Mock that order idea in $_GET request params. --- .../test-class-woocommerce-subscriptions.php | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php index 345ade8a06f..cf72457387d 100644 --- a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php +++ b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php @@ -278,15 +278,25 @@ public function test_override_selected_currency_return_currency_code_when_renewa $this->assertSame( 'JPY', $this->woocommerce_subscriptions->override_selected_currency( false ) ); } - // Returns code due to GET states there is a subscription switch like on the product page after clicking upgrade/downgrade button. - public function SKIP_test_override_selected_currency_return_currency_code_when_switch_initiated() { + // Test correct currency when shopper clicks upgrade/downgrade button in My Account – "switch". + public function test_override_selected_currency_return_currency_code_for_switch_request() { + // Reset/clear any previous mocked state. $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_cart_contains_resubscribe( false ); - $_GET['switch-subscription'] = 42; - $_GET['_wcsnonce'] = wp_create_nonce( 'wcs_switch_request' ); - update_post_meta( 42, '_order_currency', 'CAD', true ); $this->mock_wcs_get_order_type_cart_items( false ); - $this->assertSame( 'CAD', $this->woocommerce_subscriptions->override_selected_currency( false ) ); + + // Set up an order with a non-default currency. + // This might need to be a subscription object, not an order. + $order = WC_Helper_Order::create_order(); + $order->set_currency( 'JPY' ); + $order->save(); + + // Blatantly hack mock request params for the test. :) + $_GET['switch-subscription'] = $order->get_id(); + $_GET['_wcsnonce'] = wp_create_nonce( 'wcs_switch_request' ); + + // update_post_meta( 42, '_order_currency', 'CAD', true ); + $this->assertSame( 'JPY', $this->woocommerce_subscriptions->override_selected_currency( false ) ); } // Returns code due to cart contains a subscription switch. From 053bf3cc5fed5b6d4dc66b9664c593ba88b85add Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 23 Jun 2022 14:29:50 +1200 Subject: [PATCH 07/16] repair multicurrency subs switch in cart unit test: - use WC_Order for mock subscription - mock subscription and cart APIs as needed --- .../test-class-woocommerce-subscriptions.php | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php index cf72457387d..96355d67f8e 100644 --- a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php +++ b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php @@ -295,17 +295,33 @@ public function test_override_selected_currency_return_currency_code_for_switch_ $_GET['switch-subscription'] = $order->get_id(); $_GET['_wcsnonce'] = wp_create_nonce( 'wcs_switch_request' ); - // update_post_meta( 42, '_order_currency', 'CAD', true ); $this->assertSame( 'JPY', $this->woocommerce_subscriptions->override_selected_currency( false ) ); } // Returns code due to cart contains a subscription switch. - public function SKIP_test_override_selected_currency_return_currency_code_when_switch_in_cart() { + public function test_override_selected_currency_return_currency_code_when_switch_in_cart() { + // Reset/clear any previous mocked state. $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_cart_contains_resubscribe( false ); - update_post_meta( 42, '_order_currency', 'CAD', true ); - $this->mock_wcs_get_order_type_cart_items( true ); - $this->assertSame( 'CAD', $this->woocommerce_subscriptions->override_selected_currency( false ) ); + $this->mock_wcs_get_order_type_cart_items( false ); + + // Mock order with custom currency for switch cart item. + // Note we're using a WC_Order as a stand-in for a true WC_Subscription. + $mock_subscription = WC_Helper_Order::create_order(); + $mock_subscription->set_currency( 'JPY' ); + $mock_subscription->save(); + + // Mock wcs_get_subscription to return our mock subscription. + WC_Subscriptions::set_wcs_get_subscription( + function ( $id ) use ( $mock_subscription ) { + return $mock_subscription; + } + ); + + // Mock cart to simulate a switch cart item referencing our subscription. + $this->mock_wcs_get_order_type_cart_items_with_value( $mock_subscription->get_id() ); + + $this->assertSame( 'JPY', $this->woocommerce_subscriptions->override_selected_currency( false ) ); } // Returns code due to cart contains a subscription resubscribe. @@ -561,6 +577,27 @@ function () use ( $product_id, $renewal_order_id ) { ); } + private function mock_wcs_get_order_type_cart_items_with_value( $switch_id = 0 ) { + WC_Subscriptions::wcs_get_order_type_cart_items( + function () use ( $switch_id ) { + if ( $switch_id ) { + return [ + [ + 'product_id' => 42, + 'key' => 'abc123', + 'subscription_switch' => [ + 'subscription_id' => $switch_id, + ], + ], + ]; + } + + return []; + } + ); + } + + private function mock_wcs_get_order_type_cart_items( $value ) { WC_Subscriptions::wcs_get_order_type_cart_items( function () use ( $value ) { From f45d6b9381bbccecd266185898a7e15508683c3c Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 23 Jun 2022 14:34:58 +1200 Subject: [PATCH 08/16] refactor mock_wcs_get_order_type_cart_items so can pass a real order id --- .../test-class-woocommerce-subscriptions.php | 37 ++++--------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php index 96355d67f8e..0ac4cf5af29 100644 --- a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php +++ b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php @@ -151,7 +151,7 @@ public function test_get_subscription_product_signup_fee_does_not_convert_price_ ->method( 'is_call_in_backtrace' ) ->with( [ 'WC_Subscriptions_Cart::set_subscription_prices_for_calculation' ] ) ->willReturn( true ); - $this->mock_wcs_get_order_type_cart_items( true ); + $this->mock_wcs_get_order_type_cart_items( 42 ); $this->assertSame( 10.0, $this->woocommerce_subscriptions->get_subscription_product_signup_fee( 10.0, $this->mock_product ) ); } @@ -167,7 +167,7 @@ public function test_get_subscription_product_signup_fee_does_not_convert_price_ [ [ 'WCS_Switch_Totals_Calculator->apportion_sign_up_fees' ] ] ) ->willReturn( false, true, true, false ); - $this->mock_wcs_get_order_type_cart_items( true ); + $this->mock_wcs_get_order_type_cart_items( 42 ); $this->woocommerce_subscriptions->switch_cart_item = 'abc123'; $this->assertSame( 10.0, $this->woocommerce_subscriptions->get_subscription_product_signup_fee( 10.0, $this->mock_product ) ); } @@ -178,7 +178,7 @@ public function test_get_subscription_product_signup_fee_does_not_convert_price_ ->expects( $this->any() ) ->method( 'is_call_in_backtrace' ) ->willReturn( false ); - $this->mock_wcs_get_order_type_cart_items( true ); + $this->mock_wcs_get_order_type_cart_items( 42 ); $this->woocommerce_subscriptions->switch_cart_item = 'abc123'; $this->mock_meta_data @@ -201,7 +201,7 @@ public function test_get_subscription_product_signup_fee_converts_price_when_met ->expects( $this->any() ) ->method( 'is_call_in_backtrace' ) ->willReturn( false ); - $this->mock_wcs_get_order_type_cart_items( true ); + $this->mock_wcs_get_order_type_cart_items( 42 ); $this->woocommerce_subscriptions->switch_cart_item = 'abc123'; $this->mock_meta_data @@ -225,7 +225,7 @@ public function test_get_subscription_product_signup_fee_converts_price_when_car ->expects( $this->any() ) ->method( 'is_call_in_backtrace' ) ->willReturn( false ); - $this->mock_wcs_get_order_type_cart_items( true ); + $this->mock_wcs_get_order_type_cart_items( 42 ); $this->woocommerce_subscriptions->switch_cart_item = 'def456'; $this->mock_product @@ -237,7 +237,7 @@ public function test_get_subscription_product_signup_fee_converts_price_when_car } public function test_maybe_disable_mixed_cart_return_no() { - $this->mock_wcs_get_order_type_cart_items( true ); + $this->mock_wcs_get_order_type_cart_items( 42 ); $this->assertSame( 'no', $this->woocommerce_subscriptions->maybe_disable_mixed_cart( 'yes' ) ); } @@ -319,7 +319,7 @@ function ( $id ) use ( $mock_subscription ) { ); // Mock cart to simulate a switch cart item referencing our subscription. - $this->mock_wcs_get_order_type_cart_items_with_value( $mock_subscription->get_id() ); + $this->mock_wcs_get_order_type_cart_items( $mock_subscription->get_id() ); $this->assertSame( 'JPY', $this->woocommerce_subscriptions->override_selected_currency( false ) ); } @@ -577,7 +577,7 @@ function () use ( $product_id, $renewal_order_id ) { ); } - private function mock_wcs_get_order_type_cart_items_with_value( $switch_id = 0 ) { + private function mock_wcs_get_order_type_cart_items( $switch_id = 0 ) { WC_Subscriptions::wcs_get_order_type_cart_items( function () use ( $switch_id ) { if ( $switch_id ) { @@ -597,27 +597,6 @@ function () use ( $switch_id ) { ); } - - private function mock_wcs_get_order_type_cart_items( $value ) { - WC_Subscriptions::wcs_get_order_type_cart_items( - function () use ( $value ) { - if ( $value ) { - return [ - [ - 'product_id' => 42, - 'key' => 'abc123', - 'subscription_switch' => [ - 'subscription_id' => 42, - ], - ], - ]; - } - - return []; - } - ); - } - private function mock_wcs_cart_contains_resubscribe( $value ) { WC_Subscriptions::wcs_cart_contains_resubscribe( function () use ( $value ) { From 3d97600b6b4bbfd700fb23692406d0c40c17bd31 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 23 Jun 2022 14:41:41 +1200 Subject: [PATCH 09/16] repair multicurrency sub resubscribe unit test: - parameterise wcs_get_order_type_cart_items so can use real order id - mock sub (order) and wcs_get_subscription to return mocked sub - it's fine right --- .../test-class-woocommerce-subscriptions.php | 45 ++++++++++++------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php index 0ac4cf5af29..0e897d19d69 100644 --- a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php +++ b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php @@ -99,7 +99,7 @@ public function woocommerce_filter_provider() { public function test_get_subscription_product_price_does_not_convert_price() { $this->mock_utils->method( 'is_call_in_backtrace' )->willReturn( true ); $this->mock_wcs_cart_contains_renewal( 42, 43 ); - $this->mock_wcs_cart_contains_resubscribe( true ); + $this->mock_wcs_cart_contains_resubscribe( 42 ); $this->assertSame( 10.0, $this->woocommerce_subscriptions->get_subscription_product_price( 10.0, $this->mock_product ) ); } @@ -139,7 +139,7 @@ public function test_get_subscription_product_price_converts_price_if_only_renew public function test_get_subscription_product_price_converts_price_if_only_resubscribe_in_cart() { $this->mock_utils->method( 'is_call_in_backtrace' )->willReturn( false ); $this->mock_wcs_cart_contains_renewal( false ); - $this->mock_wcs_cart_contains_resubscribe( true ); + $this->mock_wcs_cart_contains_resubscribe( 42 ); $this->mock_multi_currency->method( 'get_price' )->with( 10.0, 'product' )->willReturn( 25.0 ); $this->assertSame( 25.0, $this->woocommerce_subscriptions->get_subscription_product_price( 10.0, $this->mock_product ) ); } @@ -303,7 +303,6 @@ public function test_override_selected_currency_return_currency_code_when_switch // Reset/clear any previous mocked state. $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_cart_contains_resubscribe( false ); - $this->mock_wcs_get_order_type_cart_items( false ); // Mock order with custom currency for switch cart item. // Note we're using a WC_Order as a stand-in for a true WC_Subscription. @@ -325,19 +324,35 @@ function ( $id ) use ( $mock_subscription ) { } // Returns code due to cart contains a subscription resubscribe. - public function SKIP_test_override_selected_currency_return_currency_code_when_resubscribe_in_cart() { + public function test_override_selected_currency_return_currency_code_when_resubscribe_in_cart() { + // Reset/clear any previous mocked state. $this->mock_wcs_cart_contains_renewal( false ); - $this->mock_wcs_cart_contains_resubscribe( true ); - update_post_meta( 42, '_order_currency', 'CAD', true ); $this->mock_wcs_get_order_type_cart_items( false ); - $this->assertSame( 'CAD', $this->woocommerce_subscriptions->override_selected_currency( false ) ); + + // Mock order with custom currency for switch cart item. + // Note we're using a WC_Order as a stand-in for a true WC_Subscription. + $mock_subscription = WC_Helper_Order::create_order(); + $mock_subscription->set_currency( 'JPY' ); + $mock_subscription->save(); + + // Mock wcs_get_subscription to return our mock subscription. + WC_Subscriptions::set_wcs_get_subscription( + function ( $id ) use ( $mock_subscription ) { + return $mock_subscription; + } + ); + + // Mock cart to simulate a resubscribe cart item referencing our subscription. + $this->mock_wcs_cart_contains_resubscribe( $mock_subscription->get_id() ); + + $this->assertSame( 'JPY', $this->woocommerce_subscriptions->override_selected_currency( false ) ); } public function test_should_convert_product_price_return_false_when_false_passed() { // Conditions added to return true, but it should return false if passed. $this->mock_utils->method( 'is_call_in_backtrace' )->willReturn( false ); $this->mock_wcs_cart_contains_renewal( 42, 43 ); - $this->mock_wcs_cart_contains_resubscribe( true ); + $this->mock_wcs_cart_contains_resubscribe( 42 ); $this->assertFalse( $this->woocommerce_subscriptions->should_convert_product_price( false, $this->mock_product ) ); } @@ -372,14 +387,14 @@ public function test_should_convert_product_price_return_false_when_resubscribe_ ) ->willReturn( true ); $this->mock_wcs_cart_contains_renewal( false ); - $this->mock_wcs_cart_contains_resubscribe( true ); + $this->mock_wcs_cart_contains_resubscribe( 42 ); $this->assertFalse( $this->woocommerce_subscriptions->should_convert_product_price( true, $this->mock_product ) ); } public function test_should_convert_product_price_return_true_when_backtrace_does_not_match() { $this->mock_utils->method( 'is_call_in_backtrace' )->willReturn( false ); $this->mock_wcs_cart_contains_renewal( 42, 43 ); - $this->mock_wcs_cart_contains_resubscribe( true ); + $this->mock_wcs_cart_contains_resubscribe( 42 ); $this->assertTrue( $this->woocommerce_subscriptions->should_convert_product_price( true, $this->mock_product ) ); } @@ -537,7 +552,7 @@ public function test_should_hide_widgets_return_true_when_renewal_in_cart() { public function test_should_hide_widgets_return_true_when_resubscribe_in_cart() { $this->mock_wcs_cart_contains_renewal( false ); - $this->mock_wcs_cart_contains_resubscribe( true ); + $this->mock_wcs_cart_contains_resubscribe( 42 ); $this->assertTrue( $this->woocommerce_subscriptions->should_hide_widgets( false ) ); } @@ -597,14 +612,14 @@ function () use ( $switch_id ) { ); } - private function mock_wcs_cart_contains_resubscribe( $value ) { + private function mock_wcs_cart_contains_resubscribe( $subscription_id = 0 ) { WC_Subscriptions::wcs_cart_contains_resubscribe( - function () use ( $value ) { - if ( $value ) { + function () use ( $subscription_id ) { + if ( $subscription_id ) { return [ 'product_id' => 42, 'subscription_resubscribe' => [ - 'subscription_id' => 42, + 'subscription_id' => $subscription_id, ], ]; } From 4a1cfc881e17a97ee2ad3e9a4a73a8569fdf7f8f Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 23 Jun 2022 14:50:47 +1200 Subject: [PATCH 10/16] remove a bunch of extraneous whitespace / php linter fixes --- .../Compatibility/WooCommerceSubscriptions.php | 2 +- .../test-class-woocommerce-subscriptions.php | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index 0910a073fa9..5719e0c7ab4 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -159,7 +159,7 @@ public function override_selected_currency( $return ) { $switch_cart_items = $this->get_subscription_switch_cart_items(); if ( 0 < count( $switch_cart_items ) ) { - $switch_cart_item = array_shift( $switch_cart_items ); + $switch_cart_item = array_shift( $switch_cart_items ); $switch_subscription = wcs_get_subscription( $switch_cart_item['subscription_switch']['subscription_id'] ); return $switch_subscription ? $switch_subscription->get_currency() : $return; } diff --git a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php index 0e897d19d69..1f6c6c73ee2 100644 --- a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php +++ b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php @@ -267,7 +267,7 @@ public function test_override_selected_currency_return_false() { // Returns code due to cart contains a subscription renewal. public function test_override_selected_currency_return_currency_code_when_renewal_in_cart() { - // Set up an order with a non-default currency. + // Set up an order with a non-default currency. $order = WC_Helper_Order::create_order(); $order->set_currency( 'JPY' ); $order->save(); @@ -284,14 +284,14 @@ public function test_override_selected_currency_return_currency_code_for_switch_ $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_cart_contains_resubscribe( false ); $this->mock_wcs_get_order_type_cart_items( false ); - + // Set up an order with a non-default currency. // This might need to be a subscription object, not an order. $order = WC_Helper_Order::create_order(); $order->set_currency( 'JPY' ); $order->save(); - - // Blatantly hack mock request params for the test. :) + + // Blatantly hack mock request params for the test. $_GET['switch-subscription'] = $order->get_id(); $_GET['_wcsnonce'] = wp_create_nonce( 'wcs_switch_request' ); @@ -303,7 +303,7 @@ public function test_override_selected_currency_return_currency_code_when_switch // Reset/clear any previous mocked state. $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_cart_contains_resubscribe( false ); - + // Mock order with custom currency for switch cart item. // Note we're using a WC_Order as a stand-in for a true WC_Subscription. $mock_subscription = WC_Helper_Order::create_order(); @@ -328,7 +328,7 @@ public function test_override_selected_currency_return_currency_code_when_resubs // Reset/clear any previous mocked state. $this->mock_wcs_cart_contains_renewal( false ); $this->mock_wcs_get_order_type_cart_items( false ); - + // Mock order with custom currency for switch cart item. // Note we're using a WC_Order as a stand-in for a true WC_Subscription. $mock_subscription = WC_Helper_Order::create_order(); @@ -573,7 +573,7 @@ public function test_should_hide_widgets_return_true_when_switch_found_in_cart() $this->assertTrue( $this->woocommerce_subscriptions->should_hide_widgets( false ) ); } - // Simulate (mock) a renewal in the cart. + // Simulate (mock) a renewal in the cart. // Pass 0 / no args to unmock. private function mock_wcs_cart_contains_renewal( $product_id = 0, $renewal_order_id = 0 ) { WC_Subscriptions::wcs_cart_contains_renewal( From 02cf94e2645b0da4425b783ed6e3c8fe728ee58d Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 23 Jun 2022 14:54:53 +1200 Subject: [PATCH 11/16] remove extraneous whitespace / php linter fixes --- .../compatibility/test-class-woocommerce-subscriptions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php index 1f6c6c73ee2..cd46d8e20eb 100644 --- a/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php +++ b/tests/unit/multi-currency/compatibility/test-class-woocommerce-subscriptions.php @@ -285,7 +285,7 @@ public function test_override_selected_currency_return_currency_code_for_switch_ $this->mock_wcs_cart_contains_resubscribe( false ); $this->mock_wcs_get_order_type_cart_items( false ); - // Set up an order with a non-default currency. + // Set up an order with a non-default currency. // This might need to be a subscription object, not an order. $order = WC_Helper_Order::create_order(); $order->set_currency( 'JPY' ); From 1dda55eb3e69cb8b99765b18d59444b1dc1d779d Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 23 Jun 2022 16:03:31 +1200 Subject: [PATCH 12/16] =?UTF-8?q?add=20local=20wrapper=20for=20wcs=5Fget?= =?UTF-8?q?=5Fsubscription()=20=E2=80=93=20attempt=20psalm=20fix:=20-=20ER?= =?UTF-8?q?ROR:=20UndefinedFunction=20$switch=5Fsubscription=20=3D=20wcs?= =?UTF-8?q?=5Fget=5Fsubscription(=20$switch=5Fcart=5Fitem['subscription=5F?= =?UTF-8?q?switch']['subscription=5Fid']=20);?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Compatibility/WooCommerceSubscriptions.php | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index 5719e0c7ab4..897685643d8 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -160,13 +160,13 @@ public function override_selected_currency( $return ) { $switch_cart_items = $this->get_subscription_switch_cart_items(); if ( 0 < count( $switch_cart_items ) ) { $switch_cart_item = array_shift( $switch_cart_items ); - $switch_subscription = wcs_get_subscription( $switch_cart_item['subscription_switch']['subscription_id'] ); + $switch_subscription = $this->get_subscription( $switch_cart_item['subscription_switch']['subscription_id'] ); return $switch_subscription ? $switch_subscription->get_currency() : $return; } $subscription_resubscribe = $this->cart_contains_resubscribe(); if ( $subscription_resubscribe ) { - $switch_subscription = wcs_get_subscription( $subscription_resubscribe['subscription_resubscribe']['subscription_id'] ); + $switch_subscription = $this->get_subscription( $subscription_resubscribe['subscription_resubscribe']['subscription_id'] ); return $switch_subscription ? $switch_subscription->get_currency() : $return; } @@ -296,6 +296,19 @@ private function get_subscription_switch_cart_items(): array { return wcs_get_order_type_cart_items( 'switch' ); } + /** + * Getter for subscription objects. + * + * @param mixed $the_subscription Post object or post ID of the order. + * @return WC_Subscription|false The subscription object, or false if it cannot be found. + */ + private function get_subscription( $subscription ): array { + if ( ! function_exists( 'wcs_get_subscription' ) ) { + return false; + } + return wcs_get_subscription( $subscription ); + } + /** * Checks $_GET superglobal for a switch id and returns it if found. * From 73780a458cdc89499bb8080c2a4ad0ad31ea5455 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 23 Jun 2022 16:14:41 +1200 Subject: [PATCH 13/16] fix mismatched param name in docblock for get_subscription wrapper --- .../multi-currency/Compatibility/WooCommerceSubscriptions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index 897685643d8..4f404934d34 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -302,11 +302,11 @@ private function get_subscription_switch_cart_items(): array { * @param mixed $the_subscription Post object or post ID of the order. * @return WC_Subscription|false The subscription object, or false if it cannot be found. */ - private function get_subscription( $subscription ): array { + private function get_subscription( $the_subscription ): array { if ( ! function_exists( 'wcs_get_subscription' ) ) { return false; } - return wcs_get_subscription( $subscription ); + return wcs_get_subscription( $the_subscription ); } /** From 76365db46e38b2db0fa60e1d7084dd831f8592aa Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Mon, 27 Jun 2022 13:55:14 +1200 Subject: [PATCH 14/16] remove incorrect return type (array) from get_subscription test helper + + clarify true type in docblock comment --- .../Compatibility/WooCommerceSubscriptions.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index 4f404934d34..583f9edc60c 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -300,9 +300,11 @@ private function get_subscription_switch_cart_items(): array { * Getter for subscription objects. * * @param mixed $the_subscription Post object or post ID of the order. - * @return WC_Subscription|false The subscription object, or false if it cannot be found. + * @return mixed The subscription object, or false if it cannot be found. + * Note: this is WC_Subscription|bool in normal use, but in tests + * we use WC_Order to simulate a subscription (hence `mixed`). */ - private function get_subscription( $the_subscription ): array { + private function get_subscription( $the_subscription ) { if ( ! function_exists( 'wcs_get_subscription' ) ) { return false; } From 6c139f619cfd465855086e9be32766c7d34dc179 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Mon, 27 Jun 2022 14:00:46 +1200 Subject: [PATCH 15/16] remove whitespace offending the linter --- .../multi-currency/Compatibility/WooCommerceSubscriptions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index 583f9edc60c..cb412819a69 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -300,8 +300,8 @@ private function get_subscription_switch_cart_items(): array { * Getter for subscription objects. * * @param mixed $the_subscription Post object or post ID of the order. - * @return mixed The subscription object, or false if it cannot be found. - * Note: this is WC_Subscription|bool in normal use, but in tests + * @return mixed The subscription object, or false if it cannot be found. + * Note: this is WC_Subscription|bool in normal use, but in tests * we use WC_Order to simulate a subscription (hence `mixed`). */ private function get_subscription( $the_subscription ) { From a5ccf31500cf6f27c3b21ed25ca2a120ed729b84 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Wed, 29 Jun 2022 11:44:08 +1200 Subject: [PATCH 16/16] =?UTF-8?q?rename=20subscription=20variable=20?= =?UTF-8?q?=E2=80=93=20it's=20resubscribe=20not=20switch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../multi-currency/Compatibility/WooCommerceSubscriptions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php index cb412819a69..c47b0d8f54d 100644 --- a/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php +++ b/includes/multi-currency/Compatibility/WooCommerceSubscriptions.php @@ -166,8 +166,8 @@ public function override_selected_currency( $return ) { $subscription_resubscribe = $this->cart_contains_resubscribe(); if ( $subscription_resubscribe ) { - $switch_subscription = $this->get_subscription( $subscription_resubscribe['subscription_resubscribe']['subscription_id'] ); - return $switch_subscription ? $switch_subscription->get_currency() : $return; + $subscription = $this->get_subscription( $subscription_resubscribe['subscription_resubscribe']['subscription_id'] ); + return $subscription ? $subscription->get_currency() : $return; } return $return;