Skip to content
This repository was archived by the owner on Apr 29, 2019. It is now read-only.

Commit 38735ac

Browse files
nmalevanecdmanners
nmalevanec
authored and
dmanners
committed
8003: Using System Value for Base Currency Results in Config Error.
1 parent 44d2562 commit 38735ac

File tree

4 files changed

+386
-4
lines changed

4 files changed

+386
-4
lines changed

‎app/code/Magento/Directory/Model/Currency.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
namespace Magento\Directory\Model;
88

9+
use Magento\Framework\App\ObjectManager;
910
use Magento\Framework\Exception\InputException;
1011
use Magento\Directory\Model\Currency\Filter;
1112

@@ -65,6 +66,11 @@ class Currency extends \Magento\Framework\Model\AbstractModel
6566
*/
6667
protected $_localeCurrency;
6768

69+
/**
70+
* @var CurrencySystemConfig
71+
*/
72+
private $currencyConfig;
73+
6874
/**
6975
* @param \Magento\Framework\Model\Context $context
7076
* @param \Magento\Framework\Registry $registry
@@ -76,6 +82,7 @@ class Currency extends \Magento\Framework\Model\AbstractModel
7682
* @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
7783
* @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
7884
* @param array $data
85+
* @param CurrencySystemConfig|null $currencyConfig
7986
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
8087
*/
8188
public function __construct(
@@ -88,7 +95,8 @@ public function __construct(
8895
\Magento\Framework\Locale\CurrencyInterface $localeCurrency,
8996
\Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
9097
\Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
91-
array $data = []
98+
array $data = [],
99+
CurrencySystemConfig $currencyConfig = null
92100
) {
93101
parent::__construct(
94102
$context,
@@ -102,6 +110,7 @@ public function __construct(
102110
$this->_directoryHelper = $directoryHelper;
103111
$this->_currencyFilterFactory = $currencyFilterFactory;
104112
$this->_localeCurrency = $localeCurrency;
113+
$this->currencyConfig = $currencyConfig ?: ObjectManager::getInstance()->get(CurrencySystemConfig::class);
105114
}
106115

107116
/**
@@ -347,7 +356,8 @@ public function getOutputFormat()
347356
*/
348357
public function getConfigAllowCurrencies()
349358
{
350-
$allowedCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_ALLOW);
359+
$allowedCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_ALLOW) ?:
360+
$this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_ALLOW);
351361
$appBaseCurrencyCode = $this->_directoryHelper->getBaseCurrencyCode();
352362
if (!in_array($appBaseCurrencyCode, $allowedCurrencies)) {
353363
$allowedCurrencies[] = $appBaseCurrencyCode;
@@ -369,7 +379,9 @@ public function getConfigAllowCurrencies()
369379
*/
370380
public function getConfigDefaultCurrencies()
371381
{
372-
$defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_DEFAULT);
382+
$defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_DEFAULT) ?:
383+
$this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_DEFAULT);
384+
373385
return $defaultCurrencies;
374386
}
375387

@@ -378,7 +390,8 @@ public function getConfigDefaultCurrencies()
378390
*/
379391
public function getConfigBaseCurrencies()
380392
{
381-
$defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_BASE);
393+
$defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_BASE) ?:
394+
$this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_BASE);
382395
return $defaultCurrencies;
383396
}
384397

‎app/code/Magento/Directory/Model/CurrencySystemConfig.php

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Directory\Model;
8+
9+
use Magento\Config\App\Config\Type\System;
10+
use Magento\Framework\App\ResourceConnection;
11+
use Magento\Store\Model\StoreManagerInterface;
12+
13+
/**
14+
* Provide system config values for allowed, base and default currencies.
15+
*/
16+
class CurrencySystemConfig
17+
{
18+
/**
19+
* @var System
20+
*/
21+
private $systemConfig;
22+
23+
/**
24+
* @var StoreManagerInterface
25+
*/
26+
private $storeManager;
27+
28+
/**
29+
* @var string
30+
*/
31+
private $path;
32+
33+
/**
34+
* Currency constructor.
35+
*
36+
* @param System $systemConfig
37+
* @param StoreManagerInterface $storeManager
38+
*/
39+
public function __construct(
40+
System $systemConfig,
41+
StoreManagerInterface $storeManager,
42+
ResourceConnection $resources
43+
) {
44+
$this->systemConfig = $systemConfig;
45+
$this->storeManager = $storeManager;
46+
}
47+
48+
/**
49+
* Retrieve config currency data by config path.
50+
*
51+
* @param string $path
52+
* @return array
53+
*/
54+
public function getConfigCurrencies(string $path)
55+
{
56+
$this->path = $path;
57+
$result = array_merge(
58+
$this->getDefaultConfigCurrencies(),
59+
$this->getWebsiteConfigCurrencies(),
60+
$this->getStoreConfigCurrencies()
61+
);
62+
sort($result);
63+
64+
return array_unique($result);
65+
}
66+
67+
/**
68+
* Get system config values as array for default scope.
69+
*
70+
* @return array
71+
*/
72+
private function getDefaultConfigCurrencies()
73+
{
74+
return $this->getConfig($this->path, 'default');
75+
}
76+
77+
/**
78+
* Get system config values as array for website scope.
79+
*
80+
* @return array
81+
*/
82+
private function getWebsiteConfigCurrencies()
83+
{
84+
$websiteResult = [[]];
85+
foreach ($this->storeManager->getWebsites() as $website) {
86+
$websiteResult[] = $this->getConfig($this->path, 'websites', $website->getId());
87+
}
88+
$websiteResult = array_merge(...$websiteResult);
89+
90+
return $websiteResult;
91+
}
92+
93+
/**
94+
* Get system config values as array for store scope.
95+
*
96+
* @return array
97+
*/
98+
private function getStoreConfigCurrencies()
99+
{
100+
$storeResult = [[]];
101+
foreach ($this->storeManager->getStores() as $store) {
102+
$storeResult[] = $this->getConfig($this->path, 'stores', $store->getId());
103+
}
104+
$storeResult = array_merge(...$storeResult);
105+
106+
return $storeResult;
107+
}
108+
109+
/**
110+
* Get system config values as array for specified scope.
111+
*
112+
* @param string $scope
113+
* @param string $scopeId
114+
* @param string $path
115+
* @return array
116+
*/
117+
private function getConfig(string $path, string $scope, string $scopeId = null)
118+
{
119+
$configPath = $scopeId ? sprintf('%s/%s/%s', $scope, $scopeId, $path) : sprintf('%s/%s', $scope, $path);
120+
121+
return explode(',', $this->systemConfig->get($configPath));
122+
}
123+
}

