Skip to content

Commit

Permalink
MAGETWO-83329: 8003: Using System Value for Base Currency Results in …
Browse files Browse the repository at this point in the history
…Config Error. #11809

 - Merge Pull Request #11809 from nmalevanec/magento2:Github-8003
 - Merged commits:
   1. 38735ac
   2. b63ceb9
   3. 3e0c785
  • Loading branch information
dmanners committed Nov 17, 2017
2 parents f2f94cf + 3e0c785 commit 537cecb
Show file tree
Hide file tree
Showing 5 changed files with 443 additions and 6 deletions.
19 changes: 13 additions & 6 deletions app/code/Magento/Directory/Model/Currency.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

namespace Magento\Directory\Model;

use Magento\Framework\App\ObjectManager;
use Magento\Framework\Exception\InputException;
use Magento\Directory\Model\Currency\Filter;

Expand Down Expand Up @@ -65,6 +66,11 @@ class Currency extends \Magento\Framework\Model\AbstractModel
*/
protected $_localeCurrency;

/**
* @var CurrencyConfig
*/
private $currencyConfig;

/**
* @param \Magento\Framework\Model\Context $context
* @param \Magento\Framework\Registry $registry
Expand All @@ -76,6 +82,7 @@ class Currency extends \Magento\Framework\Model\AbstractModel
* @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
* @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
* @param array $data
* @param CurrencyConfig $currencyConfig
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
Expand All @@ -88,7 +95,8 @@ public function __construct(
\Magento\Framework\Locale\CurrencyInterface $localeCurrency,
\Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
array $data = []
array $data = [],
CurrencyConfig $currencyConfig = null
) {
parent::__construct(
$context,
Expand All @@ -102,6 +110,7 @@ public function __construct(
$this->_directoryHelper = $directoryHelper;
$this->_currencyFilterFactory = $currencyFilterFactory;
$this->_localeCurrency = $localeCurrency;
$this->currencyConfig = $currencyConfig ?: ObjectManager::getInstance()->get(CurrencyConfig::class);
}

/**
Expand Down Expand Up @@ -347,7 +356,7 @@ public function getOutputFormat()
*/
public function getConfigAllowCurrencies()
{
$allowedCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_ALLOW);
$allowedCurrencies = $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_ALLOW);
$appBaseCurrencyCode = $this->_directoryHelper->getBaseCurrencyCode();
if (!in_array($appBaseCurrencyCode, $allowedCurrencies)) {
$allowedCurrencies[] = $appBaseCurrencyCode;
Expand All @@ -369,17 +378,15 @@ public function getConfigAllowCurrencies()
*/
public function getConfigDefaultCurrencies()
{
$defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_DEFAULT);
return $defaultCurrencies;
return $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_DEFAULT);
}

/**
* @return array
*/
public function getConfigBaseCurrencies()
{
$defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_BASE);
return $defaultCurrencies;
return $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_BASE);
}

