Skip to content

Commit

Permalink
mccy: round to lowest denomination before applying rounding from sett…
Browse files Browse the repository at this point in the history 
…ings

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.
  • Loading branch information
@reykjalin
reykjalin committed Dec 20, 2024
1 parent 915fcc8 commit ff38f59
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 49 deletions.
68 changes: 26 additions & 42 deletions includes/multi-currency/MultiCurrency.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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();
Expand All @@ -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.
*
Expand Down
15 changes: 8 additions & 7 deletions tests/unit/multi-currency/test-class-multi-currency.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,26 +621,26 @@ 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() {
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.
// 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() {
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.
// 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' ) );
}

/**
Expand Down Expand Up @@ -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 ],
Expand Down

0 comments on commit ff38f59

Please sign in to comment.