Correctly save Product Custom Option values

Author: Jeroen van Leusden

app/code/Magento/Catalog/Model/Product/Option.php

@@ -8,6 +8,7 @@
 
 use Magento\Catalog\Api\Data\ProductCustomOptionInterface;
 use Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface;
+use Magento\Catalog\Api\Data\ProductCustomOptionValuesInterfaceFactory;
 use Magento\Catalog\Api\Data\ProductInterface;
 use Magento\Catalog\Model\Product;
 use Magento\Catalog\Model\ResourceModel\Product\Option\Value\Collection;
@@ -102,6 +103,11 @@ class Option extends AbstractExtensibleModel implements ProductCustomOptionInter
      */
     private $metadataPool;
 
+    /**
+     * @var ProductCustomOptionValuesInterfaceFactory
+     */
+    private $customOptionValuesFactory;
+
     /**
      * @param \Magento\Framework\Model\Context $context
      * @param \Magento\Framework\Registry $registry
@@ -114,6 +120,7 @@ class Option extends AbstractExtensibleModel implements ProductCustomOptionInter
      * @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
      * @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
      * @param array $data
+     * @param ProductCustomOptionValuesInterfaceFactory|null $customOptionValuesFactory
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -127,12 +134,16 @@ public function __construct(
         Option\Validator\Pool $validatorPool,
         \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
         \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
-        array $data = []
+        array $data = [],
+        ProductCustomOptionValuesInterfaceFactory $customOptionValuesFactory = null
     ) {
         $this->productOptionValue = $productOptionValue;
         $this->optionTypeFactory = $optionFactory;
         $this->validatorPool = $validatorPool;
         $this->string = $string;
+        $this->customOptionValuesFactory = $customOptionValuesFactory ?:
+            \Magento\Framework\App\ObjectManager::getInstance()->get(ProductCustomOptionValuesInterfaceFactory::class);
+
         parent::__construct(
             $context,
             $registry,
@@ -390,20 +401,21 @@ public function beforeSave()
      */
     public function afterSave()
     {
-        $this->getValueInstance()->unsetValues();
         $values = $this->getValues() ?: $this->getData('values');
         if (is_array($values)) {
             foreach ($values as $value) {
-                if ($value instanceof \Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface) {
+                if ($value instanceof ProductCustomOptionValuesInterface) {
                     $data = $value->getData();
                 } else {
                     $data = $value;
                 }
-                $this->getValueInstance()->addValue($data);
-            }
 
-            $this->getValueInstance()->setOption($this)->saveValues();
-        } elseif ($this->getGroupByType($this->getType()) == self::OPTION_GROUP_SELECT) {
+                $this->customOptionValuesFactory->create()
+                    ->addValue($data)
+                    ->setOption($this)
+                    ->saveValues();
+            }
+        } elseif ($this->getGroupByType($this->getType()) === self::OPTION_GROUP_SELECT) {
             throw new LocalizedException(__('Select type options required values rows.'));
         }
 
@@ -804,7 +816,7 @@ public function setImageSizeY($imageSizeY)
     }
 
     /**
-     * @param \Magento\Catalog\Api\Data\ProductCustomOptionValuesInterface[] $values
+     * @param ProductCustomOptionValuesInterface[] $values
      * @return $this
      */
     public function setValues(array $values = null)

app/code/Magento/Catalog/Model/Product/Option/Value.php

@@ -76,6 +76,7 @@ class Value extends AbstractModel implements \Magento\Catalog\Api\Data\ProductCu
      * @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
      * @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
      * @param array $data
+     * @param CustomOptionPriceCalculator|null $customOptionPriceCalculator
      */
     public function __construct(
         \Magento\Framework\Model\Context $context,
@@ -89,6 +90,7 @@ public function __construct(
         $this->_valueCollectionFactory = $valueCollectionFactory;
         $this->customOptionPriceCalculator = $customOptionPriceCalculator
             ?? \Magento\Framework\App\ObjectManager::getInstance()->get(CustomOptionPriceCalculator::class);
+
         parent::__construct(
             $context,
             $registry,
@@ -201,27 +203,21 @@ public function getProduct()
      */
     public function saveValues()
     {
+        $option = $this->getOption();
+
         foreach ($this->getValues() as $value) {
-            $optionValue = clone $this;
-            $optionValue->isDeleted(false);
-
-            $optionValue->setData(
-                $value
-            )->setData(
-                'option_id',
-                $optionValue->getOption()->getId()
-            )->setData(
-                'store_id',
-                $optionValue->getOption()->getStoreId()
-            );
-
-            if ($optionValue->getData('is_delete') == '1') {
-                if ($optionValue->getId()) {
-                    $optionValue->deleteValues($optionValue->getId());
-                    $optionValue->delete();
+            $this->isDeleted(false);
+            $this->setData($value)
+                ->setData('option_id', $option->getId())
+                ->setData('store_id', $option->getStoreId());
+
+            if ((bool) $this->getData('is_delete') === true) {
+                if ($this->getId()) {
+                    $this->deleteValues($this->getId());
+                    $this->delete();
                 }
             } else {
-                $optionValue->save();
+                $this->save();
             }
         }