/**
Expand Down
99 changes: 99 additions & 0 deletions app/code/Magento/Directory/Model/CurrencyConfig.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Directory\Model;

use Magento\Framework\App\Area;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\State;
use Magento\Store\Model\ScopeInterface;
use Magento\Store\Model\StoreManagerInterface;

/**
* Provide config values for allowed, base and default currencies.
*/
class CurrencyConfig
{
/**
* @var State
*/
private $appState;

/**
* @var ScopeConfigInterface
*/
private $config;

/**
* @var StoreManagerInterface
*/
private $storeManager;

/**
* CurrencyConfig constructor.
*
* @param State $appState
* @param ScopeConfigInterface $config
* @param StoreManagerInterface $storeManager
*/
public function __construct(
State $appState,
ScopeConfigInterface $config,
StoreManagerInterface $storeManager
) {
$this->appState = $appState;
$this->config = $config;
$this->storeManager = $storeManager;
}

/**
* Retrieve config currency data by config path.
*
* @param string $path
* @return array
*/
public function getConfigCurrencies(string $path)
{
$result = $this->appState->getAreaCode() === Area::AREA_ADMINHTML
? $this->getConfigForAllStores($path)
: $this->getConfigForCurrentStore($path);
sort($result);

return array_unique($result);
}

/**
* Get allowed, base and default currency codes for all stores.
*
* @param string $path
* @return array
*/
private function getConfigForAllStores(string $path)
{
$storesResult = [[]];
foreach ($this->storeManager->getStores() as $store) {
$storesResult[] = explode(
',',
$this->config->getValue($path, ScopeInterface::SCOPE_STORE, $store->getCode())
);
}

return array_merge(...$storesResult);
}

/**
* Get allowed, base and default currency codes for current store.
*
* @param string $path
* @return mixed
*/
private function getConfigForCurrentStore(string $path)
{
$store = $this->storeManager->getStore();

return explode(',', $this->config->getValue($path, ScopeInterface::SCOPE_STORE, $store->getCode()));
}
}
2 changes: 2 additions & 0 deletions app/code/Magento/Directory/Model/ResourceModel/Currency.php
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ public function saveRates($rates)
* @param string $path
* @return array
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
* @deprecated because doesn't take into consideration scopes and system config values.
* @see \Magento\Directory\Model\CurrencyConfig::getConfigCurrencies()
*/
public function getConfigCurrencies($model, $path)
{
Expand Down
127 changes: 127 additions & 0 deletions app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Directory\Test\Unit\Model;

use Magento\Config\App\Config\Type\System;
use Magento\Directory\Model\CurrencyConfig;
use Magento\Framework\App\Area;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\State;
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
use Magento\Store\Api\Data\StoreInterface;
use Magento\Store\Model\StoreManagerInterface;
use PHPUnit\Framework\TestCase;

/**
* Provide tests for CurrencyConfig model.
*/
class CurrencyConfigTest extends TestCase
{
/**
* @var CurrencyConfig
*/
private $testSubject;

/**
* @var System|\PHPUnit_Framework_MockObject_MockObject
*/
private $config;

/**
* @var StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $storeManager;

/**
* @var State|\PHPUnit_Framework_MockObject_MockObject
*/
private $appState;

/**
* @inheritdoc
*/
protected function setUp()
{
$this->config = $this->getMockBuilder(ScopeConfigInterface::class)
->disableOriginalConstructor()
->getMock();
$this->storeManager = $this->getMockBuilder(StoreManagerInterface::class)
->setMethods(['getStores', 'getWebsites'])
->disableOriginalConstructor()
->getMockForAbstractClass();
$this->appState = $this->getMockBuilder(State::class)
->disableOriginalConstructor()
->getMock();
$objectManager = new ObjectManager($this);
$this->testSubject = $objectManager->getObject(
CurrencyConfig::class,
[
'storeManager' => $this->storeManager,
'appState' => $this->appState,
'config' => $this->config,
]
);
}

/**
* Test get currency config for admin and storefront areas.
*
* @dataProvider getConfigCurrenciesDataProvider
* @return void
*/
public function testGetConfigCurrencies(string $areCode)
{
$path = 'test/path';
$expected = ['ARS', 'AUD', 'BZD'];

$this->appState->expects(self::once())
->method('getAreaCode')
->willReturn($areCode);

/** @var StoreInterface|\PHPUnit_Framework_MockObject_MockObject $store */
$store = $this->getMockBuilder(StoreInterface::class)
->setMethods(['getCode'])
->disableOriginalConstructor()
->getMockForAbstractClass();
$store->expects(self::once())
->method('getCode')
->willReturn('testCode');

if ($areCode === Area::AREA_ADMINHTML) {
$this->storeManager->expects(self::once())
->method('getStores')
->willReturn([$store]);
} else {
$this->storeManager->expects(self::once())
->method('getStore')
->willReturn($store);
}

$this->config->expects(self::once())
->method('getValue')
->with(
self::identicalTo($path)
)->willReturn('ARS,AUD,BZD');

$result = $this->testSubject->getConfigCurrencies($path);

self::assertEquals($expected, $result);
}

/**
* Provide test data for getConfigCurrencies test.
*
* @return array
*/
public function getConfigCurrenciesDataProvider()
{
return [
['areaCode' => Area::AREA_ADMINHTML],
['areaCode' => Area::AREA_FRONTEND],
];
}
}
Loading

0 comments on commit 537cecb

Please sign in to comment.