MAGETWO-85696: Re saving product attribute [backport 2.1] #11618

Author: Oleksii Korshenko

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

@@ -5,79 +5,95 @@
  * See COPYING.txt for license details.
  */
 
-// @codingStandardsIgnoreFile
-
 namespace Magento\Catalog\Controller\Adminhtml\Product\Attribute;
 
-use Magento\Framework\Exception\AlreadyExistsException;
+use Magento\Backend\App\Action\Context;
+use Magento\Backend\Model\View\Result\Redirect;
+use Magento\Catalog\Controller\Adminhtml\Product\Attribute;
+use Magento\Catalog\Model\Product\AttributeSet\BuildFactory;
+use Magento\Catalog\Helper\Product;
+use Magento\Catalog\Api\Data\ProductAttributeInterface;
+use Magento\Catalog\Model\ResourceModel\Eav\AttributeFactory;
+use Magento\Eav\Model\Entity\Attribute\Set;
+use Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\Validator;
+use Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory;
+use Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\CollectionFactory;
+use Magento\Framework\Cache\FrontendInterface;
 use Magento\Framework\Controller\ResultFactory;
+use Magento\Framework\Controller\Result\Json;
+use Magento\Framework\Exception\AlreadyExistsException;
+use Magento\Framework\Exception\LocalizedException;
+use Magento\Framework\Filter\FilterManager;
+use Magento\Framework\Registry;
+use Magento\Framework\View\LayoutFactory;
+use Magento\Framework\View\Result\PageFactory;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
-class Save extends \Magento\Catalog\Controller\Adminhtml\Product\Attribute
+class Save extends Attribute
 {
     /**
-     * @var \Magento\Catalog\Model\Product\AttributeSet\BuildFactory
+     * @var BuildFactory
      */
     protected $buildFactory;
 
     /**
-     * @var \Magento\Framework\Filter\FilterManager
+     * @var FilterManager
      */
     protected $filterManager;
 
     /**
-     * @var \Magento\Catalog\Helper\Product
+     * @var Product
      */
     protected $productHelper;
 
     /**
-     * @var \Magento\Catalog\Model\ResourceModel\Eav\AttributeFactory
+     * @var AttributeFactory
      */
     protected $attributeFactory;
 
     /**
-     * @var \Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory
+     * @var ValidatorFactory
      */
     protected $validatorFactory;
 
     /**
-     * @var \Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\CollectionFactory
+     * @var CollectionFactory
      */
     protected $groupCollectionFactory;
 
     /**
-     * @var \Magento\Framework\View\LayoutFactory
+     * @var LayoutFactory
      */
     private $layoutFactory;
 
     /**
-     * @param \Magento\Backend\App\Action\Context $context
-     * @param \Magento\Framework\Cache\FrontendInterface $attributeLabelCache
-     * @param \Magento\Framework\Registry $coreRegistry
-     * @param \Magento\Catalog\Model\Product\AttributeSet\BuildFactory $buildFactory
-     * @param \Magento\Framework\View\Result\PageFactory $resultPageFactory
-     * @param \Magento\Catalog\Model\ResourceModel\Eav\AttributeFactory $attributeFactory
-     * @param \Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory $validatorFactory
-     * @param \Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\CollectionFactory $groupCollectionFactory
-     * @param \Magento\Framework\Filter\FilterManager $filterManager
-     * @param \Magento\Catalog\Helper\Product $productHelper
-     * @param \Magento\Framework\View\LayoutFactory $layoutFactory
+     * @param Context $context
+     * @param FrontendInterface $attributeLabelCache
+     * @param Registry $coreRegistry
+     * @param BuildFactory $buildFactory
+     * @param PageFactory $resultPageFactory
+     * @param AttributeFactory $attributeFactory
+     * @param ValidatorFactory $validatorFactory
+     * @param CollectionFactory $groupCollectionFactory
+     * @param FilterManager $filterManager
+     * @param Product $productHelper
+     * @param LayoutFactory $layoutFactory
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
-        \Magento\Backend\App\Action\Context $context,
-        \Magento\Framework\Cache\FrontendInterface $attributeLabelCache,
-        \Magento\Framework\Registry $coreRegistry,
-        \Magento\Framework\View\Result\PageFactory $resultPageFactory,
-        \Magento\Catalog\Model\Product\AttributeSet\BuildFactory $buildFactory,
-        \Magento\Catalog\Model\ResourceModel\Eav\AttributeFactory $attributeFactory,
-        \Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory $validatorFactory,
-        \Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\CollectionFactory $groupCollectionFactory,
-        \Magento\Framework\Filter\FilterManager $filterManager,
-        \Magento\Catalog\Helper\Product $productHelper,
-        \Magento\Framework\View\LayoutFactory $layoutFactory
+        Context $context,
+        FrontendInterface $attributeLabelCache,
+        Registry $coreRegistry,
+        PageFactory $resultPageFactory,
+        BuildFactory $buildFactory,
+        AttributeFactory $attributeFactory,
+        ValidatorFactory $validatorFactory,
+        CollectionFactory $groupCollectionFactory,
+        FilterManager $filterManager,
+        Product $productHelper,
+        LayoutFactory $layoutFactory
     ) {
         parent::__construct($context, $attributeLabelCache, $coreRegistry, $resultPageFactory);
         $this->buildFactory = $buildFactory;
@@ -90,7 +106,7 @@ public function __construct(
     }
 
     /**
-     * @return \Magento\Backend\Model\View\Result\Redirect
+     * @return Redirect
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      * @SuppressWarnings(PHPMD.NPathComplexity)
      * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
@@ -107,31 +123,43 @@ public function execute()
                 $name = trim($name);
 
                 try {
-                    /** @var $attributeSet \Magento\Eav\Model\Entity\Attribute\Set */
+                    /** @var $attributeSet Set */
                     $attributeSet = $this->buildFactory->create()
                         ->setEntityTypeId($this->_entityTypeId)
                         ->setSkeletonId($setId)
                         ->setName($name)
                         ->getAttributeSet();
-
                 } catch (AlreadyExistsException $alreadyExists) {
-                    $this->messageManager->addError(__('An attribute set named \'%1\' already exists.', $name));
+                    $this->messageManager->addErrorMessage(__('An attribute set named \'%1\' already exists.', $name));
                     $this->_session->setAttributeData($data);
                     return $this->returnResult('catalog/*/edit', ['_current' => true], ['error' => true]);
-                } catch (\Magento\Framework\Exception\LocalizedException $e) {
-                    $this->messageManager->addError($e->getMessage());
+                } catch (LocalizedException $e) {
+                    $this->messageManager->addErrorMessage($e->getMessage());
                 } catch (\Exception $e) {
-                    $this->messageManager->addException($e, __('Something went wrong while saving the attribute.'));
+                    $this->messageManager->addExceptionMessage(
+                        $e,
+                        __('Something went wrong while saving the attribute.')
+                    );
                 }
             }
 
             $attributeId = $this->getRequest()->getParam('attribute_id');
