From 38735ac4b3200e81c5e7f28beb4ec7f4b76916b8 Mon Sep 17 00:00:00 2001 From: magento-engcom-team Date: Fri, 10 Nov 2017 13:08:36 +0200 Subject: [PATCH] 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 + ); + } +}