Skip to content

Commit

Permalink
ENGCOM-6460: Fixed ability to save configuration in field without label
Browse files Browse the repository at this point in the history
#25985

 - Merge Pull Request #25985 from AndreyChorniy/magento2:fix-23899-system-xml-validation
 - Merged commits:
   1. c273067
   2. eb40893
   3. cfd7771
   4. 1308074
   5. 5e6c595
   6. f30a06a
  • Loading branch information
magento-engcom-team committed Jan 8, 2020
2 parents 01ab2cd + f30a06a commit c4953a7
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 33 deletions.
3 changes: 2 additions & 1 deletion app/code/Magento/Config/Model/Config/Structure.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -398,7 +399,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;

Expand Down
128 changes: 98 additions & 30 deletions app/code/Magento/Config/Test/Unit/Model/Config/StructureTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ class StructureTest extends \PHPUnit\Framework\TestCase
*/
protected $_structureData;

/**
* @inheritdoc
*/
protected function setUp()
{
$this->_flyweightFactory = $this->getMockBuilder(FlyweightFactory::class)
Expand Down Expand Up @@ -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']]]];

Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -234,21 +253,33 @@ 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();

$this->assertEquals($elementMock, $this->_model->getElementByConfigPath('section_1/group_level_1/field_3'));
}

/**
* Build mock element
*
* @return Mock
*/
private function getElementPathReturnsProperElementByPath()
Expand All @@ -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',
Expand Down Expand Up @@ -342,15 +378,25 @@ 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();

$this->assertEquals($elementMock, $this->_model->getElement('section_1/group_level_1/field_3'));
$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();

Expand Down Expand Up @@ -393,6 +439,8 @@ public function testGetFieldPathsByAttribute($attributeName, $attributeValue, $p
}

/**
* DataProvider
*
* @return array
*/
public function getFieldPathsByAttributeDataProvider()
Expand All @@ -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',
'field_5'
]
]
]
];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,14 @@
],
'_elementType' => 'field',
],
'field_5' => [
'field_5' => [
'id' => 'field_5',
'translate' => 'label',
'showInWebsite' => '1',
'type' => 'text',
'label' => '',
'_elementType' => 'field',
],
],
],
'_elementType' => 'group',
],
Expand All @@ -190,6 +190,29 @@
],
'_elementType' => 'section',
],
'section_3' => [
'id' => 'section_3',
'type' => 'text',
'_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',
]
]
]
]
]
],
],
],
Expand Down
6 changes: 6 additions & 0 deletions app/code/Magento/Config/Test/Unit/Model/_files/system_2.xml
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,11 @@
</depends>
</group>
</section>
<section id="section_3" type="text">
<group id="group_5" type="text" showInDefault="1" showInWebsite="1" showInStore="1">
<field id="field_5" showInWebsite="1" type="text">
</field>
</group>
</section>
</system>
</config>

0 comments on commit c4953a7

Please sign in to comment.