Merge branch 'MC-41758' of https://github.com/magento-l3/magento2ce into PR-20210423

Author: chittima

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

@@ -158,6 +158,7 @@ public function save(\Magento\Catalog\Api\Data\ProductCustomOptionInterface $opt
         $option->setData('product_id', $product->getData($metadata->getLinkField()));
         $option->setData('store_id', $product->getStoreId());
 
+        $backedOptions = $option->getValues();
         if ($option->getOptionId()) {
             $options = $product->getOptions();
             if (!$options) {
@@ -174,6 +175,9 @@ public function save(\Magento\Catalog\Api\Data\ProductCustomOptionInterface $opt
             }
             $originalValues = $persistedOption->getValues();
             $newValues = $option->getData('values');
+            if (!$newValues) {
+                $newValues = $this->getOptionValues($option);
+            }
             if ($newValues) {
                 if (isset($originalValues)) {
                     $newValues = $this->markRemovedValues($newValues, $originalValues);
@@ -182,6 +186,8 @@ public function save(\Magento\Catalog\Api\Data\ProductCustomOptionInterface $opt
             }
         }
         $option->save();
+        // Required for API response data consistency
+        $option->setValues($backedOptions);
         return $option;
     }
 
@@ -249,4 +255,28 @@ private function getHydratorPool()
         }
         return $this->hydratorPool;
     }
+
+    /**
+     * Get Option values from property
+     *
+     * Gets Option values stored in property, modifies for needed format and clears the property
+     *
+     * @param \Magento\Catalog\Api\Data\ProductCustomOptionInterface $option
+     * @return array|null
+     */
+    private function getOptionValues(\Magento\Catalog\Api\Data\ProductCustomOptionInterface $option): ?array
+    {
+        if ($option->getValues() === null) {
+            return null;
+        }
+
+        $optionValues = [];
+
+        foreach ($option->getValues() as $optionValue) {
+            $optionValues[] = $optionValue->getData();
+        }
+        $option->setValues(null);
+
+        return $optionValues;
+    }
 }

app/code/Magento/Catalog/Test/Unit/Model/Product/Option/RepositoryTest.php

@@ -262,7 +262,7 @@ public function testSave()
             ->getMock();
         $optionCollection->expects($this->once())->method('getProductOptions')->willReturn([$this->optionMock]);
         $this->optionCollectionFactory->expects($this->once())->method('create')->willReturn($optionCollection);
-        $this->optionMock->expects($this->once())->method('getValues')->willReturn([
+        $this->optionMock->expects($this->exactly(2))->method('getValues')->willReturn([
             $originalValue1,
             $originalValue2,
             $originalValue3
@@ -291,7 +291,7 @@ public function testSaveWhenOptionTypeWasChanged()
             ->getMock();
         $optionCollection->expects($this->once())->method('getProductOptions')->willReturn([$this->optionMock]);
         $this->optionCollectionFactory->expects($this->once())->method('create')->willReturn($optionCollection);
-        $this->optionMock->expects($this->once())->method('getValues')->willReturn(null);
+        $this->optionMock->expects($this->exactly(2))->method('getValues')->willReturn(null);
         $this->assertEquals($this->optionMock, $this->optionRepository->save($this->optionMock));
     }
 }

dev/tests/api-functional/testsuite/Magento/Catalog/Api/ProductCustomOptionRepositoryTest.php

@@ -301,13 +301,15 @@ public function testUpdate()
 
     /**
      * @param string $optionType
-     *
+     * @param bool $includedExisting
+     * @param int $expectedOptionValuesCount
+     * @throws \Magento\Framework\Exception\NoSuchEntityException
      * @magentoApiDataFixture Magento/Catalog/_files/product_with_options.php
      * @magentoAppIsolation enabled
      * @dataProvider validOptionDataProvider
      * @SuppressWarnings(PHPMD.UnusedLocalVariable)
      */
-    public function testUpdateOptionAddingNewValue($optionType)
+    public function testUpdateOptionAddingNewValue($optionType, $includedExisting, $expectedOptionValuesCount)
     {
         $fixtureOption = null;
         $valueData = [
@@ -334,17 +336,21 @@ public function testUpdateOptionAddingNewValue($optionType)
         }
 
         $values = [];
-        foreach ($option->getValues() as $key => $value) {
-            $values[] =
-                [
-                    'price' => $value->getPrice(),
-                    'price_type' => $value->getPriceType(),
-                    'sku' => $value->getSku(),
-                    'title' => $value->getTitle(),
-                    'sort_order' => $value->getSortOrder(),
-                ];
-        }
         $values[] = $valueData;
+        // Keeps the existing Option Values when adding a new Option Value
+        if ($includedExisting) {
+            foreach ($option->getValues() as $key => $value) {
+                $values[] =
+                    [
+                        'price' => $value->getPrice(),
+                        'price_type' => $value->getPriceType(),
+                        'sku' => $value->getSku(),
+                        'title' => $value->getTitle(),
+                        'sort_order' => $value->getSortOrder(),
+                    ];
+            }
+        }
+
         $data = [
             'product_sku' => $option->getProductSku(),
             'title' => $option->getTitle(),
@@ -375,21 +381,31 @@ public function testUpdateOptionAddingNewValue($optionType)
             $valueObject = $this->_webApiCall($serviceInfo, ['option' => $data]);
         }
 
-        $values = end($valueObject['values']);
+        $values = reset($valueObject['values']);
         $this->assertEquals($valueData['price'], $values['price']);
         $this->assertEquals($valueData['price_type'], $values['price_type']);
         $this->assertEquals($valueData['sku'], $values['sku']);
         $this->assertEquals('New Option Title', $values['title']);
         $this->assertEquals(100, $values['sort_order']);
+
+        $product = $productRepository->get('simple', false, null, true);
+        // Assert correct number of Option Values after Option is updated
+        foreach ($product->getOptions() as $option) {
+            if ($option->getId() === $fixtureOption->getId()) {
+                $this->assertEquals($expectedOptionValuesCount, count($option->getValues()));
+            }
+        }
     }
 
     public function validOptionDataProvider()
     {
         return [
-            'drop_down' => ['drop_down'],
-            'checkbox' => ['checkbox'],
-            'radio' => ['radio'],
-            'multiple' => ['multiple']
+            'drop_down including previous values' => ['drop_down', true, 3],
+            'drop_down with new value only' => ['drop_down', false, 1],
+            'checkbox including previous values' => ['checkbox', true, 3],
+            'checkbox with new value only' => ['checkbox', false, 1],
+            'radio including previous values' => ['radio', true, 3],
+            'multiple with new value only' => ['multiple', false, 1],
         ];
     }