Skip to content

Commit 81d1302

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 f55f411 commit 81d1302

File tree

4 files changed

+270
-5
lines changed

4 files changed

+270
-5
lines changed

‎app/code/Magento/Catalog/Block/Adminhtml/Product/Helper/Form/Gallery/Content.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,15 +209,22 @@ public function getImagesJson()
209209
*/
210210
private function sortImagesByPosition($images)
211211
{
212-
if (is_array($images)) {
212+
$nullPositions = [];
213+
foreach ($images as $index => $image) {
214+
if ($image['position'] === null) {
215+
$nullPositions[] = $image;
216+
unset($images[$index]);
217+
}
218+
}
219+
if (is_array($images) && !empty($images)) {
213220
usort(
214221
$images,
215222
function ($imageA, $imageB) {
216223
return ($imageA['position'] < $imageB['position']) ? -1 : 1;
217224
}
218225
);
219226
}
220-
return $images;
227+
return array_merge($images, $nullPositions);
221228
}
222229

223230
/**

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

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ public function execute($entity, $arguments = [])
6464

6565
$this->addMediaDataToProduct(
6666
$entity,
67-
$mediaEntries
67+
$this->sortMediaEntriesByPosition($mediaEntries)
6868
);
69-
69+
7070
return $entity;
7171
}
7272

@@ -108,7 +108,7 @@ public function getAttribute()
108108
* Find default value
109109
*
110110
* @param string $key
111-
* @param string[] &$image
111+
* @param string[] $image
112112
* @return string
113113
* @deprecated 101.0.1
114114
* @since 101.0.0
@@ -121,4 +121,30 @@ protected function findDefaultValue($key, &$image)
121121

122122
return '';
123123
}
124+
125+
/**
126+
* Sort media entries by position
127+
*
128+
* @param array $mediaEntries
129+
* @return array
130+
*/
131+
private function sortMediaEntriesByPosition(array $mediaEntries): array
132+
{
133+
$mediaEntriesWithNullPositions = [];
134+
foreach ($mediaEntries as $index => $mediaEntry) {
135+
if ($mediaEntry['position'] === null) {
136+
$mediaEntriesWithNullPositions[] = $mediaEntry;
137+
unset($mediaEntries[$index]);
138+
}
139+
}
140+
if (!empty($mediaEntries)) {
141+
usort(
142+
$mediaEntries,
143+
function ($entryA, $entryB) {
144+
return ($entryA['position'] < $entryB['position']) ? -1 : 1;
145+
}
146+
);
147+
}
148+
return array_merge($mediaEntries, $mediaEntriesWithNullPositions);
149+
}
124150
}

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

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,20 @@
66

77
namespace Magento\Catalog\Block\Adminhtml\Product\Helper\Form\Gallery;
88

9+
use Magento\Catalog\Api\Data\ProductInterface;
910
use Magento\Catalog\Api\ProductRepositoryInterface;
1011
use Magento\Catalog\Block\Adminhtml\Product\Helper\Form\Gallery;
1112
use Magento\Catalog\Model\Product;
13+
use Magento\Catalog\Model\Product\Gallery\UpdateHandler;
1214
use Magento\Framework\App\Request\DataPersistorInterface;
1315
use Magento\Framework\Registry;
16+
use Magento\Store\Api\StoreRepositoryInterface;
17+
use Magento\Store\Model\Store;
1418
use Magento\TestFramework\Helper\Bootstrap;
1519

1620
/**
1721
* @magentoAppArea adminhtml
22+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
1823
*/
1924
class ContentTest extends \PHPUnit\Framework\TestCase
2025
{
@@ -35,6 +40,16 @@ class ContentTest extends \PHPUnit\Framework\TestCase
3540
*/
3641
private $dataPersistor;
3742

43+
/**
44+
* @var StoreRepositoryInterface
45+
*/
46+
private $storeRepository;
47+
48+
/**
49+
* @var ProductRepositoryInterface
50+
*/
51+
private $productRepository;
52+
3853
/**
3954
* @inheritdoc
4055
*/
@@ -51,6 +66,8 @@ protected function setUp(): void
5166
$this->block->setElement($gallery);
5267
$this->registry = Bootstrap::getObjectManager()->get(Registry::class);
5368
$this->dataPersistor = Bootstrap::getObjectManager()->get(DataPersistorInterface::class);
69+
$this->storeRepository = Bootstrap::getObjectManager()->create(StoreRepositoryInterface::class);
70+
$this->productRepository = Bootstrap::getObjectManager()->get(ProductRepositoryInterface::class);
5471
}
5572

