Change URL Key behaviour to allow the same key to be used more then once

[dverkade]

The behaviour of the URL key is different in Magento 1 and Magento 2. This PR will fix this, so that users will not get the widley reported "URL key for specified store already exists" error message anymore.

Description

This PR makes it possible to use the same URL key for a product more then once. This is a real life scenario and occurs quite often when migrating from Magento 1 to Magento 2 with the data migration tool. This tool will dump the same data into Magento 2 and after importing this data, this will result in the "URL key for specified store already exists" when editing categories, moving categories, editing products, etc. because the same URL key is used more then once (which was possible in Magento 1).

This PR checks the URL key as entered by the user. When the URL key already exists it will append the product entity id to the URL. So you can have a product name "Acme" with the same URL key and ID 1 and a product called "Acme" with the same URL key and ID 2. For the product with ID one, the URL in the rewrite table will be "acme.html", for the second product it will be "acme-2.html".

Also removed adding URL rewrites for the root categories, resulting in a URL starting with /product-name, which could conflict with the canonical URL being generated. Since the are both basically the same URL's. Added a check while generating the Anchor Categories URL's to not generate it for any root categories.

This PR does not have the same behaviour as Magento 1 where every (configurable?) product saved added a new entry into the url_rewrite table with a new number at the end.

Fixed Issues

This PR fixes a lot of issues which are reported with the "URL key for specified store already exists" error. I have listed a couple below, but this is not limited to this list.

1. URL key for specified store already exists - over 51 bugs reported - still not fixed #10265
2. Behaviour of URL Key field is different in Magento 2 then in Magento 1 #10242
3. URL key for specified store already exists. While adding new product with same name as of older product. #9465
4. Magento2 URL key for specified store already exists. #9313
5. 'URL key for specified store already exists.' cannot save category and products. 'URL key not generate when save categories or products also when import prducts'. #8937
6. URL key for specified store already exists error on product save with same name #8679
7. Provide more details for "URL key for specified store already exists." error #8564
8. URL key for specified store already exists in Magento 2.1.1 #6671

Manual testing scenarios

1. Create 2 products with the same URL key and save, edit and delete them.
2. Put these products in multiple categories and move the categories around. All URL's are correctly written to the url_rewrite table.

[hostep]

I don't agree with these proposed changes. Url keys's should be unique and should not be auto-generated when a duplicate is found, it should fail to save the product when something like that is detected.

What does need to be improved:

• the error message should clearly state what duplicate url key is found, this doesn't happen right now and is very annoying when manipulating multiple products and you have no idea what product causes the problem
• duplicated url keys over different storeviews should be allowed (we can import them, but we get errors when trying to manipulate such products through the adminhtml after we imported them)
• the data migration tool should warn users that they have duplicate url's and don't allow to go further until they fix everything (either manually, or using some kind of algorithm)

Just my 2 cents :)

[dverkade]

@hostep Thank you for your suggestion, the error message can be much more clear then it currently is. However autogenerating the URL when the primary one is not availabe has been a feature in Magento 1. It's also works the same in Wordpress, the leading CMS / Blog platform worldwide. It has the same behaviour, no error when using the same page title twice, but appending the URL with a number automatically. So it's something users are expecting and are ready to deal with. It's upto the @magento-team to determine which way they want to move forward with the URL rewrites. In my opinion we should preserve the functionality offered by Magento 1, since users are accustomed to it and it makes migrating from M1 to M2 a lot easier. Above all it's common best practice in a number of top used e-commerce, cms and blog platforms worldwide.

[hostep]

It's not because people are used to a certain behaviour, that it is a good one :)

I just got to think about a possible case where your proposed logic could go wrong.
Suppose you are importing a bunch of products:

SKU	Name	Url Key
PRODUCT_A	Product A	product-a
PRODUCT_B1	Product B	product-b
PRODUCT_B2	Product B	product-b

With your proposed solution, PRODUCT_B2's url key should automatically been changed to 'product-b-{someid}'

Suppose a day later, you import the same products again, but in a different order:

SKU	Name	Url Key
PRODUCT_A	Product A	product-a
PRODUCT_B2	Product B	product-b
PRODUCT_B1	Product B	product-b

Now what will happen? PRODUCT_B2's url key will change to 'product-b' and PRODUCT_B1's url key will change to 'product-b-{some-other-id}'

This means that a product's url has changed without the shop owner knowing about it and previous url's linking to the original product will now end up somewhere else.

This is the biggest issue I have with the proposed changes, that url's can change 'out of the blue' and that's a very bad idea (that was also a huge problem in Magento1 btw ;))
Url's should never change by itself, unless you specifically change it yourself and are aware of the consequences.

Anyway, this just popped into my head, haven't really thought it trough a lot, but I think it's very dangerous to let url key's be generated automatically should a duplicate be detected.

