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

Author: Mykola Palamar

app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php

@@ -109,7 +109,7 @@ public function execute()
                 $response,
                 $options
             );
-            $valueOptions = is_array($options['value']) ? $options['value'] : [];
+            $valueOptions = (isset($options['value']) && is_array($options['value'])) ? $options['value'] : [];
             $this->checkEmptyOption($response, $valueOptions);
         }
 

app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php

@@ -15,7 +15,6 @@
 use Magento\Framework\ObjectManagerInterface;
 use Magento\Framework\View\LayoutFactory;
 use Magento\Framework\View\LayoutInterface;
-use Magento\Framework\DataObject;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -62,24 +61,20 @@ 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();
@@ -91,9 +86,10 @@ protected function setUp()
             ->getMock();
         $this->layoutMock = $this->getMockBuilder(LayoutInterface::class)
             ->getMockForAbstractClass();
-        $this->responseDataObject = $this->getMockBuilder(DataObject::class)
-            ->setMethods(null)
-            ->getMock();
+
+        $this->contextMock->expects($this->any())
+            ->method('getObjectManager')
+            ->willReturn($this->objectManagerMock);
     }
 
     /**
@@ -104,17 +100,13 @@ 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', 'multipleselect' => 'option'],
-                'attributeResource' => $this->attributeMock,
-                'attributeSetResource' => $this->attributeSetMock,
-                'escaper' => $this->escaperMock,
-                'responseObject' => $this->responseDataObject,
+            'context' => $this->contextMock,
+            'attributeLabelCache' => $this->attributeLabelCacheMock,
+            'coreRegistry' => $this->coreRegistryMock,
+            'resultPageFactory' => $this->resultPageFactoryMock,
+            'resultJsonFactory' => $this->resultJsonFactoryMock,
+            'layoutFactory' => $this->layoutFactoryMock,
+            'multipleAttributeList' => ['select' => 'option']
             ]
         );
     }
@@ -128,9 +120,22 @@ 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();
@@ -150,11 +155,13 @@ public function testExecute()
     /**
      * @dataProvider provideUniqueData
      * @param array $options
+     * @param boolean $isError
      * @throws \Magento\Framework\Exception\NotFoundException
      */
-    public function testUniqueValidation(array $options)
+    public function testUniqueValidation(array $options, $isError)
     {
-        $this->requestMock->expects($this->any())
+        $countFunctionCalls = ($isError) ? 6 : 5;
+        $this->requestMock->expects($this->exactly($countFunctionCalls))
             ->method('getParam')
             ->willReturnMap([
                 ['frontend_label', null, null],
@@ -164,10 +171,19 @@ public function testUniqueValidation(array $options)
                 ['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);
@@ -189,7 +205,7 @@ public function provideUniqueData()
                         "option_1" => "",
                         "option_2" => "",
                     ]
-                ]
+                ], false
             ],
             'valid options' => [
                 [
@@ -203,7 +219,7 @@ public function provideUniqueData()
                         "option_1" => "",
                         "option_2" => "",
                     ]
-                ]
+                ], false
             ],
             'duplicate options' => [
                 [
@@ -217,7 +233,7 @@ public function provideUniqueData()
                         "option_1" => "",
                         "option_2" => "",
                     ]
-                ]
+                ], true
             ],
             'duplicate and deleted' => [
                 [
@@ -231,7 +247,7 @@ public function provideUniqueData()
                         "option_1" => "1",
                         "option_2" => "",
                     ]
-                ]
+                ], false
             ],
         ];
     }
@@ -253,9 +269,13 @@ public function testEmptyOption(array $options, $result)
                 ['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]
+                ['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'],
             ]);
 
+        $this->objectManagerMock->expects($this->once())
+            ->method('create')
+            ->willReturn($this->attributeMock);
+
         $this->attributeMock->expects($this->once())
             ->method('loadByCode')
             ->willReturnSelf();
@@ -289,9 +309,7 @@ public function provideEmptyOption()
                 ],
                 (object) [
                     'error' => true,
-                    'message' => [
-                        'The value of Admin scope can\'t be empty.'
-                    ]
+                    'message' => 'The value of Admin scope can\'t be empty.',
                 ]
             ],
             'not empty admin scope options' => [