known issue Commerce: bug when calculating costToUpgrade()

[LaCollision]

Hi team,

I think I may have found a bug in Commerce, where the cost to upgrade a package is calculated.

In \IPS\nexus\Package->costToUpgrade(), at line 3334:

try
{
	$currency = $purchase->original_invoice->currency;
}
catch ( \Exception $e )
{
	$currency = $purchase->member->defaultCurrency();
}

The $currency used for the calculation is either the purchase original currency, or the member default currency.

But then, at line 3423, a difference is calculated:

$diff = $purchase->renewals->diff( $renewalTermToUse );

 

And if the $currency calculated above is different from the current package currency, this difference throws an exception:

DomainException
currencies_dont_match

/applications/nexus/sources/Purchase/RenewalTerm.php 257

 

In /applications/nexus/sources/Purchase/RenewalTerm.php, at line 257:

/* Sanity check */
if ( $this->cost->currency !== $otherTerm->cost->currency )
{
	throw new \DomainException('currencies_dont_match');
}

 

That's because the 2 RenewalTerms don't have the same currency.

 

Let's get an example:

1. I create a purchase with renewals, for instance in US $.
   So we have $purchase->original_invoice->currency = 'USD'
    
2. Later, I change the purchase renewals for a new currency; Euros for instance.
    
3. Now, I cannot upgrade my initial purchase, as a 'currencies_dont_match' exception is thrown.

 

This exception is thrown because the difference in the cost to upgrade is calculated between a purchase in USD, and my current package currency, which is Euros.

To solve this issue, we should have in \IPS\nexus\Package->costToUpgrade(), at line 3334:

$currency = $purchase->renewal_currency;

… instead of :

try
{
	$currency = $purchase->original_invoice->currency;
}
catch ( \Exception $e )
{
	$currency = $purchase->member->defaultCurrency();
}

 

In this way, the difference will be correctly calculated between two identical currencies, rather than between the currency of the original invoice and my current currency.

Please note that this bug may be present in different parts of Commerce.
There are indeed several instances of '->original_invoice->currency' in Commerce source code, and I think some of them might lead to the 'currencies_dont_match' exception.

Thank you for your help!

[LaCollision]

I can confirm that this bug is present in other parts of Commerce.

For instance in \IPS\nexus\Package->upgradeDowngrade(), at line 3524:

try
{
	$currency = $purchase->original_invoice->currency;
}
catch ( \OutOfRangeException $e )
{
	$currency = $purchase->member->defaultCurrency();
}

 

Here, the consequence of this bug is that after upgrading/downgrading, the purchase ends up with a currency different from its current currency.
If we go back to the example in my message above, then a purchase currently in Euros will end up in US dollars after the upgrade/downgrade.

To solve this bug, the solution is the same:

$currency = $purchase->renewal_currency;

 

Thank you,

[Marc]

Thank you for bringing this issue to our attention! I can confirm this should be further reviewed and I have logged an internal bug report for our development team to investigate and address as necessary, in a future maintenance release.