ENGCOM-3278: #75: fix stoping on errors #130

Author: sidolov

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

@@ -86,7 +86,7 @@ private function processValidationResult($validationResult, $resultBlock)
                 $resultBlock->addError(
                     __('Data validation failed. Please fix the following errors and upload the file again.')
                 );
-                $this->addErrorMessages($resultBlock, $errorAggregator);
+
                 if ($errorAggregator->getErrorsCount()) {
                     $this->addMessageToSkipErrors($resultBlock);
                 }
@@ -100,6 +100,8 @@ private function processValidationResult($validationResult, $resultBlock)
                     $errorAggregator->getErrorsCount()
                 )
             );
+            
+            $this->addErrorMessages($resultBlock, $errorAggregator);
         } else {
             if ($errorAggregator->getErrorsCount()) {
                 $this->collectErrors($resultBlock);

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

@@ -312,7 +312,7 @@ public function getOperationResultMessages(ProcessingErrorAggregatorInterface $v
     {
         $messages = [];
         if ($this->getProcessedRowsCount()) {
-            if ($validationResult->getErrorsCount()) {
+            if ($validationResult->isErrorLimitExceeded()) {
                 $messages[] = __('Data validation failed. Please fix the following errors and upload the file again.');
 
                 // errors info
@@ -630,16 +630,7 @@ public function validateSource(\Magento\ImportExport\Model\Import\AbstractSource
         $messages = $this->getOperationResultMessages($errorAggregator);
         $this->addLogComment($messages);
 
-        $errorsCount = $errorAggregator->getErrorsCount();
-        $result = !$errorsCount;
-        $validationStrategy = $this->getData(self::FIELD_NAME_VALIDATION_STRATEGY);
-        if ($errorsCount
-            && $validationStrategy === ProcessingErrorAggregatorInterface::VALIDATION_STRATEGY_SKIP_ERRORS
-        ) {
-            $this->messageManager->addWarningMessage(__('Skipped errors: %1', $errorsCount));
-            $result = true;
-        }
-
+        $result = !$errorAggregator->isErrorLimitExceeded();
         if ($result) {
             $this->addLogComment(__('Import data validation is complete.'));
         }

app/code/Magento/ImportExport/Model/Import/ErrorProcessing/ProcessingErrorAggregator.php

@@ -61,6 +61,8 @@ public function __construct(
     }
 
     /**
+     * Add error via code and level
+     *
      * @param string $errorCode
      * @param string $errorLevel
      * @param int|null $rowNumber
@@ -96,6 +98,8 @@ public function addError(
     }
 
     /**
+     * Add row to be skipped during import
+     *
      * @param int $rowNumber
      * @return $this
      */
@@ -110,6 +114,8 @@ public function addRowToSkip($rowNumber)
     }
 
     /**
+     * Add specific row to invalid list via row number
+     *
      * @param int $rowNumber
      * @return $this
      */
@@ -126,6 +132,8 @@ protected function processInvalidRow($rowNumber)
     }
 
     /**
+     * Add error message template
+     *
      * @param string $code
      * @param string $template
      * @return $this
@@ -138,6 +146,8 @@ public function addErrorMessageTemplate($code, $template)
     }
 
     /**
+     * Check if row is invalid by row number
+     *
      * @param int $rowNumber
      * @return bool
      */
@@ -147,6 +157,8 @@ public function isRowInvalid($rowNumber)
     }
 
     /**
+     * Get number of invalid rows
+     *
      * @return int
      */
     public function getInvalidRowsCount()
@@ -155,6 +167,8 @@ public function getInvalidRowsCount()
     }
 
     /**
+     * Initialize validation strategy
+     *
      * @param string $validationStrategy
      * @param int $allowedErrorCount
      * @return $this
@@ -178,6 +192,8 @@ public function initValidationStrategy($validationStrategy, $allowedErrorCount =
     }
 
     /**
+     * Check if import has to be terminated
+     *
      * @return bool
      */
     public function hasToBeTerminated()
@@ -186,15 +202,17 @@ public function hasToBeTerminated()
     }
 
     /**
+     * Check if error limit has been exceeded
+     *
      * @return bool
      */
     public function isErrorLimitExceeded()
     {
         $isExceeded = false;
-        $errorsCount = $this->getErrorsCount([ProcessingError::ERROR_LEVEL_NOT_CRITICAL]);
+        $errorsCount = $this->getErrorsCount();
         if ($errorsCount > 0
             && $this->validationStrategy == self::VALIDATION_STRATEGY_STOP_ON_ERROR
-            && $errorsCount >= $this->allowedErrorsCount
+            && $errorsCount > $this->allowedErrorsCount
         ) {
             $isExceeded = true;
         }
@@ -203,6 +221,8 @@ public function isErrorLimitExceeded()
     }
 
     /**
+     * Check if import has a fatal error
+     *
      * @return bool
      */
     public function hasFatalExceptions()
@@ -211,6 +231,8 @@ public function hasFatalExceptions()
     }
 
     /**
+     * Get all errors from an import process
+     *
      * @return ProcessingError[]
      */
     public function getAllErrors()
@@ -228,6 +250,8 @@ public function getAllErrors()
     }
 
     /**
+     * Get a specific set of errors via codes
+     *
      * @param string[] $codes
      * @return ProcessingError[]
      */
@@ -244,6 +268,8 @@ public function getErrorsByCode(array $codes)
     }
 
     /**
+     * Get an error via row number
+     *
      * @param int $rowNumber
      * @return ProcessingError[]
      */
