ENGCOM-2870: [Forwardport] Unable to change attribute type from swatch #17750

Author: Stanislav Idolov

app/code/Magento/Swatches/Model/Plugin/EavAttribute.php

@@ -9,6 +9,7 @@
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Exception\InputException;
 use Magento\Framework\Serialize\Serializer\Json;
+use Magento\Swatches\Model\ResourceModel\Swatch as SwatchResource;
 use Magento\Swatches\Model\Swatch;
 
 /**
@@ -18,6 +19,11 @@ class EavAttribute
 {
     const DEFAULT_STORE_ID = 0;
 
+    /**
+     * @var SwatchResource
+     */
+    private $swatchResource;
+
     /**
      * Base option title used for string operations to detect is option already exists or new
      */
@@ -64,17 +70,20 @@ class EavAttribute
      * @param \Magento\Swatches\Model\SwatchFactory $swatchFactory
      * @param \Magento\Swatches\Helper\Data $swatchHelper
      * @param Json|null $serializer
+     * @param SwatchResource|null $swatchResource
      */
     public function __construct(
         \Magento\Swatches\Model\ResourceModel\Swatch\CollectionFactory $collectionFactory,
         \Magento\Swatches\Model\SwatchFactory $swatchFactory,
         \Magento\Swatches\Helper\Data $swatchHelper,
-        Json $serializer = null
+        Json $serializer = null,
+        SwatchResource $swatchResource = null
     ) {
         $this->swatchCollectionFactory = $collectionFactory;
         $this->swatchFactory = $swatchFactory;
         $this->swatchHelper = $swatchHelper;
         $this->serializer = $serializer ?: ObjectManager::getInstance()->create(Json::class);
+        $this->swatchResource = $swatchResource ?: ObjectManager::getInstance()->create(SwatchResource::class);
     }
 
     /**
@@ -148,6 +157,7 @@ protected function setProperOptionsArray(Attribute $attribute)
      * Prepare attribute for conversion from any swatch type to dropdown
      *
      * @param Attribute $attribute
+     * @throws \Magento\Framework\Exception\LocalizedException
      * @return void
      */
     protected function convertSwatchToDropdown(Attribute $attribute)
@@ -157,6 +167,8 @@ protected function convertSwatchToDropdown(Attribute $attribute)
             if (!empty($additionalData)) {
                 $additionalData = $this->serializer->unserialize($additionalData);
                 if (is_array($additionalData) && isset($additionalData[Swatch::SWATCH_INPUT_TYPE_KEY])) {
+                    $option = $attribute->getOption() ?: [];
+                    $this->cleanEavAttributeOptionSwatchValues($option);
                     unset($additionalData[Swatch::SWATCH_INPUT_TYPE_KEY]);
                     $attribute->setData('additional_data', $this->serializer->serialize($additionalData));
                 }
@@ -235,6 +247,8 @@ protected function saveSwatchParams(Attribute $attribute)
     {
         if ($this->swatchHelper->isVisualSwatch($attribute)) {
             $this->processVisualSwatch($attribute);
+            $attributeOptions = $attribute->getOptiontext() ?: [];
+            $this->cleanTextSwatchValuesAfterSwitch($attributeOptions);
         } elseif ($this->swatchHelper->isTextSwatch($attribute)) {
             $this->processTextualSwatch($attribute);
         }
@@ -267,6 +281,33 @@ protected function processVisualSwatch(Attribute $attribute)
         }
     }
 
