Correctly provide list of swatch attributes for product #11703

app/code/Magento/Swatches/Helper/Attribute.php

@@ -0,0 +1,136 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\Swatches\Helper;
+
+use Magento\Framework\Serialize\SerializerInterface;
+use Magento\Eav\Model\Entity\Attribute\AbstractAttribute;
+use Magento\Swatches\Model\Swatch;
+
+/**
+ * Provide list of swatch attributes for product.
+ */
+class Attribute
+{
+    /**
+     * @var SerializerInterface
+     */
+    protected $serializer;
+
+    /**
+     * @param SerializerInterface $serializer
+     */
+    public function __construct(
+        SerializerInterface $serializer
+    ) {
+        $this->serializer = $serializer;
+    }
+
+    /**
+     * Get data key which should populated to attribute entity from "additional_data" field
+     *
+     * @return array
+     */
+    public function getEavAttributeAdditionalDataKeys()
+    {
+        return [
+            Swatch::SWATCH_INPUT_TYPE_KEY,
+            'update_product_preview_image',
+            'use_product_image_for_swatch'
+        ];
+    }
+
+    /**
+     * @param AbstractAttribute $attribute
+     * @return $this
+     */
+    public function assembleAdditionalDataEavAttribute(AbstractAttribute $attribute)
+    {
+        $initialAdditionalData = [];
+        $additionalData = (string) $attribute->getData('additional_data');
+        if (!empty($additionalData)) {
+            $additionalData = $this->serializer->unserialize($additionalData);
+            if (is_array($additionalData)) {
+                $initialAdditionalData = $additionalData;
+            }
+        }
+
+        $dataToAdd = [];
+        foreach ($this->getEavAttributeAdditionalDataKeys() as $key) {
+            $dataValue = $attribute->getData($key);
+            if (null !== $dataValue) {
+                $dataToAdd[$key] = $dataValue;
+            }
+        }
+        $additionalData = array_merge($initialAdditionalData, $dataToAdd);
+        $attribute->setData('additional_data', $this->serializer->serialize($additionalData));
+        return $this;
+    }
+
+    /**
+     * Populate eav attribute with additional data from "additional_data" field
+     *
+     * @param AbstractAttribute $attribute
+     * @return $this
+     */
+    public function populateAdditionalDataEavAttribute(AbstractAttribute $attribute)
+    {
+        $serializedAdditionalData = $attribute->getData('additional_data');
+
+        if ($serializedAdditionalData) {
+            $additionalData = $this->serializer->unserialize($serializedAdditionalData);
+
+            if (isset($additionalData) && is_array($additionalData)) {
+                foreach ($this->getEavAttributeAdditionalDataKeys() as $key) {
+                    if (isset($additionalData[$key])) {
+                        $attribute->setData($key, $additionalData[$key]);
+                    }
+                }
+            }
+        }
+
+        return $this;
+    }
+
+    /**
+     * Check if an attribute is Swatch
+     *
+     * @param AbstractAttribute $attribute
+     * @return bool
+     */
+    public function isSwatchAttribute(AbstractAttribute $attribute)
+    {
+        $result = $this->isVisualSwatch($attribute) || $this->isTextSwatch($attribute);
+        return $result;
+    }
+
+    /**
+     * Is attribute Visual Swatch
+     *
+     * @param AbstractAttribute $attribute
+     * @return bool
+     */
+    public function isVisualSwatch(AbstractAttribute $attribute)
+    {
+        if (!$attribute->hasData(Swatch::SWATCH_INPUT_TYPE_KEY)) {
+            $this->populateAdditionalDataEavAttribute($attribute);
+        }
+        return $attribute->getData(Swatch::SWATCH_INPUT_TYPE_KEY) == Swatch::SWATCH_INPUT_TYPE_VISUAL;
+    }
+
+    /**
+     * Is attribute Textual Swatch
+     *
+     * @param AbstractAttribute $attribute
+     * @return bool
+     */
+    public function isTextSwatch(AbstractAttribute $attribute)
+    {
+        if (!$attribute->hasData(\Magento\Swatches\Model\Swatch::SWATCH_INPUT_TYPE_KEY)) {
+            $this->populateAdditionalDataEavAttribute($attribute);
+        }
+        return $attribute->getData(Swatch::SWATCH_INPUT_TYPE_KEY) == Swatch::SWATCH_INPUT_TYPE_TEXT;
+    }
+}

