Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset skippedRows array in clear method #7590

Merged

Conversation

ccasciotti
Copy link
Contributor

@ccasciotti ccasciotti commented Nov 28, 2016

Added skippedRows array in clear method, missing to clear this array can cause an issue when you programmatically import more than one file, especially in a foreach loop because this object is treated as singleton. In this case, in the next file of your loop, magento checks for skipped rows and skips rows based on previous file. A pseudo-code example is following:

$files = ['file1.csv', 'file2.csv'];

foreach ($files as $file) {
.... // validate file
.... // import file
// if some file has skipped rows, skippedRows array is persistent between all files because is not cleared with clear() method called in validateData() method
}

Added skippedRows array in clear method, missing to clear this array can cause an issue when you programmatically import more than one file, especially in a foreach loop because this object is treated as singleton. In this case, in the next file of your loop, magento checks for skipped rows and skips rows based on previous file. A pseudo-code example is following:

$files = ['file1.csv', 'file2.csv'];

foreach ($files as $file) {
    .... // validate file
    .... // import file
    // if some file has skipped rows, skippedRows array is persistent between all files because is not cleared with clear() method called in validateData() method
}
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Nov 28, 2016

CLA assistant check
All committers have signed the CLA.

@okorshenko
Copy link
Contributor

@ccasciotti thank you for your contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants