#22934 Improved sitemap product generation logic

[sergiy-v]

Description

Improved sitemap product generation logic for case when the 'Use Categories Path for Product URLs' is enabled.

If the 'Use Categories Path for Product URLs' is enabled the following URL generation logic for the product uses:

• if product is assigned to category (categories) use any URL with category path
• if product is not assigned to category (categories) use native product URL

Fixed Issues

1. Incorrect work of "Use Categories Path for Product URLs" in sitemap generation. #22934: Incorrect work of "Use Categories Path for Product URLs" in sitemap generation.
2. Wrong sitemap product url #4788: Wrong sitemap product url

Manual testing scenarios (*)

Create two categories:

• category-a
• category-b

Create three simple products:

• simple-a, assigne it to category-a and category-b categories
• simple-b, assigne it to category-b category
• simple-c (should not be assigned to any category)

Case #1: the 'Use Categories Path for Product URLs' is enabled

1. Enable the 'Use Categories Path for Product URLs' (set to YES in Configuration -> Catalog -> Catalog -> Search Engine Optimization).
2. Generate sitemap.
3. Check the result:

http(s)://your-magento-instance/category-b/simple-a.html
http(s)://your-magento-instance/category-b/simple-b.html
http(s)://your-magento-instance/simple-c.html

Case #2: the 'Use Categories Path for Product URLs' is disabled

1. Disable the 'Use Categories Path for Product URLs' (set to NO in Configuration -> Catalog -> Catalog -> Search Engine Optimization).
2. Generate sitemap.
3. Check the result:

http(s)://your-magento-instance/simple-a.html
http(s)://your-magento-instance/simple-b.html
http(s)://your-magento-instance/simple-c.html

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 are green)

[m2-assistant]

Hi @sergiy-v. 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 give me test instance - deploy test instance based on PR changes
• @magento give me 2.3-develop instance - deploy vanilla Magento instance

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

[magento-cicd2]

All committers have signed the CLA.

[hostep]

Just some thought here:

We sometimes enable this "Use categories path for product urls" setting on certain shops, but also always enable "Use Canonical Link Meta Tag For Products". This means that for the end user he will see an url with categories while navigating the shop, but search engines will only care about the canonical url's. Those are url's without the category path.

So if both of these settings are enabled, I would expect the sitemap to contain the canonical url's and not the url's with the category path.

Can you double check if that is the case after this PR?

[Stepa4man]

Hi @sergiy-v,
Thank you for your contribution!
Please sign Contributor License Agreement.

[magento-engcom-team]

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

[hostep]

So if both of these settings are enabled, I would expect the sitemap to contain the canonical url's and not the url's with the category path.

Can you double check if that is the case after this PR?

Since this got approved without an answer to my question above, I just did some small testing.
And it's like I thought, with the settings Use categories path for product urls and Use Canonical Link Meta Tag For Products both enabled, it still generates the product url with category path and puts it in the sitemap.

I'm not saying this will cause issues for SEO, but search engines would then have to go visit 2 product pages for every product (the first one from the sitemap, and the second when they find the canonical url on that first one and then discard the first one).
What are people's thoughts on this? Shouldn't we just put the canonical url in the sitemap when the config setting Use Canonical Link Meta Tag For Products is enabled?
This only applies for product url's btw.

---

Then a second issue I'm noticing is this case:

1. Set Use Categories Path for Product URLs => Yes
2. Set Generate "category/product" URL Rewrites => No
3. Set Use Canonical Link Meta Tag For Products => No

In this case, I would expect the product url in the sitemap to include the category path, just like it does on the frontend. But here the category path isn't part of the product url in the sitemap.
That option Generate "category/product" URL Rewrites is a new one (introduced by MC-4244) which isn't released in a stable version of Magento yet, but we should consider that one as well in this PR I'm guessing?

[sergiy-v]

@hostep very good points from your side and sorry for delay with answer.
I think it will be better to discuss such changes in the related issue (or create new one), because the PR was created for issue requirements (description) so it will be a good idea to discuss any related changes with person who opened the issue and then add needed changes or create new issue for it.
Thank you.

[soleksii]

✔️ QA Passed

[hostep]

@AlexWorking : before this PR gets merged, can you also check my comments above? Thanks! :)

[AlexWorking]

Hi, @sergiy-v, @hostep. Sorry for delay I've been on a small vacation. I was asked to recheck the issue of #4788 whether it is still reproducible or not. The recheck showed that the initial behavior described in the issue was not reproduced anymore by just following the steps given. Also I could not reproduce an updated by Valerii Naida behavior. But I detected another incorrect behavior (that was done by the way with Use Canonical Link Meta Tag For Products set to No). Therefore I closed #4788 and created #22934. That's all. Though hostep's arguments look logical enough, unfortunately I'm so to say not good enough an expert in those url rewrites and sitemappings:) So forgive me please but I don't dare to suggest, approve or disapprove something in the current situation.

[hostep]

Thanks @AlexWorking for the answer!

How should we proceed then here?

The second case I brought up looks like a missing feature not being handled which is almost exactly the same as what this PR is trying to fix, so that one should get handled in this PR as well I assume?

And the first case is something which needs to be discussed by people who understand more about SEO and what use cases sitemaps.xml files have I guess, but it is also strongly related to this PR itself, no? As it would cause a regression if this PR gets approved without handling this?

[ghost]

@magento-cicd2 unfortunately, only members of the maintainers team are allowed to unassign developers from the pull request

[ghost]

@magento-cicd2 unfortunately, only members of the maintainers team are allowed to unassign developers from the pull request

[engcom-Bravo]

Hi @hostep. Seems I do agree with You, but these questions are better be addressed to a Product Owner, who I am not, at least yet)).

[hostep]

@engcom-Bravo: ok, can you try to include a product owner in the discussion then please? Thanks! 🙂

[engcom-Bravo]

I'll try @hostep, but can't guarantee it'll be already today. (Yes as You might have guessed I'm AlexWorking)) )

[engcom-Bravo]

@hostep, I've just invited an architect on SEO Alex Paliarush @paliarush to join the discussion.

[engcom-Bravo]

Also, @hostep please take a note of this page if You've been not familiar with it. In may become in useful.

[m2-assistant]

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

[hostep]

Since this got merged without considering my comments, I created a follow up ticket: #23363