Skip to content

Commit 867c49b

Browse files
author
Sergey Semenov
committed
MAGETWO-62947: Can't ignore errors during customer Import
1 parent 36a4d76 commit 867c49b

File tree

4 files changed

+372
-32
lines changed

4 files changed

+372
-32
lines changed

‎app/code/Magento/ImportExport/Controller/Adminhtml/Import/Validate.php

Lines changed: 77 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@
77

88
use Magento\ImportExport\Controller\Adminhtml\ImportResult as ImportResultController;
99
use Magento\ImportExport\Model\Import;
10-
use Magento\ImportExport\Block\Adminhtml\Import\Frame\Result as ImportResultBlock;
10+
use Magento\ImportExport\Block\Adminhtml\Import\Frame\Result;
1111
use Magento\Framework\Controller\ResultFactory;
1212
use Magento\Framework\App\Filesystem\DirectoryList;
1313
use Magento\ImportExport\Model\Import\Adapter as ImportAdapter;
14+
use Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingErrorAggregatorInterface;
1415

1516
class Validate extends ImportResultController
1617
{
@@ -29,7 +30,7 @@ public function execute()
2930
$data = $this->getRequest()->getPostValue();
3031
/** @var \Magento\Framework\View\Result\Layout $resultLayout */
3132
$resultLayout = $this->resultFactory->create(ResultFactory::TYPE_LAYOUT);
32-
/** @var $resultBlock ImportResultBlock */
33+
/** @var $resultBlock Result */
3334
$resultBlock = $resultLayout->getLayout()->getBlock('import.frame.result');
3435
if ($data) {
3536
// common actions
@@ -66,36 +67,27 @@ public function execute()
6667
}
6768

6869
/**
70+
* Process validation result and add required error or success messages to Result block
71+
*
6972
* @param bool $validationResult
70-
* @param ImportResultBlock $resultBlock
73+
* @param Result $resultBlock
7174
* @return void
7275
*/
7376
private function processValidationResult($validationResult, $resultBlock)
7477
{
7578
$import = $this->getImport();
76-
if (!$import->getProcessedRowsCount()) {
77-
if (!$import->getErrorAggregator()->getErrorsCount()) {
78-
$resultBlock->addError(__('This file is empty. Please try another one.'));
79+
$errorAggregator = $import->getErrorAggregator();
80+
81+
if ($import->getProcessedRowsCount()) {
82+
if ($validationResult) {
83+
$this->addMessageForValidResult($resultBlock);
7984
} else {
80-
foreach ($import->getErrorAggregator()->getAllErrors() as $error) {
81-
$resultBlock->addError($error->getErrorMessage());
82-
}
83-
}
84-
} else {
85-
$errorAggregator = $import->getErrorAggregator();
86-
if (!$validationResult) {
8785
$resultBlock->addError(
8886
__('Data validation failed. Please fix the following errors and upload the file again.')
8987
);
9088
$this->addErrorMessages($resultBlock, $errorAggregator);
91-
} else {
92-
if ($import->isImportAllowed()) {
93-
$resultBlock->addSuccess(
94-
__('File is valid! To start import process press "Import" button'),
95-
true
96-
);
97-
} else {
98-
$resultBlock->addError(__('The file is valid, but we can\'t import it for some reason.'));
89+
if ($errorAggregator->getErrorsCount()) {
90+
$this->addMessageToSkipErrors($resultBlock);
9991
}
10092
}
10193
$resultBlock->addNotice(
@@ -107,6 +99,12 @@ private function processValidationResult($validationResult, $resultBlock)
10799
$errorAggregator->getErrorsCount()
108100
)
109101
);
102+
} else {
103+
if ($errorAggregator->getErrorsCount()) {
104+
$this->collectErrors($resultBlock);
105+
} else {
106+
$resultBlock->addError(__('This file is empty. Please try another one.'));
107+
}
110108
}
111109
}
112110

@@ -121,4 +119,62 @@ private function getImport()
121119
}
122120
return $this->import;
123121
}
122+
123+
/**
124+
* Add error message to Result block and allow 'Import' button
125+
*
126+
* If validation strategy is equal to 'validation-skip-errors' and validation error limit is not exceeded,
127+
* then add error message and allow 'Import' button.
128+
*
129+
* @param Result $resultBlock
130+
* @return void
131+
*/
132+
private function addMessageToSkipErrors(Result $resultBlock)
133+
{
134+
$import = $this->getImport();
135+
if ($import->getValidationStrategy() == ProcessingErrorAggregatorInterface::VALIDATION_STRATEGY_SKIP_ERRORS
136+
&& !$import->getErrorAggregator()->hasFatalExceptions()
137+
) {
138+
$resultBlock->addSuccess(
139+
__('Please fix errors and re-upload file or simply press "Import" button to skip rows with errors'),
140+
true
141+
);
142+
}
143+
}
144+
145+
/**
146+
* Add success message to Result block
147+
*
148+
* 1. Add message for case when imported data was checked and result is valid.
149+
* 2. Add message for case when imported data was checked and result is valid, but import is not allowed.
150+
*
151+
* @param Result $resultBlock
152+
* @return void
153+
*/
154+
private function addMessageForValidResult(Result $resultBlock)
155+
{
156+
if ($this->getImport()->isImportAllowed()) {
157+
$resultBlock->addSuccess(__('File is valid! To start import process press "Import" button'), true);
158+
} else {
159+
$resultBlock->addError(__('The file is valid, but we can\'t import it for some reason.'));
160+
}
161+
}
162+
163+
/**
164+
* Collect errors and add error messages to Result block
165+
*
166+
* Get all errors from ProcessingErrorAggregatorInterface and add appropriated error messages
167+
* to Result block.
168+
*
169+
* @param Result $resultBlock
170+
* @return void
171+
*/
172+
private function collectErrors(Result $resultBlock)
173+
{
174+
$import = $this->getImport();
175+
$errors = $import->getErrorAggregator()->getAllErrors();
176+
foreach ($errors as $error) {
177+
$resultBlock->addError($error->getErrorMessage());
178+
}
179+
}
124180
}

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,19 +561,28 @@ protected function _removeBom($sourceFile)
561561
}
562562

