[Forwardport] Fix Magento_ImportExport not supporting unicode characters in column names #15721
Conversation
|
|
| @@ -831,7 +831,7 @@ public function validateData() | |||
| if (!$this->isAttributeParticular($columnName)) { | |||
| if (trim($columnName) == '') { | |||
| $emptyHeaderColumns[] = $columnNumber; | |||
| } elseif (!preg_match('/^[a-z][a-z0-9_]*$/', $columnName)) { | |||
| } elseif (!preg_match('/^[a-z][\w]*$/u', $columnName)) { | |||
There was a problem hiding this comment.
Why first character is supposed to be a Latin letter?
There was a problem hiding this comment.
I'd say that a regex like '/^[\w]+$/u' would work perfectly fine as well, but I wanted to keep old behavior for whatever reason it was designed this way. Should I change it to '/^[\w]+$/u'?
There was a problem hiding this comment.
Yes. So that name like µg_vitamin_a_µg is also supported. Not sure it can start from digit or underscore though, could you check attribute validation when you create/update it via Admin UI please?
There was a problem hiding this comment.
Sure thing. I'll get back to you.
There was a problem hiding this comment.
The attribute code validation when saving happens right here: Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php#L161. The regex seems to be ^[a-z\x{600}-\x{6FF}][a-z\x{600}-\x{6FF}_0-9]{0,30}$.
It starts to look like we need a validation class for attribute codes in general, so we can unify this stuff. But that's a story for another time.
@orlangur What's your suggestion for now?
There was a problem hiding this comment.
@tdgroot ok, so it's not "Magento_ImportExport not supporting unicode characters in column names" actually but such attribute code are generally forbidden. I've raised an internal discussion to understand the correct approach.
There was a problem hiding this comment.
@orlangur sure, I'd like to add something to it. I created the attribute in my example using the Magento API methods. To me it seems that validation of the attribute code shouldn't happen in a controller, but rather in the EAV model/repository class.
|
Checking with @VladimirZaets and @sidolov on this. |
|
@orlangur , @tdgroot as there were no updates on attribute code format I will accept this pull request as a port from 2.2 to syncranize the validation between 2.2 and 2.3. Please open a nwe pull request to fix the first letter attribute code validation in case there will be any result of technical discussion. Also, it might be useful to reach out to architeture team opening a pull request to https://github.com/magento/architecture with the proposal. |
|
Hi @sivaschenko, thank you for the review. |
orlangur
added
Progress: needs update
Progress: reject
and removed
Progress: needs update
labels
|
@sivaschenko please do not change PR status without communication with others, especially when it is This PR must be rejected according to discussion in maintainers channel and original one must be reverted for both |
Original Pull Request
#15197
Description
Column names like
vitamin_a_µgwere being marked invalid.Manual testing scenarios
vitamin_a_µgContribution checklist