Skip to content

Commit 373bd01

Browse files
ishakhsuvarovishakhsuvarov
authored
Merge pull request #1637 from magento-engcom/2.2-develop-prs
[EngCom] Public Pull Requests - 2.2-develop - MAGETWO-82954: Admin product search - Pressing enter does not submit #4696 #11827 - MAGETWO-82953: Fix #11581: Reference to wrong / non-existing class #11830 - MAGETWO-82887: X-Magento-Tags header containing whitespaces causes exception #11767 - MAGETWO-82886: Improve error reporting for products images import. #11779 - MAGETWO-82754: Magento setup:install interactive shell #11425 - MAGETWO-81594: Issue #6924: Unmask exception message during product import #11363
2 parents 73c7ac2 + eeb0e6f commit 373bd01

File tree

14 files changed

+344
-19
lines changed

14 files changed

+344
-19
lines changed

‎app/code/Magento/CatalogImportExport/Model/Import/Product.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,6 +2042,7 @@ protected function uploadMediaFiles($fileName, $renameFileOff = false)
20422042
$res = $this->_getUploader()->move($fileName, $renameFileOff);
20432043
return $res['file'];
20442044
} catch (\Exception $e) {
2045+
$this->_logger->critical($e);
20452046
return '';
20462047
}
20472048
}

‎app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/ProductTest.php

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,6 +1194,70 @@ public function testParseAttributesWithWrappedValuesWillReturnsLowercasedAttribu
11941194
$this->assertArrayNotHasKey('PARAM2', $attributes);
11951195
}
11961196