563563
/**
564-
* Validates source file and returns validation result.
564+
* Validates source file and returns validation result
565+
*
566+
* Before validate data the method requires to initialize error aggregator (ProcessingErrorAggregatorInterface)
567+
* with 'validation strategy' and 'allowed error count' values to allow using this parameters in validation process.
565568
*
566569
* @param \Magento\ImportExport\Model\Import\AbstractSource $source
567570
* @return bool
568571
*/
569572
public function validateSource(\Magento\ImportExport\Model\Import\AbstractSource $source)
570573
{
571574
$this->addLogComment(__('Begin data validation'));
575+
576+
$errorAggregator = $this->getErrorAggregator();
577+
$errorAggregator->initValidationStrategy(
578+
$this->getData(self::FIELD_NAME_VALIDATION_STRATEGY),
579+
$this->getData(self::FIELD_NAME_ALLOWED_ERROR_COUNT)
580+
);
581+
572582
try {
573583
$adapter = $this->_getEntityAdapter()->setSource($source);
574-
$errorAggregator = $adapter->validateData();
584+
$adapter->validateData();
575585
} catch (\Exception $e) {
576-
$errorAggregator = $this->getErrorAggregator();
577586
$errorAggregator->addError(
578587
\Magento\ImportExport\Model\Import\Entity\AbstractEntity::ERROR_CODE_SYSTEM_EXCEPTION,
579588
ProcessingError::ERROR_LEVEL_CRITICAL,

‎app/code/Magento/ImportExport/Test/Unit/Controller/Adminhtml/Import/ValidateTest.php

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
<?php
2+
/**
3+
* Copyright © 2013-2017 Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\ImportExport\Test\Unit\Controller\Adminhtml\Import;
7+
8+
class ValidateTest extends \PHPUnit_Framework_TestCase
9+
{
10+
/**
11+
* @var \Magento\Backend\App\Action\Context|\PHPUnit_Framework_MockObject_MockObject
12+
*/
13+
private $contextMock;
14+
15+
/**
16+
* @var \Magento\ImportExport\Model\Report\ReportProcessorInterface|\PHPUnit_Framework_MockObject_MockObject
17+
*/
18+
private $reportProcessorMock;
19+
20+
/**
21+
* @var \Magento\ImportExport\Model\History|\PHPUnit_Framework_MockObject_MockObject
22+
*/
23+
private $historyMock;
24+
25+
/**
26+
* @var \Magento\ImportExport\Helper\Report|\PHPUnit_Framework_MockObject_MockObject
27+
*/
28+
private $reportHelperMock;
29+
30+
/**
31+
* @var \Magento\ImportExport\Controller\Adminhtml\Import\Validate
32+
*/
33+
private $validate;
34+
35+
/**
36+
* @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject
37+
*/
38+
private $requestMock;
39+
40+
/**
41+
* @var \Magento\Framework\Controller\ResultFactory|\PHPUnit_Framework_MockObject_MockObject
42+
*/
43+
private $resultFactoryMock;
44+
45+
/**
46+
* @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject
47+
*/
48+
private $messageManagerMock;
49+
50+
protected function setUp()
51+
{
52+
$this->requestMock = $this->getMockBuilder(\Magento\Framework\App\Request\Http::class)
53+
->disableOriginalConstructor()
54+
->setMethods([
55+
'getPostValue',
56+
'isPost',
57+
])
58+
->getMock();
59+
60+
$this->resultFactoryMock = $this->getMockBuilder(\Magento\Framework\Controller\ResultFactory::class)
61+
->disableOriginalConstructor()
62+
->getMock();
63+
64+
$this->messageManagerMock = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class)
65+
->getMockForAbstractClass();
66+
67+
$this->contextMock = $this->getMockBuilder(\Magento\Backend\App\Action\Context::class)
68+
->disableOriginalConstructor()
69+
->getMock();
70+
$this->contextMock->expects($this->any())
71+
->method('getRequest')
72+
->willReturn($this->requestMock);
73+
$this->contextMock->expects($this->any())
74+
->method('getResultFactory')
75+
->willReturn($this->resultFactoryMock);
76+
$this->contextMock->expects($this->any())
77+
->method('getMessageManager')
78+
->willReturn($this->messageManagerMock);
79+
80+
$this->reportProcessorMock = $this->getMockBuilder(
81+
\Magento\ImportExport\Model\Report\ReportProcessorInterface::class
82+
)
83+
->getMockForAbstractClass();
84+
85+
$this->historyMock = $this->getMockBuilder(\Magento\ImportExport\Model\History::class)
86+
->disableOriginalConstructor()
87+
->getMock();
88+
89+
$this->reportHelperMock = $this->getMockBuilder(\Magento\ImportExport\Helper\Report::class)
90+
->disableOriginalConstructor()
91+
->getMock();
92+
93+
$this->validate = new \Magento\ImportExport\Controller\Adminhtml\Import\Validate(
94+
$this->contextMock,
95+
$this->reportProcessorMock,
96+
$this->historyMock,
97+
$this->reportHelperMock
98+
);
99+
}
100+
101+
/**
102+
* Test execute() method
103+
*
104+
* Check the case in which no data was posted.
105+
*/
106+
public function testNoDataWasPosted()
107+
{
108+
$data = null;
109+
110+
$this->requestMock->expects($this->once())
111+
->method('getPostValue')
112+
->willReturn($data);
113+
114+
$resultBlock = $this->getMockBuilder(\Magento\ImportExport\Block\Adminhtml\Import\Frame\Result::class)
115+
->disableOriginalConstructor()
116+
->getMock();
117+
118+
$layoutMock = $this->getMockBuilder(\Magento\Framework\View\LayoutInterface::class)
119+
->getMockForAbstractClass();
120+
$layoutMock->expects($this->once())
121+
->method('getBlock')
122+
->with('import.frame.result')
123+
->willReturn($resultBlock);
124+
125+
$resultLayoutMock = $this->getMockBuilder(\Magento\Framework\View\Result\Layout::class)
126+
->disableOriginalConstructor()
127+
->getMock();
128+
$resultLayoutMock->expects($this->once())
129+
->method('getLayout')
130+
->willReturn($layoutMock);
131+
132+
$resultRedirectMock = $this->getMockBuilder(\Magento\Backend\Model\View\Result\Redirect::class)
133+
->disableOriginalConstructor()
134+
->getMock();
135+
$resultRedirectMock->expects($this->once())
136+
->method('setPath')
137+
->with('adminhtml/*/index');
138+
139+
$this->resultFactoryMock->expects($this->exactly(2))
140+
->method('create')
141+
->willReturnMap([
142+
[\Magento\Framework\Controller\ResultFactory::TYPE_LAYOUT, [], $resultLayoutMock],
143+
[\Magento\Framework\Controller\ResultFactory::TYPE_REDIRECT, [], $resultRedirectMock],
144+
]);
145+
146+
$this->messageManagerMock->expects($this->once())
147+
->method('addError')
148+
->with(__('Sorry, but the data is invalid or the file is not uploaded.'));
149+
150+
$this->assertEquals($resultRedirectMock, $this->validate->execute());
151+
}
152+
153+
/**
154+
* Test execute() method
155+
*
156+
* Check the case in which the import file was not uploaded.
157+
*/
158+
public function testFileWasNotUploaded()
159+
{
160+
$data = false;
161+
162+
$this->requestMock->expects($this->once())
163+
->method('getPostValue')
164+
->willReturn($data);
165+
$this->requestMock->expects($this->once())
166+
->method('isPost')
167+
->willReturn(true);
168+
169+
$resultBlock = $this->getMockBuilder(\Magento\ImportExport\Block\Adminhtml\Import\Frame\Result::class)
170+
->disableOriginalConstructor()
171+
->getMock();
172+
$resultBlock->expects($this->once())
173+
->method('addError')
174+
->with(__('The file was not uploaded.'));
175+
176+
$layoutMock = $this->getMockBuilder(\Magento\Framework\View\LayoutInterface::class)
177+
->getMockForAbstractClass();
178+
$layoutMock->expects($this->once())
179+
->method('getBlock')
180+
->with('import.frame.result')
181+
->willReturn($resultBlock);
182+
183+
$resultLayoutMock = $this->getMockBuilder(\Magento\Framework\View\Result\Layout::class)
184+
->disableOriginalConstructor()
185+
->getMock();
186+
$resultLayoutMock->expects($this->once())
187+
->method('getLayout')
188+
->willReturn($layoutMock);
189+
190+
$this->resultFactoryMock->expects($this->once())
191+
->method('create')
192+
->with(\Magento\Framework\Controller\ResultFactory::TYPE_LAYOUT)
193+
->willReturn($resultLayoutMock);
194+
195+
$this->assertEquals($resultLayoutMock, $this->validate->execute());
196+
}
197+
}

0 commit comments

Comments
 (0)