From 16839952eed2596243ad2f883364d28620f36e28 Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Wed, 25 Oct 2017 18:17:55 +0300 Subject: [PATCH] MAGETWO-5015: Report error csv doesn't work when trying to import a csv file with semicolon delimiter. --- .../Magento/ImportExport/Helper/Report.php | 11 +++++++- .../Magento/ImportExport/Model/Report/Csv.php | 3 ++- .../Test/Unit/Helper/ReportTest.php | 25 +++++++++++++++++++ .../Test/Unit/Model/Report/CsvTest.php | 14 ++++++++++- .../Adminhtml/Import/ValidateTest.php | 23 +++++++++++++---- .../incorrect_catalog_product_comma.csv | 2 ++ .../incorrect_catalog_product_semicolon.csv | 2 ++ 7 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/_files/incorrect_catalog_product_comma.csv create mode 100644 dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/_files/incorrect_catalog_product_semicolon.csv diff --git a/app/code/Magento/ImportExport/Helper/Report.php b/app/code/Magento/ImportExport/Helper/Report.php index 708aa2e39df9a..92eaecfc16903 100644 --- a/app/code/Magento/ImportExport/Helper/Report.php +++ b/app/code/Magento/ImportExport/Helper/Report.php @@ -7,7 +7,6 @@ namespace Magento\ImportExport\Helper; use Magento\Framework\App\Filesystem\DirectoryList; -use Magento\Framework\Stdlib\DateTime; use Magento\ImportExport\Model\Import; /** @@ -127,4 +126,14 @@ protected function getFilePath($filename) { return $this->varDirectory->getRelativePath(Import::IMPORT_HISTORY_DIR . $filename); } + + /** + * Get csv delimiter from request. + * + * @return string + */ + public function getDelimiter() + { + return $this->_request->getParam(Import::FIELD_FIELD_SEPARATOR, ','); + } } diff --git a/app/code/Magento/ImportExport/Model/Report/Csv.php b/app/code/Magento/ImportExport/Model/Report/Csv.php index f45910e519847..86c68d7c4df77 100644 --- a/app/code/Magento/ImportExport/Model/Report/Csv.php +++ b/app/code/Magento/ImportExport/Model/Report/Csv.php @@ -130,7 +130,8 @@ protected function createSourceCsvModel($sourceFile) return $this->sourceCsvFactory->create( [ 'file' => $sourceFile, - 'directory' => $this->filesystem->getDirectoryWrite(DirectoryList::VAR_DIR) + 'directory' => $this->filesystem->getDirectoryWrite(DirectoryList::VAR_DIR), + 'delimiter' => $this->reportHelper->getDelimiter(), ] ); } diff --git a/app/code/Magento/ImportExport/Test/Unit/Helper/ReportTest.php b/app/code/Magento/ImportExport/Test/Unit/Helper/ReportTest.php index baaf0071e54ea..52b6bdcfc5ee7 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Helper/ReportTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Helper/ReportTest.php @@ -44,12 +44,21 @@ class ReportTest extends \PHPUnit\Framework\TestCase */ protected $report; + /** + * @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject + */ + private $requestMock; + /** * Set up */ protected function setUp() { $this->context = $this->createMock(\Magento\Framework\App\Helper\Context::class); + $this->requestMock = $this->getMockBuilder(\Magento\Framework\App\Request\Http::class) + ->disableOriginalConstructor() + ->getMock(); + $this->context->expects($this->any())->method('getRequest')->willReturn($this->requestMock); $this->timezone = $this->createPartialMock( \Magento\Framework\Stdlib\DateTime\Timezone::class, ['date', 'getConfigTimezone', 'diff', 'format'] @@ -159,4 +168,20 @@ public function testGetReportSize() $result = $this->report->getReportSize('file'); $this->assertNull($result); } + + /** + * Test getDelimiter() take into consideration request param '_import_field_separator'. + */ + public function testGetDelimiter() + { + $testDelimiter = 'some delimiter'; + $this->requestMock->expects($this->once()) + ->method('getParam') + ->with($this->identicalTo(\Magento\ImportExport\Model\Import::FIELD_FIELD_SEPARATOR)) + ->willReturn($testDelimiter); + $this->assertEquals( + $testDelimiter, + $this->report->getDelimiter() + ); + } } diff --git a/app/code/Magento/ImportExport/Test/Unit/Model/Report/CsvTest.php b/app/code/Magento/ImportExport/Test/Unit/Model/Report/CsvTest.php index a305127f8461b..cf961d033946e 100644 --- a/app/code/Magento/ImportExport/Test/Unit/Model/Report/CsvTest.php +++ b/app/code/Magento/ImportExport/Test/Unit/Model/Report/CsvTest.php @@ -45,8 +45,10 @@ class CsvTest extends \PHPUnit\Framework\TestCase protected function setUp() { $objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $testDelimiter = 'some_delimiter'; $this->reportHelperMock = $this->createMock(\Magento\ImportExport\Helper\Report::class); + $this->reportHelperMock->expects($this->any())->method('getDelimiter')->willReturn($testDelimiter); $this->outputCsvFactoryMock = $this->createPartialMock( \Magento\ImportExport\Model\Export\Adapter\CsvFactory::class, @@ -65,7 +67,17 @@ protected function setUp() [23 => 'first error'], [27 => 'second error'] ); - $this->sourceCsvFactoryMock->expects($this->any())->method('create')->willReturn($this->sourceCsvMock); + $this->sourceCsvFactoryMock + ->expects($this->any()) + ->method('create') + ->with( + [ + 'file' => 'some_file_name', + 'directory' => null, + 'delimiter' => $testDelimiter + ] + ) + ->willReturn($this->sourceCsvMock); $this->filesystemMock = $this->createMock(\Magento\Framework\Filesystem::class); diff --git a/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/ValidateTest.php b/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/ValidateTest.php index f30663a199a67..552a23a0c1fd5 100644 --- a/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/ValidateTest.php +++ b/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/ValidateTest.php @@ -18,10 +18,11 @@ class ValidateTest extends \Magento\TestFramework\TestCase\AbstractBackendContro * @dataProvider validationDataProvider * @param string $fileName * @param string $message + * @param string $delimiter * @backupGlobals enabled * @magentoDbIsolation enabled */ - public function testValidationReturn($fileName, $message) + public function testValidationReturn($fileName, $message, $delimiter) { $validationStrategy = ProcessingErrorAggregatorInterface::VALIDATION_STRATEGY_STOP_ON_ERROR; @@ -36,7 +37,7 @@ public function testValidationReturn($fileName, $message) $this->getRequest()->setPostValue('behavior', 'append'); $this->getRequest()->setPostValue(Import::FIELD_NAME_VALIDATION_STRATEGY, $validationStrategy); $this->getRequest()->setPostValue(Import::FIELD_NAME_ALLOWED_ERROR_COUNT, 0); - $this->getRequest()->setPostValue('_import_field_separator', ','); + $this->getRequest()->setPostValue('_import_field_separator', $delimiter); /** @var \Magento\TestFramework\App\Filesystem $filesystem */ $filesystem = $this->_objectManager->get(\Magento\Framework\Filesystem::class); @@ -83,12 +84,24 @@ public function validationDataProvider() return [ [ 'file_name' => 'catalog_product.csv', - 'message' => 'File is valid' + 'message' => 'File is valid', + 'delimiter' => ',', ], [ 'file_name' => 'test.txt', - 'message' => '\'txt\' file extension is not supported' - ] + 'message' => '\'txt\' file extension is not supported', + 'delimiter' => ',', + ], + [ + 'file_name' => 'incorrect_catalog_product_comma.csv', + 'message' => 'Download full report', + 'delimiter' => ',', + ], + [ + 'file_name' => 'incorrect_catalog_product_semicolon.csv', + 'message' => 'Download full report', + 'delimiter' => ';', + ], ]; } } diff --git a/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/_files/incorrect_catalog_product_comma.csv b/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/_files/incorrect_catalog_product_comma.csv new file mode 100644 index 0000000000000..67114a40b2244 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/_files/incorrect_catalog_product_comma.csv @@ -0,0 +1,2 @@ +sku,store_view_code,attribute_set_code,product_type,categories,product_websites,name,description,short_description,weight,product_online,tax_class_name,visibility,price,special_price,special_price_from_date,special_price_to_date,url_key,meta_title,meta_keywords,meta_description,base_image,base_image_label,small_image,small_image_label,thumbnail_image,thumbnail_image_label,swatch_image,swatch_image_label,created_at,updated_at,new_from_date,new_to_date,display_product_options_in,map_price,msrp_price,map_enabled,gift_message_available,custom_design,custom_design_from,custom_design_to,custom_layout_update,page_layout,product_options_container,msrp_display_actual_price_type,country_of_manufacture,additional_attributes,qty,out_of_stock_qty,use_config_min_qty,is_qty_decimal,allow_backorders,use_config_backorders,min_cart_qty,use_config_min_sale_qty,max_cart_qty,use_config_max_sale_qty,is_in_stock,notify_on_stock_below,use_config_notify_stock_qty,manage_stock,use_config_manage_stock,use_config_qty_increments,qty_increments,use_config_enable_qty_inc,enable_qty_increments,is_decimal_divided,website_id,related_skus,related_position,crosssell_skus,crosssell_position,upsell_skus,upsell_position,additional_images,additional_image_labels,hide_from_product_page,custom_options,bundle_price_type,bundle_sku_type,bundle_price_view,bundle_weight_type,bundle_values,bundle_shipment_type,configurable_variations,configurable_variation_labels,associated_skus +Simple,,Default,simple,"Default Category/New",base,Simple,,,,1,"Taxable Goods","Catalo g, Search",100.0000,,,,simple,Simple,Simple,"Simple ",,,,,,,,,"10/25/17, 8:21 AM","10/25/17, 8:21 AM",,,"Block after Info Column",,,,"Use config",,,,,,,,,,100.0000,0.0000,1,0,0,1,1.0000,1,10000.0000,1,1,1.0000,1,1,1,1,1.0000,1,0,0,0,,,,,,,,,,,,,,,,,,, diff --git a/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/_files/incorrect_catalog_product_semicolon.csv b/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/_files/incorrect_catalog_product_semicolon.csv new file mode 100644 index 0000000000000..d0a0b8639cf78 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/ImportExport/Controller/Adminhtml/Import/_files/incorrect_catalog_product_semicolon.csv @@ -0,0 +1,2 @@ +sku;store_view_code;attribute_set_code;product_type;categories;product_websites;name;description;short_description;weight;product_online;tax_class_name;visibility;price;special_price;special_price_from_date;special_price_to_date;url_key;meta_title;meta_keywords;meta_description;base_image;base_image_label;small_image;small_image_label;thumbnail_image;thumbnail_image_label;swatch_image;swatch_image_label;created_at;updated_at;new_from_date;new_to_date;display_product_options_in;map_price;msrp_price;map_enabled;gift_message_available;custom_design;custom_design_from;custom_design_to;custom_layout_update;page_layout;product_options_container;msrp_display_actual_price_type;country_of_manufacture;additional_attributes;qty;out_of_stock_qty;use_config_min_qty;is_qty_decimal;allow_backorders;use_config_backorders;min_cart_qty;use_config_min_sale_qty;max_cart_qty;use_config_max_sale_qty;is_in_stock;notify_on_stock_below;use_config_notify_stock_qty;manage_stock;use_config_manage_stock;use_config_qty_increments;qty_increments;use_config_enable_qty_inc;enable_qty_increments;is_decimal_divided;website_id;related_skus;related_position;crosssell_skus;crosssell_position;upsell_skus;upsell_position;additional_images;additional_image_labels;hide_from_product_page;custom_options;bundle_price_type;bundle_sku_type;bundle_price_view;bundle_weight_type;bundle_values;bundle_shipment_type;configurable_variations;configurable_variation_labels;associated_skus +Simple;;Default;simple;"Default Category/New";base;Simple;;;;1;"Taxable Goods";"Catalo g, Search";100.0000;;;;simple;Simple;Simple;"Simple ";;;;;;;;;"10/25/17, 8,21 AM";"10/25/17, 8,21 AM";;;"Block after Info Column";;;;"Use config";;;;;;;;;;100.0000;0.0000;1;0;0;1;1.0000;1;10000.0000;1;1;1.0000;1;1;1;1;1.0000;1;0;0;0;;;;;;;;;;;;;;;;;;;