Merge branch 'MC-18815' of https://github.com/magento-mpi/magento2ce into PR-16-08-2019

Author: Spandana Chittimala

app/code/Magento/CatalogImportExport/Model/Import/Product/MediaGalleryProcessor.php

@@ -284,6 +284,42 @@ private function initMediaGalleryResources()
         }
     }
 
+    /**
+     * Get the last media position for each product from the given list
+     *
+     * @param int $storeId
+     * @param array $productIds
+     * @return array
+     */
+    private function getLastMediaPositionPerProduct(int $storeId, array $productIds): array
+    {
+        $result = [];
+        if ($productIds) {
+            $productKeyName = $this->getProductEntityLinkField();
+            // this result could be achieved by using GROUP BY. But there is no index on position column, therefore
+            // it can be slower than the implementation below
+            $positions = $this->connection->fetchAll(
+                $this->connection
+                    ->select()
+                    ->from($this->mediaGalleryValueTableName, [$productKeyName, 'position'])
+                    ->where("$productKeyName IN (?)", $productIds)
+                    ->where('value_id is not null')
+                    ->where('store_id = ?', $storeId)
+            );
+            // Make sure the result contains all product ids even if the product has no media files
+            $result = array_fill_keys($productIds, 0);
+            // Find the largest position for each product
+            foreach ($positions as $record) {
+                $productId = $record[$productKeyName];
+                $result[$productId] = $result[$productId] < $record['position']
+                    ? $record['position']
+                    : $result[$productId];
+            }
+        }
+
+        return $result;
+    }
+
     /**
      * Save media gallery data per store.
      *
@@ -301,24 +337,30 @@ private function processMediaPerStore(
     ) {
         $multiInsertData = [];
         $dataForSkinnyTable = [];
+        $lastMediaPositionPerProduct = $this->getLastMediaPositionPerProduct(
+            $storeId,
+            array_unique(array_merge(...array_values($valueToProductId)))
+        );
+
         foreach ($mediaGalleryData as $mediaGalleryRows) {
             foreach ($mediaGalleryRows as $insertValue) {
-                foreach ($newMediaValues as $value_id => $values) {
+                foreach ($newMediaValues as $valueId => $values) {
                     if ($values['value'] == $insertValue['value']) {
-                        $insertValue['value_id'] = $value_id;
+                        $insertValue['value_id'] = $valueId;
                         $insertValue[$this->getProductEntityLinkField()]
                             = array_shift($valueToProductId[$values['value']]);
-                        unset($newMediaValues[$value_id]);
+                        unset($newMediaValues[$valueId]);
                         break;
                     }
                 }
                 if (isset($insertValue['value_id'])) {
+                    $productId = $insertValue[$this->getProductEntityLinkField()];
                     $valueArr = [
                         'value_id' => $insertValue['value_id'],
                         'store_id' => $storeId,
-                        $this->getProductEntityLinkField() => $insertValue[$this->getProductEntityLinkField()],
+                        $this->getProductEntityLinkField() => $productId,
                         'label' => $insertValue['label'],
-                        'position' => $insertValue['position'],
+                        'position' => $lastMediaPositionPerProduct[$productId] + $insertValue['position'],
                         'disabled' => $insertValue['disabled'],
                     ];
                     $multiInsertData[] = $valueArr;

dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php

@@ -626,6 +626,7 @@ function ($input) {
                         explode(',', $optionData)
                     )
                 );
+                // phpcs:ignore Magento2.Performance.ForeachArrayMerge.ForeachArrayMerge
                 $option = array_merge(...$option);
 
                 if (!empty($option['type']) && !empty($option['name'])) {
@@ -692,12 +693,14 @@ protected function mergeWithExistingData(
                 }
             } else {
                 $existingOptionId = array_search($optionKey, $expectedOptions);
+                // phpcs:ignore Magento2.Performance.ForeachArrayMerge.ForeachArrayMerge
                 $expectedData[$existingOptionId] = array_merge(
                     $this->getOptionData($option),
                     $expectedData[$existingOptionId]
                 );
                 if ($optionValues) {
                     foreach ($optionValues as $optionKey => $optionValue) {
+                        // phpcs:ignore Magento2.Performance.ForeachArrayMerge.ForeachArrayMerge
                         $expectedValues[$existingOptionId][$optionKey] = array_merge(
                             $optionValue,
                             $expectedValues[$existingOptionId][$optionKey]
@@ -842,6 +845,58 @@ public function testSaveMediaImage()
         $this->assertEquals('Additional Image Label Two', $additionalImageTwoItem->getLabel());
     }
 
+    /**
+     * Test that new images should be added after the existing ones.
+     *
+     * @magentoDataFixture mediaImportImageFixture
+     * @magentoAppIsolation enabled
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
+     */
+    public function testNewImagesShouldBeAddedAfterExistingOnes()
+    {
+        $this->importDataForMediaTest('import_media.csv');
+
+        $product = $this->getProductBySku('simple_new');
+
+        $items = array_values($product->getMediaGalleryImages()->getItems());
+
+        $images = [
+            ['file' => '/m/a/magento_image.jpg', 'label' => 'Image Label'],
+            ['file' => '/m/a/magento_small_image.jpg', 'label' => 'Small Image Label'],
+            ['file' => '/m/a/magento_thumbnail.jpg', 'label' => 'Thumbnail Label'],
+            ['file' => '/m/a/magento_additional_image_one.jpg', 'label' => 'Additional Image Label One'],
+            ['file' => '/m/a/magento_additional_image_two.jpg', 'label' => 'Additional Image Label Two'],
+        ];
+
+        $this->assertCount(5, $items);
+        $this->assertEquals(
+            $images,
+            array_map(
+                function (\Magento\Framework\DataObject $item) {
+                    return $item->toArray(['file', 'label']);
+                },
+                $items
+            )
+        );
+
+        $this->importDataForMediaTest('import_media_additional_images.csv');
+        $product->cleanModelCache();
+        $product = $this->getProductBySku('simple_new');
+        $items = array_values($product->getMediaGalleryImages()->getItems());
+        $images[] = ['file' => '/m/a/magento_additional_image_three.jpg', 'label' => ''];
+        $images[] = ['file' => '/m/a/magento_additional_image_four.jpg', 'label' => ''];
+        $this->assertCount(7, $items);
+        $this->assertEquals(
+            $images,
+            array_map(
+                function (\Magento\Framework\DataObject $item) {
+                    return $item->toArray(['file', 'label']);
+                },
+                $items
+            )
+        );
+    }
+
     /**
      * Test that errors occurred during importing images are logged.
      *
@@ -892,6 +947,14 @@ public static function mediaImportImageFixture()
                 'source' => __DIR__ . '/_files/magento_additional_image_two.jpg',
                 'dest' => $dirPath . '/magento_additional_image_two.jpg',
             ],
+            [
+                'source' => __DIR__ . '/_files/magento_additional_image_three.jpg',
+                'dest' => $dirPath . '/magento_additional_image_three.jpg',
+            ],
+            [
+                'source' => __DIR__ . '/_files/magento_additional_image_four.jpg',
+                'dest' => $dirPath . '/magento_additional_image_four.jpg',
+            ],
         ];
 
         foreach ($items as $item) {
@@ -2023,7 +2086,17 @@ private function importDataForMediaTest(string $fileName, int $expectedErrors =
         $this->assertTrue($errors->getErrorsCount() == 0);
 
         $this->_model->importData();
-        $this->assertTrue($this->_model->getErrorAggregator()->getErrorsCount() == $expectedErrors);
+        $this->assertEquals(
+            $expectedErrors,
+            $this->_model->getErrorAggregator()->getErrorsCount(),
+            array_reduce(
+                $this->_model->getErrorAggregator()->getAllErrors(),
+                function ($output, $error) {
+                    return "$output\n{$error->getErrorMessage()}";
+                },
+                ''
+            )
+        );
     }
 
     /**

dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/import_media_additional_images.csv

@@ -0,0 +1,2 @@
+sku,additional_images
+simple_new,"magento_additional_image_three.jpg,magento_additional_image_four.jpg"