1197+
/**
1198+
* Test that errors occurred during importing images are logged.
1199+
*
1200+
* @param string $fileName
1201+
* @param bool $throwException
1202+
* @dataProvider uploadMediaFilesDataProvider
1203+
*/
1204+
public function testUploadMediaFiles(string $fileName, bool $throwException)
1205+
{
1206+
$exception = new \Exception();
1207+
$expectedFileName = $fileName;
1208+
if ($throwException) {
1209+
$expectedFileName = '';
1210+
$this->_logger->expects($this->once())->method('critical')->with($exception);
1211+
}
1212+
1213+
$fileUploaderMock = $this
1214+
->getMockBuilder(\Magento\CatalogImportExport\Model\Import\Uploader::class)
1215+
->disableOriginalConstructor()
1216+
->getMock();
1217+
1218+
$fileUploaderMock
1219+
->expects($this->once())
1220+
->method('move')
1221+
->willReturnCallback(
1222+
function ($name) use ($throwException, $exception) {
1223+
if ($throwException) {
1224+
throw $exception;
1225+
}
1226+
return ['file' => $name];
1227+
}
1228+
);
1229+
1230+
$this->setPropertyValue(
1231+
$this->importProduct,
1232+
'_fileUploader',
1233+
$fileUploaderMock
1234+
);
1235+
1236+
$actualFileName = $this->invokeMethod(
1237+
$this->importProduct,
1238+
'uploadMediaFiles',
1239+
[$fileName]
1240+
);
1241+
1242+
$this->assertEquals(
1243+
$expectedFileName,
1244+
$actualFileName
1245+
);
1246+
}
1247+
1248+
/**
1249+
* Data provider for testUploadMediaFiles.
1250+
*
1251+
* @return array
1252+
*/
1253+
public function uploadMediaFilesDataProvider()
1254+
{
1255+
return [
1256+
['test1.jpg', false],
1257+
['test2.jpg', true],
1258+
];
1259+
}
1260+
11971261
public function getImagesFromRowDataProvider()
11981262
{
11991263
return [

‎app/code/Magento/ImportExport/Model/Import.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,6 @@ public function validateSource(\Magento\ImportExport\Model\Import\AbstractSource
567567
ProcessingError::ERROR_LEVEL_CRITICAL,
568568
null,
569569
null,
570-
null,
571570
$e->getMessage()
572571
);
573572
}

‎app/code/Magento/Sales/Model/Order/Pdf/Creditmemo.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,12 @@ public function __construct(
7575
protected function _drawHeader(\Zend_Pdf_Page $page)
7676
{
7777
$this->_setFontRegular($page, 10);
78-
$page->setFillColor(new \Zend_Pdf_Color_RGB(0.93, 0.92, 0.92));
78+
$page->setFillColor(new \Zend_Pdf_Color_Rgb(0.93, 0.92, 0.92));
7979
$page->setLineColor(new \Zend_Pdf_Color_GrayScale(0.5));
8080
$page->setLineWidth(0.5);
8181
$page->drawRectangle(25, $this->y, 570, $this->y - 30);
8282
$this->y -= 10;
83-
$page->setFillColor(new \Zend_Pdf_Color_RGB(0, 0, 0));
83+
$page->setFillColor(new \Zend_Pdf_Color_Rgb(0, 0, 0));
8484

8585
//columns headers
8686
$lines[0][] = ['text' => __('Products'), 'feed' => 35];

‎app/code/Magento/Sales/Model/Order/Pdf/Invoice.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ protected function _drawHeader(\Zend_Pdf_Page $page)
8282
{
8383
/* Add table head */
8484
$this->_setFontRegular($page, 10);
85-
$page->setFillColor(new \Zend_Pdf_Color_RGB(0.93, 0.92, 0.92));
85+
$page->setFillColor(new \Zend_Pdf_Color_Rgb(0.93, 0.92, 0.92));
8686
$page->setLineColor(new \Zend_Pdf_Color_GrayScale(0.5));
8787
$page->setLineWidth(0.5);
8888
$page->drawRectangle(25, $this->y, 570, $this->y - 15);
8989
$this->y -= 10;
90-
$page->setFillColor(new \Zend_Pdf_Color_RGB(0, 0, 0));
90+
$page->setFillColor(new \Zend_Pdf_Color_Rgb(0, 0, 0));
9191

9292
//columns headers
9393
$lines[0][] = ['text' => __('Products'), 'feed' => 35];

‎app/code/Magento/Sales/Model/Order/Pdf/Shipment.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,12 @@ protected function _drawHeader(\Zend_Pdf_Page $page)
8080
{
8181
/* Add table head */
8282
$this->_setFontRegular($page, 10);
83-
$page->setFillColor(new \Zend_Pdf_Color_RGB(0.93, 0.92, 0.92));
83+
$page->setFillColor(new \Zend_Pdf_Color_Rgb(0.93, 0.92, 0.92));
8484
$page->setLineColor(new \Zend_Pdf_Color_GrayScale(0.5));
8585
$page->setLineWidth(0.5);
8686
$page->drawRectangle(25, $this->y, 570, $this->y - 15);
8787
$this->y -= 10;
88-
$page->setFillColor(new \Zend_Pdf_Color_RGB(0, 0, 0));
88+
$page->setFillColor(new \Zend_Pdf_Color_Rgb(0, 0, 0));
8989

9090
//columns headers
9191
$lines[0][] = ['text' => __('Products'), 'feed' => 100];

‎app/code/Magento/Ui/view/base/web/templates/grid/filters/filters.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
<scope args="chips" render=""/>
1616

17-
<div class="admin__data-grid-filters-wrap" data-part="filter-form" css="_show: hasVisible() && $collapsible.opened()">
17+
<div class="admin__data-grid-filters-wrap" data-part="filter-form" css="_show: hasVisible() && $collapsible.opened()" keyboard="{ 13: apply }">
1818
<fieldset class="admin__fieldset admin__data-grid-filters">
1919
<legend class="admin__filters-legend">
2020
<span translate="'Advanced filter'"/>

‎dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
use Magento\Framework\Filesystem;
2727
use Magento\ImportExport\Model\Import;
2828
use Magento\Store\Model\Store;
29-
use Magento\UrlRewrite\Model\UrlRewrite;
29+
use Psr\Log\LoggerInterface;
3030

3131
/**
3232
* Class ProductTest
@@ -62,11 +62,20 @@ class ProductTest extends \Magento\TestFramework\Indexer\TestCase
6262
*/
6363
protected $objectManager;
6464

65+
/**
66+
* @var LoggerInterface|\PHPUnit_Framework_MockObject_MockObject
67+
*/
68+
private $logger;
69+
6570
protected function setUp()
6671
{
6772
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
73+
$this->logger = $this->getMockBuilder(LoggerInterface::class)
74+
->disableOriginalConstructor()
75+
->getMock();
6876
$this->_model = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
69-
\Magento\CatalogImportExport\Model\Import\Product::class
77+
\Magento\CatalogImportExport\Model\Import\Product::class,
78+
['logger' => $this->logger]
7079
);
7180
parent::setUp();
7281
}
@@ -659,6 +668,21 @@ public function testSaveMediaImage()
659668
$this->assertEquals('Additional Image Label Two', $additionalImageTwoItem->getLabel());
660669
}
661670

671+
/**
672+
* Test that errors occurred during importing images are logged.
673+
*
674+
* @magentoDataIsolation enabled
675+
* @magentoAppIsolation enabled
676+
* @magentoDataFixture mediaImportImageFixture
677+
* @magentoDataFixture mediaImportImageFixtureError
678+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
679+
*/
680+
public function testSaveMediaImageError()
681+
{
682+
$this->logger->expects(self::once())->method('critical');
683+
$this->importDataForMediaTest('import_media.csv', 1);
684+
}
685+
662686
/**
663687
* Copy fixture images into media import directory
664688
*/
@@ -717,6 +741,30 @@ public static function mediaImportImageFixtureRollback()
717741
$mediaDirectory->delete('catalog');
718742
}
719743

744+
/**
745+
* Copy incorrect fixture image into media import directory.
746+
*/
747+
public static function mediaImportImageFixtureError()
748+
{
749+
/** @var \Magento\Framework\Filesystem\Directory\Write $mediaDirectory */
750+
$mediaDirectory = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
751+
\Magento\Framework\Filesystem::class
752+
)->getDirectoryWrite(
753+
DirectoryList::MEDIA
754+
);
755+
$dirPath = $mediaDirectory->getAbsolutePath('import');
756+
$items = [
757+
[
758+
'source' => __DIR__ . '/_files/magento_additional_image_error.jpg',
759+
'dest' => $dirPath . '/magento_additional_image_two.jpg',
760+
],
761+
];
762+
foreach ($items as $item) {
763+
copy($item['source'], $item['dest']);
764+
}
765+
}
766+
767+
720768
/**
721769
* Export CSV string to array
722770
*
@@ -1564,8 +1612,9 @@ public function testProductWithWrappedAdditionalAttributes()
15641612
* Import and check data from file
15651613
*
15661614
* @param string $fileName
1615+
* @param int $expectedErrors
15671616
*/
1568-
private function importDataForMediaTest($fileName)
1617+
private function importDataForMediaTest(string $fileName, int $expectedErrors = 0)
15691618
{
15701619
$filesystem = $this->objectManager->create(\Magento\Framework\Filesystem::class);
15711620
$directory = $filesystem->getDirectoryWrite(DirectoryList::ROOT);
@@ -1603,7 +1652,7 @@ private function importDataForMediaTest($fileName)
16031652
$this->assertTrue($errors->getErrorsCount() == 0);
16041653

16051654
$this->_model->importData();
1606-
$this->assertTrue($this->_model->getErrorAggregator()->getErrorsCount() == 0);
1655+
$this->assertTrue($this->_model->getErrorAggregator()->getErrorsCount() == $expectedErrors);
16071656
}
16081657

16091658
/**

‎dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/magento_additional_image_error.jpg

Loading

‎dev/tests/integration/testsuite/Magento/ImportExport/Model/ImportTest.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,31 @@ public function testValidateSourceException()
135135
$this->_model->validateSource($source);
136136
}
137137

138+
public function testValidateSourceExceptionMessage()
139+
{
140+
$exceptionMessage = 'Test Exception Message.';
141+
142+
$validationStrategy = ProcessingErrorAggregatorInterface::VALIDATION_STRATEGY_STOP_ON_ERROR;
143+
$this->_model->setEntity('catalog_product');
144+
$this->_model->setData(\Magento\ImportExport\Model\Import::FIELD_NAME_VALIDATION_STRATEGY, $validationStrategy);
145+
$this->_model->setData(\Magento\ImportExport\Model\Import::FIELD_NAME_ALLOWED_ERROR_COUNT, 0);
146+
147+
/** @var \Magento\ImportExport\Model\Import\AbstractSource|\PHPUnit_Framework_MockObject_MockObject $source */
148+
$source = $this->getMockForAbstractClass(
149+
\Magento\ImportExport\Model\Import\AbstractSource::class,
150+
[['sku', 'name']]
151+
);
152+
$source->expects($this->any())->method('_getNextRow')->willThrowException(
153+
new \Exception($exceptionMessage)
154+
);
155+
156+
$this->assertFalse($this->_model->validateSource($source));
157+
$this->assertEquals(
158+
$exceptionMessage,
159+
$this->_model->getErrorAggregator()->getAllErrors()[0]->getErrorMessage()
160+
);
161+
}
162+
138163
public function testGetEntity()
139164
{
140165
$entityName = 'entity_name';

‎lib/internal/Magento/Framework/Config/Converter/Dom/Flat.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ public function convert(\DOMNode $source, $basePath = '')
102102
}
103103
} else {
104104
if ($result) {
105-
$result['value'] = $value;
105+
$result['value'] = trim($value);
106106
} else {
107-
$result = $value;
107+
$result = trim($value);
108108
}
109109
}
110110
return $result;

‎lib/internal/Magento/Framework/Config/Test/Unit/_files/converter/dom/flat/result.php

Lines changed: 7 additions & 0 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+
// @codingStandardsIgnoreFile
67
return [
78
'root' => [
89
'node_one' => [
@@ -11,14 +12,20 @@
1112
'subnode' => [
1213
['attributeThree' => '30'],
1314
['attributeThree' => '40', 'attributeFour' => '40', 'value' => 'Value1'],
15+
['attributeThree' => '50', 'value' => 'value_from_new_line'],
16+
['attributeThree' => '60', 'value' => 'auto_formatted_by_ide_value_due_to_line_size_restriction']
1417
],
1518
'books' => ['attributeFive' => '50'],
1619
],
1720
'multipleNode' => [
1821
'one' => ['id' => 'one', 'name' => 'name1', 'value' => '1'],
1922
'two' => ['id' => 'two', 'name' => 'name2', 'value' => '2'],
23+
'three' => ['id' => 'three', 'name' => 'name3', 'value' => 'value_from_new_line'],
24+
'four' => ['id' => 'four', 'name' => 'name4', 'value' => 'auto_formatted_by_ide_value_due_to_line_size_restriction'],
2025
],
2126
'someOtherVal' => '',
2227
'someDataVal' => '',
28+
'valueFromNewLine' => 'value_from_new_line',
29+
'autoFormattedValue' => 'auto_formatted_by_ide_value_due_to_line_size_restriction'
2330
]
2431
];

