magento-engcom / import-export-improvements

Open Software License 3.0
31 stars 29 forks source link

Magento 2.2.2-dev - CSV Import, skip errors not working #75

Open piotrekkaminski opened 6 years ago

piotrekkaminski commented 6 years ago

From @JustBeYou on December 19, 2017 14:36

Preconditions

  1. Magento version is 2.2.2 as specified in the title and I think is the only relevant info about the environment
  2. PHP 7.0.25
  3. MySQL Ver 15.1 Distrib 10.1.29-MariaDB
  4. Cent OS 7 (Linux 3.10.0-693.11.1.el7.x86_64 # 1 SMP Mon Dec 4 23:52:40 UTC 2017 x86_64 GNU/Linux)

Steps to reproduce

  1. Go to System > Data Transfer > Import
  2. Select 'Products' for 'Entity type'
  3. Select 'Add/Update' behavior and 'Skip error entries' for strategy
  4. Select some arbitrary number for 'Allowed Errors Count' bigger than your actual error count from CSV file
  5. Select a CSV file to import with few errors, but not more than the number specified in 'Allowed Errors Count'
  6. Check the data

Expected result

  1. Should let me import the CSV file ignoring the errors

Actual result

  1. The checking data phase just fail and I can't continue to the actual import screenshot_20171219_155548 screenshot_20171219_155627

I did some research on this problem and I've found that it was reported somehow in earlier versions (2.1.x), but the issues I've found on this bug didn't come to some solution. The first I've found is #5831, but it is closed because it's marked as a duplicate of this #5334, but this is closed too as it is not clear enough.

I want to know if is this is fixed in any 2.2.x > 2.2.2 version (just supposing, I think 2.2.2 is the last release) or if is anyone already working on it? If no one is working I could try to fix it as I am already looking at Magento/ImportExport/Model/Import because I think the problem is here, but I don't want to waste my time if there is already fixed in another version.

Have a nice day!

Copied from original issue: magento/magento2#12808

piotrekkaminski commented 6 years ago

From @PieterCappelle on December 21, 2017 21:43

Issue is not fixed in 2.2-develop and 2.3-develop. So yes, its an issue. Will you fix this with PR?

piotrekkaminski commented 6 years ago

From @JustBeYou on December 22, 2017 13:53

OK then. I am going to make a fork and try to get a fix. I will create a pull request when I'm done.

piotrekkaminski commented 6 years ago

From @PieterCappelle on December 22, 2017 14:46

Let me know if I can help you @JustBeYou :)

piotrekkaminski commented 6 years ago

From @magento-engcom-team on December 22, 2017 15:30

@JustBeYou, thank you for your report. We've created internal ticket(s) MAGETWO-85966 to track progress on the issue.

piotrekkaminski commented 6 years ago

From @JustBeYou on December 23, 2017 15:26

Fixed in #12867. I am waiting for a review.

piotrekkaminski commented 6 years ago

From @PieterCappelle on December 23, 2017 18:54

Nice @JustBeYou! If accepted please create backport to 2.2 too :-) #thanks!

piotrekkaminski commented 6 years ago

From @JustBeYou on December 23, 2017 20:51

@PieterCappelle I developed the bug-fix on 2.2. Should I create a pull request for 2.1 and 2.3 too?

piotrekkaminski commented 6 years ago

From @PieterCappelle on December 24, 2017 8:48

Sorry. Yes, if accepted, please create backport for 2.1 & 2.3. Currently we are setting up a team from developers to fix issues and refactor the current import of Magento2. You can always join our channel on slack #import-export. We also have custom repository > https://github.com/magento-engcom/import-export-improvements/projects

Have a nice day today! 24th :)

dmanners commented 5 years ago

At the moment the class app/code/Magento/ImportExport/Model/Import.php is using the method getErrorsCount to define if there have been errors or not. I think in our case we should replace the usage of getErrorsCount with isErrorLimitExceeded.

The isErrorLimitExceeded has the following code in 2.3-develop.

    /**
     * @return bool
     */
    public function isErrorLimitExceeded()
    {
        $isExceeded = false;
        $errorsCount = $this->getErrorsCount([ProcessingError::ERROR_LEVEL_NOT_CRITICAL]);
        if ($errorsCount > 0
            && $this->validationStrategy == self::VALIDATION_STRATEGY_STOP_ON_ERROR
            && $errorsCount >= $this->allowedErrorsCount
        ) {
            $isExceeded = true;
        }

        return $isExceeded;
    }

This means we will check for critical errors and also if the validation strategy is Stop on Error.