From 38735ac4b3200e81c5e7f28beb4ec7f4b76916b8 Mon Sep 17 00:00:00 2001 From: magento-engcom-team Date: Fri, 10 Nov 2017 13:08:36 +0200 Subject: [PATCH 1/3] 8003: Using System Value for Base Currency Results in Config Error. --- app/code/Magento/Directory/Model/Currency.php | 21 ++- .../Directory/Model/CurrencySystemConfig.php | 123 +++++++++++++++++ .../Test/Unit/Model/CurrencyConfigTest.php | 116 ++++++++++++++++ .../Model/CurrencySystemConfigTest.php | 130 ++++++++++++++++++ 4 files changed, 386 insertions(+), 4 deletions(-) create mode 100644 app/code/Magento/Directory/Model/CurrencySystemConfig.php create mode 100644 app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php create mode 100644 dev/tests/integration/testsuite/Magento/Directory/Model/CurrencySystemConfigTest.php diff --git a/app/code/Magento/Directory/Model/Currency.php b/app/code/Magento/Directory/Model/Currency.php index a8df4936b8fae..700f6b24f8674 100644 --- a/app/code/Magento/Directory/Model/Currency.php +++ b/app/code/Magento/Directory/Model/Currency.php @@ -6,6 +6,7 @@ namespace Magento\Directory\Model; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Exception\InputException; use Magento\Directory\Model\Currency\Filter; @@ -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 @@ -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( @@ -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, @@ -102,6 +110,7 @@ public function __construct( $this->_directoryHelper = $directoryHelper; $this->_currencyFilterFactory = $currencyFilterFactory; $this->_localeCurrency = $localeCurrency; + $this->currencyConfig = $currencyConfig ?: ObjectManager::getInstance()->get(CurrencySystemConfig::class); } /** @@ -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; @@ -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; } @@ -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; } diff --git a/app/code/Magento/Directory/Model/CurrencySystemConfig.php b/app/code/Magento/Directory/Model/CurrencySystemConfig.php new file mode 100644 index 0000000000000..ce2829a489b4b --- /dev/null +++ b/app/code/Magento/Directory/Model/CurrencySystemConfig.php @@ -0,0 +1,123 @@ +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)); + } +} diff --git a/app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php b/app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php new file mode 100644 index 0000000000000..2a81cc23b7a15 --- /dev/null +++ b/app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php @@ -0,0 +1,116 @@ +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); + } +} diff --git a/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencySystemConfigTest.php b/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencySystemConfigTest.php new file mode 100644 index 0000000000000..e01fd1f5deff9 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencySystemConfigTest.php @@ -0,0 +1,130 @@ +currency = Bootstrap::getObjectManager()->get(CurrencyModel::class); + $this->configResource = Bootstrap::getObjectManager()->get(ConfigInterface::class); + } + + /** + * Test CurrencySystemConfig won't read system config, if values present in DB. + */ + public function testGetConfigCurrenciesWithDbValues() + { + $this->clearCurrencyConfig(); + $allowedCurrencies = 'BDT,BNS,BTD,USD'; + $baseCurrency = 'BDT'; + $this->configResource->saveConfig( + $this->baseCurrencyPath, + $baseCurrency, + ScopeInterface::SCOPE_STORE, + 0 + ); + $this->configResource->saveConfig( + $this->defaultCurrencyPath, + $baseCurrency, + ScopeInterface::SCOPE_STORE, + 0 + ); + $this->configResource->saveConfig( + $this->allowedCurrenciesPath, + $allowedCurrencies, + ScopeInterface::SCOPE_STORE, + 0 + ); + + $expected = [ + 'allowed' => explode(',', $allowedCurrencies), + 'base' => [$baseCurrency], + 'default' => [$baseCurrency], + ]; + self::assertEquals($expected['allowed'], $this->currency->getConfigAllowCurrencies()); + self::assertEquals($expected['base'], $this->currency->getConfigBaseCurrencies()); + self::assertEquals($expected['default'], $this->currency->getConfigDefaultCurrencies()); + } + + /** + * Test CurrencySystemConfig will read system config, if values don't present in DB. + */ + public function testGetConfigCurrenciesWithNoDbValues() + { + $this->clearCurrencyConfig(); + $expected = [ + 'allowed' => [0 => 'EUR',3 => 'USD'], + 'base' => ['USD'], + 'default' => ['USD'], + ]; + self::assertEquals($expected['allowed'], $this->currency->getConfigAllowCurrencies()); + self::assertEquals($expected['base'], $this->currency->getConfigBaseCurrencies()); + self::assertEquals($expected['default'], $this->currency->getConfigDefaultCurrencies()); + } + + /** + * Remove currency config form Db. + * + * @return void + */ + private function clearCurrencyConfig() + { + $this->configResource->deleteConfig( + $this->allowedCurrenciesPath, + ScopeInterface::SCOPE_STORE, + 0 + ); + $this->configResource->deleteConfig( + $this->baseCurrencyPath, + ScopeInterface::SCOPE_STORE, + 0 + ); + $this->configResource->deleteConfig( + $this->defaultCurrencyPath, + ScopeInterface::SCOPE_STORE, + 0 + ); + } +} From b63ceb99d650e23d62c6a03230f01ff7f85b6920 Mon Sep 17 00:00:00 2001 From: magento-engcom-team Date: Mon, 13 Nov 2017 16:06:20 +0200 Subject: [PATCH 2/3] 8003: Using System Value for Base Currency Results in Config Error. --- app/code/Magento/Directory/Model/Currency.php | 20 +- .../Directory/Model/CurrencyConfig.php | 99 +++++++++ .../Directory/Model/CurrencySystemConfig.php | 123 ----------- .../Model/ResourceModel/Currency.php | 2 + .../Test/Unit/Model/CurrencyConfigTest.php | 105 +++++---- .../Directory/Model/CurrencyConfigTest.php | 202 ++++++++++++++++++ .../Model/CurrencySystemConfigTest.php | 130 ----------- 7 files changed, 368 insertions(+), 313 deletions(-) create mode 100644 app/code/Magento/Directory/Model/CurrencyConfig.php delete mode 100644 app/code/Magento/Directory/Model/CurrencySystemConfig.php create mode 100644 dev/tests/integration/testsuite/Magento/Directory/Model/CurrencyConfigTest.php delete mode 100644 dev/tests/integration/testsuite/Magento/Directory/Model/CurrencySystemConfigTest.php diff --git a/app/code/Magento/Directory/Model/Currency.php b/app/code/Magento/Directory/Model/Currency.php index 700f6b24f8674..356b3db639b74 100644 --- a/app/code/Magento/Directory/Model/Currency.php +++ b/app/code/Magento/Directory/Model/Currency.php @@ -67,7 +67,7 @@ class Currency extends \Magento\Framework\Model\AbstractModel protected $_localeCurrency; /** - * @var CurrencySystemConfig + * @var CurrencyConfig */ private $currencyConfig; @@ -82,7 +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 + * @param CurrencyConfig|null $currencyConfig * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -96,7 +96,7 @@ public function __construct( \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null, \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null, array $data = [], - CurrencySystemConfig $currencyConfig = null + CurrencyConfig $currencyConfig = null ) { parent::__construct( $context, @@ -110,7 +110,7 @@ public function __construct( $this->_directoryHelper = $directoryHelper; $this->_currencyFilterFactory = $currencyFilterFactory; $this->_localeCurrency = $localeCurrency; - $this->currencyConfig = $currencyConfig ?: ObjectManager::getInstance()->get(CurrencySystemConfig::class); + $this->currencyConfig = $currencyConfig ?: ObjectManager::getInstance()->get(CurrencyConfig::class); } /** @@ -356,8 +356,7 @@ public function getOutputFormat() */ public function getConfigAllowCurrencies() { - $allowedCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_ALLOW) ?: - $this->currencyConfig->getConfigCurrencies(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; @@ -379,10 +378,7 @@ public function getConfigAllowCurrencies() */ public function getConfigDefaultCurrencies() { - $defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_DEFAULT) ?: - $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_DEFAULT); - - return $defaultCurrencies; + return $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_DEFAULT); } /** @@ -390,9 +386,7 @@ public function getConfigDefaultCurrencies() */ public function getConfigBaseCurrencies() { - $defaultCurrencies = $this->_getResource()->getConfigCurrencies($this, self::XML_PATH_CURRENCY_BASE) ?: - $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_BASE); - return $defaultCurrencies; + return $this->currencyConfig->getConfigCurrencies(self::XML_PATH_CURRENCY_BASE); } /** diff --git a/app/code/Magento/Directory/Model/CurrencyConfig.php b/app/code/Magento/Directory/Model/CurrencyConfig.php new file mode 100644 index 0000000000000..fdb561c224170 --- /dev/null +++ b/app/code/Magento/Directory/Model/CurrencyConfig.php @@ -0,0 +1,99 @@ +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())); + } +} diff --git a/app/code/Magento/Directory/Model/CurrencySystemConfig.php b/app/code/Magento/Directory/Model/CurrencySystemConfig.php deleted file mode 100644 index ce2829a489b4b..0000000000000 --- a/app/code/Magento/Directory/Model/CurrencySystemConfig.php +++ /dev/null @@ -1,123 +0,0 @@ -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)); - } -} diff --git a/app/code/Magento/Directory/Model/ResourceModel/Currency.php b/app/code/Magento/Directory/Model/ResourceModel/Currency.php index ac0716fc4e67e..ffbcce11cb4f6 100644 --- a/app/code/Magento/Directory/Model/ResourceModel/Currency.php +++ b/app/code/Magento/Directory/Model/ResourceModel/Currency.php @@ -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) { diff --git a/app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php b/app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php index 2a81cc23b7a15..9b52bae26f90f 100644 --- a/app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php +++ b/app/code/Magento/Directory/Test/Unit/Model/CurrencyConfigTest.php @@ -7,110 +7,121 @@ namespace Magento\Directory\Test\Unit\Model; use Magento\Config\App\Config\Type\System; -use Magento\Directory\Model\CurrencySystemConfig; +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\Api\Data\WebsiteInterface; use Magento\Store\Model\StoreManagerInterface; use PHPUnit\Framework\TestCase; /** - * Provide tests for CurrencySystemConfig model. + * Provide tests for CurrencyConfig model. */ class CurrencyConfigTest extends TestCase { /** - * @var CurrencySystemConfig + * @var CurrencyConfig */ private $testSubject; /** * @var System|\PHPUnit_Framework_MockObject_MockObject */ - private $systemConfig; + private $config; /** * @var StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject */ private $storeManager; + /** + * @var State|\PHPUnit_Framework_MockObject_MockObject + */ + private $appState; + /** * @inheritdoc */ protected function setUp() { - $this->systemConfig = $this->getMockBuilder(System::class) + $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( - CurrencySystemConfig::class, + CurrencyConfig::class, [ - 'systemConfig' => $this->systemConfig, '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() + public function testGetConfigCurrencies(string $areCode) { $path = 'test/path'; - $expected = [ - 0 => 'ARS', - 1 => 'AUD', - 3 => 'BZD', - 4 => 'CAD', - 5 => 'CLP', - 6 => 'EUR', - 7 => 'USD', - ]; + $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(['getId']) + ->setMethods(['getCode']) ->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); + ->method('getCode') + ->willReturn('testCode'); - $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' - ); + 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->storeManager->expects(self::once()) - ->method('getStores') - ->willReturn([$store]); - $this->storeManager->expects(self::once()) - ->method('getWebsites') - ->willReturn([$website]); + $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], + ]; + } } diff --git a/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencyConfigTest.php b/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencyConfigTest.php new file mode 100644 index 0000000000000..b620d9097b4be --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencyConfigTest.php @@ -0,0 +1,202 @@ +currency = Bootstrap::getObjectManager()->get(CurrencyModel::class); + $this->config = Bootstrap::getObjectManager()->get(ConfigInterface::class); + } + + /** + * Test get currency config for admin and storefront areas. + * + * @dataProvider getConfigCurrenciesDataProvider + * @magentoDataFixture Magento/Store/_files/store.php + * @magentoDbIsolation disabled + * @param string $areaCode + * @param array $expected + * @return void + */ + public function testGetConfigCurrencies(string $areaCode, array $expected) + { + /** @var State $appState */ + $appState = Bootstrap::getObjectManager()->get(State::class); + $appState->setAreaCode($areaCode); + $store = Bootstrap::getObjectManager()->get(Store::class); + $store->load('test', 'code'); + $this->clearCurrencyConfig(); + $this->setStoreConfig($store->getId()); + $storeManager = Bootstrap::getObjectManager()->get(StoreManagerInterface::class); + $storeManager->setCurrentStore($store->getId()); + + if ($areaCode === Area::AREA_ADMINHTML) { + self::assertEquals($expected['allowed'], $this->currency->getConfigAllowCurrencies()); + self::assertEquals($expected['base'], $this->currency->getConfigBaseCurrencies()); + self::assertEquals($expected['default'], $this->currency->getConfigDefaultCurrencies()); + } else { + /** @var StoreManagerInterface $storeManager */ + $storeManager = Bootstrap::getObjectManager()->get(StoreManagerInterface::class); + foreach ($storeManager->getStores() as $store) { + $storeManager->setCurrentStore($store->getId()); + self::assertEquals( + $expected[$store->getCode()]['allowed'], + $this->currency->getConfigAllowCurrencies() + ); + self::assertEquals( + $expected[$store->getCode()]['base'], + $this->currency->getConfigBaseCurrencies() + ); + self::assertEquals( + $expected[$store->getCode()]['default'], + $this->currency->getConfigDefaultCurrencies() + ); + } + } + } + + /** + * Provide test data for getConfigCurrencies test. + * + * @return array + */ + public function getConfigCurrenciesDataProvider() + { + return [ + [ + 'areaCode' => Area::AREA_ADMINHTML, + 'expected' => [ + 'allowed' => ['BDT', 'BNS', 'BTD', 'EUR', 'USD'], + 'base' => ['BDT', 'USD'], + 'default' => ['BDT', 'USD'], + ], + ], + [ + 'areaCode' => Area::AREA_FRONTEND, + 'expected' => [ + 'default' => [ + 'allowed' => ['EUR', 'USD'], + 'base' => ['USD'], + 'default' => ['USD'], + ], + 'test' => [ + 'allowed' => ['BDT', 'BNS', 'BTD', 'USD'], + 'base' => ['BDT'], + 'default' => ['BDT'], + ], + ], + ], + ]; + } + + /** + * Remove currency config form Db. + * + * @return void + */ + private function clearCurrencyConfig() + { + $storeManager = Bootstrap::getObjectManager()->get(StoreManagerInterface::class); + foreach ($storeManager->getStores() as $store) { + $this->config->deleteConfig( + $this->allowedCurrenciesPath, + 'stores', + $store->getId() + ); + $this->config->deleteConfig( + $this->baseCurrencyPath, + 'stores', + $store->getId() + ); + $this->config->deleteConfig( + $this->defaultCurrencyPath, + 'stores', + $store->getId() + ); + } + } + + /** + * Set allowed, base and default currency config values for given store. + * + * @param string $storeId + * @return void + */ + private function setStoreConfig(string $storeId) + { + $allowedCurrencies = 'BDT,BNS,BTD'; + $baseCurrency = 'BDT'; + $this->config->saveConfig( + $this->baseCurrencyPath, + $baseCurrency, + 'stores', + $storeId + ); + $this->config->saveConfig( + $this->defaultCurrencyPath, + $baseCurrency, + 'stores', + $storeId + ); + $this->config->saveConfig( + $this->allowedCurrenciesPath, + $allowedCurrencies, + 'stores', + $storeId + ); + Bootstrap::getObjectManager()->get(ReinitableConfigInterface::class)->reinit(); + Bootstrap::getObjectManager()->create(StoreManagerInterface::class)->reinitStores(); + } + + protected function tearDown() + { + $this->clearCurrencyConfig(); + } +} diff --git a/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencySystemConfigTest.php b/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencySystemConfigTest.php deleted file mode 100644 index e01fd1f5deff9..0000000000000 --- a/dev/tests/integration/testsuite/Magento/Directory/Model/CurrencySystemConfigTest.php +++ /dev/null @@ -1,130 +0,0 @@ -currency = Bootstrap::getObjectManager()->get(CurrencyModel::class); - $this->configResource = Bootstrap::getObjectManager()->get(ConfigInterface::class); - } - - /** - * Test CurrencySystemConfig won't read system config, if values present in DB. - */ - public function testGetConfigCurrenciesWithDbValues() - { - $this->clearCurrencyConfig(); - $allowedCurrencies = 'BDT,BNS,BTD,USD'; - $baseCurrency = 'BDT'; - $this->configResource->saveConfig( - $this->baseCurrencyPath, - $baseCurrency, - ScopeInterface::SCOPE_STORE, - 0 - ); - $this->configResource->saveConfig( - $this->defaultCurrencyPath, - $baseCurrency, - ScopeInterface::SCOPE_STORE, - 0 - ); - $this->configResource->saveConfig( - $this->allowedCurrenciesPath, - $allowedCurrencies, - ScopeInterface::SCOPE_STORE, - 0 - ); - - $expected = [ - 'allowed' => explode(',', $allowedCurrencies), - 'base' => [$baseCurrency], - 'default' => [$baseCurrency], - ]; - self::assertEquals($expected['allowed'], $this->currency->getConfigAllowCurrencies()); - self::assertEquals($expected['base'], $this->currency->getConfigBaseCurrencies()); - self::assertEquals($expected['default'], $this->currency->getConfigDefaultCurrencies()); - } - - /** - * Test CurrencySystemConfig will read system config, if values don't present in DB. - */ - public function testGetConfigCurrenciesWithNoDbValues() - { - $this->clearCurrencyConfig(); - $expected = [ - 'allowed' => [0 => 'EUR',3 => 'USD'], - 'base' => ['USD'], - 'default' => ['USD'], - ]; - self::assertEquals($expected['allowed'], $this->currency->getConfigAllowCurrencies()); - self::assertEquals($expected['base'], $this->currency->getConfigBaseCurrencies()); - self::assertEquals($expected['default'], $this->currency->getConfigDefaultCurrencies()); - } - - /** - * Remove currency config form Db. - * - * @return void - */ - private function clearCurrencyConfig() - { - $this->configResource->deleteConfig( - $this->allowedCurrenciesPath, - ScopeInterface::SCOPE_STORE, - 0 - ); - $this->configResource->deleteConfig( - $this->baseCurrencyPath, - ScopeInterface::SCOPE_STORE, - 0 - ); - $this->configResource->deleteConfig( - $this->defaultCurrencyPath, - ScopeInterface::SCOPE_STORE, - 0 - ); - } -} From 3e0c785820acd4bcae6f0b2c3ff1bc706bc10499 Mon Sep 17 00:00:00 2001 From: magento-engcom-team Date: Tue, 14 Nov 2017 10:23:34 +0200 Subject: [PATCH 3/3] 8003: Using System Value for Base Currency Results in Config Error. --- app/code/Magento/Directory/Model/Currency.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Directory/Model/Currency.php b/app/code/Magento/Directory/Model/Currency.php index 356b3db639b74..0b5b836b4ac93 100644 --- a/app/code/Magento/Directory/Model/Currency.php +++ b/app/code/Magento/Directory/Model/Currency.php @@ -82,7 +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|null $currencyConfig + * @param CurrencyConfig $currencyConfig * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct(