From ab3f767c530763c34bbb0132fd56c2bbf2fa1a16 Mon Sep 17 00:00:00 2001 From: Riccardo Tempesta Date: Mon, 25 Jun 2018 22:01:00 +0200 Subject: [PATCH 1/2] Rework for PR https://github.com/magento/magento2/pull/16222 . When running 'app:config:dump', admin fields get disabled, but clicking save will process them anyway This behaivour causes a validation check on empty fields since the admin form fields are disabled. This commit checks when the field is read only and skips its save. --- app/code/Magento/Config/Model/Config.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/app/code/Magento/Config/Model/Config.php b/app/code/Magento/Config/Model/Config.php index 11ad1ef2a43e9..0721d05e5d910 100644 --- a/app/code/Magento/Config/Model/Config.php +++ b/app/code/Magento/Config/Model/Config.php @@ -5,8 +5,10 @@ */ namespace Magento\Config\Model; +use Magento\Config\Model\Config\Reader\Source\Deployed\SettingChecker; use Magento\Config\Model\Config\Structure\Element\Group; use Magento\Config\Model\Config\Structure\Element\Field; +use Magento\Framework\App\ObjectManager; /** * Backend config model @@ -80,6 +82,11 @@ class Config extends \Magento\Framework\DataObject */ protected $_storeManager; + /** + * @var Config\Reader\Source\Deployed\SettingChecker + */ + private $settingChecker; + /** * @param \Magento\Framework\App\Config\ReinitableConfigInterface $config * @param \Magento\Framework\Event\ManagerInterface $eventManager @@ -88,6 +95,7 @@ class Config extends \Magento\Framework\DataObject * @param \Magento\Config\Model\Config\Loader $configLoader * @param \Magento\Framework\App\Config\ValueFactory $configValueFactory * @param \Magento\Store\Model\StoreManagerInterface $storeManager + * @param Config\Reader\Source\Deployed\SettingChecker|null $settingChecker * @param array $data */ public function __construct( @@ -98,6 +106,7 @@ public function __construct( \Magento\Config\Model\Config\Loader $configLoader, \Magento\Framework\App\Config\ValueFactory $configValueFactory, \Magento\Store\Model\StoreManagerInterface $storeManager, + SettingChecker $settingChecker = null, array $data = [] ) { parent::__construct($data); @@ -108,6 +117,7 @@ public function __construct( $this->_configLoader = $configLoader; $this->_configValueFactory = $configValueFactory; $this->_storeManager = $storeManager; + $this->settingChecker = $settingChecker ?: ObjectManager::getInstance()->get(SettingChecker::class); } /** @@ -351,6 +361,16 @@ protected function _processGroup( // use extra memory $fieldsetData = []; foreach ($groupData['fields'] as $fieldId => $fieldData) { + $isReadOnly = $this->settingChecker->isReadOnly( + $groupPath . '/' . $fieldId, + $this->getScope(), + $this->getScopeCode() + ); + + if ($isReadOnly) { + continue; + } + $field = $this->getField($sectionPath, $groupId, $fieldId); /** @var \Magento\Framework\App\Config\ValueInterface $backendModel */ $backendModel = $field->hasBackendModel() From af78c2fe5a02e5f521f1c984f303063544ef71f6 Mon Sep 17 00:00:00 2001 From: Riccardo Tempesta Date: Tue, 26 Jun 2018 20:51:46 +0200 Subject: [PATCH 2/2] Test coverage for read only fields --- .../Config/Test/Unit/Model/ConfigTest.php | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Config/Test/Unit/Model/ConfigTest.php b/app/code/Magento/Config/Test/Unit/Model/ConfigTest.php index 5e11e46439c8b..fcc1ff8b9c70c 100644 --- a/app/code/Magento/Config/Test/Unit/Model/ConfigTest.php +++ b/app/code/Magento/Config/Test/Unit/Model/ConfigTest.php @@ -60,6 +60,11 @@ class ConfigTest extends \PHPUnit\Framework\TestCase */ protected $_configStructure; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $_settingsChecker; + protected function setUp() { $this->_eventManagerMock = $this->createMock(\Magento\Framework\Event\ManagerInterface::class); @@ -79,7 +84,7 @@ protected function setUp() $this->_transFactoryMock = $this->createPartialMock( \Magento\Framework\DB\TransactionFactory::class, - ['create'] + ['create', 'addObject'] ); $this->_appConfigMock = $this->createMock(\Magento\Framework\App\Config\ReinitableConfigInterface::class); $this->_configLoaderMock = $this->createPartialMock( @@ -90,6 +95,9 @@ protected function setUp() $this->_storeManager = $this->getMockForAbstractClass(\Magento\Store\Model\StoreManagerInterface::class); + $this->_settingsChecker = $this + ->createMock(\Magento\Config\Model\Config\Reader\Source\Deployed\SettingChecker::class); + $this->_model = new \Magento\Config\Model\Config( $this->_appConfigMock, $this->_eventManagerMock, @@ -97,7 +105,8 @@ protected function setUp() $this->_transFactoryMock, $this->_configLoaderMock, $this->_dataFactoryMock, - $this->_storeManager + $this->_storeManager, + $this->_settingsChecker ); } @@ -149,6 +158,49 @@ public function testSaveToCheckAdminSystemConfigChangedSectionEvent() $this->_model->save(); } + public function testDoNotSaveReadOnlyFields() + { + $transactionMock = $this->createMock(\Magento\Framework\DB\Transaction::class); + $this->_transFactoryMock->expects($this->any())->method('create')->will($this->returnValue($transactionMock)); + + $this->_settingsChecker->expects($this->any())->method('isReadOnly')->will($this->returnValue(true)); + $this->_configLoaderMock->expects($this->any())->method('getConfigByPath')->will($this->returnValue([])); + + $this->_model->setGroups(['1' => ['fields' => ['key' => ['data']]]]); + $this->_model->setSection('section'); + + $group = $this->createMock(\Magento\Config\Model\Config\Structure\Element\Group::class); + $group->method('getPath')->willReturn('section/1'); + + $field = $this->createMock(\Magento\Config\Model\Config\Structure\Element\Field::class); + $field->method('getGroupPath')->willReturn('section/1'); + $field->method('getId')->willReturn('key'); + + $this->_configStructure->expects($this->at(0)) + ->method('getElement') + ->with('section/1') + ->will($this->returnValue($group)); + $this->_configStructure->expects($this->at(1)) + ->method('getElement') + ->with('section/1') + ->will($this->returnValue($group)); + $this->_configStructure->expects($this->at(2)) + ->method('getElement') + ->with('section/1/key') + ->will($this->returnValue($field)); + + $backendModel = $this->createPartialMock( + \Magento\Framework\App\Config\Value::class, + ['addData'] + ); + $this->_dataFactoryMock->expects($this->any())->method('create')->will($this->returnValue($backendModel)); + + $this->_transFactoryMock->expects($this->never())->method('addObject'); + $backendModel->expects($this->never())->method('addData'); + + $this->_model->save(); + } + public function testSaveToCheckScopeDataSet() { $transactionMock = $this->createMock(\Magento\Framework\DB\Transaction::class);