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.

Sign up for GitHub

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

Jump to bottom

[2.3] Magento 2 Store Code validation regex: doesn't support uppercase letters in store code #12010

Merged
merged 3 commits into from
Nov 6, 2017
Merged

[2.3] Magento 2 Store Code validation regex: doesn't support uppercase letters in store code #12010

merged 3 commits into from
Nov 6, 2017

Conversation

manuelson
Copy link
Contributor

Description

Magento 2 Store Code validation regex: doesn't support uppercase letters in store code

Fixed Issues (if relevant)

  1. Magento 2 Store Code validation regex: doesn't support uppercase letters in store code #11996: Magento 2 Store Code validation regex: doesn't support uppercase letters in store code

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)

@manuelson manuelson changed the title Magento 2 Store Code validation regex: doesn't support uppercase letters in store code [2.3] Magento 2 Store Code validation regex: doesn't support uppercase letters in store code Nov 4, 2017
@vkublytskyi vkublytskyi self-assigned this Nov 4, 2017
@vkublytskyi vkublytskyi added this to the November 2017 milestone Nov 4, 2017
@vkublytskyi vkublytskyi added Release Line: 2.3 2.2.x Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 4, 2017
vkublytskyi
vkublytskyi suggested changes Nov 4, 2017
View reviewed changes
Copy link

@vkublytskyi vkublytskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistent behavior please provide also similar changes in the \Magento\Store\Model\ResourceModel\Website::_beforeSave

app/code/Magento/Store/Model/Store.php Outdated
@@ -463,10 +463,10 @@ protected function _getValidationRulesBeforeSave()
$storeLabelRule->setMessage(__('Name is required'), \Zend_Validate_NotEmpty::IS_EMPTY);
$validator->addRule($storeLabelRule, 'name');

$storeCodeRule = new \Zend_Validate_Regex('/^[a-z]+[a-z0-9_]*$/');
$storeCodeRule = new \Zend_Validate_Regex('/^[a-zA-Z]+[a-z0-9_]*$/');
Copy link

@vkublytskyi vkublytskyi Nov 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expression will not allow code with uppercase letter after number or underscore (e.g. a1A). Instead of modifying regexp it's better to apply modifier i (PCRE_CASELESS) to make case-insensitive comparison.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@zack6849 zack6849 Nov 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest you change the regex to be

$storeCodeRule = new \Zend_Validate_Regex('/^[A-z]+[a-z0-9_]*$/');

[A-z] is the same as [a-zA-Z] but shorter and more concise

app/code/Magento/Store/Model/Store.php Outdated
$storeCodeRule->setMessage(
__(
'The store code may contain only letters (a-z), numbers (0-9) or underscore (_),'
'The store code may contain only letters (a-zA-Z), numbers (0-9) or underscore (_),'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not modify the description as this will break existed translations (will be backward incompatible change). As string does not say "lowercase letters" it is also applicable for updated implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@magento-team magento-team merged commit d486cb1 into magento:2.3-develop Nov 6, 2017
magento-team pushed a commit that referenced this pull request Nov 6, 2017
@ishakhsuvarov
MAGETWO-83276: [2.3] Magento 2 Store Code validation regex: doesn't s…
6912631 
…upport uppercase letters in store code #12010
magento-team pushed a commit that referenced this pull request Nov 6, 2017
@ishakhsuvarov
Merge pull request #1667 from magento-engcom/develop-prs
4ee8196 
[EngCom] Public Pull Requests - develop
 - MAGETWO-83276: [2.3] Magento 2 Store Code validation regex: doesn't support uppercase letters in store code #12010
 - MAGETWO-82951: Check attribute unique between same fields in magento commerce [backport] #11622
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zack6849 zack6849 zack6849 left review comments

@vkublytskyi vkublytskyi vkublytskyi requested changes

Assignees

@vkublytskyi vkublytskyi

Labels
Progress: accept Release Line: 2.3 Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Milestone
November 2017
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@manuelson @vkublytskyi @zack6849 @magento-team