Skip to content

Commit 263f71d

Browse files
nmalevanecnmalevanec
committed
#7372: Product images gets removed from "Images And Videos" after validation alert.
1 parent a39e37f commit 263f71d

File tree

7 files changed

+567
-12
lines changed

7 files changed

+567
-12
lines changed

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

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
*/
1414
namespace Magento\Catalog\Block\Adminhtml\Product\Helper\Form;
1515

16+
use Magento\Framework\App\ObjectManager;
17+
use Magento\Framework\App\Request\DataPersistorInterface;
1618
use Magento\Framework\Registry;
1719
use Magento\Catalog\Model\Product;
1820
use Magento\Eav\Model\Entity\Attribute;
@@ -68,6 +70,11 @@ class Gallery extends \Magento\Framework\View\Element\AbstractBlock
6870
*/
6971
protected $registry;
7072

73+
/**
74+
* @var DataPersistorInterface
75+
*/
76+
private $dataPersistor;
77+
7178
/**
7279
* @param \Magento\Framework\View\Element\Context $context
7380
* @param \Magento\Store\Model\StoreManagerInterface $storeManager
@@ -80,11 +87,13 @@ public function __construct(
8087
\Magento\Store\Model\StoreManagerInterface $storeManager,
8188
Registry $registry,
8289
\Magento\Framework\Data\Form $form,
83-
$data = []
90+
$data = [],
91+
DataPersistorInterface $dataPersistor = null
8492
) {
8593
$this->storeManager = $storeManager;
8694
$this->registry = $registry;
8795
$this->form = $form;
96+
$this->dataPersistor = $dataPersistor ?: ObjectManager::getInstance()->get(DataPersistorInterface::class);
8897
parent::__construct($context, $data);
8998
}
9099

@@ -104,7 +113,24 @@ public function getElementHtml()
104113
*/
105114
public function getImages()
106115
{
107-
return $this->registry->registry('current_product')->getData('media_gallery') ?: null;
116+
$images = $this->registry->registry('current_product')->getData('media_gallery') ?: null;
117+
if ($images === null) {
118+
$images = ((array)$this->dataPersistor->get('catalog_product'))['product']['media_gallery'] ?? null;
119+
}
120+
121+
return $images;
122+
}
123+
124+
/**
125+
* Get value for given type.
126+
*
127+
* @param string $type
128+
* @return string|null
129+
*/
130+
public function getImageValue(string $type)
131+
{
132+
$product = (array)$this->dataPersistor->get('catalog_product');
133+
return $product['product'][$type] ?? null;
108134
}
109135

110136
/**

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,11 @@ public function getImageTypes()
193193
$imageTypes = [];
194194
foreach ($this->getMediaAttributes() as $attribute) {
195195
/* @var $attribute \Magento\Eav\Model\Entity\Attribute */
196+
$value = $this->getElement()->getDataObject()->getData($attribute->getAttributeCode())
197+
?: $this->getElement()->getImageValue($attribute->getAttributeCode());
196198
$imageTypes[$attribute->getAttributeCode()] = [
197199
'code' => $attribute->getAttributeCode(),
198-
'value' => $this->getElement()->getDataObject()->getData($attribute->getAttributeCode()),
200+
'value' => $value,
199201
'label' => $attribute->getFrontend()->getLabel(),
200202
'scope' => __($this->getElement()->getScopeLabel($attribute)),
201203
'name' => $this->getElement()->getAttributeFieldName($attribute),

‎app/code/Magento/Catalog/Controller/Adminhtml/Product/Save.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
namespace Magento\Catalog\Controller\Adminhtml\Product;
88

99
use Magento\Backend\App\Action;
10+
use Magento\Catalog\Api\Data\ProductInterface;
1011
use Magento\Catalog\Controller\Adminhtml\Product;
1112
use Magento\Store\Model\StoreManagerInterface;
1213
use Magento\Framework\App\Request\DataPersistorInterface;
@@ -146,11 +147,13 @@ public function execute()
146147
} catch (\Magento\Framework\Exception\LocalizedException $e) {
147148
$this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e);
148149
$this->messageManager->addExceptionMessage($e);
150+
$data = $this->persistMediaData($product, $data);
149151
$this->getDataPersistor()->set('catalog_product', $data);
150152
$redirectBack = $productId ? true : 'new';
151153
} catch (\Exception $e) {
152154
$this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e);
153155
$this->messageManager->addErrorMessage($e->getMessage());
156+
$data = $this->persistMediaData($product, $data);
154157
$this->getDataPersistor()->set('catalog_product', $data);
155158
$redirectBack = $productId ? true : 'new';
156159
}
@@ -279,4 +282,36 @@ protected function getDataPersistor()
279282