‎app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Directory\Test\Unit\Model;
8+
9+
use Magento\Config\App\Config\Type\System;
10+
use Magento\Directory\Model\CurrencySystemConfig;
11+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
12+
use Magento\Store\Api\Data\StoreInterface;
13+
use Magento\Store\Api\Data\WebsiteInterface;
14+
use Magento\Store\Model\StoreManagerInterface;
15+
use PHPUnit\Framework\TestCase;
16+
17+
/**
18+
* Provide tests for CurrencySystemConfig model.
19+
*/
20+
class CurrencyConfigTest extends TestCase
21+
{
22+
/**
23+
* @var CurrencySystemConfig
24+
*/
25+
private $testSubject;
26+
27+
/**
28+
* @var System|\PHPUnit_Framework_MockObject_MockObject
29+
*/
30+
private $systemConfig;
31+
32+
/**
33+
* @var StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject
34+
*/
35+
private $storeManager;
36+
37+
/**
38+
* @inheritdoc
39+
*/
40+
protected function setUp()
41+
{
42+
$this->systemConfig = $this->getMockBuilder(System::class)
43+
->disableOriginalConstructor()
44+
->getMock();
45+
$this->storeManager = $this->getMockBuilder(StoreManagerInterface::class)
46+
->setMethods(['getStores', 'getWebsites'])
47+
->disableOriginalConstructor()
48+
->getMockForAbstractClass();
49+
$objectManager = new ObjectManager($this);
50+
$this->testSubject = $objectManager->getObject(
51+
CurrencySystemConfig::class,
52+
[
53+
'systemConfig' => $this->systemConfig,
54+
'storeManager' => $this->storeManager,
55+
]
56+
);
57+
}
58+
59+
/**
60+
* @return void
61+
*/
62+
public function testGetConfigCurrencies()
63+
{
64+
$path = 'test/path';
65+
$expected = [
66+
0 => 'ARS',
67+
1 => 'AUD',
68+
3 => 'BZD',
69+
4 => 'CAD',
70+
5 => 'CLP',
71+
6 => 'EUR',
72+
7 => 'USD',
73+
];
74+
75+
/** @var StoreInterface|\PHPUnit_Framework_MockObject_MockObject $store */
76+
$store = $this->getMockBuilder(StoreInterface::class)
77+
->setMethods(['getId'])
78+
->disableOriginalConstructor()
79+
->getMockForAbstractClass();
80+
$store->expects(self::once())
81+
->method('getId')
82+
->willReturn(1);
83+
84+
/** @var WebsiteInterface|\PHPUnit_Framework_MockObject_MockObject $website */
85+
$website = $this->getMockBuilder(WebsiteInterface::class)
86+
->setMethods(['getId'])
87+
->disableOriginalConstructor()
88+
->getMockForAbstractClass();
89+
$website->expects(self::once())
90+
->method('getId')
91+
->willReturn(1);
92+
93+
$this->systemConfig->expects(self::exactly(3))
94+
->method('get')
95+
->withConsecutive(
96+
self::identicalTo('default/test/path'),
97+
self::identicalTo('websites/1/test/path'),
98+
self::identicalTo('stores/1/test/path')
99+
)->willReturnOnConsecutiveCalls(
100+
'USD,EUR',
101+
'AUD,ARS',
102+
'BZD,CAD,AUD,CLP'
103+
);
104+
105+
$this->storeManager->expects(self::once())
106+
->method('getStores')
107+
->willReturn([$store]);
108+
$this->storeManager->expects(self::once())
109+
->method('getWebsites')
110+
->willReturn([$website]);
111+
112+
$result = $this->testSubject->getConfigCurrencies($path);
113+
114+
self::assertEquals($expected, $result);
115+
}
116+
}

0 commit comments

Comments
 (0)