‎lib/internal/Magento/Framework/Config/Test/Unit/_files/converter/dom/flat/source.xml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@
99
<node_one attributeOne = '10' attributeTwo = '20'>
1010
<subnode attributeThree = '30'></subnode>
1111
<subnode attributeThree = '40' attributeFour = '40' >Value1</subnode>
12+
<subnode attributeThree = '50'>
13+
value_from_new_line
14+
</subnode>
15+
<subnode attributeThree = '60'>auto_formatted_by_ide_value_due_to_line_size_restriction
16+
</subnode>
1217
<books attributeFive = '50' />
1318
</node_one>
1419
<multipleNode id="one" name="name1">
@@ -19,4 +24,18 @@
1924
</multipleNode>
2025
<someOtherVal></someOtherVal>
2126
<someDataVal><![CDATA[]]></someDataVal>
27+
<multipleNode id="three" name="name3">
28+
<value>
29+
value_from_new_line
30+
</value>
31+
</multipleNode>
32+
<multipleNode id="four" name="name4">
33+
<value>auto_formatted_by_ide_value_due_to_line_size_restriction
34+
</value>
35+
</multipleNode>
36+
<valueFromNewLine>
37+
value_from_new_line
38+
</valueFromNewLine>
39+
<autoFormattedValue>auto_formatted_by_ide_value_due_to_line_size_restriction
40+
</autoFormattedValue>
2241
</root>

0 commit comments

Comments
 (0)