+    /**
+     * Clean swatch option values after switching to the dropdown type.
+     *
+     * @param array $attributeOptions
+     * @param int|null  $swatchType
+     * @throws \Magento\Framework\Exception\LocalizedException
+     */
+    private function cleanEavAttributeOptionSwatchValues(array $attributeOptions, int $swatchType = null)
+    {
+        if (count($attributeOptions) && isset($attributeOptions['value'])) {
+            $optionsIDs = array_keys($attributeOptions['value']);
+
+            $this->swatchResource->clearSwatchOptionByOptionIdAndType($optionsIDs, $swatchType);
+        }
+    }
+
+    /**
+     * Cleaning the text type of swatch option values after switching.
+     *
+     * @param array $attributeOptions
+     * @throws \Magento\Framework\Exception\LocalizedException
+     */
+    private function cleanTextSwatchValuesAfterSwitch(array $attributeOptions)
+    {
+        $this->cleanEavAttributeOptionSwatchValues($attributeOptions, Swatch::SWATCH_TYPE_TEXTUAL);
+    }
+
     /**
      * @param string $value
      * @return int
@@ -432,7 +473,7 @@ protected function validateOptions(Attribute $attribute)
             $options = $attribute->getData('optiontext');
         }
         if ($options && !$this->isOptionsValid($options, $attribute)) {
-            throw new InputException(__('Admin is a required field in the each row'));
+            throw new InputException(__('Admin is a required field in each row'));
         }
         return true;
     }

app/code/Magento/Swatches/Model/ResourceModel/Swatch.php

@@ -37,4 +37,24 @@ public function saveDefaultSwatchOption($id, $defaultValue)
             $this->getConnection()->update($this->getTable('eav_attribute'), $bind, $where);
         }
     }
+
+    /**
+     * Cleaned swatch option values when switching to dropdown input type.
+     *
+     * @param array $optionIDs
+     * @param int $type
+     * @throws \Magento\Framework\Exception\LocalizedException
+     */
+    public function clearSwatchOptionByOptionIdAndType(array $optionIDs, int $type = null)
+    {
+        if (count($optionIDs)) {
+            foreach ($optionIDs as $optionId) {
+                $where = ['option_id' => $optionId];
+                if ($type !== null) {
+                    $where['type = ?'] = $type;
+                }
+                $this->getConnection()->delete($this->getMainTable(), $where);
+            }
+        }
+    }
 }

app/code/Magento/Swatches/Test/Unit/Model/Plugin/EavAttributeTest.php

@@ -191,7 +191,7 @@ public function testBeforeSaveTextSwatch()
 
     /**
      * @expectedException \Magento\Framework\Exception\InputException
-     * @expectedExceptionMessage Admin is a required field in the each row
+     * @expectedExceptionMessage Admin is a required field in each row
      */
     public function testBeforeSaveWithFailedValidation()
     {

app/code/Magento/Swatches/Test/Unit/Model/SwatchAttributesProviderTest.php

@@ -3,10 +3,10 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\Swatches\Test\Unit\Model;
 
 use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
-use Magento\Catalog\Model\ResourceModel\Eav\Attribute;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use Magento\Swatches\Model\SwatchAttributeCodes;
 use Magento\Swatches\Model\SwatchAttributesProvider;
@@ -35,26 +35,26 @@ class SwatchAttributesProviderTest extends \PHPUnit\Framework\TestCase
     private $productMock;
 
     /**
-     * @var SwatchAttributeType|\PHPUnit_Framework_MockObject_MockObject
+     * @var SwatchAttributeType | \PHPUnit_Framework_MockObject_MockObject
      */
-    private $swatchTypeCheckerMock;
+    private $swatchTypeChecker;
 
     protected function setUp()
     {
         $this->typeConfigurable = $this->createPartialMock(
             Configurable::class,
-            ['getConfigurableAttributes', 'getCodes']
+            ['getConfigurableAttributes', 'getCodes', 'getProductAttribute']
         );
 
         $this->swatchAttributeCodes = $this->createMock(SwatchAttributeCodes::class);
 
         $this->productMock = $this->createPartialMock(\Magento\Catalog\Model\Product::class, ['getId', 'getTypeId']);
-        $this->swatchTypeCheckerMock = $this->createMock(SwatchAttributeType::class);
+        $this->swatchTypeChecker = $this->createMock(SwatchAttributeType::class);
 
         $this->swatchAttributeProvider = (new ObjectManager($this))->getObject(SwatchAttributesProvider::class, [
             'typeConfigurable' => $this->typeConfigurable,
             'swatchAttributeCodes' => $this->swatchAttributeCodes,
-            'swatchTypeChecker' => $this->swatchTypeCheckerMock,
+            'swatchTypeChecker' => $this->swatchTypeChecker,
         ]);
     }
 
@@ -64,8 +64,9 @@ public function testProvide()
         $this->productMock->method('getTypeId')
             ->willReturn(Configurable::TYPE_CODE);
 
-        $productAttributeMock = $this->getMockBuilder(Attribute::class)
+        $attributeMock =  $this->getMockBuilder(\Magento\Catalog\Model\ResourceModel\Eav\Attribute::class)
             ->disableOriginalConstructor()
+            ->setMethods(['setStoreId', 'getData', 'setData', 'getSource', 'hasData'])
             ->getMock();
 
         $configAttributeMock = $this->createPartialMock(
@@ -78,7 +79,7 @@ public function testProvide()
 
         $configAttributeMock
             ->method('getProductAttribute')
-            ->willReturn($productAttributeMock);
+            ->willReturn($attributeMock);
 
         $this->typeConfigurable
             ->method('getConfigurableAttributes')
@@ -90,12 +91,10 @@ public function testProvide()
             ->method('getCodes')
             ->willReturn($swatchAttributes);
 
-        $this->swatchTypeCheckerMock->expects($this->once())->method('isSwatchAttribute')->willReturn(true);
+        $this->swatchTypeChecker->expects($this->once())->method('isSwatchAttribute')->willReturn(true);
+
         $result = $this->swatchAttributeProvider->provide($this->productMock);
 
-        $this->assertEquals(
-            [1 => $productAttributeMock],
-            $result
-        );
+        $this->assertEquals([1 => $attributeMock], $result);
     }
 }

