MAGETWO-56487: [GitHub] Multi-select option attribute values do not appear in admin #5877

Mykola Palamar · Mykola Palamar · commit 1d5d114d32aa · 2017-07-05T14:10:59.000+03:00
diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php
@@ -104,10 +104,13 @@ public function execute()
         $multipleOption = null == $multipleOption ? 'select' : $multipleOption;
 
         if (isset($this->multipleAttributeList[$multipleOption]) && !(null == ($multipleOption))) {
+            $options = $this->getRequest()->getParam($this->multipleAttributeList[$multipleOption]);
             $this->checkUniqueOption(
                 $response,
-                $this->getRequest()->getParam($this->multipleAttributeList[$multipleOption])
+                $options
             );
+            $valueOptions = is_array($options['value']) ? $options['value'] : [];
+            $this->checkEmptyOption($response, $valueOptions);
         }
 
         return $this->resultJsonFactory->create()->setJsonData($response->toJson());
@@ -155,13 +158,29 @@ private function setMessageToResponse($response, $messages)
     private function checkUniqueOption(DataObject $response, array $options = null)
     {
         if (is_array($options)
-            && !empty($options['value'])
-            && !empty($options['delete'])
+            && isset($options['value'])
+            && isset($options['delete'])
             && !$this->isUniqueAdminValues($options['value'], $options['delete'])
         ) {
             $this->setMessageToResponse($response, [__("The value of Admin must be unique.")]);
             $response->setError(true);
         }
         return $this;
     }
+
+    /**
+     * Check that admin does not try to create option with empty admin scope option.
+     *
+     * @param DataObject $response
+     * @param array $optionsForCheck
+     */
+    private function checkEmptyOption(DataObject $response, array $optionsForCheck = null)
+    {
+        foreach ($optionsForCheck as $optionValues) {
+            if (isset($optionValues[0]) && $optionValues[0] == '') {
+                $this->setMessageToResponse($response, [__("The value of Admin scope can't be empty.")]);
+                $response->setError(true);
+            }
+        }
+    }
 }
diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/DataProvider.php b/app/code/Magento/Catalog/Model/Product/Attribute/DataProvider.php
@@ -5,15 +5,14 @@
  */
 namespace Magento\Catalog\Model\Product\Attribute;
 
-use Magento\Catalog\Model\Category;
 use Magento\Framework\Stdlib\ArrayManager;
 use Magento\Store\Api\StoreRepositoryInterface;
 use Magento\Catalog\Model\ResourceModel\Product\Attribute\CollectionFactory;
 use Magento\Eav\Model\Entity\Attribute as EavAttribute;
-use Magento\Ui\Component\Form;
 use Magento\Ui\Component\Form\Element\Input;
 use Magento\Ui\Component\Form\Field;
 use Magento\Ui\Component\Form\Element\DataType\Text;
+use Magento\Store\Model\Store;
 
 /**
  * Data provider for the form of adding new product attribute.
@@ -149,40 +148,36 @@ private function customizeOptions($meta)
         $sortOrder = 1;
         foreach ($this->storeRepository->getList() as $store) {
             $storeId = $store->getId();
-
+            $storeLabelConfiguration = [
+                'dataType' => 'text',
+                'formElement' => 'input',
+                'component' => 'Magento_Catalog/js/form/element/input',
+                'template' => 'Magento_Catalog/form/element/input',
+                'prefixName' => 'option.value',
+                'prefixElementName' => 'option_',
+                'suffixName' => (string)$storeId,
+                'label' => $store->getName(),
+                'sortOrder' => $sortOrder,
+                'componentType' => Field::NAME,
+            ];
+            // JS code can't understand 'required-entry' => false|null, we have to avoid even empty property.
+            if ($store->getCode() == Store::ADMIN_CODE) {
+                $storeLabelConfiguration['validation'] = [
+                    'required-entry' => true,
+                ];
+            }
             $meta['attribute_options_select_container']['children']['attribute_options_select']['children']
             ['record']['children']['value_option_' . $storeId] = $this->arrayManager->set(
                 'arguments/data/config',
                 [],
-                [
-                    'dataType' => 'text',
-                    'formElement' => 'input',
-                    'component' => 'Magento_Catalog/js/form/element/input',
-                    'template' => 'Magento_Catalog/form/element/input',
-                    'prefixName' => 'option.value',
-                    'prefixElementName' => 'option_',
-                    'suffixName' => (string)$storeId,
-                    'label' => $store->getName(),
-                    'sortOrder' => $sortOrder,
-                    'componentType' => Field::NAME,
-                ]
+                $storeLabelConfiguration
             );
+
             $meta['attribute_options_multiselect_container']['children']['attribute_options_multiselect']['children']
             ['record']['children']['value_option_' . $storeId] = $this->arrayManager->set(
                 'arguments/data/config',
                 [],
-                [
-                    'dataType' => 'text',
-                    'formElement' => 'input',
-                    'component' => 'Magento_Catalog/js/form/element/input',
-                    'template' => 'Magento_Catalog/form/element/input',
-                    'prefixName' => 'option.value',
-                    'prefixElementName' => 'option_',
-                    'suffixName' => (string)$storeId,
-                    'label' => $store->getName(),
-                    'sortOrder' => $sortOrder,
-                    'componentType' => Field::NAME,
-                ]
+                $storeLabelConfiguration
             );
             ++$sortOrder;
         }
diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php
@@ -15,6 +15,7 @@
 use Magento\Framework\ObjectManagerInterface;
 use Magento\Framework\View\LayoutFactory;
 use Magento\Framework\View\LayoutInterface;
+use Magento\Framework\DataObject;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -61,20 +62,24 @@ class ValidateTest extends AttributeTest
      */
     protected $layoutMock;
 
