Skip to content

Commit c4a39ff

Browse files
anzinanzin
committed
Fixed code after review
1 parent cf2b81a commit c4a39ff

File tree

10 files changed

+95
-57
lines changed

10 files changed

+95
-57
lines changed

‎app/code/Magento/Catalog/Helper/Product/View.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public function initProductLayout(ResultPage $resultPage, $product, $params = nu
178178
$pageConfig->setPageLayout($settings->getPageLayout());
179179
}
180180

181-
$urlSafeSku = is_string($product->getSku()) ? rawurlencode($product->getSku()) : '';
181+
$urlSafeSku = rawurlencode($product->getSku());
182182

183183
// Load default page handles and page configurations
184184
if ($params && $params->getBeforeHandles()) {

‎app/code/Magento/ReleaseNotification/Model/Condition/CanViewNotification.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,13 @@ public function isVisible(array $arguments)
8080
$userId = $this->session->getUser()->getId();
8181
$cacheKey = self::$cachePrefix . $userId;
8282
$value = $this->cacheStorage->load($cacheKey);
83+
8384
if ($value === false) {
84-
$lastViewVersion = $this->viewerLogger->get($userId)->getLastViewVersion() ?: '';
85-
$productMetadataVersion = $this->productMetadata->getVersion() ?: '';
86-
$value = version_compare($lastViewVersion, $productMetadataVersion, '<');
85+
$value = version_compare(
86+
$this->viewerLogger->get($userId)->getLastViewVersion(),
87+
$this->productMetadata->getVersion(),
88+
'<'
89+
);
8790
$this->cacheStorage->save(false, $cacheKey);
8891
}
8992
return (bool)$value;

‎app/code/Magento/ReleaseNotification/Test/Unit/Model/Condition/CanViewNotificationTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public function isVisibleProvider()
122122
return [
123123
[false, '2.2.1-dev', '999.999.999-alpha'],
124124
[true, '2.2.1-dev', '2.0.0'],
125-
[true, '2.2.1-dev', null],
125+
[true, '2.2.1-dev', '2.2.0'],
126126
[false, '2.2.1-dev', '2.2.1'],
127127
[true, '2.2.1-dev', '2.2.0'],
128128
[true, '2.3.0', '2.2.0'],

‎dev/tests/integration/testsuite/Magento/Catalog/Helper/Product/ViewTest.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,10 @@ public function testInitProductLayout()
7777
$uniqid = uniqid();
7878
/** @var $product \Magento\Catalog\Model\Product */
7979
$product = $this->objectManager->create(\Magento\Catalog\Model\Product::class);
80-
$product->setTypeId(\Magento\Catalog\Model\Product\Type::DEFAULT_TYPE)->setId(99)->setUrlKey($uniqid);
80+
$product->setTypeId(\Magento\Catalog\Model\Product\Type::DEFAULT_TYPE)
81+
->setId(99)
82+
->setSku('test-sku')
83+
->setUrlKey($uniqid);
8184
/** @var $objectManager \Magento\TestFramework\ObjectManager */
8285
$objectManager = $this->objectManager;
8386
$objectManager->get(\Magento\Framework\Registry::class)->register('product', $product);

‎dev/tests/integration/testsuite/Magento/Framework/Image/Adapter/InterfaceTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,13 @@ protected function _getFixture($pattern)
115115
*/
116116
protected function _isFormatSupported($image, $adapter)
117117
{
118-
if (!$image || !file_exists($image)) {
118+
if ($image === null || !file_exists($image)) {
119119
return false;
120120
}
121121
$data = pathinfo($image);
122+
$supportedTypes = $adapter->getSupportedFormats();
122123

123-
return isset($data['extension']) && in_array(strtolower($data['extension']), $adapter->getSupportedFormats());
124+
return isset($data['extension']) && in_array(strtolower($data['extension']), $supportedTypes);
124125
}
125126

126127
/**

‎dev/tests/integration/testsuite/Magento/Framework/Translate/InlineTest.php‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Framework\Translate;
78

89
class InlineTest extends \PHPUnit\Framework\TestCase
@@ -116,10 +117,6 @@ public function processResponseBodyDataProvider()
116117
$originalText = file_get_contents(__DIR__ . '/_files/_inline_page_original.html');
117118
$expectedText = file_get_contents(__DIR__ . '/_files/_inline_page_expected.html');
118119

119-
$package = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
120-
\Magento\Framework\View\DesignInterface::class
121-
)->getDesignTheme()->getPackageCode();
122-
$expectedText = str_replace('{{design_package}}', is_string($package) ? $package : '', $expectedText);
123120
return [
124121
'plain text' => ['text with no translations and tags', 'text with no translations and tags'],
125122
'html string' => [$originalText, $expectedText],

‎lib/internal/Magento/Framework/Image/Adapter/Gd2.php‎

Lines changed: 76 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ protected function _reset()
6161
*/
6262
public function open($filename)
6363
{
64-
if (!$filename || !file_exists($filename)) {
64+
if ($filename === null || !file_exists($filename)) {
6565
throw new FileSystemException(
66-
new Phrase('File "%1" does not exist.', [$this->_fileName])
66+
new Phrase('File "%1" does not exist.', [$filename])
6767
);
6868
}
6969
if (!$filename || filesize($filename) === 0 || !$this->validateURLScheme($filename)) {
@@ -188,7 +188,7 @@ public function save($destination = null, $newName = null)
188188
} else {
189189
$newImage = imagecreate($this->_imageSrcWidth, $this->_imageSrcHeight);
190190
}
191-
$this->_fillBackgroundColor($newImage);
191+
$this->fillBackgroundColor($newImage);
192192
imagecopy($newImage, $this->_imageHandler, 0, 0, 0, 0, $this->_imageSrcWidth, $this->_imageSrcHeight);
193193
$this->imageDestroy();
194194
$this->_imageHandler = $newImage;
@@ -265,64 +265,98 @@ private function _getCallback($callbackType, $fileType = null, $unsupportedText
265265
* Returns a color identifier.
266266
*
267267
* @param resource &$imageResourceTo
268+
*
268269
* @return void
269270
* @throws \InvalidArgumentException
270-
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
271-
* @SuppressWarnings(PHPMD.NPathComplexity)
272271
*/
273-
private function _fillBackgroundColor(&$imageResourceTo)
272+
private function fillBackgroundColor(&$imageResourceTo): void
274273
{
275274
// try to keep transparency, if any
276275
if ($this->_keepTransparency) {
277276
$isAlpha = false;
278277
$transparentIndex = $this->_getTransparency($this->_imageHandler, $this->_fileType, $isAlpha);
278+
279279
try {
280-
// fill truecolor png with alpha transparency
280+
// fill true color png with alpha transparency
281281
if ($isAlpha) {
282-
if (!imagealphablending($imageResourceTo, false)) {
283-
throw new \InvalidArgumentException('Failed to set alpha blending for PNG image.');
284-
}
285-
$transparentAlphaColor = imagecolorallocatealpha($imageResourceTo, 0, 0, 0, 127);
286-
if (false === $transparentAlphaColor) {
287-
throw new \InvalidArgumentException('Failed to allocate alpha transparency for PNG image.');
288-
}
289-
if (!imagefill($imageResourceTo, 0, 0, $transparentAlphaColor)) {
290-
throw new \InvalidArgumentException('Failed to fill PNG image with alpha transparency.');
291-
}
292-
if (!imagesavealpha($imageResourceTo, true)) {
293-
throw new \InvalidArgumentException('Failed to save alpha transparency into PNG image.');
294-
}
295-
} elseif (false !== $transparentIndex) {
296-
// fill image with indexed non-alpha transparency
297-
$transparentColor = false;
298-
if ($transparentIndex >= 0 && $transparentIndex <= imagecolorstotal($this->_imageHandler)) {
299-
list($r, $g, $b) = array_values(imagecolorsforindex($this->_imageHandler, $transparentIndex));
300-
$transparentColor = imagecolorallocate($imageResourceTo, $r, $g, $b);
301-
}
302-
if (false === $transparentColor) {
303-
throw new \InvalidArgumentException('Failed to allocate transparent color for image.');
304-
}
305-
if (!imagefill($imageResourceTo, 0, 0, $transparentColor)) {
306-
throw new \InvalidArgumentException('Failed to fill image with transparency.');
307-
}
308-
imagecolortransparent($imageResourceTo, $transparentColor);
282+
$this->applyAlphaTransparency($imageResourceTo);
283+
284+
return;
285+
}
286+
287+
if ($transparentIndex !== false) {
288+
$this->applyTransparency($imageResourceTo, $transparentIndex);
289+
290+
return;
309291
}
310292
// phpcs:ignore Magento2.CodeAnalysis.EmptyBlock.DetectedCatch
311293
} catch (\Exception $e) {
312294
// fallback to default background color
313295
}
314296
}
315297
list($red, $green, $blue) = $this->_backgroundColor;
298+
$red = $red !== null ? $red : 0;
299+
$green = $green !== null ? $green : 0;
300+
$blue = $blue !== null ? $blue : 0;
301+
$color = imagecolorallocate($imageResourceTo, $red, $green, $blue);
316302

317-
if ($imageResourceTo && $red && $green && $blue) {
318-
$color = imagecolorallocate($imageResourceTo, $red, $green, $blue);
303+
if (!imagefill($imageResourceTo, 0, 0, $color)) {
304+
throw new \InvalidArgumentException("Failed to fill image background with color {$red} {$green} {$blue}.");
305+
}
306+
}
319307

320-
if (!$color && !imagefill($imageResourceTo, 0, 0, $color)) {
321-
throw new \InvalidArgumentException(
322-
"Failed to fill image background with color {$red} {$green} {$blue}."
323-
);
324-
}
308+
/**
309+
* Method to apply alpha transparency for image.
310+
*
311+
* @param resource $imageResourceTo
312+
*
313+
* @return void
314+
* @SuppressWarnings(PHPMD.LongVariable)
315+
*/
316+
private function applyAlphaTransparency(&$imageResourceTo): void
317+
{
318+
if (!imagealphablending($imageResourceTo, false)) {
319+
throw new \InvalidArgumentException('Failed to set alpha blending for PNG image.');
320+
}
321+
$transparentAlphaColor = imagecolorallocatealpha($imageResourceTo, 0, 0, 0, 127);
322+
323+
if (false === $transparentAlphaColor) {
324+
throw new \InvalidArgumentException('Failed to allocate alpha transparency for PNG image.');
325+
}
326+
327+
if (!imagefill($imageResourceTo, 0, 0, $transparentAlphaColor)) {
328+
throw new \InvalidArgumentException('Failed to fill PNG image with alpha transparency.');
329+
}
330+
331+
if (!imagesavealpha($imageResourceTo, true)) {
332+
throw new \InvalidArgumentException('Failed to save alpha transparency into PNG image.');
333+
}
334+
}
335+
336+
/**
337+
* Method to apply transparency for image.
338+
*
339+
* @param resource $imageResourceTo
340+
* @param int $transparentIndex
341+
*
342+
* @return void
343+
*/
344+
private function applyTransparency(&$imageResourceTo, $transparentIndex): void
345+
{
346+
// fill image with indexed non-alpha transparency
347+
$transparentColor = false;
348+
349+
if ($transparentIndex >= 0 && $transparentIndex <= imagecolorstotal($this->_imageHandler)) {
350+
list($red, $green, $blue) = array_values(imagecolorsforindex($this->_imageHandler, $transparentIndex));
351+
$transparentColor = imagecolorallocate($imageResourceTo, (int) $red, (int) $green, (int) $blue);
352+
}
353+
if (false === $transparentColor) {
354+
throw new \InvalidArgumentException('Failed to allocate transparent color for image.');
355+
}
356+
if (!imagefill($imageResourceTo, 0, 0, $transparentColor)) {
357+
throw new \InvalidArgumentException('Failed to fill image with transparency.');
325358
}
359+
imagecolortransparent($imageResourceTo, $transparentColor);
326360
}
327361

328362
/**
@@ -398,7 +432,7 @@ public function resize($frameWidth = null, $frameHeight = null)
398432
}
399433

400434
// fill new image with required color
401-
$this->_fillBackgroundColor($newImage);
435+
$this->fillBackgroundColor($newImage);
402436

403437
if ($this->_imageHandler) {
404438
// resample source image and copy it into new frame

‎lib/internal/Magento/Framework/Serialize/Serializer/Json.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function unserialize($string)
3636
{
3737
if ($string === null) {
3838
throw new \InvalidArgumentException(
39-
sprintf('Invalid parameter: string must be a string type, null given.')
39+
sprintf('Unable to unserialize value. Error: Parameter must be a string type, null given.')
4040
);
4141
}
4242
$result = json_decode($string, true);

‎lib/internal/Magento/Framework/Serialize/Test/Unit/Serializer/JsonTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public function testUnserializeException($value)
101101
$errorMessage = 'Unable to unserialize value.';
102102

103103
if ($value === null) {
104-
$errorMessage = 'Invalid parameter: string must be a string type, null given.';
104+
$errorMessage = 'Unable to unserialize value. Error: Parameter must be a string type, null given.';
105105
}
106106
$this->expectException('InvalidArgumentException');
107107
$this->expectExceptionMessage($errorMessage);

‎lib/internal/Magento/Framework/View/Model/Layout/Merge.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,7 @@ public function getCacheId()
988988
private function extractHandlers(): void
989989
{
990990
foreach ($this->updates as $update) {
991-
$updateXml = false;
991+
$updateXml = null;
992992

993993
try {
994994
$updateXml = is_string($update) ? $this->_loadXmlString($update) : false;

0 commit comments

Comments
 (0)