app/code/Magento/Swatches/i18n/en_US.csv

@@ -1,4 +1,4 @@
-"Admin is a required field in the each row","Admin is a required field in the each row"
+"Admin is a required field in each row","Admin is a required field in each row"
 "Update Product Preview Image","Update Product Preview Image"
 "Filtering by this attribute will update the product image on catalog page","Filtering by this attribute will update the product image on catalog page"
 "Use Product Image for Swatch if Possible","Use Product Image for Swatch if Possible"

app/code/Magento/Ui/i18n/en_US.csv

@@ -180,7 +180,7 @@ CSV,CSV
 "Please enter a valid value from list","Please enter a valid value from list"
 "Please enter valid SKU key.","Please enter valid SKU key."
 "Please enter a valid number.","Please enter a valid number."
-"Admin is a required field in the each row.","Admin is a required field in the each row."
+"Admin is a required field in each row.","Admin is a required field in each row."
 "Please fix this field.","Please fix this field."
 "Please enter a valid date (ISO).","Please enter a valid date (ISO)."
 "Please enter only digits.","Please enter only digits."

lib/web/i18n/en_US.csv

@@ -95,7 +95,7 @@ Submit,Submit
 "Please enter valid SKU key.","Please enter valid SKU key."
 "Please enter a valid number.","Please enter a valid number."
 "This is required field","This is required field"
-"Admin is a required field in the each row.","Admin is a required field in the each row."
+"Admin is a required field in each row.","Admin is a required field in each row."
 "Password cannot be the same as email address.","Password cannot be the same as email address."
 "Please fix this field.","Please fix this field."
 "Please enter a valid email address.","Please enter a valid email address."

lib/web/mage/validation.js

@@ -1562,15 +1562,15 @@
         ],
         'required-text-swatch-entry': [
             tableSingleValidation,
-            $.mage.__('Admin is a required field in the each row.')
+            $.mage.__('Admin is a required field in each row.')
         ],
         'required-visual-swatch-entry': [
             tableSingleValidation,
-            $.mage.__('Admin is a required field in the each row.')
+            $.mage.__('Admin is a required field in each row.')
         ],
         'required-dropdown-attribute-entry': [
             tableSingleValidation,
-            $.mage.__('Admin is a required field in the each row.')
+            $.mage.__('Admin is a required field in each row.')
         ],
         'validate-item-quantity': [
             function (value, element, params) {