Magento2: \Magento\Directory\Model\PriceCurrency::format() fails without conversion rate

Created on 12 Oct 2016  路  6Comments  路  Source: magento/magento2

The issue is about the implementation of the method \Magento\Framework\Pricing\PriceCurrencyInterface::format().

Even though no conversion takes place during the formatting of the given amount, the method only works if a conversion rate from the base currency to the specified currency is configured.

There is no technical need for this limitation.

Preconditions

A currency code without a configured conversion rate to the base currency.

Steps to reproduce

Call the method \Magento\Directory\Model\PriceCurrency::format() with a currency code without a configured conversion rate to the base currency.

Expected result

The specified amount should be formatted in the specified currency.

Actual result

The specified amount is surprisingly rendered in the base currency.

Further information

The method format() looks like this:

public function format(
    $amount,
    $includeContainer = true,
    $precision = self::DEFAULT_PRECISION,
    $scope = null,
    $currency = null
) {
    return $this->getCurrency($scope, $currency)
        ->formatPrecision($amount, $precision, [], $includeContainer);
}

This problem occurs in the getCurrency() method where the constraint that the currency needs a configured conversion rate is applies:

public function getCurrency($scope = null, $currency = null)
{
    if ($currency instanceof Currency) {
        $currentCurrency = $currency;
    } elseif (is_string($currency)) {
        $currency = $this->currencyFactory->create()
            ->load($currency);
        $baseCurrency = $this->getStore($scope)
            ->getBaseCurrency();
        $currentCurrency = $baseCurrency->getRate($currency) ? $currency : $baseCurrency;
    } else {
        $currentCurrency = $this->getStore($scope)
            ->getCurrentCurrency();
    }

    return $currentCurrency;
}

With a currency code the middle conditional branch is executed:

        $currency = $this->currencyFactory->create()
            ->load($currency);
        $baseCurrency = $this->getStore($scope)
            ->getBaseCurrency();
        $currentCurrency = $baseCurrency->getRate($currency) ? $currency : $baseCurrency;

This rule makes sense in some contexts, but not in the context of formatting a currency value.

Proposed solution:

Extract the loading of the given currency into a separate method and call that from format() instead of the current getCurrency() method. This way there is no behavioral backward compatibility break for methods calling getCurrency() elsewhere.

Tax Fixed in 2.2.x Clear Description Confirmed Format is valid Ready for Work Reproduced on 2.1.x Reproduced on 2.2.x Reproduced on 2.3.x bug report

Most helpful comment

Hi @Vinai. Thank you for your report.
The issue has been fixed in magento-engcom/magento2ce#1022 by @nmalevanec in 2.2-develop branch
Related commits:

  • 2c4fa0771676b562f84de41069d64aff648172ba
  • ed5ffedd4cd28492f299e5d059414dd3d2540b8e

The fix will be available with the upcoming patch release.

All 6 comments

@Vinai thank you for your feedback.
Please identify which version of Magento you are running.

According to contributor guide, tickets without response for two weeks should be closed.
If this issue still reproducible please feel free to create the new one: format new issue according to the Issue reporting guidelines: with steps to reproduce, actual result and expected result and specify Magento version.

@veloraven please reopen this one. This applies to any 2.x version of Magento as can easily be seen here https://github.com/magento/magento2/commits/develop/lib/internal/Magento/Framework/Pricing/PriceCurrencyInterface.php

@Vinai We've created internal ticket MAGETWO-63501 to address this issue.

@Vinai, thank you for your report.
We've created internal ticket(s) MAGETWO-63501 to track progress on the issue.

Hi @Vinai. Thank you for your report.
The issue has been fixed in magento-engcom/magento2ce#1022 by @nmalevanec in 2.2-develop branch
Related commits:

  • 2c4fa0771676b562f84de41069d64aff648172ba
  • ed5ffedd4cd28492f299e5d059414dd3d2540b8e

The fix will be available with the upcoming patch release.

Was this page helpful?
0 / 5 - 0 ratings