URL Regeneration Fails on Store-Level URL Keys - Products

[BarnyShergold]

Preconditions (*)

1. Magento 2.3.5-p1 (Cloud Edition)
2. Magento 2.4-develop

Steps to reproduce (*)

We have a module that will regenerate the URLs of products enmasse - this is useful for many reasons but we have discovered that on a site where Store Views use different URL keys (category) incorrect URLs can be generated. This occurs when product URLS contain category URLs.

The module uses the Core Magento urlRewriteGenerator to do this -

  public function regenerateEntity($products, $storeId)
   {
       $this->getConnection()->beginTransaction();
       $currentProductId = 0;

       try {
           $urls = [];
           /** @var \Magento\Catalog\Model\Product $product */
           foreach ($products as $product) {
               $currentProductId = $product->getId();
               $this->logger->info(\__('Regeneration of product %1', $product->getSku()));

               $product->setStoreId($storeId);

               if ($product->getVisibility() !== Visibility::VISIBILITY_NOT_VISIBLE) {
                   $urls = \array_merge($urls, $this->urlRewriteGenerator->generate($product));
               }
           }

           if (!empty($urls)) {
               $this->urlPersist->replace($urls);
           }

           $this->getConnection()->commit();
       } catch (\Throwable $e) {
           $this->getConnection()->rollback();
           $message = \__(
               'Error occurred for product id %1, store id %2: %3',
               $currentProductId,
               $storeId,
               $e->getMessage()
           );
           $this->logger->error($message);
       }
   }

We have also found that the same bugs below cause URLS to be created incorrectly for a product when an Admin user switches Visibility Status.

1. Ensure Category URLs in Product URLS is active
2. Create 3 level category - Men > Collections > Premium
3. Add product to this category set visibility to "Not Visible"
4. Create a new Store View - Italian.
5. Edit Categories from Admin and switch to Italian Store View
6. Change url_key on Men to uomo
7. Change url_key on Collections to collezioni
8. Change product from "Not Visible" to "Catalog, Search"

Expected result (*)

1. Find the record for this product on this category in url_rewrite.
2. Record should show for request path : uomo/collezioni/premium/

Actual result (*)

1. Record shows : men/collections/premium/

The Regeneration script causes the same issue. Also the script can cause TWO URLs to appear for a product - one in the StoreView language and one in the Default View language - both work but Magento uses the Default View one.

BUGS FOUND -

ProductScopeRewriteGenerator::getCategoryWithOverriddenUrlKey - The problem is that the method checks to see if the category has an overridden URL key for that store view. If not then it uses the Default Category. The flaw here is that the category in question could be several layers down and whilst the URL key might still be the default, the higher level ones might not.
EG as above on a 3rd Level category :
men/collections/premium/
In Italian this is :
uomo/collezioni/premium/
So Magento sees the key 'premium' as the same as default, uses default level and thus the Italian site has the English URLS
SOLUTION - The method should always return the store view level category.

AnchorUrlRewriteGenerator::generate
This method handles rewrites for anchor categories. However, where it gets the category (line 71) it always gets the default level. This means the URL created and inserted is the Default Store one.
Result is that a product can have TWO URLs - one in the language of the Store View and one in the language of the default store. Both work, but Magento seems to want to use the Default Store one on the front-end
SOLUTION - This line should pass the $storeId.

---

Please provide Severity assessment for the Issue as Reporter. This information will help during Confirmation and Issue triage processes.

• Severity: S0 - Affects critical data or functionality and leaves users without workaround.
• Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
• Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
• Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
• Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

[m2-assistant]

Hi @BarnyShergold. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

• Summary of the issue
• Information on your environment
• Steps to reproduce
• Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

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

Please, add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

[hostep]

Hi @BarnyShergold

Can you verify if #26784 fixes your issue?

It's been a known issue since Magento 2.0.0 was released, I've been complaining about this bug since 2016 and finally after many years and many, many, many, many Pull Request attempts, it looks like it will finally be fixed in 2.4.0 (it was previously already fixed in 2.2.10, but no idea if the fix will ever come to 2.3.x ... though)

[pocallaghan]

@hostep #26784 partly fixes it by ensuring that the url_path should end up right for all stores. #28676 as linked by @BarnyShergold fixes a related issue whereby saving a category in a scope with an overloaded url_key, uses that categories url_path from the current store when generating the URL of products for all stores that don't have an overloaded url_key. I've composer patched this slightly differently locally by forcing \Magento\CatalogUrlRewrite\Service\V1\StoreViewService::doesEntityHaveOverriddenUrlKeyForStore to always return true and thus always fetch the category from the repo with the right store ID.

[BarnyShergold]

@pocallaghan I originally had a similar fix - however if you just have that returning zero then one of the Integration Tests fails due to the way category data is handled when MOVE is executed. The fix on #28676 solves the bug and allows the tests to pass

[pocallaghan]

@BarnyShergold yer, I have since moved the change due to an issue caused elsewhere. I have to say URL handling in M2 is a bit of a tire fire if you have more than one store. I've ran in to so many different issues.

[hostep]

@pocallaghan, exactly! Feel free to checkout the module we wrote for detecting various issues with incorrect url data in the database: https://github.com/baldwin-agency/magento2-module-url-data-integrity-checker (incorrect url rewrite detection isn't part of it yet, because it's pretty complex and also because I have not that much free time to work on it)