280283
return $this->dataPersistor;
281284
}
285+
286+
/**
287+
* Persist media gallery on error, in order to show already saved images on next run.
288+
*
289+
* @param ProductInterface $product
290+
* @param array $data
291+
* @return array
292+
*/
293+
private function persistMediaData(ProductInterface $product, array $data)
294+
{
295+
$mediaGallery = $product->getData('media_gallery');
296+
if (!empty($mediaGallery['images'])) {
297+
foreach ($mediaGallery['images'] as $key => $image) {
298+
if (!isset($image['new_file'])) {
299+
//Remove duplicates.
300+
unset($mediaGallery['images'][$key]);
301+
}
302+
}
303+
$data['product']['media_gallery'] = $mediaGallery;
304+
$fields = [
305+
'image',
306+
'small_image',
307+
'thumbnail',
308+
'swatch_image',
309+
];
310+
foreach ($fields as $field) {
311+
$data['product'][$field] = $product->getData($field);
312+
}
313+
}
314+
315+
return $data;
316+
}
282317
}

‎app/code/Magento/Catalog/Test/Unit/Block/Adminhtml/Product/Helper/Form/Gallery/ContentTest.php

Lines changed: 144 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
namespace Magento\Catalog\Test\Unit\Block\Adminhtml\Product\Helper\Form\Gallery;
77

88
use Magento\Catalog\Block\Adminhtml\Product\Helper\Form\Gallery\Content;
9-
use Magento\Framework\Filesystem;
9+
use Magento\Catalog\Model\Entity\Attribute;
10+
use Magento\Catalog\Model\Product;
1011
use Magento\Framework\Phrase;
1112

1213
class ContentTest extends \PHPUnit\Framework\TestCase
@@ -219,4 +220,146 @@ public function testGetImagesJsonWithException()
219220

