-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
11615: Fix UrlRewriteGenerator for anchor categories #20826
Conversation
Hi @obuchowski. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
Let's also link to the corresponding PR for 2.2-develop: #14344 |
Thank you @hostep |
Hi @VladimirZaets, thank you for the review. |
@obuchowski thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository. |
@VladimirZaets thank you guys. Pull requests accepting process has significantly accelerated compared with previous years. Great work. |
✔️ QA passed |
Hi @obuchowski , we found huge performance degradation on the scenario:: |
@sidolov: how can this change have influence on getting category data? Are url rewrites being generated while just getting category data? That wouldn't make any sense? |
@obuchowski, I am closing this PR now due to inactivity. |
Hi @obuchowski, thank you for your contribution! |
Closing this is ridiculous, I'm reopening. |
Hi @obuchowski. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
@obuchowski please sign the Adobe CLA so that we can further process this PR |
Hi @obuchowski, |
@obuchowski unfortunately, we will not be able to process this PR without signed CLA. |
I run performance tests on this branch, everything is ✅ |
@lenaorobei: so why has this ticket taken sooo long to get processed, first Magento staff is complaining that it causes performance issues and refuses to merge it, and now all of the sudden there's no problem anymore? If that's the only thing blocking this PR from merging, I'll create a new one in a couple of days. |
@hostep it's not an excuse, but the reason is a huge amount of open PRs. CLA is the only thing. Sorry about that and I hope for your understanding. |
Added a new PR: #26784 Unless @obuchowski starts responding again, we can probably close this one? |
Hi @obuchowski, |
Hi @obuchowski, thank you for your contribution! |
A commit itself is pretty straightforward. Need to pass storeId param to category repository so resulted category has a correct url path.
Description (*)
For not default stores adding a category for a product may result in wrong url rewrite for a product in category's anchor parent .
Fixed Issues (if relevant)
Manual testing scenarios (*)
Note that there is a correct result if a product is added from a category form.
Contribution checklist (*)