[korostii]

@hostep, just because you personally don't feel like using this functionality - doesn't mean other people should be deprived from it, as well.
I mean, it's rather easy to satisfy everyone by having this feature optional: disabled by default, yet easy to enable by flicking a simple switch somewhere in config settings.

[dverkade]

@hostep @korostii I don't think the system itself should apply any restrictions. If you want to make sure that every product has a URL without the ID included it can be checked while doing the import. The changes in this PR will set the URL key once and it will only be changed if a category is moved or the URL key for the specified product is changed. It will also create redirects for the original URL's, so that they will be preserved.

[orlangur]

Looks like you didn't check changes introduced in scope of #6671 which improve URL Rewrites functionality in the right direction (available in 2.2.0-preview, for example, d3b1de3 commit).

I personally vote for reject here as this PR simply removes full merchant control over URLs generated which was the whole point of realtime URL Rewrites generation introduction.

I appreciate the efforts invested to embody your idea into life but generally it is better to conduct a technical discussion before doing any controversial changes, just to avoid waste of such efforts (recent example - whether we can validate emails by just presence of @ in Magento 2).

[dverkade]

Hi @orlangur,

No, I did not miss the changes in the commit you are referencing. I have created issue #10242 for this. It was designated a feature request by the @magento-team and @choukalos. So in order to get things up to speed I have created this PR to resolve this issue. The @magento-team can decide which way they want to go with the URL rewrites. But I'm suggesting that an URL key should no be unique, especially for the following reasons:

• This is a change in functionality from Magento 1, users want to use it in the same way (see the number of issues regarding URL keys already submitted), so we should listen to those users and preserve functionality.
• Other platforms such as Wordpress use the same logic, where the same URL key can be used appended by a number.
• The system should not limit the use of the URL key. It's upto the user or the developer to make sure every URL key is unique of they want to do so. So a message telling them that the key is in use by another product, category or CMS page would indeed be helpfull.

[orlangur]

I mean, it's rather easy to satisfy everyone by having this feature optional: disabled by default, yet easy to enable by flicking a simple switch somewhere in config settings.

I don't think making such a crucial thing as a switch in core is a good idea. What if you want to switch it back at some point? It means starting work on any Magento 2 shop you'll have to check it first just to understand how system behaves.

Implementation is not supplied with any tests, just removed few broken ones.

Adding any feature to core means it will be fully maintained by core team later, preserving backward-compatibility and doing any required adoption with future features.

So, I think such functionality should be better implemented as a third-party module. I wouldn't make it a switch in Admin UI, but making it an option in Setup seems a viable solution to me.

[orlangur]

@dverkade #10265 is the best illustration on why people reported those issues. Error message is simply not verbose enough to allow merchant perform necessary changes.

I have nothing to add here, let's just wait for the response of Magento Officials.

[dverkade]

Implementation is not supplied with any tests, just removed few broken ones.

@orlangur Unfortunately you did not check the description of this PR or the commit comments. The test where changed because in the Anchor rewrite generator I have removed the current Magento behaviour where it will generate URL's for the website root category. This is not necessary and should be removed. It will create duplicate entries in the url_rewrite table such as the URL "/product-name.html". So the total number of URL's being generated is less.

We will wait in see how the @magento-team wants to go forward with this. However when so much users do not understand the way the system works. Is that the users or the systems fault? In my opinion it's the last and the system should take the users into account. So that's the reason for these proposed changes.

[korostii]

@orlangur

What if you want to switch it back at some point? It means starting work on any Magento 2 shop you'll have to check it first just to understand how system behaves.

What's wrong about that? Would you rather hunt down for custom code, 3rd-party modules or even patches to the core - whatever the previous maintainer might've used in order to reach the very same goal precisely because this feature is not provided out of the box? How is that any better?
Also please keep in mind that any 3rd-party code is almost certain to have more bugs (since less people test and maintain it).

[korostii]

At least we can agree on that other point you make along the way:

it is better to conduct a technical discussion before doing any controversial changes, just to avoid waste of such efforts (recent example - whether we can validate emails by just presence of @ in Magento 2).

However I'd like to see it work both ways, meaning I wish @magento-team would also announce any future (possibly controversial) changes before they implement them and put them into a release, so that the community could at least warn them about possible issues in advance.
For example, that catalog:image:resize disaster in 2.1.6 could've been averted by publishing an RC, hearing some feedback and then simply hiding the "improvement" they developed behind a config setting shortly before the release, as well.
Having a public 2.1.8-preview branch is a good first step in that direction.

[orlangur]

Would you rather hunt down for custom code, 3rd-party modules or even patches to the core - whatever the previous maintainer might've used in order to reach the very same goal precisely because this feature is not provided out of the box? How is that any better?

This "feature" is not provided out of the box simply because it is not needed out of the box.

