Skip to content

Commit

Permalink
8003: Using System Value for Base Currency Results in Config Error.
Browse files Browse the repository at this point in the history
  • Loading branch information
nmalevanec authored and dmanners committed Nov 17, 2017
1 parent 44d2562 commit 38735ac
Show file tree
Hide file tree
Showing 4 changed files with 386 additions and 4 deletions.
21 changes: 17 additions & 4 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 CurrencySystemConfig
*/
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 CurrencySystemConfig|null $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 = [],
CurrencySystemConfig $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(CurrencySystemConfig::class);
}

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

return $defaultCurrencies;
}

Expand All @@ -378,7 +390,8 @@ public function getConfigDefaultCurrencies()
*/
public function getConfigBaseCurrencies()
{
$defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_BASE);
$defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_BASE) ?:
$this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_BASE);
return $defaultCurrencies;
}

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

namespace Magento\Directory\Model;

use Magento\Config\App\Config\Type\System;
use Magento\Framework\App\ResourceConnection;
use Magento\Store\Model\StoreManagerInterface;

/**
* Provide system config values for allowed, base and default currencies.
*/
class CurrencySystemConfig
{
/**
* @var System
*/
private $systemConfig;

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

/**
* @var string
*/
private $path;

/**
* Currency constructor.
*
* @param System $systemConfig
* @param StoreManagerInterface $storeManager
*/
public function __construct(
System $systemConfig,
StoreManagerInterface $storeManager,
ResourceConnection $resources
) {
$this->systemConfig = $systemConfig;
$this->storeManager = $storeManager;
}

/**
* Retrieve config currency data by config path.
*
* @param string $path
* @return array
*/
public function getConfigCurrencies(string $path)
{
$this->path = $path;
$result = array_merge(
$this->getDefaultConfigCurrencies(),
$this->getWebsiteConfigCurrencies(),
$this->getStoreConfigCurrencies()
);
sort($result);

return array_unique($result);
}

/**
* Get system config values as array for default scope.
*
* @return array
*/
private function getDefaultConfigCurrencies()
{
return $this->getConfig($this->path, 'default');
}

/**
* Get system config values as array for website scope.
*
* @return array
*/
private function getWebsiteConfigCurrencies()
{
$websiteResult = [[]];
foreach ($this->storeManager->getWebsites() as $website) {
$websiteResult[] = $this->getConfig($this->path, 'websites', $website->getId());
}
$websiteResult = array_merge(...$websiteResult);

return $websiteResult;
}

/**
* Get system config values as array for store scope.
*
* @return array
*/
private function getStoreConfigCurrencies()
{
$storeResult = [[]];
foreach ($this->storeManager->getStores() as $store) {
$storeResult[] = $this->getConfig($this->path, 'stores', $store->getId());
}
$storeResult = array_merge(...$storeResult);

return $storeResult;
}

/**
* Get system config values as array for specified scope.
*
* @param string $scope
* @param string $scopeId
* @param string $path
* @return array
*/
private function getConfig(string $path, string $scope, string $scopeId = null)
{
$configPath = $scopeId ? sprintf('%s/%s/%s', $scope, $scopeId, $path) : sprintf('%s/%s', $scope, $path);

return explode(',', $this->systemConfig->get($configPath));
}
}
116 changes: 116 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,116 @@
<?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\CurrencySystemConfig;
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
use Magento\Store\Api\Data\StoreInterface;
use Magento\Store\Api\Data\WebsiteInterface;
use Magento\Store\Model\StoreManagerInterface;
use PHPUnit\Framework\TestCase;

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

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

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

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

/**
* @return void
*/
public function testGetConfigCurrencies()
{
$path = 'test/path';
$expected = [
0 => 'ARS',
1 => 'AUD',
3 => 'BZD',
4 => 'CAD',
5 => 'CLP',
6 => 'EUR',
7 => 'USD',
];

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

/** @var WebsiteInterface|\PHPUnit_Framework_MockObject_MockObject $website */
$website = $this->getMockBuilder(WebsiteInterface::class)
->setMethods(['getId'])
->disableOriginalConstructor()
->getMockForAbstractClass();
$website->expects(self::once())
->method('getId')
->willReturn(1);

$this->systemConfig->expects(self::exactly(3))
->method('get')
->withConsecutive(
self::identicalTo('default/test/path'),
self::identicalTo('websites/1/test/path'),
self::identicalTo('stores/1/test/path')
)->willReturnOnConsecutiveCalls(
'USD,EUR',
'AUD,ARS',
'BZD,CAD,AUD,CLP'
);

$this->storeManager->expects(self::once())
->method('getStores')
->willReturn([$store]);
$this->storeManager->expects(self::once())
->method('getWebsites')
->willReturn([$website]);

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

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

0 comments on commit 38735ac

Please sign in to comment.