220221
$this->assertSame(json_encode($imagesResult), $this->content->getImagesJson());
221222
}
223+
224+
/**
225+
* Test GetImageTypes() will return value for given attribute from data persistor.
226+
*
227+
* @return void
228+
*/
229+
public function testGetImageTypesFromDataPersistor()
230+
{
231+
$attributeCode = 'thumbnail';
232+
$value = 'testImageValue';
233+
$scopeLabel = 'testScopeLabel';
234+
$label = 'testLabel';
235+
$name = 'testName';
236+
$expectedTypes = [
237+
$attributeCode => [
238+
'code' => $attributeCode,
239+
'value' => $value,
240+
'label' => $label,
241+
'name' => $name,
242+
],
243+
];
244+
$product = $this->getMockBuilder(Product::class)
245+
->disableOriginalConstructor()
246+
->getMock();
247+
$product->expects($this->once())
248+
->method('getData')
249+
->with($this->identicalTo($attributeCode))
250+
->willReturn(null);
251+
$mediaAttribute = $this->getMediaAttribute($label, $attributeCode);
252+
$product->expects($this->once())
253+
->method('getMediaAttributes')
254+
->willReturn([$mediaAttribute]);
255+
$this->galleryMock->expects($this->exactly(2))
256+
->method('getDataObject')
257+
->willReturn($product);
258+
$this->galleryMock->expects($this->once())
259+
->method('getImageValue')
260+
->with($this->identicalTo($attributeCode))
261+
->willReturn($value);
262+
$this->galleryMock->expects($this->once())
263+
->method('getScopeLabel')
264+
->with($this->identicalTo($mediaAttribute))
265+
->willReturn($scopeLabel);
266+
$this->galleryMock->expects($this->once())
267+
->method('getAttributeFieldName')
268+
->with($this->identicalTo($mediaAttribute))
269+
->willReturn($name);
270+
$this->getImageTypesAssertions($attributeCode, $scopeLabel, $expectedTypes);
271+
}
272+
273+
/**
274+
* Test GetImageTypes() will return value for given attribute from product.
275+
*
276+
* @return void
277+
*/
278+
public function testGetImageTypesFromProduct()
279+
{
280+
$attributeCode = 'thumbnail';
281+
$value = 'testImageValue';
282+
$scopeLabel = 'testScopeLabel';
283+
$label = 'testLabel';
284+
$name = 'testName';
285+
$expectedTypes = [
286+
$attributeCode => [
287+
'code' => $attributeCode,
288+
'value' => $value,
289+
'label' => $label,
290+
'name' => $name,
291+
],
292+
];
293+
$product = $this->getMockBuilder(Product::class)
294+
->disableOriginalConstructor()
295+
->getMock();
296+
$product->expects($this->once())
297+
->method('getData')
298+
->with($this->identicalTo($attributeCode))
299+
->willReturn($value);
300+
$mediaAttribute = $this->getMediaAttribute($label, $attributeCode);
301+
$product->expects($this->once())
302+
->method('getMediaAttributes')
303+
->willReturn([$mediaAttribute]);
304+
$this->galleryMock->expects($this->exactly(2))
305+
->method('getDataObject')
306+
->willReturn($product);
307+
$this->galleryMock->expects($this->never())
308+
->method('getImageValue');
309+
$this->galleryMock->expects($this->once())
310+
->method('getScopeLabel')
311+
->with($this->identicalTo($mediaAttribute))
312+
->willReturn($scopeLabel);
313+
$this->galleryMock->expects($this->once())
314+
->method('getAttributeFieldName')
315+
->with($this->identicalTo($mediaAttribute))
316+
->willReturn($name);
317+
$this->getImageTypesAssertions($attributeCode, $scopeLabel, $expectedTypes);
318+
}
319+
320+
/**
321+
* Perform assertions.
322+
*
323+
* @param string $attributeCode
324+
* @param string $scopeLabel
325+
* @param array $expectedTypes
326+
* @return void
327+
*/
328+
private function getImageTypesAssertions(string $attributeCode, string $scopeLabel, array $expectedTypes)
329+
{
330+
$this->content->setElement($this->galleryMock);
331+
$result = $this->content->getImageTypes();
332+
$scope = $result[$attributeCode]['scope'];
333+
$this->assertSame($scopeLabel, $scope->getText());
334+
unset($result[$attributeCode]['scope']);
335+
$this->assertSame($expectedTypes, $result);
336+
}
337+
338+
/**
339+
* Get media attribute mock.
340+
*
341+
* @param string $label
342+
* @param string $attributeCode
343+
* @return \PHPUnit_Framework_MockObject_MockObject
344+
*/
345+
private function getMediaAttribute(string $label, string $attributeCode)
346+
{
347+
$frontend = $this->getMockBuilder(Product\Attribute\Frontend\Image::class)
348+
->disableOriginalConstructor()
349+
->getMock();
350+
$frontend->expects($this->once())
351+
->method('getLabel')
352+
->willReturn($label);
353+
$mediaAttribute = $this->getMockBuilder(Attribute::class)
354+
->disableOriginalConstructor()
355+
->getMock();
356+
$mediaAttribute->expects($this->any())
357+
->method('getAttributeCode')
358+
->willReturn($attributeCode);
359+
$mediaAttribute->expects($this->once())
360+
->method('getFrontend')
361+
->willReturn($frontend);
362+
363+
return $mediaAttribute;
364+
}
222365
}