5673
public function testGetUploader()
@@ -120,6 +137,119 @@ public function getImagesAndImageTypesDataProvider()
120137
];
121138
}
122139

140+
/**
141+
* Tests images positions in store view
142+
*
143+
* @magentoDataFixture Magento/Catalog/_files/product_with_image.php
144+
* @magentoDataFixture Magento/Store/_files/second_store.php
145+
* @dataProvider imagesPositionStoreViewDataProvider
146+
* @param string $addFromStore
147+
* @param array $newImages
148+
* @param string $viewFromStore
149+
* @param array $expectedImages
150+
* @return void
151+
* @throws \Magento\Framework\Exception\NoSuchEntityException
152+
*/
153+
public function testImagesPositionStoreView(
154+
string $addFromStore,
155+
array $newImages,
156+
string $viewFromStore,
157+
array $expectedImages
158+
): void {
159+
$storeId = (int)$this->storeRepository->get($addFromStore)->getId();
160+
$product = $this->getProduct($storeId);
161+
$images = $product->getData('media_gallery')['images'];
162+
$images = array_merge($images, $newImages);
163+
$product->setData('media_gallery', ['images' => $images]);
164+
$updateHandler = Bootstrap::getObjectManager()->create(UpdateHandler::class);
165+
$updateHandler->execute($product);
166+
$storeId = (int)$this->storeRepository->get($viewFromStore)->getId();
167+
$product = $this->getProduct($storeId);
168+
$this->registry->register('current_product', $product);
169+
$actualImages = array_map(
170+
function ($item) {
171+
return [
172+
'file' => $item['file'],
173+
'label' => $item['label'],
174+
'position' => $item['position'],
175+
];
176+
},
177+
json_decode($this->block->getImagesJson(), true)
178+
);
179+
$this->assertEquals($expectedImages, array_values($actualImages));
180+
}
181+
182+
/**
183+
* @return array[]
184+
*/
185+
public function imagesPositionStoreViewDataProvider(): array
186+
{
187+
return [
188+
[
189+
'fixture_second_store',
190+
[
191+
[
192+
'file' => '/m/a/magento_small_image.jpg',
193+
'position' => 2,
194+
'label' => 'New Image Alt Text',
195+
'disabled' => 0,
196+
'media_type' => 'image'
197+
]
198+
],
199+
'default',
200+
[
201+
[
202+
'file' => '/m/a/magento_image.jpg',
203+
'label' => 'Image Alt Text',
204+
'position' => 1,
205+
],
206+
[
207+
'file' => '/m/a/magento_small_image.jpg',
208+
'label' => null,
209+
'position' => null,
210+
],
211+
]
212+
],
213+
[
214+
'fixture_second_store',
215+
[
216+
[
217+
'file' => '/m/a/magento_small_image.jpg',
218+
'position' => 2,
219+
'label' => 'New Image Alt Text',
220+
'disabled' => 0,
221+
'media_type' => 'image'
222+
]
223+
],
224+
'fixture_second_store',
225+
[
226+
[
227+
'file' => '/m/a/magento_image.jpg',
228+
'label' => 'Image Alt Text',
229+
'position' => 1,
230+
],
231+
[
232+
'file' => '/m/a/magento_small_image.jpg',
233+
'label' => 'New Image Alt Text',
234+
'position' => 2,
235+
],
236+
]
237+
]
238+
];
239+
}
240+
241+
/**
242+
* Returns product for testing.
243+
*
244+
* @param int $storeId
245+
* @param string $sku
246+
* @return ProductInterface
247+
*/
248+
private function getProduct(int $storeId = Store::DEFAULT_STORE_ID, string $sku = 'simple'): ProductInterface
249+
{
250+
return $this->productRepository->get($sku, false, $storeId, true);
251+
}
252+
123253
/**
124254
* Prepare product, and set it to registry and data persistor.
125255
*

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use Magento\Catalog\Api\Data\ProductInterface;
1111
use Magento\Catalog\Api\ProductRepositoryInterface;
12+
use Magento\Catalog\Model\Product\Gallery\UpdateHandler;
1213
use Magento\Catalog\Model\ResourceModel\Product as ProductResource;
1314
use Magento\Framework\Serialize\Serializer\Json;
1415
use Magento\Framework\View\LayoutInterface;
@@ -392,6 +393,107 @@ public function galleryImagesOnStoreViewDataProvider(): array
392393
];
393394
}
394395

396+
/**
397+
* Tests images positions in store view
398+
*
399+
* @magentoDataFixture Magento/Catalog/_files/product_with_image.php
400+
* @magentoDataFixture Magento/Store/_files/second_store.php
401+
* @magentoConfigFixture default/web/url/catalog_media_url_format image_optimization_parameters
402+
* @dataProvider imagesPositionStoreViewDataProvider
403+
* @param string $addFromStore
404+
* @param array $newImages
405+
* @param string $viewFromStore
406+
* @param array $expectedImages
407+
* @return void
408+
*/
409+
public function testImagesPositionStoreView(
410+
string $addFromStore,
411+
array $newImages,
412+
string $viewFromStore,
413+
array $expectedImages
414+
): void {
415+
$storeId = (int)$this->storeRepository->get($addFromStore)->getId();
416+
$product = $this->getProduct($storeId);
417+
$images = $product->getData('media_gallery')['images'];
418+
$images = array_merge($images, $newImages);
419+
$product->setData('media_gallery', ['images' => $images]);
420+
$updateHandler = Bootstrap::getObjectManager()->create(UpdateHandler::class);
421+
$updateHandler->execute($product);
422+
$storeId = (int)$this->storeRepository->get($viewFromStore)->getId();
423+
$product = $this->getProduct($storeId);
424+
$this->block->setData('product', $product);
425+
$actualImages = array_map(
426+
function ($item) {
427+
return [
428+
'img' => parse_url($item['img'], PHP_URL_PATH),
429+
'caption' => $item['caption'],
430+
'position' => $item['position'],
431+
];
432+
},
433+
$this->serializer->unserialize($this->block->getGalleryImagesJson())
434+
);
435+
$this->assertEquals($expectedImages, array_values($actualImages));
436+
}
437+
438+
/**
439+
* @return array[]
440+
*/
441+
public function imagesPositionStoreViewDataProvider(): array
442+
{
443+
return [
444+
[
445+
'fixture_second_store',
446+
[
447+
[
448+
'file' => '/m/a/magento_small_image.jpg',
449+
'position' => 2,
450+
'label' => 'New Image Alt Text',
451+
'disabled' => 0,
452+
'media_type' => 'image'
453+
]
454+
],
455+
'default',
456+
[
457+
[
458+
'img' => '/media/catalog/product/m/a/magento_image.jpg',
459+
'caption' => 'Image Alt Text',
460+
'position' => 1,
461+
],
462+
[
463+
'img' => '/media/catalog/product/m/a/magento_small_image.jpg',
464+
'caption' => 'Simple Product',
465+
'position' => null,
466+
],
467+
]
468+
],
469+
[
470+
'fixture_second_store',
471+
[
472+
[
473+
'file' => '/m/a/magento_small_image.jpg',
474+
'position' => 2,
475+
'label' => 'New Image Alt Text',
476+
'disabled' => 0,
477+
'media_type' => 'image'
478+
]
479+
],
480+
'fixture_second_store',
481+
[
482+
[
483+
'img' => '/media/catalog/product/m/a/magento_image.jpg',
484+
'caption' => 'Image Alt Text',
485+
'position' => 1,
486+
],
487+
[
488+
'img' => '/media/catalog/product/m/a/magento_small_image.jpg',
489+
'caption' => 'New Image Alt Text',
490+
'position' => 2,
491+
],
492+
]
493+
]
494+
];
495+
}
496+
395497
/**
396498
* Updates product gallery images and saves product.
397499
*

0 commit comments

Comments
 (0)