Skip to content

Commit 3163fae

Browse files
bubasumabubasuma
committed
MC-38951: Images positions are inconsistent across store-views if images were added in a store-view level
- Fix images positions for default scope if image were added in store view level
1 parent 4294a06 commit 3163fae

File tree

7 files changed

+126
-19
lines changed

7 files changed

+126
-19
lines changed

‎app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -267,29 +267,50 @@ protected function processNewAndExistingImages($product, array &$images)
267267
{
268268
foreach ($images as &$image) {
269269
if (empty($image['removed'])) {
270+
$isNew = empty($image['value_id']);
270271
$data = $this->processNewImage($product, $image);
271272

272-
if (!$product->isObjectNew()) {
273-
$this->resourceModel->deleteGalleryValueInStore(
274-
$image['value_id'],
275-
$product->getData($this->metadata->getLinkField()),
276-
$product->getStoreId()
277-
);
278-
}
279273
// Add per store labels, position, disabled
280274
$data['value_id'] = $image['value_id'];
281275
$data['label'] = isset($image['label']) ? $image['label'] : '';
282-
$data['position'] = isset($image['position']) ? (int)$image['position'] : 0;
276+
$data['position'] = isset($image['position']) && $image['position'] !== ''
277+
? (int)$image['position']
278+
: null;
283279
$data['disabled'] = isset($image['disabled']) ? (int)$image['disabled'] : 0;
284280
$data['store_id'] = (int)$product->getStoreId();
285281

286282
$data[$this->metadata->getLinkField()] = (int)$product->getData($this->metadata->getLinkField());
287283

288-
$this->resourceModel->insertGalleryValueInStore($data);
284+
$this->saveGalleryStoreValue($product, $data);
285+
if ($isNew && $data['store_id'] !== Store::DEFAULT_STORE_ID) {
286+
$dataForDefaultScope = $data;
287+
$dataForDefaultScope['store_id'] = Store::DEFAULT_STORE_ID;
288+
$dataForDefaultScope['disabled'] = 0;
289+
$dataForDefaultScope['label'] = null;
290+
$this->saveGalleryStoreValue($product, $dataForDefaultScope);
291+
}
289292
}
290293
}
291294
}
292295

296+
/**
297+
* Save media gallery store value
298+
*
299+
* @param Product $product
300+
* @param array $data
301+
*/
302+
private function saveGalleryStoreValue(Product $product, array $data): void
303+
{
304+
if (!$product->isObjectNew()) {
305+
$this->resourceModel->deleteGalleryValueInStore(
306+
$data['value_id'],
307+
$data[$this->metadata->getLinkField()],
308+
$data['store_id']
309+
);
310+
}
311+
$this->resourceModel->insertGalleryValueInStore($data);
312+
}
313+
293314
/**
294315
* Processes image as new.
295316
*

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1807,7 +1807,7 @@ protected function _saveProducts()
18071807
if ($column === self::COL_MEDIA_IMAGE) {
18081808
$rowData[$column][] = $uploadedFile;
18091809
}
1810-
$mediaGallery[$storeId][$rowSku][$uploadedFile] = [
1810+
$mediaGalleryStoreData = [
18111811
'attribute_id' => $this->getMediaGalleryAttributeId(),
18121812
'label' => isset($rowLabels[$column][$columnImageKey])
18131813
? $rowLabels[$column][$columnImageKey]
@@ -1817,6 +1817,15 @@ protected function _saveProducts()
18171817
? $imageHiddenStates[$columnImage] : '0',
18181818
'value' => $uploadedFile,
18191819
];
1820+
$mediaGallery[$storeId][$rowSku][$uploadedFile] = $mediaGalleryStoreData;
1821+
// Add record for default scope if it does not exist
1822+
if (!($mediaGallery[Store::DEFAULT_STORE_ID][$rowSku][$uploadedFile] ?? [])) {
1823+
//Set label and disabled values to their default values
1824+
$mediaGalleryStoreData['label'] = null;
1825+
$mediaGalleryStoreData['disabled'] = 0;
1826+
$mediaGallery[Store::DEFAULT_STORE_ID][$rowSku][$uploadedFile] = $mediaGalleryStoreData;
1827+
}
1828+
18201829
}
18211830
}
18221831
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Magento\Framework\App\ResourceConnection;
1313
use Magento\Framework\EntityManager\MetadataPool;
1414
use Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingErrorAggregatorInterface;
15+
use Magento\Store\Model\Store;
1516

1617
/**
1718
* Process and saves images during import.
@@ -259,7 +260,10 @@ private function prepareMediaGalleryValueData(
259260
$position = $data['position'];
260261
$storeId = $data['store_id'];
261262
$mediaGalleryValueData[$index]['value_id'] = $productIdMediaValueIdMap[$productId][$value];
262-
$mediaGalleryValueData[$index]['position'] = $position + ($lastPositions[$storeId][$productId] ?? 0);
263+
$lastPosition = $lastPositions[$storeId][$productId]
264+
?? $lastPositions[Store::DEFAULT_STORE_ID][$productId]
265+
?? 0;
266+
$mediaGalleryValueData[$index]['position'] = $position + $lastPosition;
263267
unset($mediaGalleryValueData[$index]['value']);
264268
}
265269
return $mediaGalleryValueData;

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,8 @@ public function testCreateWithNotDefaultStoreId()
235235
$this->assertEquals($updatedImage['file'], $targetProduct->getData('image'));
236236
// No values for default store view were provided
237237
$this->assertNull($updatedImage['label_default']);
238-
$this->assertNull($updatedImage['position_default']);
239-
$this->assertNull($updatedImage['disabled_default']);
238+
$this->assertEquals(1, $updatedImage['position_default']);
239+
$this->assertEquals(0, $updatedImage['disabled_default']);
240240
}
241241

242242
/**
@@ -483,7 +483,9 @@ public function testCreateThrowsExceptionIfProvidedImageHasWrongMimeType()
483483
public function testCreateThrowsExceptionIfTargetProductDoesNotExist()
484484
{
485485
$this->expectException(\Exception::class);
486-
$this->expectExceptionMessage('The product that was requested doesn\'t exist. Verify the product and try again.');
486+
$this->expectExceptionMessage(
487+
'The product that was requested doesn\'t exist. Verify the product and try again.'
488+
);
487489

488490
$this->createServiceInfo['rest']['resourcePath'] = '/V1/products/wrong_product_sku/media';
489491

@@ -538,7 +540,9 @@ public function testCreateThrowsExceptionIfProvidedImageNameContainsForbiddenCha
538540
public function testUpdateThrowsExceptionIfTargetProductDoesNotExist()
539541
{
540542
$this->expectException(\Exception::class);
541-
$this->expectExceptionMessage('The product that was requested doesn\'t exist. Verify the product and try again.');
543+
$this->expectExceptionMessage(
544+
'The product that was requested doesn\'t exist. Verify the product and try again.'
545+
);
542546

543547
$this->updateServiceInfo['rest']['resourcePath'] = '/V1/products/wrong_product_sku/media'
544548
. '/' . 'wrong-sku';
@@ -592,7 +596,9 @@ public function testUpdateThrowsExceptionIfThereIsNoImageWithGivenId()
592596
public function testDeleteThrowsExceptionIfTargetProductDoesNotExist()
593597
{
594598
$this->expectException(\Exception::class);
595-
$this->expectExceptionMessage('The product that was requested doesn\'t exist. Verify the product and try again.');
599+
$this->expectExceptionMessage(
600+
'The product that was requested doesn\'t exist. Verify the product and try again.'
601+
);
596602

597603
$this->deleteServiceInfo['rest']['resourcePath'] = '/V1/products/wrong_product_sku/media/9999';
598604
$requestData = [
@@ -782,6 +788,6 @@ public function testAddProductVideo()
782788
$this->assertEquals(1, $updatedImage['position']);
783789
$this->assertEquals(0, $updatedImage['disabled']);
784790
$this->assertStringStartsWith('/t/e/test_image', $updatedImage['file']);
785-
$this->assertEquals($videoContent, array_intersect($updatedImage, $videoContent));
791+
$this->assertEquals($videoContent, array_intersect_key($updatedImage, $videoContent));
786792
}
787793
}

‎dev/tests/integration/testsuite/Magento/Catalog/Block/Adminhtml/Product/Helper/Form/Gallery/ContentTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ public function imagesPositionStoreViewDataProvider(): array
206206
[
207207
'file' => '/m/a/magento_small_image.jpg',
208208
'label' => null,
209-
'position' => null,
209+
'position' => 2,
210210
],
211211
]
212212
],

‎dev/tests/integration/testsuite/Magento/Catalog/Block/Product/View/GalleryTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ public function imagesPositionStoreViewDataProvider(): array
462462
[
463463
'img' => '/media/catalog/product/m/a/magento_small_image.jpg',
464464
'caption' => 'Simple Product',
465-
'position' => null,
465+
'position' => 2,
466466
],
467467
]
468468
],

‎dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Gallery/UpdateHandlerTest.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,73 @@ public function updateImageDataProvider(): array
593593
];
594594
}
595595

596+
/**
597+
* Tests that images positions are inconsistent across store-views if images were added in a store-view level
598+
*
599+
* @magentoDataFixture Magento/Catalog/_files/product_with_image.php
600+
* @magentoDataFixture Magento/Store/_files/second_store.php
601+
* @return void
602+
*/
603+
public function testAddImageInStoreView(): void
604+
{
605+
$secondStoreId = (int)$this->storeRepository->get('fixture_second_store')->getId();
606+
$existingImagePath = '/m/a/magento_image.jpg';
607+
$newImagePath = '/m/a/magento_small_image.jpg';
608+
$product = $this->getProduct($secondStoreId);
609+
$images = $product->getData('media_gallery')['images'];
610+
$newImage = [
611+
'file' => $newImagePath,
612+
'position' => 2,
613+
'label' => 'New Image Alt Text',
614+
'disabled' => 0,
615+
'media_type' => 'image'
616+
];
617+
$images[] = $newImage;
618+
$product->setData('media_gallery', ['images' => $images]);
619+
$this->updateHandler->execute($product);
620+
$product = $this->getProduct(Store::DEFAULT_STORE_ID);
621+
$expectedImages = [
622+
[
623+
'file' => $existingImagePath,
624+
'label' => 'Image Alt Text',
625+
'position' => 1
626+
],
627+
[
628+
'file' => $newImagePath,
629+
'label' => null,
630+
'position' => 2
631+
],
632+
];
633+
$actualImages = array_map(
634+
function (\Magento\Framework\DataObject $item) {
635+
return $item->toArray(['file', 'label', 'position']);
636+
},
637+
$product->getMediaGalleryImages()->getItems()
638+
);
639+
$this->assertEquals($expectedImages, array_values($actualImages));
640+
$product->cleanModelCache();
641+
$product = $this->getProduct($secondStoreId);
642+
$expectedImages = [
643+
[
644+
'file' => $existingImagePath,
645+
'label' => 'Image Alt Text',
646+
'position' => 1
647+
],
648+
[
649+
'file' => $newImagePath,
650+
'label' => 'New Image Alt Text',
651+
'position' => 2
652+
],
653+
];
654+
$actualImages = array_map(
655+
function (\Magento\Framework\DataObject $item) {
656+
return $item->toArray(['file', 'label', 'position']);
657+
},
658+
$product->getMediaGalleryImages()->getItems()
659+
);
660+
$this->assertEquals($expectedImages, array_values($actualImages));
661+
}
662+
596663
/**
597664
* Check product image link and product image exist
598665
*

0 commit comments

Comments
 (0)