Skip to content

Commit

Permalink
Fix rounding error with deposit products (#9876)
Browse files Browse the repository at this point in the history
  • Loading branch information
reykjalin authored Dec 6, 2024
1 parent 81ee27e commit a842ba4
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 7 deletions.
4 changes: 4 additions & 0 deletions changelog/fix-rounding-error-with-deposit-products
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fix

Ceil product prices after applying currency conversion, but before charm pricing and price rounding from settings is applied.
38 changes: 38 additions & 0 deletions includes/multi-currency/MultiCurrency.php
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,12 @@ 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;
Expand Down Expand Up @@ -1356,6 +1361,39 @@ 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.
*
Expand Down
17 changes: 10 additions & 7 deletions tests/unit/multi-currency/test-class-multi-currency.php
Original file line number Diff line number Diff line change
Expand Up @@ -620,24 +620,27 @@ public function test_get_price_returns_converted_coupon_price_without_adjustment
WC()->session->set( WCPay\MultiCurrency\MultiCurrency::CURRENCY_SESSION_KEY, 'GBP' );
add_filter( 'wcpay_multi_currency_apply_charm_only_to_products', '__return_false' );

// 0.708099 * 10 = 7,08099
$this->assertSame( 7.08099, $this->multi_currency->get_price( '10.0', 'coupon' ) );
// 0.708099 * 10 = 7.08099.
// ceil( 7.08099, 2 ) = 7.09.
$this->assertSame( 7.09, $this->multi_currency->get_price( '10.0', 'coupon' ) );
}

public function test_get_price_returns_converted_exchange_rate_without_adjustments() {
WC()->session->set( WCPay\MultiCurrency\MultiCurrency::CURRENCY_SESSION_KEY, 'GBP' );
add_filter( 'wcpay_multi_currency_apply_charm_only_to_products', '__return_false' );

// 0.708099 * 10 = 7,08099
$this->assertSame( 7.08099, $this->multi_currency->get_price( '10.0', 'exchange_rate' ) );
// 0.708099 * 10 = 7.08099.
// ceil( 7.08099, 2 ) = 7.09.
$this->assertSame( 7.09, $this->multi_currency->get_price( '10.0', 'exchange_rate' ) );
}

public function test_get_price_returns_converted_tax_price() {
WC()->session->set( WCPay\MultiCurrency\MultiCurrency::CURRENCY_SESSION_KEY, 'GBP' );
add_filter( 'wcpay_multi_currency_apply_charm_only_to_products', '__return_false' );

// 0.708099 * 10 = 7,08099
$this->assertSame( 7.08099, $this->multi_currency->get_price( '10.0', 'tax' ) );
// 0.708099 * 10 = 7.08099.
// ceil( 7.08099, 2 ) = 7.09.
$this->assertSame( 7.09, $this->multi_currency->get_price( '10.0', 'tax' ) );
}

/**
Expand Down Expand Up @@ -1014,7 +1017,7 @@ 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.2499 ],
[ '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.2499', '0.25', 5.25 ],
[ '5.2500', '0.25', 5.25 ],
[ '5.2501', '0.25', 5.50 ],
Expand Down

0 comments on commit a842ba4

Please sign in to comment.