From my experience once you solved all problems in your invalid data after migration and having understanding how URL Rewrites are generated in Magento 2 the things go smoothly.

Having some switches in core for such a crucial things makes predictability of system behavior quite low.

Also please keep in mind that any 3rd-party code is almost certain to have more bugs (since less people test and maintain it).

That's the point. There is no need to waste core team efforts to keep features needed only for those with poorly migrated data / misunderstanding of URL Rewrites behavior in Magento 2. Magento should promote best practices and not some hacky solutions bringing more surprises to developers than a real value.

[korostii]

This "feature" is not provided out of the box simply because it is not needed out of the box.

Clearly, people are struggling - numerous issues both her and on stackexchange speak about that loudly. You aren't denying that, are you?
Clearly, it's feature that's missing. Adding it would be seen as an improvement by many users.

Having some switches in core for such a crucial things makes predictability of system behavior quite low.

I would continue to argue that it's less harmful (than forcing users to do core hacks themselves instead).

There is no need to waste core team efforts

a) Initial code is provided by community in this case.
b) [irony] It's not as if they're putting much effort into bugfixing lately, anyway. [/Irony]
Seriously though, is all you're saying goes down to "I don't need it, thus there's no reason to have it available for eveyrone else" ?

Magento should promote best practices and not some hacky solutions bringing more surprises to developers than a real value.

Last time I checked, developers aren't the only people using Magento. There are also the merchants.

[korostii]

So, I think such functionality should be better implemented as a third-party module. I wouldn't make it a switch in Admin UI, but making it an option in Setup seems a viable solution to me.

Oops, missed that one at first. You mean having it selected during Magento's initial installation and storing in env.php, right? Yes, that makes sense.
As long as it's part of the core, of course (not sure if it's even currently possible to let user choose anything during a module installation - in any case, that would be "some hacky solution").

[nntoan]

@dverkade Can you please provide a patch for this PR ?

[daenarys3]

@dverkade

This PR seems to inform us of how things work, but not how to fix it. I have a category giving me the URL key error. I tried to change the URL key, and unchecked creating a permanent redirect, and resave, and it still gave me the same error.

If we are getting this error in a category, what is a quick fix for it? Do you have a step by step processes?

Also I agree with @korostii , I'm not a developer using this site, there are lots of us here that used M1 that are suddenly faced with the task of having to learn way more than we ever needed to know previously, just to get M2 functional.

[orlangur]

@dverkade, thanks for your endeavor to improve Magento functionality as you see it!

As noted previously the problem this PR aims to solve is already fixed in 2.2.0 in more appropriate way.

Proposed system behavior change was evaluated and not approved. Before introducing any controversial changes it is always a good idea to get it designed and described as 'up for grabs' ticket first.

Generally, system behaves normally in current implementation if your data is valid in the first place. If https://github.com/magento/data-migration-tool produces invalid data in some cases, it should be reported instead of hacking core to automatically "fix" invalid data.

[korostii]

it is always a good idea to get it designed and described as 'up for grabs' ticket first

That's new. Could you please elaborate on that?
From past experience, feature requests and suggested improvements (and anything controversial would probably fall under one of these categories) are not welcome in the "issues" section of this repository.

Has that officially changed recently?

[orlangur]

Nothing has changed as far as I know. Difference between "I want ..." and "Here is the problem, here is the design, let's just do it" is the key.

Some examples: 9582 10863 (not linking intentionally as our discussion is unrelated)

[korostii]

The tickets you mention aren't a very good example as far as I'm concerned.
Why? because they're created by Magento employees.
Let me explain this: these tasks might've been up for internal discussion, but they seem to have been approved before the got put up on GitHub.
As in "We want ... but we don't fell like contributing any resources to it. But hey! it's approved so at least you know we'll merge it in afterwards".

This doesn't really help with putting new suggested improvements up for wide discussion and Magento's internal approval.
If they were created by community members, they'd be moved to forums... and that's it, really.
Short of PRs, the official channels currently seem to give little to none feedback on whatever the developer community has to say.

[orlangur]

https://github.com/magento/magento2/issues?q=is%3Aopen+is%3Aissue+label%3A"up+for+grabs" actually, some of them were proposed by Community members and then approved. As far as I understand current process, improvements should be discussed on Community Forums and onsite events and after that they may appear as 'up for grabs'. Issue reporter should not be confused with people who actually raise some topic.

[korostii]

actually, some of them were proposed by Community members and then approved

Well, I count 4 issues like that over whole 2017 to date. Those are negligible.
( and the earlier ones can't be considered "current" process).

Issue reporter should not be confused with people who actually raise some topic
should be discussed on Community Forums and onsite events

IMHO That's all very shady and overcomplicated.
We're in the 21st century. You shouldn't need to track down the mainterners in person and corner them with your suggestions in order to get some attention.