app/code/Magento/Swatches/Helper/Data.php

@@ -19,6 +19,7 @@
 use Magento\Swatches\Model\ResourceModel\Swatch\CollectionFactory as SwatchCollectionFactory;
 use Magento\Swatches\Model\Swatch;
 use Magento\Swatches\Model\SwatchAttributesProvider;
+use Magento\Swatches\Helper\Attribute as AttributeHelper;
 
 /**
  * Class Helper Data
@@ -76,30 +77,25 @@ class Data
      */
     private $swatchAttributesProvider;
 
-    /**
-     * Data key which should populated to Attribute entity from "additional_data" field
-     *
-     * @var array
-     */
-    protected $eavAttributeAdditionalDataKeys = [
-        Swatch::SWATCH_INPUT_TYPE_KEY,
-        'update_product_preview_image',
-        'use_product_image_for_swatch'
-    ];
-
     /**
      * Serializer to/from JSON.
      *
      * @var Json
      */
     private $serializer;
 
+    /**
+     * @var AttributeHelper
+     */
+    protected $attributeHelper;
+
     /**
      * @param CollectionFactory $productCollectionFactory
      * @param ProductRepositoryInterface $productRepository
      * @param StoreManagerInterface $storeManager
      * @param SwatchCollectionFactory $swatchCollectionFactory
      * @param Image $imageHelper
+     * @param AttributeHelper $attributeHelper
      * @param Json|null $serializer
      * @param SwatchAttributesProvider $swatchAttributesProvider
      */
