From c273067b6a6236883ff623f40589b298c461692d Mon Sep 17 00:00:00 2001 From: "a.chorniy" Date: Wed, 11 Dec 2019 10:19:04 +0200 Subject: [PATCH 1/5] fix for system xml. --- app/code/Magento/Config/Model/Config/Structure.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Config/Model/Config/Structure.php b/app/code/Magento/Config/Model/Config/Structure.php index a380dc82a7c5e..a51a26a80ca46 100644 --- a/app/code/Magento/Config/Model/Config/Structure.php +++ b/app/code/Magento/Config/Model/Config/Structure.php @@ -398,7 +398,7 @@ private function getFieldsRecursively(array $elements = []) $this->getFieldsRecursively($element['children']) ); } else { - if ($element['_elementType'] === 'field' && isset($element['label'])) { + if ($element['_elementType'] === 'field') { $structurePath = (isset($element['path']) ? $element['path'] . '/' : '') . $element['id']; $configPath = isset($element['config_path']) ? $element['config_path'] : $structurePath; From eb408932576b485b668d8c44b0e2bacdda764eff Mon Sep 17 00:00:00 2001 From: "a.chorniy" Date: Wed, 11 Dec 2019 12:16:54 +0200 Subject: [PATCH 2/5] add ignore phpcs:ignore Magento2.Performance.ForeachArrayMerge.ForeachArrayMerge --- app/code/Magento/Config/Model/Config/Structure.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/Config/Model/Config/Structure.php b/app/code/Magento/Config/Model/Config/Structure.php index a51a26a80ca46..a16920f0dc527 100644 --- a/app/code/Magento/Config/Model/Config/Structure.php +++ b/app/code/Magento/Config/Model/Config/Structure.php @@ -292,6 +292,7 @@ public function getFieldPathsByAttribute($attributeName, $attributeValue) foreach ($section['children'] as $group) { if (isset($group['children'])) { $path = $section['id'] . '/' . $group['id']; + // phpcs:ignore Magento2.Performance.ForeachArrayMerge.ForeachArrayMerge $result = array_merge( $result, $this->_getGroupFieldPathsByAttribute( From cfd7771232d328da6c5ed6903b53984e06b027e7 Mon Sep 17 00:00:00 2001 From: Nazar Klovanych Date: Mon, 6 Jan 2020 15:37:41 +0200 Subject: [PATCH 3/5] Cover changes with unit test --- .../Test/Unit/Model/Config/StructureTest.php | 128 ++++++++++++++---- .../Unit/Model/_files/converted_config.php | 14 +- 2 files changed, 108 insertions(+), 34 deletions(-) diff --git a/app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php b/app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php index a17faf8f35883..93f143b99a42d 100644 --- a/app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php +++ b/app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php @@ -50,6 +50,9 @@ class StructureTest extends \PHPUnit\Framework\TestCase */ protected $_structureData; + /** + * @inheritdoc + */ protected function setUp() { $this->_flyweightFactory = $this->getMockBuilder(FlyweightFactory::class) @@ -82,7 +85,12 @@ protected function setUp() ); } - public function testGetTabsBuildsSectionTree() + /** + * Verify tabs build section tree + * + * @return void + */ + public function testGetTabsBuildsSectionTree(): void { $expected = ['tab1' => ['children' => ['section1' => ['tab' => 'tab1']]]]; @@ -108,7 +116,12 @@ public function testGetTabsBuildsSectionTree() $this->assertEquals($this->_tabIteratorMock, $model->getTabs()); } - public function testGetSectionList() + /** + * Verify get section list method + * + * @return void + */ + public function testGetSectionList(): void { $expected = [ 'section1_child_id_1' => true, @@ -152,6 +165,8 @@ public function testGetSectionList() } /** + * Verify Get Element return empty element if element is requested + * * @param string $path * @param string $expectedType * @param string $expectedId @@ -174,6 +189,8 @@ public function testGetElementReturnsEmptyElementIfNotExistingElementIsRequested } /** + * Verify get Element return empty by path element if not exist + * * @param string $path * @param string $expectedType * @param string $expectedId @@ -196,6 +213,8 @@ public function testGetElementReturnsEmptyByConfigPathElementIfNotExistingElemen } /** + * Verify Element return e,pty element if not exists + * * @param string $expectedType * @param string $expectedId * @param string $expectedPath @@ -234,14 +253,24 @@ public function emptyElementDataProvider() ]; } - public function testGetElementReturnsProperElementByPath() + /** + * Verify get element returns proper element by path + * + * @return void + */ + public function testGetElementReturnsProperElementByPath(): void { $elementMock = $this->getElementPathReturnsProperElementByPath(); $this->assertEquals($elementMock, $this->_model->getElement('section_1/group_level_1/field_3')); } - public function testGetElementByConfigPathReturnsProperElementByPath() + /** + * Verify get element by config path return proper path + * + * @return void + */ + public function testGetElementByConfigPathReturnsProperElementByPath(): void { $elementMock = $this->getElementPathReturnsProperElementByPath(); @@ -249,6 +278,8 @@ public function testGetElementByConfigPathReturnsProperElementByPath() } /** + * Build mock element + * * @return Mock */ private function getElementPathReturnsProperElementByPath() @@ -271,7 +302,12 @@ private function getElementPathReturnsProperElementByPath() return $elementMock; } - public function testGetElementByPathPartsIfSectionDataIsEmpty() + /** + * Verefy get element by path part + * + * @return void + */ + public function testGetElementByPathPartsIfSectionDataIsEmpty(): void { $fieldData = [ 'id' => 'field_3', @@ -342,7 +378,12 @@ public function testGetFirstSectionReturnsFirstAllowedSection() $this->assertEquals('currentSection', $this->_model->getFirstSection()->getData()); } - public function testGetElementReturnsProperElementByPathCachesObject() + /** + * Verify get element return element by path caches object + * + * @return void + */ + public function testGetElementReturnsProperElementByPathCachesObject(): void { $elementMock = $this->getElementReturnsProperElementByPathCachesObject(); @@ -350,7 +391,12 @@ public function testGetElementReturnsProperElementByPathCachesObject() $this->assertEquals($elementMock, $this->_model->getElement('section_1/group_level_1/field_3')); } - public function testGetElementByConfigPathReturnsProperElementByPathCachesObject() + /** + * Verify Get Element by id returns proper element + * + * @return void + */ + public function testGetElementByConfigPathReturnsProperElementByPathCachesObject(): void { $elementMock = $this->getElementReturnsProperElementByPathCachesObject(); @@ -393,6 +439,8 @@ public function testGetFieldPathsByAttribute($attributeName, $attributeValue, $p } /** + * DataProvider + * * @return array */ public function getFieldPathsByAttributeDataProvider() @@ -411,33 +459,53 @@ public function getFieldPathsByAttributeDataProvider() ]; } - public function testGetFieldPaths() + /** + * Verify get Fields paths method + * + * @dataProvider getFieldPaths + * @param array $expected + * @return void + */ + public function testGetFieldPaths(array $expected): void { - $expected = [ - 'section/group/field2' => [ - 'field_2' - ], - 'field_3' => [ - 'field_3', - 'field_3' - ], - 'field_3_1' => [ - 'field_3_1' - ], - 'field_3_1_1' => [ - 'field_3_1_1' - ], - 'section/group/field4' => [ - 'field_4', - ], - 'field_5' => [ - 'field_5', - ], - ]; - $this->assertSame( $expected, $this->_model->getFieldPaths() ); } + + /** + * dataprovider for Field Paths + * + * @return array + */ + public function getFieldPaths(): array + { + return [ + [ + [ + 'section/group/field2' => [ + 'field_2' + ], + 'field_3' => [ + 'field_3', + 'field_3' + ], + 'field_3_1' => [ + 'field_3_1' + ], + 'field_3_1_1' => [ + 'field_3_1_1' + ], + 'section/group/field4' => [ + 'field_4', + ], + 'field_5' => [ + 'field_5', + ], + 'section_3' => ['section_3'] + ] + ] + ]; + } } diff --git a/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php b/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php index ef2447338dc07..8f4b152e4dcd1 100644 --- a/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php +++ b/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php @@ -76,7 +76,7 @@ 'translate' => 'label', 'showInWebsite' => '1', 'backend_model' => - \Magento\Config\Model\Config\Backend\Encrypted::class, + \Magento\Config\Model\Config\Backend\Encrypted::class, 'type' => 'text', 'label' => 'Field 3.1.1', '_elementType' => 'field', @@ -158,14 +158,14 @@ ], '_elementType' => 'field', ], - 'field_5' => [ + 'field_5' => [ 'id' => 'field_5', 'translate' => 'label', 'showInWebsite' => '1', 'type' => 'text', 'label' => '', '_elementType' => 'field', - ], + ], ], '_elementType' => 'group', ], @@ -190,7 +190,13 @@ ], '_elementType' => 'section', ], - ], + 'section_3' => [ + 'id' => 'section_3', + 'type' => 'text', + 'tab' => 'tab_1', + '_elementType' => 'field' + ], + ], ], ], ]; From 5e6c59522154c4c72db3d6f2472869bf33657cd8 Mon Sep 17 00:00:00 2001 From: Nazar Klovanych Date: Mon, 6 Jan 2020 15:58:44 +0200 Subject: [PATCH 4/5] Static test fix --- .../Config/Test/Unit/Model/_files/converted_config.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php b/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php index 8f4b152e4dcd1..54fd5719c4881 100644 --- a/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php +++ b/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php @@ -75,8 +75,7 @@ 'id' => 'field_3_1_1', 'translate' => 'label', 'showInWebsite' => '1', - 'backend_model' => - \Magento\Config\Model\Config\Backend\Encrypted::class, + 'backend_model' => \Magento\Config\Model\Config\Backend\Encrypted::class, 'type' => 'text', 'label' => 'Field 3.1.1', '_elementType' => 'field', @@ -196,7 +195,7 @@ 'tab' => 'tab_1', '_elementType' => 'field' ], - ], + ], ], ], ]; From f30a06a6695f590f261953d2b576efcb1ce512a3 Mon Sep 17 00:00:00 2001 From: Nazar Klovanych Date: Tue, 7 Jan 2020 21:05:30 +0200 Subject: [PATCH 5/5] Refactoring config fixtures --- .../Test/Unit/Model/Config/StructureTest.php | 4 +-- .../Unit/Model/_files/converted_config.php | 26 ++++++++++++++++--- .../Test/Unit/Model/_files/system_2.xml | 6 +++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php b/app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php index 93f143b99a42d..57e57fcd2503f 100644 --- a/app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php +++ b/app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php @@ -502,8 +502,8 @@ public function getFieldPaths(): array ], 'field_5' => [ 'field_5', - ], - 'section_3' => ['section_3'] + 'field_5' + ] ] ] ]; diff --git a/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php b/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php index 54fd5719c4881..ed6cc868f8d08 100644 --- a/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php +++ b/app/code/Magento/Config/Test/Unit/Model/_files/converted_config.php @@ -75,7 +75,8 @@ 'id' => 'field_3_1_1', 'translate' => 'label', 'showInWebsite' => '1', - 'backend_model' => \Magento\Config\Model\Config\Backend\Encrypted::class, + 'backend_model' => + \Magento\Config\Model\Config\Backend\Encrypted::class, 'type' => 'text', 'label' => 'Field 3.1.1', '_elementType' => 'field', @@ -192,9 +193,26 @@ 'section_3' => [ 'id' => 'section_3', 'type' => 'text', - 'tab' => 'tab_1', - '_elementType' => 'field' - ], + '_elementType' => 'section', + 'children' => [ + 'group_5' => [ + 'id' => 'group_5', + 'type' => 'text', + 'showInDefault' => 1, + 'showInWebsite' => 1, + 'showInStore' => 1, + '_elementType' => 'group', + 'children' => [ + 'field_5' => [ + 'id' => 'field_5', + 'showInWebsite' => '1', + 'type' => 'text', + '_elementType' => 'field', + ] + ] + ] + ] + ] ], ], ], diff --git a/app/code/Magento/Config/Test/Unit/Model/_files/system_2.xml b/app/code/Magento/Config/Test/Unit/Model/_files/system_2.xml index c4001f47ced0b..715d7cf4e8a61 100644 --- a/app/code/Magento/Config/Test/Unit/Model/_files/system_2.xml +++ b/app/code/Magento/Config/Test/Unit/Model/_files/system_2.xml @@ -86,5 +86,11 @@ +
+ + + + +