‎app/code/Magento/Catalog/Test/Unit/Block/Adminhtml/Product/Helper/Form/GalleryTest.php

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
*/
66
namespace Magento\Catalog\Test\Unit\Block\Adminhtml\Product\Helper\Form;
77

8+
use Magento\Framework\App\Request\DataPersistorInterface;
9+
810
class GalleryTest extends \PHPUnit\Framework\TestCase
911
{
1012
/**
@@ -32,18 +34,27 @@ class GalleryTest extends \PHPUnit\Framework\TestCase
3234
*/
3335
protected $objectManager;
3436

37+
/**
38+
* @var DataPersistorInterface|\PHPUnit_Framework_MockObject_MockObject
39+
*/
40+
private $dataPersistorMock;
41+
3542
public function setUp()
3643
{
3744
$this->registryMock = $this->createMock(\Magento\Framework\Registry::class);
3845
$this->productMock = $this->createPartialMock(\Magento\Catalog\Model\Product::class, ['getData']);
3946
$this->formMock = $this->createMock(\Magento\Framework\Data\Form::class);
40-
47+
$this->dataPersistorMock = $this->getMockBuilder(DataPersistorInterface::class)
48+
->disableOriginalConstructor()
49+
->setMethods(['get'])
50+
->getMockForAbstractClass();
4151
$this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
4252
$this->gallery = $this->objectManager->getObject(
4353
\Magento\Catalog\Block\Adminhtml\Product\Helper\Form\Gallery::class,
4454
[
4555
'registry' => $this->registryMock,
46-
'form' => $this->formMock
56+
'form' => $this->formMock,
57+
'dataPersistor' => $this->dataPersistorMock
4758
]
4859
);
4960
}
@@ -70,6 +81,68 @@ public function testGetImages()
7081
$this->assertSame($mediaGallery, $this->gallery->getImages());
7182
}
7283

84+
/**
85+
* Test getImages() will try get data from data persistor, if it's absent in registry.
86+
*
87+
* @return void
88+
*/
89+
public function testGetImagesWithDataPersistor()
90+
{
91+
$product = [
92+
'product' => [
93+
'media_gallery' => [
94+
'images' => [
95+
[
96+
'value_id' => '1',
97+
'file' => 'image_1.jpg',
98+
'media_type' => 'image',
99+
],
100+
[
101+
'value_id' => '2',
102+
'file' => 'image_2.jpg',
103+
'media_type' => 'image',
104+
],
105+
],
106+
],
107+
],
108+
];
109+
$this->registryMock->expects($this->once())->method('registry')->willReturn($this->productMock);
110+
$this->productMock->expects($this->once())->method('getData')->willReturn(null);
111+
$this->dataPersistorMock->expects($this->once())
112+
->method('get')
113+
->with($this->identicalTo('catalog_product'))
114+
->willReturn($product);
115+
116+
$this->assertSame($product['product']['media_gallery'], $this->gallery->getImages());
117+
}
118+
119+
/**
120+
* Test get image value from data persistor in case it's absent in product from registry.
121+
*
122+
* @return void
123+
*/
124+
public function testGetImageValue()
125+
{
126+
$product = [
127+
'product' => [
128+
'media_gallery' => [
129+
'images' => [
130+
'value_id' => '1',
131+
'file' => 'image_1.jpg',
132+
'media_type' => 'image',
133+
],
134+
],
135+
'small' => 'testSmallImage',
136+
'thumbnail' => 'testThumbnail'
137+
]
138+
];
139+
$this->dataPersistorMock->expects($this->once())
140+
->method('get')
141+
->with($this->identicalTo('catalog_product'))
142+
->willReturn($product);
143+
$this->assertSame($product['product']['small'], $this->gallery->getImageValue('small'));
144+
}
145+
73146
public function testGetDataObject()
74147
{
75148
$this->registryMock->expects($this->once())->method('registry')->willReturn($this->productMock);

0 commit comments

Comments
 (0)