@@ -109,6 +105,7 @@ public function __construct(
         StoreManagerInterface $storeManager,
         SwatchCollectionFactory $swatchCollectionFactory,
         Image $imageHelper,
+        AttributeHelper $attributeHelper,
         Json $serializer = null,
         SwatchAttributesProvider $swatchAttributesProvider = null
     ) {
@@ -120,6 +117,7 @@ public function __construct(
         $this->serializer = $serializer ?: ObjectManager::getInstance()->create(Json::class);
         $this->swatchAttributesProvider = $swatchAttributesProvider
             ?: ObjectManager::getInstance()->get(SwatchAttributesProvider::class);
+        $this->attributeHelper = $attributeHelper;
     }
 
     /**
@@ -128,44 +126,7 @@ public function __construct(
      */
     public function assembleAdditionalDataEavAttribute(Attribute $attribute)
     {
-        $initialAdditionalData = [];
-        $additionalData = (string) $attribute->getData('additional_data');
-        if (!empty($additionalData)) {
-            $additionalData = $this->serializer->unserialize($additionalData);
-            if (is_array($additionalData)) {
-                $initialAdditionalData = $additionalData;
-            }
-        }
-
-        $dataToAdd = [];
-        foreach ($this->eavAttributeAdditionalDataKeys as $key) {
-            $dataValue = $attribute->getData($key);
-            if (null !== $dataValue) {
-                $dataToAdd[$key] = $dataValue;
-            }
-        }
-        $additionalData = array_merge($initialAdditionalData, $dataToAdd);
-        $attribute->setData('additional_data', $this->serializer->serialize($additionalData));
-        return $this;
-    }
-
-    /**
-     * @param Attribute $attribute
-     * @return $this
-     */
-    private function populateAdditionalDataEavAttribute(Attribute $attribute)
-    {
-        $serializedAdditionalData = $attribute->getData('additional_data');
-        if ($serializedAdditionalData) {
-            $additionalData = $this->serializer->unserialize($serializedAdditionalData);
-            if (isset($additionalData) && is_array($additionalData)) {
-                foreach ($this->eavAttributeAdditionalDataKeys as $key) {
-                    if (isset($additionalData[$key])) {
-                        $attribute->setData($key, $additionalData[$key]);
-                    }
-                }
-            }
-        }
+        $this->attributeHelper->assembleAdditionalDataEavAttribute($attribute);
         return $this;
     }
 
@@ -527,8 +488,7 @@ public function isProductHasSwatch(Product $product)
      */
     public function isSwatchAttribute(Attribute $attribute)
     {
-        $result = $this->isVisualSwatch($attribute) || $this->isTextSwatch($attribute);
-        return $result;
+        return $this->attributeHelper->isSwatchAttribute($attribute);
     }
 
     /**
@@ -539,10 +499,7 @@ public function isSwatchAttribute(Attribute $attribute)
      */
     public function isVisualSwatch(Attribute $attribute)
     {
-        if (!$attribute->hasData(Swatch::SWATCH_INPUT_TYPE_KEY)) {
-            $this->populateAdditionalDataEavAttribute($attribute);
-        }
-        return $attribute->getData(Swatch::SWATCH_INPUT_TYPE_KEY) == Swatch::SWATCH_INPUT_TYPE_VISUAL;
+        return $this->attributeHelper->isVisualSwatch($attribute);
     }
 
     /**
@@ -553,10 +510,7 @@ public function isVisualSwatch(Attribute $attribute)
      */
     public function isTextSwatch(Attribute $attribute)
     {
-        if (!$attribute->hasData(Swatch::SWATCH_INPUT_TYPE_KEY)) {
-            $this->populateAdditionalDataEavAttribute($attribute);
-        }
-        return $attribute->getData(Swatch::SWATCH_INPUT_TYPE_KEY) == Swatch::SWATCH_INPUT_TYPE_TEXT;
+        return $this->attributeHelper->isTextSwatch($attribute);
     }
 
     /**

app/code/Magento/Swatches/Model/SwatchAttributesProvider.php

@@ -8,6 +8,7 @@
 use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
 use Magento\Catalog\Model\Product;
 use Magento\ConfigurableProduct\Model\Product\Type\Configurable\Attribute;
+use Magento\Swatches\Helper\Attribute as AttributeHelper;
 
 /**
  * Provide list of swatch attributes for product.
@@ -30,16 +31,24 @@ class SwatchAttributesProvider
      */
     private $attributesPerProduct;
 
+    /**
+     * @var AttributeHelper
+     */
+    protected $attributeHelper;
+
     /**
      * @param Configurable $typeConfigurable
      * @param SwatchAttributeCodes $swatchAttributeCodes
+     * @param AttributeHelper $attributeHelper
      */
     public function __construct(
         Configurable $typeConfigurable,
-        SwatchAttributeCodes $swatchAttributeCodes
+        SwatchAttributeCodes $swatchAttributeCodes,
+        AttributeHelper $attributeHelper
     ) {
         $this->typeConfigurable = $typeConfigurable;
         $this->swatchAttributeCodes = $swatchAttributeCodes;
+        $this->attributeHelper = $attributeHelper;
     }
 
     /**
@@ -61,8 +70,12 @@ public function provide(Product $product)
             $swatchAttributes = [];
             foreach ($configurableAttributes as $configurableAttribute) {
                 if (array_key_exists($configurableAttribute->getAttributeId(), $swatchAttributeCodeMap)) {
-                    $swatchAttributes[$configurableAttribute->getAttributeId()]
-                        = $configurableAttribute->getProductAttribute();
+                    $productAttribute = $configurableAttribute->getProductAttribute();
+
+                    // Check if product attribute is actually an swatch attribute
+                    if ($productAttribute && $this->attributeHelper->isSwatchAttribute($productAttribute)) {
+                        $swatchAttributes[$configurableAttribute->getAttributeId()] = $productAttribute;
+                    }
                 }
             }
             $this->attributesPerProduct[$product->getId()] = $swatchAttributes;

app/code/Magento/Swatches/Test/Unit/Helper/DataTest.php

@@ -58,6 +58,11 @@ class DataTest extends \PHPUnit\Framework\TestCase
      */
     private $swatchAttributesProvider;
 
+    /**
+     * @var \Magento\Swatches\Helper\Attribute|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $attributeHelper;
+
     protected function setUp()
     {
         $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
@@ -115,6 +120,10 @@ protected function setUp()
         $this->swatchAttributesProvider = $this->getMockBuilder(SwatchAttributesProvider::class)
             ->disableOriginalConstructor()
             ->getMock();
+        $this->attributeHelper = $this->objectManager->getObject(
+            \Magento\Swatches\Helper\Attribute::class,
+            ['serializer' => $serializer]
+        );
 
         $this->swatchHelperObject = $this->objectManager->getObject(
             \Magento\Swatches\Helper\Data::class,
@@ -127,6 +136,7 @@ protected function setUp()
                 'imageHelper' => $this->imageHelperMock,
                 'serializer' => $serializer,
                 'swatchAttributesProvider' => $this->swatchAttributesProvider,
+                'attributeHelper' => $this->attributeHelper
             ]
         );
         $this->objectManager->setBackwardCompatibleProperty(