@@ -258,6 +284,8 @@ public function getErrorByRowNumber($rowNumber)
     }
 
     /**
+     * Get a set rows via a set of error codes
+     *
      * @param array $errorCode
      * @param array $excludedCodes
      * @param bool $replaceCodeWithMessage
@@ -292,6 +320,8 @@ public function getRowsGroupedByErrorCode(
     }
 
     /**
+     * Get the max allowed error count
+     *
      * @return int
      */
     public function getAllowedErrorsCount()
@@ -300,6 +330,8 @@ public function getAllowedErrorsCount()
     }
 
     /**
+     * Get current error count
+     *
      * @param string[] $errorLevels
      * @return int
      */
@@ -318,6 +350,8 @@ public function getErrorsCount(
     }
 
     /**
+     * Clear the error aggregator
+     *
      * @return $this
      */
     public function clear()
@@ -331,6 +365,8 @@ public function clear()
     }
 
     /**
+     * Check if an error has already been added to the aggregator
+     *
      * @param int $rowNum
      * @param string $errorCode
      * @param string $columnName
@@ -348,6 +384,8 @@ protected function isErrorAlreadyAdded($rowNum, $errorCode, $columnName = null)
     }
 
     /**
+     * Build an error message via code, message and column name
+     *
      * @param string $errorCode
      * @param string $errorMessage
      * @param string $columnName
@@ -369,6 +407,8 @@ protected function getErrorMessage($errorCode, $errorMessage, $columnName)
     }
 
     /**
+     * Process the error statistics for a given error level
+     *
      * @param string $errorLevel
      * @return $this
      */

app/code/Magento/ImportExport/Test/Unit/Model/Import/ErrorProcessing/ProcessingErrorAggregatorTest.php

@@ -216,6 +216,7 @@ public function testIsErrorLimitExceededTrue()
      */
     public function testIsErrorLimitExceededFalse()
     {
+        $this->model->initValidationStrategy('validation-stop-on-errors', 5);
         $this->model->addError('systemException');
         $this->model->addError('systemException', 'critical', 7, 'Some column name', 'Message', 'Description');
         $this->model->addError('systemException', 'critical', 4, 'Some column name', 'Message', 'Description');

dev/tests/functional/tests/app/Magento/AdvancedPricingImportExport/Test/TestCase/ImportDataNegativeTest.xml

@@ -19,7 +19,7 @@
                 <item name="entity" xsi:type="string">Advanced Pricing</item>
                 <item name="behavior" xsi:type="string">Add/Update</item>
                 <item name="validation_strategy" xsi:type="string">Stop on Error</item>
-                <item name="allowed_error_count" xsi:type="string">10</item>
+                <item name="allowed_error_count" xsi:type="string">1</item>
                 <item name="import_field_separator" xsi:type="string">,</item>
                 <item name="import_multiple_value_separator" xsi:type="string">,</item>
                 <item name="import_file" xsi:type="array">

dev/tests/functional/tests/app/Magento/AdvancedPricingImportExport/Test/_files/template/pricing/advanced_incorrect.php

@@ -14,5 +14,13 @@
             'tier_price' => 'text',
             'tier_price_value_type' => 'Fixed',
         ],
+        'data_1' => [
+            'sku' => '%sku%',
+            'tier_price_website' => "All Websites [USD]",
+            'tier_price_customer_group' => 'ALL GROUPS',
+            'tier_price_qty' => '3',
+            'tier_price' => 'text',
+            'tier_price_value_type' => 'Fixed',
+        ],
     ],
 ];