Remove URL rewrites when changing product from visible to not visible

[dverkade]

Description

Fix for #19293 deletes url rewrite data when a product is switched from visible to not visible.

I don't think we still want to have URL rewrites of not visible products in the url rewrite table. This PR makes sure that data that is present in the URL rewrite table will be removed when a product changes visibility.

Fixed Issues

1. Invisible products do not update url_rewrite properly #19293: Invisible products do not update url_rewrite properly

Manual testing scenarios (*)

1. Create a product in Magento with the URL key "product-a" and set the product to be visible.
2. URL key for product-a should be present in the URL rewrite table.
3. Changed the visibility of the product to not visible and save the product.
4. All references of "product-a" should be removed from the URL rewrite table.

[magento-engcom-team]

Hi @dverkade. 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

[magento-engcom-team]

Hi @VladimirZaets, thank you for the review.
ENGCOM-3504 has been created to process this Pull Request

[orlangur]

Such change is not correct. URL Rewrites are intentionally created in advance so that product may become visible at any moment without any conflict.

[dverkade]

@orlangur unfortunately you are not correct. When you create a new product and set it to "Not visible", then the URL rewrites are not created in the URL rewrite table. So your comment about creating them in advance is false.

What happens if you first create the product where the product is visible on the frontend and then switch the product to become not visible, the generated URL keys are staying in the URL rewrite table. This can cause all kinds of issues, for instance when you create another new product with the same URL key it says that the URL key is already taken. Then you go into the first product, change the URL key, but this is not being updated because the product is not visible, so the product keeps the old URL key in the URL rewrite database and will only change this when the product is set to visible again.

This PR resolves this issue, by removing the generated URL keys for the product when you switch a product from visible to not visible.

[orlangur]

When you create a new product and set it to "Not visible", then the URL rewrites are not created in the URL rewrite table

@dverkade this is probably not correct behavior if I'm not mixing it up with enable/disable product.

the generated URL keys are staying in the URL rewrite table. This can cause all kinds of issues, for instance when you create another new product with the same URL key it says that the URL key is already taken

This is what I was talking about. Product may become visible again at any moment, showing validation message for colliding product is OK.

Then you go into the first product, change the URL key, but this is not being updated because the product is not visible, so the product keeps the old URL key in the URL rewrite database and will only change this when the product is set to visible again.

And this does not seem to be correct behavior.

[VladimirZaets]

Hi @dverkade, I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for collaboration

[m2-assistant]

Hi @dverkade, 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.

[dverkade]

Hi @VladimirZaets

This issue was labeled "Needs update" 19 days ago, but I really don't know what kind of updates you're expecting here? The PR works as described

[p-bystritsky]

@davidverholen @VladimirZaets PR updated, please review.

[orlangur]

@dverkade implementation is incorrect. First please adjust issue according to comments (#19293 (comment)), then change implementation accordingly.

[ghost]

@dverkade unfortunately, only members of the maintainers team are allowed to assign developers to the pull request

[dverkade]

@dverkade implementation is incorrect. First please adjust issue according to comments (#19293 (comment)), then change implementation accordingly.

No, this is incorrect. When the product is created with visibility invisible the URL key is not created. It should be deleted when a product visibility is changed. Magento already approved / reviewed this requests, so no need to change the behaviour of this PR.

[orlangur]

When the product is created with visibility invisible the URL key is not created

It must be created.

Magento already approved / reviewed this requests, so no need to change the behaviour of this PR.

Issue mentioned in this PR is not confirmed, please provide some links. Probably those changes must be reverted as they introduced undesired system behavior.

[sivaschenko]

@piotrekkaminski @chernenm can you please share your opinion from the product side:
Should url rewrites be created and preserved for not visible products or should they be removed once a product becomes not visible?

[orlangur]

Context: URL rewrites were created for disabled and not visible individually products initially so that they may be enabled or made visible without collision afterwards.

[dverkade]

Context: URL rewrites were created for disabled and not visible individually products initially so that they may be enabled or made visible without collision afterwards.

Incorrect. When a product has a visibility of "Not visible individually" no entries are created for this product in the URL rewrite table. Entries for URL rewrites are only created once the visibility of the product has been changed.

If you the product has been visible and the entries in the URL Rewrite table are present they are not updated or deleted once the visibility of the product will be changed again to "Not visible individually". This PR resolves this issue by deleting the URL rewrites once is set to "Not visible individually"

[orlangur]

@dverkade,

When a product has a visibility of "Not visible individually" no entries are created for this product in the URL rewrite table

And this is a bug which needs to be actually fixed... Instead of removing what is not supposed to be removed.

[dverkade]

And this is a bug which needs to be actually fixed... Instead of removing what is not supposed to be removed.

That's just your opinion. It's up to Magento product owners to determine the expected behaviour. That's not something for you to decide. There's a valid use case for not having the entries for the URL rewrites. If the URL rewrites are stored for products which are not visible this can create collisions when adding or changing products with the same URL key as the one which is not visible. Expected behaviour from a merchant's perspective would be that this URL key is still available, because when he visits the URL of the invisible product on the frontend this will result in a 404 page. By not having this URL key available can be unexpected behaviour for this user.

[orlangur]

That's just your opinion. It's up to Magento product owners to determine the expected behaviour. That's not something for you to decide

Nope 🤣 Me and the team actually implemented this functionality and I'm saying how it is intended to work. Of course, business requirements could be changed later but for now it looks to me that just some random PRs were merged which broke system behavior.

By not having this URL key available can be unexpected behaviour for this user.

No, if it gives a proper error message.

[piotrekkaminski]

All, thanks for your input. In this case I think URL rewrites should be created for invisible product. A reasonable use case would be you prepare a promo CMS page, you need to know the final URL the product will have - but want to have the product invisible unless you publish the promo CMS page. The current behavior seems inconsistent and should be fixed - URL rewrites for non-visible products should behave exactly like for visible, created on creation, updated on url key change.

[dverkade]

I'm closing this PR since this is not the desired behaviour as documented by the Magento product owner.

[m2-assistant]

Hi @dverkade, 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.

[orlangur]

@dverkade this PR can still be adjusted to a defined requirements. Could you please adjust bug report accordingly?