+    /**
+     * @var DataObject
+     */
+    protected $responseDataObject;
+
     protected function setUp()
     {
         parent::setUp();
         $this->resultJsonFactoryMock = $this->getMockBuilder(ResultJsonFactory::class)
             ->disableOriginalConstructor()
+            ->setMethods(['create'])
             ->getMock();
         $this->resultJson = $this->getMockBuilder(ResultJson::class)
             ->disableOriginalConstructor()
             ->getMock();
         $this->layoutFactoryMock = $this->getMockBuilder(LayoutFactory::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->objectManagerMock = $this->getMockBuilder(ObjectManagerInterface::class)
-            ->getMockForAbstractClass();
         $this->attributeMock = $this->getMockBuilder(Attribute::class)
             ->disableOriginalConstructor()
             ->getMock();
@@ -86,10 +91,9 @@ protected function setUp()
             ->getMock();
         $this->layoutMock = $this->getMockBuilder(LayoutInterface::class)
             ->getMockForAbstractClass();
-
-        $this->contextMock->expects($this->any())
-            ->method('getObjectManager')
-            ->willReturn($this->objectManagerMock);
+        $this->responseDataObject = $this->getMockBuilder(DataObject::class)
+            ->setMethods(null)
+            ->getMock();
     }
 
     /**
@@ -100,13 +104,17 @@ protected function getModel()
         return $this->objectManager->getObject(
             Validate::class,
             [
-            'context' => $this->contextMock,
-            'attributeLabelCache' => $this->attributeLabelCacheMock,
-            'coreRegistry' => $this->coreRegistryMock,
-            'resultPageFactory' => $this->resultPageFactoryMock,
-            'resultJsonFactory' => $this->resultJsonFactoryMock,
-            'layoutFactory' => $this->layoutFactoryMock,
-            'multipleAttributeList' => ['select' => 'option']
+                'context' => $this->contextMock,
+                'attributeLabelCache' => $this->attributeLabelCacheMock,
+                'coreRegistry' => $this->coreRegistryMock,
+                'resultPageFactory' => $this->resultPageFactoryMock,
+                'resultJsonFactory' => $this->resultJsonFactoryMock,
+                'layoutFactory' => $this->layoutFactoryMock,
+                'multipleAttributeList' => ['select' => 'option', 'multipleselect' => 'option'],
+                'attributeResource' => $this->attributeMock,
+                'attributeSetResource' => $this->attributeSetMock,
+                'escaper' => $this->escaperMock,
+                'responseObject' => $this->responseDataObject,
             ]
         );
     }
@@ -120,22 +128,9 @@ public function testExecute()
                 ['attribute_code', null, 'test_attribute_code'],
                 ['new_attribute_set_name', null, 'test_attribute_set_name'],
             ]);
-        $this->objectManagerMock->expects($this->exactly(2))
-            ->method('create')
-            ->willReturnMap([
-                [\Magento\Catalog\Model\ResourceModel\Eav\Attribute::class, [], $this->attributeMock],
-                [\Magento\Eav\Model\Entity\Attribute\Set::class, [], $this->attributeSetMock]
-            ]);
         $this->attributeMock->expects($this->once())
             ->method('loadByCode')
             ->willReturnSelf();
-        $this->requestMock->expects($this->once())
-            ->method('has')
-            ->with('new_attribute_set_name')
-            ->willReturn(true);
-        $this->attributeSetMock->expects($this->once())
-            ->method('setEntityTypeId')
-            ->willReturnSelf();
         $this->attributeSetMock->expects($this->once())
             ->method('load')
             ->willReturnSelf();
@@ -155,13 +150,11 @@ public function testExecute()
     /**
      * @dataProvider provideUniqueData
      * @param array $options
-     * @param boolean $isError
      * @throws \Magento\Framework\Exception\NotFoundException
      */
-    public function testUniqueValidation(array $options, $isError)
+    public function testUniqueValidation(array $options)
     {
-        $countFunctionCalls = ($isError) ? 6 : 5;
-        $this->requestMock->expects($this->exactly($countFunctionCalls))
+        $this->requestMock->expects($this->any())
             ->method('getParam')
             ->willReturnMap([
                 ['frontend_label', null, null],
@@ -171,19 +164,10 @@ public function testUniqueValidation(array $options, $isError)
                 ['message_key', null, Validate::DEFAULT_MESSAGE_KEY]
             ]);
 
-        $this->objectManagerMock->expects($this->once())
-            ->method('create')
-            ->willReturn($this->attributeMock);
-
         $this->attributeMock->expects($this->once())
             ->method('loadByCode')
             ->willReturnSelf();
 
-        $this->requestMock->expects($this->once())
-            ->method('has')
-            ->with('new_attribute_set_name')
-            ->willReturn(false);
-
         $this->resultJsonFactoryMock->expects($this->once())
             ->method('create')
             ->willReturn($this->resultJson);
@@ -205,7 +189,7 @@ public function provideUniqueData()
                         "option_1" => "",
                         "option_2" => "",
                     ]
-                ], false
+                ]
             ],
             'valid options' => [
                 [
@@ -219,7 +203,7 @@ public function provideUniqueData()
                         "option_1" => "",
                         "option_2" => "",
                     ]
-                ], false
+                ]
             ],
             'duplicate options' => [
                 [
@@ -233,7 +217,7 @@ public function provideUniqueData()
                         "option_1" => "",
                         "option_2" => "",
                     ]
-                ], true
+                ]
             ],
             'duplicate and deleted' => [
                 [
@@ -247,8 +231,79 @@ public function provideUniqueData()
                         "option_1" => "1",
                         "option_2" => "",
                     ]
-                ], false
+                ]
             ],
         ];
     }
