search

magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.51k stars 9.31k forks source link

URL rewrites get wiped out for products w/ store-specific URL keys when saving categories #11078

Closed CNanninga closed 4 years ago

CNanninga commented 7 years ago

Preconditions

Steps to reproduce

  1. Create and save a product (e.g., SKU has-special-key) with the URL key "url-test-one", placing it in the categories Store A Root -> Category 1 and Store B Root -> Category 2.
  2. Edit has-special-key and switch to the Store B view scope. Change the URL key to "url-test-one-override" and save.
  3. The table url_rewrite now has at least two records for has-special-key:
    1. Request path "url-test-one.html" for the Store A view
    2. Request path "url-test-one-override.html" for the Store B view
  4. Create and save a product (e.g., SKU no-special-key) with the URL key "url-test-two", placing it in the categories Store A Root -> Category 1 and Store B Root -> Category 2.
  5. The table url_rewrite now also has at least two records for no-special-key:
    1. Request path "url-test-two.html" for the Store A view
    2. Request path "url-test-two.html" for the Store B view
  6. Edit the category Store B Root -> Category 2. Change the URL key in order to trigger regeneration of products belonging to the category, and save.

Expected result

The url_rewrite table should still have the following records:

  1. Two for has-special-key:
    1. Request path "url-test-one.html" for the Store A view
    2. Request path "url-test-one-override.html" for the Store B view
  2. Two for no-special-key:
    1. Request path "url-test-two.html" for the Store A view
    2. Request path "url-test-two.html" for the Store B view

Actual result

The url_rewrite table is missing the Store B record for has-special-key (the one that should have the request path "url-test-one-override.html").

Details

This happens because of a combination of two factors:

  1. Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator::generateForGlobalScope contains a check for $this->storeViewService->doesEntityHaveOverriddenUrlKeyForStore and does not proceed to generate rewrite paths for any store view where the URL key is different than the global scope. (Presumably because, the product model having been loaded at the default scope, it does not contain the appropriate values to generate rewrites for the given store.)
  2. Magento\UrlRewrite\Model\Storage\DbStorage::createFilterDataBasedOnUrls uses the generated list of URL rewrites to be created to create an array of filters to use in deleting existing rewrite records. The filters simply specify entity IDs and store IDs.

In the reproduction scenario outlined above: the check in generateForGlobalScope means no new URL rewrite records will be created for has-special-key in Store B, because it has an overriding URL key for that store. If that were the only product in the category being saved, we wouldn't have a problem. However, no-special-key does not have an overriding URL key on Store B, so it does have new URL rewrite records for that store. And so createFilterDataBasedOnUrls ends up returning a filter array that will essentially say "delete all product rewrite records for SKUs has-special-key and no-special-key and for Store A and Store B. has-special-key's rewrite record for Store B gets deleted, but no new record takes its place.

Possible solutions

Two possible solutions are immediately apparent but have their own problems:

  1. Modify Magento\UrlRewrite\Model\Storage\DbStorage::createFilterDataBasedOnUrls so that the format of the filter is more specific about combinations of products and stores that should be deleted. But this would require re-factoring all code that uses the results of this method.
  2. Modify Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator::generateForGlobalScope to avoid the check for an overriding URL key and allow generateForSpecificStoreView to run regardless. Also modify generateForSpecificStoreView to load up a new instance of the product in the right store scope, if the product's store scope doesn't match the passed store ID and if an overriding URL key exists. But if there are many such products with overriding URL keys on specific stores, this solution could result in unacceptable performance as many individual product loads are done.
magento-engcom-team commented 7 years ago

@CNanninga, thank you for your report. We've created internal ticket(s) MAGETWO-80144 to track progress on the issue.

hostep commented 7 years ago

Just informational, a long time ago we ran into similar problems (we still do, but work around them somehow) and I tried creating a PR, which seems to be related to the possible solution number 2 which @CNanninga suggests, but the PR wasn't very good as it broke stuff on other places, so I closed it. I can't remember all the details anymore, but maybe it helps somehow: https://github.com/magento/magento2/pull/7667

maksim-grib commented 6 years ago

@magento-engcom-team are any updates on this issue? it is open for more then a year...

m2-assistant[bot] commented 4 years ago

Hi @engcom-Echo. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

engcom-Echo commented 4 years ago

Hello All

I am not able to reproduce this issue on the 2.4-develop branch by provided steps.

Testing scenario:

Preconditions:

  1. Magento install with two separate stores (Store A and Store B), with two separate catalog root categories (Store A Root and Store B Root)
  2. At least one top-level category under each root category (Store A Root -> Category 1, Store B Root -> Category 2)
  3. Store is configured not to include categories in product URL paths (uncertain whether this matters)

Steps to reproduce:

  1. Create and save a product (e.g., SKU has-special-key) with the URL key "url-test-one", placing it in the categories Store A Root -> Category 1 and Store B Root -> Category 2.
  2. Edit has-special-key and switch to the Store B view scope. Change the URL key to "url-test-one-override" and save.
  3. The table url_rewrite now has at least two records for has-special-key:
    1. Request path "url-test-one.html" for the Store A view
    2. Request path "url-test-one-override.html" for the Store B view
  4. Create and save a product (e.g., SKU no-special-key) with the URL key "url-test-two", placing it in the categories Store A Root -> Category 1 and Store B Root -> Category 2.
  5. The table url_rewrite now also has at least two records for no-special-key:
    1. Request path "url-test-two.html" for the Store A view
    2. Request path "url-test-two.html" for the Store B view
  6. Edit the category Store B Root -> Category 2. Change the URL key in order to trigger regeneration of products belonging to the category, and save.

Result The url_rewrite table have the following records:

  1. Two for has-special-key:
    1. Request path "url-test-one.html" for the Store A view
    2. Request path "url-test-one-override.html" for the Store B view
  2. Two for no-special-key:
    1. Request path "url-test-two.html" for the Store A view
    2. Request path "url-test-two.html" for the Store B view

Screenshot_35 Please feel free to comment, reopen or create new ticket according to the Issue reporting guidelines if you are still facing this issue on the latest 2.4-develop branch. Thank you for collaboration.