-            $attributeCode = $this->getRequest()->getParam('attribute_code')
-                ?: $this->generateCode($this->getRequest()->getParam('frontend_label')[0]);
+
+            /** @var $model ProductAttributeInterface */
+            $model = $this->attributeFactory->create();
+            if ($attributeId) {
+                $model->load($attributeId);
+            }
+            $attributeCode = $model && $model->getId()
+                ? $model->getAttributeCode()
+                : $this->getRequest()->getParam('attribute_code');
+            $attributeCode = $attributeCode ?: $this->generateCode($this->getRequest()->getParam('frontend_label')[0]);
             if (strlen($attributeCode) > 0) {
-                $validatorAttrCode = new \Zend_Validate_Regex(['pattern' => '/^[a-z][a-z_0-9]{0,30}$/']);
+                $validatorAttrCode = new \Zend_Validate_Regex(
+                    ['pattern' => '/^[a-z][a-z_0-9]{0,30}$/']
+                );
                 if (!$validatorAttrCode->isValid($attributeCode)) {
-                    $this->messageManager->addError(
+                    $this->messageManager->addErrorMessage(
                         __(
                             'Attribute code "%1" is invalid. Please use only letters (a-z), ' .
                             'numbers (0-9) or underscore(_) in this field, first character should be a letter.',
@@ -149,11 +177,11 @@ public function execute()
 
             //validate frontend_input
             if (isset($data['frontend_input'])) {
-                /** @var $inputType \Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\Validator */
+                /** @var $inputType Validator */
                 $inputType = $this->validatorFactory->create();
                 if (!$inputType->isValid($data['frontend_input'])) {
                     foreach ($inputType->getMessages() as $message) {
-                        $this->messageManager->addError($message);
+                        $this->messageManager->addErrorMessage($message);
                     }
                     return $this->returnResult(
                         'catalog/*/edit',
@@ -163,18 +191,14 @@ public function execute()
                 }
             }
 
-            /* @var $model \Magento\Catalog\Model\ResourceModel\Eav\Attribute */
-            $model = $this->attributeFactory->create();
-
             if ($attributeId) {
-                $model->load($attributeId);
                 if (!$model->getId()) {
-                    $this->messageManager->addError(__('This attribute no longer exists.'));
+                    $this->messageManager->addErrorMessage(__('This attribute no longer exists.'));
                     return $this->returnResult('catalog/*/', [], ['error' => true]);
                 }
                 // entity type check
                 if ($model->getEntityTypeId() != $this->_entityTypeId) {
-                    $this->messageManager->addError(__('We can\'t update the attribute.'));
+                    $this->messageManager->addErrorMessage(__('We can\'t update the attribute.'));
                     $this->_session->setAttributeData($data);
                     return $this->returnResult('catalog/*/', [], ['error' => true]);
                 }
@@ -196,7 +220,7 @@ public function execute()
 
             $data += ['is_filterable' => 0, 'is_filterable_in_search' => 0, 'apply_to' => []];
 
-            if (is_null($model->getIsUserDefined()) || $model->getIsUserDefined() != 0) {
+            if ($model->getIsUserDefined() === null || $model->getIsUserDefined() != 0) {
                 $data['backend_type'] = $model->getBackendTypeByInput($data['frontend_input']);
             }
 
@@ -242,7 +266,7 @@ public function execute()
 
             try {
                 $model->save();
-                $this->messageManager->addSuccess(__('You saved the product attribute.'));
+                $this->messageManager->addSuccessMessage(__('You saved the product attribute.'));
 
                 $this->_attributeLabelCache->clean();
                 $this->_session->setAttributeData(false);
@@ -253,7 +277,7 @@ public function execute()
                         '_current' => true,
                         'product_tab' => $this->getRequest()->getParam('product_tab'),
                     ];
-                    if (!is_null($attributeSet)) {
+                    if ($attributeSet !== null) {
                         $requestParams['new_attribute_set_id'] = $attributeSet->getId();
                     }
                     return $this->returnResult('catalog/product/addAttribute', $requestParams, ['error' => false]);
@@ -266,7 +290,7 @@ public function execute()
                 }
                 return $this->returnResult('catalog/*/', [], ['error' => false]);
             } catch (\Exception $e) {
-                $this->messageManager->addError($e->getMessage());
+                $this->messageManager->addErrorMessage($e->getMessage());
                 $this->_session->setAttributeData($data);
                 return $this->returnResult(
                     'catalog/*/edit',
@@ -282,7 +306,7 @@ public function execute()
      * @param string $path
      * @param array $params
      * @param array $response
-     * @return \Magento\Framework\Controller\Result\Json|\Magento\Backend\Model\View\Result\Redirect
+     * @return Json|Redirect
      */
     private function returnResult($path = '', array $params = [], array $response = [])
     {
@@ -295,7 +319,6 @@ private function returnResult($path = '', array $params = [], array $response =
             return $this->resultFactory->create(ResultFactory::TYPE_JSON)->setData($response);
         }
         return $this->resultFactory->create(ResultFactory::TYPE_REDIRECT)->setPath($path, $params);
-
     }
 
     /**

dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/AttributeTest.php

@@ -128,12 +128,12 @@ public function testAttributeWithoutId()
      */
     public function testWrongAttributeCode()
     {
-        $postData = $this->_getAttributeData() + ['attribute_id' => '2', 'attribute_code' => '_()&&&?'];
+        $postData = $this->_getAttributeData() + ['attribute_code' => '_()&&&?'];
         $this->getRequest()->setPostValue($postData);
         $this->dispatch('backend/catalog/product_attribute/save');
         $this->assertEquals(302, $this->getResponse()->getHttpResponseCode());
         $this->assertContains(
-            'catalog/product_attribute/edit/attribute_id/2',
+            'catalog/product_attribute/edit',
             $this->getResponse()->getHeader('Location')->getFieldValue()
         );
         /** @var \Magento\Framework\Message\Collection $messages */