From 836cb3042c77d7b5899b4ed48373490da678be8b Mon Sep 17 00:00:00 2001 From: Ben Robie Date: Mon, 9 Oct 2017 22:59:05 -0500 Subject: [PATCH 1/9] Defaulting missing alt-text for a product to use the product name. https://github.com/magento/magento2/issues/9931 --- app/code/Magento/Catalog/Block/Product/View/Gallery.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Block/Product/View/Gallery.php b/app/code/Magento/Catalog/Block/Product/View/Gallery.php index 44dd3b9f97cbf..661132457b97e 100644 --- a/app/code/Magento/Catalog/Block/Product/View/Gallery.php +++ b/app/code/Magento/Catalog/Block/Product/View/Gallery.php @@ -116,7 +116,7 @@ public function getGalleryImagesJson() 'thumb' => $image->getData('small_image_url'), 'img' => $image->getData('medium_image_url'), 'full' => $image->getData('large_image_url'), - 'caption' => $image->getLabel(), + 'caption' => ($image->getLabel() ?: $this->getProduct()->getName()), 'position' => $image->getPosition(), 'isMain' => $this->isMainImage($image), 'type' => str_replace('external-', '', $image->getMediaType()), From 3381ec061baa7cdc4710ab2d025b046e06eb0f57 Mon Sep 17 00:00:00 2001 From: Ben Robie Date: Tue, 10 Oct 2017 08:58:16 -0500 Subject: [PATCH 2/9] Adding unit test coverage for Catalog/Block/Product/View/Gallery's getGalleryImagesJson() function https://github.com/magento/magento2/issues/9931 --- .../Unit/Block/Product/View/GalleryTest.php | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php b/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php index ec7779fcbb781..b3fbe7d040eef 100644 --- a/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php @@ -77,6 +77,83 @@ protected function mockContext() ->willReturn($this->registry); } + public function testGetGalleryImagesJsonWithLabel(){ + $this->prepareGetGalleryImagesJsonMocks(); + $json = $this->model->getGalleryImagesJson(); + $decodedJson = json_decode($json, true); + $this->assertEquals('product_page_image_small_url', $decodedJson[0]['thumb']); + $this->assertEquals('product_page_image_medium_url', $decodedJson[0]['img']); + $this->assertEquals('product_page_image_large_url', $decodedJson[0]['full']); + $this->assertEquals('test_label', $decodedJson[0]['caption']); + $this->assertEquals('2', $decodedJson[0]['position']); + $this->assertEquals(false, $decodedJson[0]['isMain']); + $this->assertEquals('test_media_type', $decodedJson[0]['type']); + $this->assertEquals('test_video_url', $decodedJson[0]['videoUrl']); + } + + public function testGetGalleryImagesJsonWithoutLabel(){ + $this->prepareGetGalleryImagesJsonMocks(false); + $json = $this->model->getGalleryImagesJson(); + $decodedJson = json_decode($json, true); + $this->assertEquals('test_product_name', $decodedJson[0]['caption']); + + } + + private function prepareGetGalleryImagesJsonMocks($hasLabel = true){ + $storeMock = $this->getMockBuilder(\Magento\Store\Model\Store::class) + ->disableOriginalConstructor() + ->getMock(); + + $productMock = $this->getMockBuilder(\Magento\Catalog\Model\Product::class) + ->disableOriginalConstructor() + ->getMock(); + + $productTypeMock = $this->getMockBuilder(\Magento\Catalog\Model\Product\Type\AbstractType::class) + ->disableOriginalConstructor() + ->getMock(); + $productTypeMock->expects($this->any()) + ->method('getStoreFilter') + ->with($productMock) + ->willReturn($storeMock); + + $productMock->expects($this->any()) + ->method('getTypeInstance') + ->willReturn($productTypeMock); + $productMock->expects($this->any()) + ->method('getMediaGalleryImages') + ->willReturn($this->getImagesCollectionWithPopulatedDataObject($hasLabel)); + $productMock->expects($this->any()) + ->method('getName') + ->willReturn('test_product_name'); + + $this->registry->expects($this->any()) + ->method('registry') + ->with('product') + ->willReturn($productMock); + + $this->imageHelper->expects($this->any()) + ->method('init') + ->willReturnMap([ + [$productMock, 'product_page_image_small', [], $this->imageHelper], + [$productMock, 'product_page_image_medium_no_frame', [], $this->imageHelper], + [$productMock, 'product_page_image_large_no_frame', [], $this->imageHelper], + ]) + ->willReturnSelf(); + $this->imageHelper->expects($this->any()) + ->method('setImageFile') + ->with('test_file') + ->willReturnSelf(); + $this->imageHelper->expects($this->at(2)) + ->method('getUrl') + ->willReturn('product_page_image_small_url'); + $this->imageHelper->expects($this->at(5)) + ->method('getUrl') + ->willReturn('product_page_image_medium_url'); + $this->imageHelper->expects($this->at(8)) + ->method('getUrl') + ->willReturn('product_page_image_large_url'); + } + public function testGetGalleryImages() { $storeMock = $this->getMockBuilder(\Magento\Store\Model\Store::class) @@ -154,4 +231,30 @@ private function getImagesCollection() return $collectionMock; } + + /** + * @return \Magento\Framework\Data\Collection + */ + private function getImagesCollectionWithPopulatedDataObject($hasLabel) + { + $collectionMock = $this->getMockBuilder(\Magento\Framework\Data\Collection::class) + ->disableOriginalConstructor() + ->getMock(); + + $items = [ + new \Magento\Framework\DataObject([ + 'file' => 'test_file', + 'label' => ($hasLabel ? 'test_label' : ''), + 'position' => '2', + 'media_type' => 'external-test_media_type', + "video_url" => 'test_video_url' + ]), + ]; + + $collectionMock->expects($this->any()) + ->method('getIterator') + ->willReturn(new \ArrayIterator($items)); + + return $collectionMock; + } } From 0f1b58f99d2eec7226b4cb1af8c099f901353b2b Mon Sep 17 00:00:00 2001 From: Ben Robie Date: Tue, 10 Oct 2017 10:01:21 -0500 Subject: [PATCH 3/9] Correcting code to conform with static code testing. https://github.com/magento/magento2/issues/9931 --- .../Catalog/Test/Unit/Block/Product/View/GalleryTest.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php b/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php index b3fbe7d040eef..707cc00d1a9cc 100644 --- a/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php @@ -77,7 +77,8 @@ protected function mockContext() ->willReturn($this->registry); } - public function testGetGalleryImagesJsonWithLabel(){ + public function testGetGalleryImagesJsonWithLabel() + { $this->prepareGetGalleryImagesJsonMocks(); $json = $this->model->getGalleryImagesJson(); $decodedJson = json_decode($json, true); @@ -91,7 +92,8 @@ public function testGetGalleryImagesJsonWithLabel(){ $this->assertEquals('test_video_url', $decodedJson[0]['videoUrl']); } - public function testGetGalleryImagesJsonWithoutLabel(){ + public function testGetGalleryImagesJsonWithoutLabel() + { $this->prepareGetGalleryImagesJsonMocks(false); $json = $this->model->getGalleryImagesJson(); $decodedJson = json_decode($json, true); @@ -99,7 +101,8 @@ public function testGetGalleryImagesJsonWithoutLabel(){ } - private function prepareGetGalleryImagesJsonMocks($hasLabel = true){ + private function prepareGetGalleryImagesJsonMocks($hasLabel = true) + { $storeMock = $this->getMockBuilder(\Magento\Store\Model\Store::class) ->disableOriginalConstructor() ->getMock(); From 7f44e668c0bb6119514f544b63e16db19a072ccb Mon Sep 17 00:00:00 2001 From: Ben Robie Date: Tue, 10 Oct 2017 21:41:20 -0500 Subject: [PATCH 4/9] Correcting code to conform with static code testing. https://github.com/magento/magento2/issues/9931 --- .../Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php b/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php index 707cc00d1a9cc..e0ba6531c8ab2 100644 --- a/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Block/Product/View/GalleryTest.php @@ -98,7 +98,6 @@ public function testGetGalleryImagesJsonWithoutLabel() $json = $this->model->getGalleryImagesJson(); $decodedJson = json_decode($json, true); $this->assertEquals('test_product_name', $decodedJson[0]['caption']); - } private function prepareGetGalleryImagesJsonMocks($hasLabel = true) From 8aeb71b9c99b241ca1376f94e9adeb0338c0e7b1 Mon Sep 17 00:00:00 2001 From: David Manners Date: Mon, 27 Nov 2017 14:32:42 +0000 Subject: [PATCH 5/9] Update image label values on product entity on admin product save --- .../Model/Product/Gallery/CreateHandler.php | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php index 03d418f3ba0d9..3be10de96ec12 100644 --- a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php +++ b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php @@ -167,8 +167,11 @@ public function execute($product, $arguments = []) if (empty($attrData) && empty($clearImages) && empty($newImages) && empty($existImages)) { continue; } + $resetLabel = false; if (in_array($attrData, $clearImages)) { $product->setData($mediaAttrCode, 'no_selection'); + $product->setData($mediaAttrCode . '_label', null); + $resetLabel = true; } if (in_array($attrData, array_keys($newImages))) { @@ -179,6 +182,11 @@ public function execute($product, $arguments = []) if (in_array($attrData, array_keys($existImages)) && isset($existImages[$attrData]['label'])) { $product->setData($mediaAttrCode . '_label', $existImages[$attrData]['label']); } + + if ($attrData === 'no_selection' && !empty($product->getData($mediaAttrCode . '_label'))) { + $product->setData($mediaAttrCode . '_label', null); + $resetLabel = true; + } if (!empty($product->getData($mediaAttrCode))) { $product->addAttributeUpdate( $mediaAttrCode, @@ -186,6 +194,19 @@ public function execute($product, $arguments = []) $product->getStoreId() ); } + if ( + in_array($mediaAttrCode, ['image', 'small_image', 'thumbnail']) + && ( + !empty($product->getData($mediaAttrCode . '_label')) + || $resetLabel === true + ) + ) { + $product->addAttributeUpdate( + $mediaAttrCode . '_label', + $product->getData($mediaAttrCode . '_label'), + $product->getStoreId() + ); + } } $product->setData($attrCode, $value); From 15113774593671c0c760d81140aa83969b9da143 Mon Sep 17 00:00:00 2001 From: David Manners Date: Mon, 27 Nov 2017 14:44:41 +0000 Subject: [PATCH 6/9] Fix code style for empty label check if statement --- .../Magento/Catalog/Model/Product/Gallery/CreateHandler.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php index 3be10de96ec12..22340801c0a6a 100644 --- a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php +++ b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php @@ -194,9 +194,8 @@ public function execute($product, $arguments = []) $product->getStoreId() ); } - if ( - in_array($mediaAttrCode, ['image', 'small_image', 'thumbnail']) - && ( + if (in_array($mediaAttrCode, ['image', 'small_image', 'thumbnail']) && + ( !empty($product->getData($mediaAttrCode . '_label')) || $resetLabel === true ) From bab1a384116544e3ef346dd377a791d6c51f84f4 Mon Sep 17 00:00:00 2001 From: David Manners Date: Wed, 29 Nov 2017 10:09:12 +0000 Subject: [PATCH 7/9] Extract image and lavel processing for media attributes into their own method --- .../Model/Product/Gallery/CreateHandler.php | 110 +++++++++++------- 1 file changed, 71 insertions(+), 39 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php index 22340801c0a6a..528ebd6632188 100644 --- a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php +++ b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php @@ -167,45 +167,13 @@ public function execute($product, $arguments = []) if (empty($attrData) && empty($clearImages) && empty($newImages) && empty($existImages)) { continue; } - $resetLabel = false; - if (in_array($attrData, $clearImages)) { - $product->setData($mediaAttrCode, 'no_selection'); - $product->setData($mediaAttrCode . '_label', null); - $resetLabel = true; - } - - if (in_array($attrData, array_keys($newImages))) { - $product->setData($mediaAttrCode, $newImages[$attrData]['new_file']); - $product->setData($mediaAttrCode . '_label', $newImages[$attrData]['label']); - } - - if (in_array($attrData, array_keys($existImages)) && isset($existImages[$attrData]['label'])) { - $product->setData($mediaAttrCode . '_label', $existImages[$attrData]['label']); - } - - if ($attrData === 'no_selection' && !empty($product->getData($mediaAttrCode . '_label'))) { - $product->setData($mediaAttrCode . '_label', null); - $resetLabel = true; - } - if (!empty($product->getData($mediaAttrCode))) { - $product->addAttributeUpdate( - $mediaAttrCode, - $product->getData($mediaAttrCode), - $product->getStoreId() - ); - } - if (in_array($mediaAttrCode, ['image', 'small_image', 'thumbnail']) && - ( - !empty($product->getData($mediaAttrCode . '_label')) - || $resetLabel === true - ) - ) { - $product->addAttributeUpdate( - $mediaAttrCode . '_label', - $product->getData($mediaAttrCode . '_label'), - $product->getStoreId() - ); - } + $this->processMediaAttribute( + $product, + $mediaAttrCode, + $clearImages, + $newImages, + $existImages + ); } $product->setData($attrCode, $value); @@ -468,4 +436,68 @@ private function getMediaAttributeCodes() } return $this->mediaAttributeCodes; } + + /** + * @param \Magento\Catalog\Model\Product $product + * @param $attrData + * @param array $clearImages + * @param $mediaAttrCode + * @param array $newImages + * @param array $existImages + */ + /** + * @param \Magento\Catalog\Model\Product $product + * @param $mediaAttrCode + * @param array $clearImages + * @param array $newImages + * @param array $existImages + */ + private function processMediaAttribute( + \Magento\Catalog\Model\Product $product, + $mediaAttrCode, + array $clearImages, + array $newImages, + array $existImages + ) { + $resetLabel = false; + $attrData = $product->getData($mediaAttrCode); + if (in_array($attrData, $clearImages)) { + $product->setData($mediaAttrCode, 'no_selection'); + $product->setData($mediaAttrCode . '_label', null); + $resetLabel = true; + } + + if (in_array($attrData, array_keys($newImages))) { + $product->setData($mediaAttrCode, $newImages[$attrData]['new_file']); + $product->setData($mediaAttrCode . '_label', $newImages[$attrData]['label']); + } + + if (in_array($attrData, array_keys($existImages)) && isset($existImages[$attrData]['label'])) { + $product->setData($mediaAttrCode . '_label', $existImages[$attrData]['label']); + } + + if ($attrData === 'no_selection' && !empty($product->getData($mediaAttrCode . '_label'))) { + $product->setData($mediaAttrCode . '_label', null); + $resetLabel = true; + } + if (in_array($mediaAttrCode, ['image', 'small_image', 'thumbnail']) && + ( + !empty($product->getData($mediaAttrCode . '_label')) + || $resetLabel === true + ) + ) { + $product->addAttributeUpdate( + $mediaAttrCode . '_label', + $product->getData($mediaAttrCode . '_label'), + $product->getStoreId() + ); + } + if (!empty($product->getData($mediaAttrCode))) { + $product->addAttributeUpdate( + $mediaAttrCode, + $product->getData($mediaAttrCode), + $product->getStoreId() + ); + } + } } From d6fa9dbdfc1c54802df495aed1ec085813198470 Mon Sep 17 00:00:00 2001 From: David Manners Date: Wed, 29 Nov 2017 11:27:48 +0000 Subject: [PATCH 8/9] Solve some complexity issues with the save handler --- .../Model/Product/Gallery/CreateHandler.php | 56 ++++++++++++------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php index 528ebd6632188..976e9e2448a3e 100644 --- a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php +++ b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php @@ -171,9 +171,17 @@ public function execute($product, $arguments = []) $product, $mediaAttrCode, $clearImages, - $newImages, - $existImages + $newImages ); + if (in_array($mediaAttrCode, ['image', 'small_image', 'thumbnail'])) { + $this->processMediaAttributeLabel( + $product, + $mediaAttrCode, + $clearImages, + $newImages, + $existImages + ); + } } $product->setData($attrCode, $value); @@ -439,12 +447,32 @@ private function getMediaAttributeCodes() /** * @param \Magento\Catalog\Model\Product $product - * @param $attrData - * @param array $clearImages * @param $mediaAttrCode + * @param array $clearImages * @param array $newImages - * @param array $existImages */ + private function processMediaAttribute( + \Magento\Catalog\Model\Product $product, + $mediaAttrCode, + array $clearImages, + array $newImages + ) { + $attrData = $product->getData($mediaAttrCode); + if (in_array($attrData, $clearImages)) { + $product->setData($mediaAttrCode, 'no_selection'); + } + + if (in_array($attrData, array_keys($newImages))) { + $product->setData($mediaAttrCode, $newImages[$attrData]['new_file']); + } + if (!empty($product->getData($mediaAttrCode))) { + $product->addAttributeUpdate( + $mediaAttrCode, + $product->getData($mediaAttrCode), + $product->getStoreId() + ); + } + } /** * @param \Magento\Catalog\Model\Product $product * @param $mediaAttrCode @@ -452,7 +480,7 @@ private function getMediaAttributeCodes() * @param array $newImages * @param array $existImages */ - private function processMediaAttribute( + private function processMediaAttributeLabel( \Magento\Catalog\Model\Product $product, $mediaAttrCode, array $clearImages, @@ -462,13 +490,11 @@ private function processMediaAttribute( $resetLabel = false; $attrData = $product->getData($mediaAttrCode); if (in_array($attrData, $clearImages)) { - $product->setData($mediaAttrCode, 'no_selection'); $product->setData($mediaAttrCode . '_label', null); $resetLabel = true; } if (in_array($attrData, array_keys($newImages))) { - $product->setData($mediaAttrCode, $newImages[$attrData]['new_file']); $product->setData($mediaAttrCode . '_label', $newImages[$attrData]['label']); } @@ -480,11 +506,8 @@ private function processMediaAttribute( $product->setData($mediaAttrCode . '_label', null); $resetLabel = true; } - if (in_array($mediaAttrCode, ['image', 'small_image', 'thumbnail']) && - ( - !empty($product->getData($mediaAttrCode . '_label')) - || $resetLabel === true - ) + if (!empty($product->getData($mediaAttrCode . '_label')) + || $resetLabel === true ) { $product->addAttributeUpdate( $mediaAttrCode . '_label', @@ -492,12 +515,5 @@ private function processMediaAttribute( $product->getStoreId() ); } - if (!empty($product->getData($mediaAttrCode))) { - $product->addAttributeUpdate( - $mediaAttrCode, - $product->getData($mediaAttrCode), - $product->getStoreId() - ); - } } } From ae5454a71ff3b01545f3e8a090d472a391f3449b Mon Sep 17 00:00:00 2001 From: David Manners Date: Wed, 29 Nov 2017 11:31:34 +0000 Subject: [PATCH 9/9] Make sure there is a space between the two new methods --- app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php index 976e9e2448a3e..cb045aee20899 100644 --- a/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php +++ b/app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php @@ -473,6 +473,7 @@ private function processMediaAttribute( ); } } + /** * @param \Magento\Catalog\Model\Product $product * @param $mediaAttrCode