Improved check when CategoryProcessor attempts to create a new category #8930
Conversation
…keys due to both uppercase and lowercase categories' name
| * @param string $string | ||
| * @return string | ||
| */ | ||
| protected function standardizeString(string $string) |
There was a problem hiding this comment.
Can you remove the "string" scalar type hint, please? :)
This is only possible with PHP 7+ and Magento's minimum supported PHP version is 5.6.*.
|
@tinogo i've removed type hint :) |
| */ | ||
| protected function standardizeString(string $string) | ||
| { | ||
| if (function_exists('mb_strtolower')) { |
There was a problem hiding this comment.
I believe composer.json for Magento already requires ext-mbstring so you may remove this check.
There was a problem hiding this comment.
i've removed the check
| * @param string $string | ||
| * @return string | ||
| */ | ||
| protected function standardizeString(string $string) |
There was a problem hiding this comment.
Removing the function_exists check would eliminate the need in the additional method.
| @@ -1529,6 +1529,9 @@ protected function _saveProducts() | |||
| $existingImages = $this->getExistingImages($bunch); | |||
|
|
|||
| foreach ($bunch as $rowNum => $rowData) { | |||
| // reset category processor's failed categories array | |||
| $this->categoryProcessor->clearFailedCategories(); | |||
There was a problem hiding this comment.
Could you please explain this change?
There was a problem hiding this comment.
this change clears failed categories' array for every bunch, avoiding to log the same failed categories every time a bunch is imported. See app/code/Magento/CatalogImportExport/Model/Import/Product.php::processRowCategories that is called for every bunch's import called in the same file at line 1603
| */ | ||
| protected function standardizeString($string) | ||
| { | ||
| if (function_exists('mb_strtolower')) { |
There was a problem hiding this comment.
I believe composer.json for Magento already requires ext-mbstring so you may remove this check.
There was a problem hiding this comment.
you're right, i've removed the check
| * @param string $string | ||
| * @return string | ||
| */ | ||
| protected function standardizeString($string) |
There was a problem hiding this comment.
Do you still need a method which just calls strtolower? I guess it is redundant now
There was a problem hiding this comment.
Yes but if in the future we have to change it with more complex checks, we just have to change the function's code instead of changing all the pieces of code where appears mb_strtolower. It's more efficient i think.
There was a problem hiding this comment.
Please, at least make it private and provide a reason behind this method in docblock. Thanks.
…te a new category #8930
|
@ccasciotti thank you for your contribution! |
Improved check perfomed by CategoryProcessor when it checks if it has to create a new category or not.
Description
The original check didn't consider that a category must be supplied with uppercase or lowecase characters. In this case, if you're trying to import a category named Foo, and a category named FOO, the system will create both categories but fails when attempts to create URL keys because the first url already exists causing an exception. Checking path existence by lowercase-ing it will avoid duplicate url keys creation attempts. I've also added a method to clear failed categories between bunches import, and updated related unit test file. Lastly, i've fixed a typo in CategoryProcessor's PHPDoc.
Contribution checklist