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

Added dash(-) as an allowed character for store code #1091

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

julienloizelet
Copy link
Contributor

@julienloizelet julienloizelet commented Jul 6, 2020

Allow dash in store code

Description (*)

Currently, it is not possible to create a store with a store code containing dash (-). When using the feature "add store code in url", this can lead to conflicts with SEO purposes.

There is a lot of literature (for example https://stackoverflow.com/a/2318376/7497291) on it and I am not a SEO specialist but what I know is that I have often been asked to add the dash in the store code and I had to rewrite this resource model. That's why I submit this PR.

Furthermore, the reason why dash is not allowed in Magento is not clear for me. The only answer I found is here :
https://magento.stackexchange.com/a/237309/50208

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

  1. Create a store with code "en-uk". This is not possible without this PR and this becomes possible with it.

Questions or comments

I am not sure it was necessary to create a Mage_Core_LTS.csv file : The text I changed is not used anymore in the source code, so I hesitated to modify directly the Mage_Core.csv file because it could be considered as dead code. But, if for some reason, somebody has specific code using this old key and need this old translated key, this will not break it. That's why I created a new csv file as suggested in the README.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@sreichel sreichel assigned sreichel and unassigned sreichel Jul 6, 2020
Copy link
Member

@colinmollenhour colinmollenhour left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks!

Copy link
Contributor

@kiatng kiatng left a comment

Choose a reason for hiding this comment

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

LGTM

@colinmollenhour colinmollenhour merged commit 32b70a6 into OpenMage:1.9.4.x Jul 24, 2020
@colinmollenhour
Copy link
Member

Thanks, @julienloizelet!

@sreichel sreichel added the Component: Core Relates to Mage_Core label Jul 24, 2020
@sreichel sreichel added this to the Release 20.0.2 / 19.4.6 milestone Aug 7, 2020
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 20, 2020
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants