AttributeCode column name length validation throws wrong error message #20766 #20767

irajneeshgupta · 2019-01-29T11:18:19Z

#20766

Description (*)

Attribute length 30 char check and error message added.

Fixed Issues (if relevant)

AttributeCode column name length validation throws wrong error message #20766: Issue title:AttributeCode column name length validation throws wrong error message
...

Manual testing scenarios (*)

...
...

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

magento-engcom-team · 2019-01-29T11:18:23Z

Hi @irajneeshgupta. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

brosenberger · 2019-01-29T11:33:56Z

@irajneeshgupta could you tell me the reason why the attribute code is restricted to 30 chars? would further be an input validation within the backend form be useful?

irajneeshgupta · 2019-01-29T12:52:36Z

@irajneeshgupta could you tell me the reason why the attribute code is restricted to 30 chars? would further be an input validation within the backend form be useful?

yes , input validation max-30 will be useful , i think 30 chars are sufficient for attribute_code.

I will update my commit.

Thanks @brosenberger

brosenberger · 2019-01-29T12:55:15Z

@irajneeshgupta in my case 30 characters are not enough as i am a bit bound by external system constraints for a customer project :-(

irajneeshgupta · 2019-01-29T13:11:42Z

@irajneeshgupta in my case 30 characters are not enough as i am a bit bound by external system constraints for a customer project :-(

https://magento.stackexchange.com/questions/7491/why-do-attribute-codes-have-a-maximum-length
please read Alan Storm comment in this thread.

brosenberger · 2019-01-29T13:27:25Z

@irajneeshgupta according to that comment all oracle versions equals and below 12.1 are effected - but neither version of oracle is officially supported by magento 2.3: https://devdocs.magento.com/guides/v2.3/install-gde/system-requirements-tech.html#database
oracle 12.1 is not supported anymore: https://en.wikipedia.org/wiki/Oracle_Database#Releases_and_versions

though there is a constraint on mysql column name lengths of 64 characters (https://dev.mysql.com/doc/refman/8.0/en/identifiers.html) which should be used instead

a comment within the code why X characters are allowed would be nice in every case

orlangur · 2019-01-29T13:43:45Z

app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php

@@ -203,7 +203,8 @@ public function execute()
                    $this->messageManager->addErrorMessage(
                        __(
                            'Attribute code "%1" is invalid. Please use only letters (a-z or A-Z), ' .
-                            'numbers (0-9) or underscore(_) in this field, first character should be a letter.',
+                            'numbers (0-9) or underscore(_) in this field, first character should be a letter, ' .
+                            'and length should be less than 30 characters.',


Please make a separate check for length which appears earlier than this.

Also, 30 chars seems to be allowed from regexp.

@orlangur sir
sure i am adding it.

@orlangur sir ,
i have added seperate length check before string validation.

$validatorAttrCode = new \Zend_Validate_Regex( ['pattern' => '/^[a-zA-Z\x{600}-\x{6FF}][a-zA-Z\x{600}-\x{6FF}_0-9]{0,30}$/u'] );

@orlangur
sir, please review the changes.
Thank you.

milindsingh · 2019-01-30T06:46:26Z

@orlangur Hi, I too issue some time due to the length limit of attribute_code (i.e 30 chars). Can it be increased?

could you tell me the reason why the attribute code is restricted to 30 chars?

orlangur · 2019-01-30T10:22:45Z

@milindsingh, nope, there could be a lot of stuff tied to it.

30 chars are sufficient for attribute_code

brosenberger · 2019-01-31T07:30:35Z

@orlangur any comments to my follow up links why 30 characters are not a hard constraint because of the old oracle version and reference to magento 1.6?

orlangur · 2019-01-31T11:41:05Z

@brosenberger thanks for pointing out, I found that I already talked on this with Olexii in #10225 (comment). Looks like this one is just a legacy check in Catalog module not in sync with others, I'll check soon and give exact direction to PR author.

irajneeshgupta · 2019-01-31T14:58:51Z

@orlangur sir,

In this PR that you referenced #10225 \Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MAX_LENGTH
was changed from 255 to 60.

Should i change the string validation to 60 chars. since it is currently checking for 30 chars.
['pattern' => '/^[a-zA-Z\x{600}-\x{6FF}][a-zA-Z\x{600}-\x{6FF}_0-9]{0,30}$/u']

irajneeshgupta · 2019-02-02T02:10:16Z

@orlangur sir,
Please review.
Thanks

brosenberger · 2019-02-04T05:24:34Z

app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php

@@ -210,7 +212,7 @@ public function execute()
            }
            if (strlen($attributeCode) > 0) {
                $validatorAttrCode = new \Zend_Validate_Regex(
-                    ['pattern' => '/^[a-zA-Z\x{600}-\x{6FF}][a-zA-Z\x{600}-\x{6FF}_0-9]{0,30}$/u']
+                    ['pattern' => '/^[a-zA-Z\x{600}-\x{6FF}][a-zA-Z\x{600}-\x{6FF}_0-9]{0,60}$/u']


when using the constant above for checking the length, it should be included here as well for the regexp (to be sure if it changes in future and not have failing this constraint which was already checked previously)

@brosenberger
you are correct , let me add maxLength i.e, 60 in regex

sivaschenko · 2019-02-13T15:03:43Z

Hi @irajneeshgupta can you please check #20526 I believe it resolves the issue. Let's close this pull request as a duplicate and collaborate to deliver #20526

irajneeshgupta · 2019-02-14T04:52:37Z

@sivaschenko
you are correct.

ghost · 2019-02-14T04:52:43Z

Hi @irajneeshgupta, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento-engcom-team added Component: Catalog Release Line: 2.3 Community Insider: Cedcommerce community-insider-contribution labels Jan 29, 2019

irajneeshgupta requested a review from aleron75 January 29, 2019 11:22

orlangur self-assigned this Jan 29, 2019

orlangur requested changes Jan 29, 2019

View reviewed changes

sidolov added the Progress: needs update label Jan 30, 2019

irajneeshgupta force-pushed the irajneeshgupta-20766 branch from 5926b55 to 299ab72 Compare January 31, 2019 07:02

brosenberger reviewed Feb 4, 2019

View reviewed changes

added check for attribute code length and error message

5a9a9e6

irajneeshgupta force-pushed the irajneeshgupta-20766 branch from 1bb907f to 5a9a9e6 Compare February 6, 2019 15:25

irajneeshgupta removed the Progress: needs update label Feb 9, 2019

irajneeshgupta requested a review from sivaschenko February 11, 2019 14:31

sivaschenko added Progress: needs update duplicate labels Feb 13, 2019

irajneeshgupta closed this Feb 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AttributeCode column name length validation throws wrong error message #20766 #20767

AttributeCode column name length validation throws wrong error message #20766 #20767

irajneeshgupta commented Jan 29, 2019 •

edited

Loading

magento-engcom-team commented Jan 29, 2019

brosenberger commented Jan 29, 2019

irajneeshgupta commented Jan 29, 2019 •

edited

Loading

brosenberger commented Jan 29, 2019

irajneeshgupta commented Jan 29, 2019

brosenberger commented Jan 29, 2019

orlangur Jan 29, 2019

irajneeshgupta Jan 29, 2019

irajneeshgupta Jan 31, 2019

irajneeshgupta Feb 7, 2019

milindsingh commented Jan 30, 2019

orlangur commented Jan 30, 2019

brosenberger commented Jan 31, 2019

orlangur commented Jan 31, 2019

irajneeshgupta commented Jan 31, 2019

irajneeshgupta commented Feb 2, 2019

brosenberger Feb 4, 2019

irajneeshgupta Feb 4, 2019 •

edited

Loading

sivaschenko commented Feb 13, 2019

irajneeshgupta commented Feb 14, 2019

ghost commented Feb 14, 2019

AttributeCode column name length validation throws wrong error message #20766 #20767

AttributeCode column name length validation throws wrong error message #20766 #20767

Conversation

irajneeshgupta commented Jan 29, 2019 • edited Loading

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

magento-engcom-team commented Jan 29, 2019

brosenberger commented Jan 29, 2019

irajneeshgupta commented Jan 29, 2019 • edited Loading

brosenberger commented Jan 29, 2019

irajneeshgupta commented Jan 29, 2019

brosenberger commented Jan 29, 2019

orlangur Jan 29, 2019

Choose a reason for hiding this comment

irajneeshgupta Jan 29, 2019

Choose a reason for hiding this comment

irajneeshgupta Jan 31, 2019

Choose a reason for hiding this comment

irajneeshgupta Feb 7, 2019

Choose a reason for hiding this comment

milindsingh commented Jan 30, 2019

orlangur commented Jan 30, 2019

brosenberger commented Jan 31, 2019

orlangur commented Jan 31, 2019

irajneeshgupta commented Jan 31, 2019

irajneeshgupta commented Feb 2, 2019

brosenberger Feb 4, 2019

Choose a reason for hiding this comment

irajneeshgupta Feb 4, 2019 • edited Loading

Choose a reason for hiding this comment

sivaschenko commented Feb 13, 2019

irajneeshgupta commented Feb 14, 2019

ghost commented Feb 14, 2019

irajneeshgupta commented Jan 29, 2019 •

edited

Loading

irajneeshgupta commented Jan 29, 2019 •

edited

Loading

irajneeshgupta Feb 4, 2019 •

edited

Loading