Nopcommerce: ConvertToPrimaryStoreCurrency bug

Created on 17 Aug 2016  路  11Comments  路  Source: nopSolutions/nopCommerce

Scenario:

  • Primary store currency is DKK
  • The primary exchange currency is EUR
  • The exchange rate for EUR is thus 1 and ~7.5 for DKK
  • I have an amount of money in EUR
  • I want to convert this amount to the primary store currency (DKK)

    Approach

  • I fetch the sourceCurrency (EUR) using the currency service

  • I convert the amount using ConvertToPrimaryStoreCurrency(amount, sourceCurrency)

    Expected result

  • I get the amount back converted to the primary store currency (DKK) from EUR ie. amount/~7.5

    Actual result

  • I get the same amount back ie. if I got 40 EUR I get 40 "DKK" back, even though the rates are correct.

bug

All 11 comments

I cannot reproduce it.

"I fetch the sourceCurrency (EUR) using the currency service". What currency do you fetch? Try to debug ConvertToPrimaryStoreCurrency method of \Nop.Services\Directory\CurrencyService.cs in order to find issue in your case

I cannot reproduce it.

Strange. Am I misunderstanding the semantics? As I understand it ConvertToPrimaryStoreCurrency converts an amount _from_ a source currency into the currency set as the primary store currency.

So say the primary store currency is DKK, and I have an amount in another currency. As I understand it ConvertToPrimaryStoreCurrency takes that amount along with the currency object of the source currency and converts the amount into DKK. Is this incorrect?

What currency do you fetch?

I fetch the currency using the currency service with the currency code "EUR" ie. I fetch the currency object representing euros, so in pseudo-C#

var euro = currencyService.GetCurrencyByCode("EUR");
var amount = 50m;
var converted = currencyService.ConvertToPrimaryStoreCurrency(amount, euro);

In my scenario I have "DKK" as primary store currency, and "EUR" as the exchange rate currency. The Rate of "EUR" is 1, and the rate of "DKK" is ~7.5. So they are materially different. Yet I get converted == 50m back out.

If I instead do

var euro = currencyService.GetCurrencyByCode("EUR");
var dkk = currencyService.GetCurrencyByCode("DKK");
var amount = 50m;
var converted = currencyService.ConvertCurrency(amount, euro, dkk);

I get the correct amount out, but those results should have been equivalent!

    public virtual decimal ConvertToPrimaryStoreCurrency(decimal amount, Currency sourceCurrencyCode)
    {
        if (sourceCurrencyCode == null)
            throw new ArgumentNullException("sourceCurrencyCode");

        var primaryStoreCurrency = GetCurrencyById(_currencySettings.PrimaryStoreCurrencyId);
        if (primaryStoreCurrency == null)
            throw new Exception("Primary store currency cannot be loaded");

        decimal result = amount;
        if (result != decimal.Zero && sourceCurrencyCode.Id != primaryStoreCurrency.Id)
        {
            decimal exchangeRate = sourceCurrencyCode.Rate;
            if (exchangeRate == decimal.Zero)
                throw new NopException(string.Format("Exchange rate not found for currency [{0}]", sourceCurrencyCode.Name));
            result = result / exchangeRate;
        }
        return result;
    }

This is the code for ConvertToPrimaryStoreCurrency. Notice that the rate of the source currency is the only rate used, not the rate for the primary store currency. The primary store currency is only used to check that it is not the same as the source currency.

So for my example it simply checks euro.Id != dkk.Id and then divides the amount with euro.Rate. This does not convert to the primary store currency, this converts to the _exchange rate_ currency. I imagine your test cases have both the exchange rate and primary store currency be the same?

To reduce points of failure I would reuse ConvertCurrency and propose something like

public virtual decimal ConvertToPrimaryStoreCurrency(decimal amount, Currency sourceCurrency)
{
    if (sourceCurrency == null)
        throw new ArgumentNullException("sourceCurrency");

    var primaryStoreCurrency = GetCurrencyById(_currencySettings.PrimaryStoreCurrencyId);
    if (primaryStoreCurrency == null)
        throw new Exception("Primary store currency cannot be loaded");

    return ConvertCurrency(amount, sourceCurrency, primaryStoreCurrency);
}

please share screenshot of your currency list page

I can't from my current position due to workplace restrictions.

That said, please read my two last comments as they clearly describe both the scenario, the point of failure in the code, and how to fix it.

could you please execute the following SQL command and share results here? SELECT * FROM [Currency]

If you insist, but the bug location has already been identified and described. The code for ConvertToPrimaryStoreCurrency does not even _touch_ the required information to be able to make the described conversion.

image

To reiterate what I have already described, ConvertToPrimaryStoreCurrency returns the amount converted to the primary exchange rate currency. It misses the step where it actually uses the primary store currency to convert it from that into the expected result.

Thorbj酶rn, you're absolutely right. After some investigation I can confirm it. I've just fixed it. Thanks!

Happy to be of assistance.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jigarsangoi picture jigarsangoi  路  4Comments

AndreiMaz picture AndreiMaz  路  4Comments

mrnams picture mrnams  路  3Comments

mariannk picture mariannk  路  5Comments

JohnsonJohnsonYuan picture JohnsonJohnsonYuan  路  5Comments