Skip to content

Commit 2194a10

Browse files
author
Stanislav Idolov
authored
ENGCOM-660: 7372: Product images gets removed from "Images And Videos" after validation alert. #1140 
2 parents 226ebee + 9ece610 commit 2194a10

File tree

7 files changed

+505
-12
lines changed

7 files changed

+505
-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: 147 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@
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

13+
/**
14+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
15+
*/
1216
class ContentTest extends \PHPUnit\Framework\TestCase
1317
{
1418
/**
@@ -219,4 +223,146 @@ public function testGetImagesJsonWithException()
219223

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

0 commit comments

Comments
 (0)