From ff38f598a9d39bed68f08df96f6c0ef112c919c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Thu, 12 Dec 2024 03:41:17 -0500 Subject: [PATCH] mccy: round to lowest denomination before applying rounding from settings This is consistent with the results of the discussions in paJDYF-g0K-p2. Previous behavior as of #9876 is to ceil the price before applying any rounding options from the settings, but based on the P2 discussions we identified that using `round` instead of `ceil` is the better approach. --- includes/multi-currency/MultiCurrency.php | 68 +++++++------------ .../test-class-multi-currency.php | 15 ++-- 2 files changed, 34 insertions(+), 49 deletions(-) diff --git a/includes/multi-currency/MultiCurrency.php b/includes/multi-currency/MultiCurrency.php index 9ab1ac0f19a..5eb9057f9e1 100644 --- a/includes/multi-currency/MultiCurrency.php +++ b/includes/multi-currency/MultiCurrency.php @@ -832,21 +832,23 @@ public function get_price( $price, string $type ): float { return (float) $price; } - // We must ceil the converted price here so that we don't introduce rounding errors when - // summing up costs. Consider, e.g. a converted price of 10.003 for a 2-decimal currency. - // A single product would cost 10.00, but 2 of them would cost 20.01, _unless_ we round - // the individual parts correctly. $converted_price = ( (float) $price ) * $currency->get_rate(); - $converted_price = $this->ceil_price_for_currency( $converted_price, $currency ); if ( 'tax' === $type || 'coupon' === $type || 'exchange_rate' === $type ) { - return $converted_price; + // We must make sure the price is rounded properly before returning it, otherwise we + // may end up with inconsistent prices in the cart. + $num_decimals = absint( + $this->localization_service->get_currency_format( + $currency->get_code() + )['num_decimals'] + ); + return round( $converted_price, $num_decimals ); } $charm_compatible_types = [ 'product', 'shipping' ]; $apply_charm_pricing = $this->get_apply_charm_only_to_products() - ? 'product' === $type - : in_array( $type, $charm_compatible_types, true ); + ? 'product' === $type + : in_array( $type, $charm_compatible_types, true ); return $this->get_adjusted_price( $converted_price, $apply_charm_pricing, $currency ); } @@ -1336,7 +1338,22 @@ public function is_initialized(): bool { * @return float The adjusted price. */ protected function get_adjusted_price( $price, $apply_charm_pricing, $currency ): float { - $price = $this->ceil_price( $price, (float) $currency->get_rounding() ); + $rounding = (float) $currency->get_rounding(); + + // If rounding is configured to be `0.00` we still need to round to the nearest lowest + // currency denomination. + // Otherwise we ceil the price to the configured rounding option. + if ( 0.00 === $rounding ) { + $num_decimals = absint( + $this->localization_service->get_currency_format( + $currency->get_code() + )['num_decimals'] + ); + + $price = round( $price, $num_decimals ); + } else { + $price = $this->ceil_price( $price, (float) $currency->get_rounding() ); + } if ( $apply_charm_pricing ) { $price += (float) $currency->get_charm(); @@ -1361,39 +1378,6 @@ protected function ceil_price( float $price, float $rounding ): float { return ceil( $price / $rounding ) * $rounding; } - /** - * Ceils the price to the precision dictated by the number of decimals in the provided currency. - * - * For example: US$10.0091 -> US$10.01, JPY 1001.01 -> JPY 1002. - * - * @param float $price The price to be ceiled. - * @param Currency $currency The currency used to figure out the ceil precision. - * - * @return float The ceiled price. - */ - protected function ceil_price_for_currency( float $price, Currency $currency ): float { - // phpcs:disable Squiz.PHP.CommentedOutCode.Found, example comments look like code. - - // Example to explain the math: - // $price = 10.003. - // expected rounding = 10.01. - - // $num_decimals = 2. - // $factor. = 10^2 = 100. - $num_decimals = absint( - $this->localization_service->get_currency_format( - $currency->get_code() - )['num_decimals'] - ); - $factor = 10 ** $num_decimals; // 10^{$num_decimals}. - - // ceil( 10.003 * $factor ) = ceil( 1_000.3 ) = 1_001. - // 1_001 / 100 = 10.01. - return ceil( $price * $factor ) / $factor; // = 10.01. - - // phpcs:enable Squiz.PHP.CommentedOutCode.Found - } - /** * Sets up the available currencies, which are alphabetical by name. * diff --git a/tests/unit/multi-currency/test-class-multi-currency.php b/tests/unit/multi-currency/test-class-multi-currency.php index a5a254ed7ec..fcedcc68e57 100644 --- a/tests/unit/multi-currency/test-class-multi-currency.php +++ b/tests/unit/multi-currency/test-class-multi-currency.php @@ -621,8 +621,8 @@ public function test_get_price_returns_converted_coupon_price_without_adjustment add_filter( 'wcpay_multi_currency_apply_charm_only_to_products', '__return_false' ); // 0.708099 * 10 = 7.08099. - // ceil( 7.08099, 2 ) = 7.09. - $this->assertSame( 7.09, $this->multi_currency->get_price( '10.0', 'coupon' ) ); + // round( 7.08099, 2 ) = 7.08. + $this->assertSame( 7.08, $this->multi_currency->get_price( '10.0', 'coupon' ) ); } public function test_get_price_returns_converted_exchange_rate_without_adjustments() { @@ -630,8 +630,8 @@ public function test_get_price_returns_converted_exchange_rate_without_adjustmen add_filter( 'wcpay_multi_currency_apply_charm_only_to_products', '__return_false' ); // 0.708099 * 10 = 7.08099. - // ceil( 7.08099, 2 ) = 7.09. - $this->assertSame( 7.09, $this->multi_currency->get_price( '10.0', 'exchange_rate' ) ); + // round( 7.08099, 2 ) = 7.08. + $this->assertSame( 7.08, $this->multi_currency->get_price( '10.0', 'exchange_rate' ) ); } public function test_get_price_returns_converted_tax_price() { @@ -639,8 +639,8 @@ public function test_get_price_returns_converted_tax_price() { add_filter( 'wcpay_multi_currency_apply_charm_only_to_products', '__return_false' ); // 0.708099 * 10 = 7.08099. - // ceil( 7.08099, 2 ) = 7.09. - $this->assertSame( 7.09, $this->multi_currency->get_price( '10.0', 'tax' ) ); + // round( 7.08099, 2 ) = 7.08. + $this->assertSame( 7.08, $this->multi_currency->get_price( '10.0', 'tax' ) ); } /** @@ -1017,7 +1017,8 @@ public function test_set_new_customer_currency_meta_does_not_update_user_meta_if public function get_price_provider() { return [ - [ '5.2499', '0.00', 5.25 ], // Even though the precision is 0.00 we make sure the amount is ceiled to the currency's number of digits. + [ '5.2401', '0.00', 5.24 ], // Even though the precision is 0.00 we make sure the amount is rounded to the currency's number of digits. + [ '5.2499', '0.00', 5.25 ], // Even though the precision is 0.00 we make sure the amount is rounded to the currency's number of digits. [ '5.2499', '0.25', 5.25 ], [ '5.2500', '0.25', 5.25 ], [ '5.2501', '0.25', 5.50 ],