Skip to content

Commit 774eb3f

Browse files
slavvkaslavvka
committed
Merge remote-tracking branch 'origin/MAGETWO-62947' into BUGS
2 parents 6e95f7f + ed8741b commit 774eb3f

File tree

6 files changed

+383
-34
lines changed

6 files changed

+383
-34
lines changed

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

Lines changed: 75 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,60 @@ 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->getErrorAggregator()->hasFatalExceptions()) {
136+
$resultBlock->addSuccess(
137+
__('Please fix errors and re-upload file or simply press "Import" button to skip rows with errors'),
138+
true
139+
);
140+
}
141+
}
142+
143+
/**
144+
* Add success message to Result block
145+
*
146+
* 1. Add message for case when imported data was checked and result is valid.
147+
* 2. Add message for case when imported data was checked and result is valid, but import is not allowed.
148+
*
149+
* @param Result $resultBlock
150+
* @return void
151+
*/
152+
private function addMessageForValidResult(Result $resultBlock)
153+
{
154+
if ($this->getImport()->isImportAllowed()) {
155+
$resultBlock->addSuccess(__('File is valid! To start import process press "Import" button'), true);
156+
} else {
157+
$resultBlock->addError(__('The file is valid, but we can\'t import it for some reason.'));
158+
}
159+
}
160+
161+
/**
162+
* Collect errors and add error messages to Result block
163+
*
164+
* Get all errors from ProcessingErrorAggregatorInterface and add appropriated error messages
165+
* to Result block.
166+
*
167+
* @param Result $resultBlock
168+
* @return void
169+
*/
170+
private function collectErrors(Result $resultBlock)
171+
{
172+
$import = $this->getImport();
173+
$errors = $import->getErrorAggregator()->getAllErrors();
174+
foreach ($errors as $error) {
175+
$resultBlock->addError($error->getErrorMessage());
176+
}
177+
}
124178
}

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

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

562562
/**
563-
* Validates source file and returns validation result.
563+
* Validates source file and returns validation result
564+
*
565+
* Before validate data the method requires to initialize error aggregator (ProcessingErrorAggregatorInterface)
566+
* with 'validation strategy' and 'allowed error count' values to allow using this parameters in validation process.
564567
*
565568
* @param \Magento\ImportExport\Model\Import\AbstractSource $source
566569
* @return bool
567570
*/
568571
public function validateSource(\Magento\ImportExport\Model\Import\AbstractSource $source)
569572
{
570573
$this->addLogComment(__('Begin data validation'));
574+
575+
$errorAggregator = $this->getErrorAggregator();
576+
$errorAggregator->initValidationStrategy(
577+
$this->getData(self::FIELD_NAME_VALIDATION_STRATEGY),
578+
$this->getData(self::FIELD_NAME_ALLOWED_ERROR_COUNT)
579+
);
580+
571581
try {
572582
$adapter = $this->_getEntityAdapter()->setSource($source);
573-
$errorAggregator = $adapter->validateData();
583+
$adapter->validateData();
574584
} catch (\Exception $e) {
575-
$errorAggregator = $this->getErrorAggregator();
576585
$errorAggregator->addError(
577586
\Magento\ImportExport\Model\Import\Entity\AbstractEntity::ERROR_CODE_SYSTEM_EXCEPTION,
578587
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 © 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)