+
+    /**
+     * Check that empty admin scope labels will trigger error.
+     *
+     * @dataProvider provideEmptyOption
+     * @param array $options
+     * @throws \Magento\Framework\Exception\NotFoundException
+     */
+    public function testEmptyOption(array $options, $result)
+    {
+        $this->requestMock->expects($this->any())
+            ->method('getParam')
+            ->willReturnMap([
+                ['frontend_label', null, null],
+                ['frontend_input', 'select', 'multipleselect'],
+                ['attribute_code', null, "test_attribute_code"],
+                ['new_attribute_set_name', null, 'test_attribute_set_name'],
+                ['option', null, $options],
+                [Validate::ARRAY_MESSAGE_KEY, Validate::DEFAULT_MESSAGE_KEY, Validate::DEFAULT_MESSAGE_KEY]
+            ]);
+
+        $this->attributeMock->expects($this->once())
+            ->method('loadByCode')
+            ->willReturnSelf();
+
+        $this->resultJsonFactoryMock->expects($this->once())
+            ->method('create')
+            ->willReturn($this->resultJson);
+
+        $this->resultJson->expects($this->once())
+            ->method('setJsonData')
+            ->willReturnArgument(0);
+
+        $response = $this->getModel()->execute();
+        $responseObject = json_decode($response);
+        $this->assertEquals($responseObject, $result);
+    }
+
+    /**
+     * Dataprovider for testEmptyOption.
+     *
+     * @return array
+     */
+    public function provideEmptyOption()
+    {
+        return [
+            'empty admin scope options' => [
+                [
+                    'value' => [
+                        "option_0" => [''],
+                    ],
+                ],
+                (object) [
+                    'error' => true,
+                    'message' => [
+                        'The value of Admin scope can\'t be empty.'
+                    ]
+                ]
+            ],
+            'not empty admin scope options' => [
+                [
+                    'value' => [
+                        "option_0" => ['asdads'],
+                    ],
+                ],
+                (object) [
+                    'error' => false,
+                ]
+            ]
+        ];
+    }
 }
diff --git a/app/code/Magento/Catalog/etc/adminhtml/di.xml b/app/code/Magento/Catalog/etc/adminhtml/di.xml
@@ -51,6 +51,7 @@
         <arguments>
             <argument name="multipleAttributeList" xsi:type="array">
                 <item name="select" xsi:type="string">option</item>
+                <item name="multiselect" xsi:type="string">option</item>
             </argument>
         </arguments>
     </type>
diff --git a/app/code/Magento/Catalog/view/adminhtml/web/template/form/element/input.html b/app/code/Magento/Catalog/view/adminhtml/web/template/form/element/input.html
@@ -17,3 +17,4 @@
             id: uid,
             disabled: disabled
     }"/>
+<label class="admin__field-error" if="error" attr="for: uid" text="error"/>
diff --git a/app/code/Magento/Ui/view/base/web/templates/dynamic-rows/templates/default.html b/app/code/Magento/Ui/view/base/web/templates/dynamic-rows/templates/default.html
@@ -22,9 +22,8 @@
             <table class="admin__dynamic-rows admin__control-table" data-role="grid" attr="{'data-index': index}">
                 <thead if="element.columnsHeader">
                     <tr>
-                        <th if="dndConfig.enabled"/>
+                        <th if="dndConfig.enabled" />
                         <th repeat="foreach: labels, item: '$label'"
-                            text="$label().label"
                             css="setClasses($label())"
                             visible="$label().visible"
                             disable="$label().disabled">
