-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Move assign category ids to beforeToHtml instead inside load collection #32390
Move assign category ids to beforeToHtml instead inside load collection #32390
Conversation
Hi @mrtuvn. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review. For more details, please, review the Magento Contributor Guide documentation. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket. 🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
@magento run all tests |
dev/tests/integration/testsuite/Magento/Catalog/Block/Product/ListTest.php
Outdated
Show resolved
Hide resolved
dev/tests/integration/testsuite/Magento/Catalog/Block/Product/ListTest.php
Show resolved
Hide resolved
dev/tests/integration/testsuite/Magento/Catalog/Block/Product/ListTest.php
Show resolved
Hide resolved
7264026
to
c0d7665
Compare
@magento run all tests |
Hi @vzabaznov @mslabko @sidolov @sivaschenko, |
@ihor-sviziev category_id setting was added as part of performance improvement |
From @sivaschenko in slack:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔ Approved
Note for QA: Please make sure this PR isn't introducing regression for performance improvement for the EE edition that was done in the scope of MC-33086.
Hi @ihor-sviziev, thank you for the review. |
✔️ QA passed |
Moving back to "accept" status. It looks like it was moved by some mistake. |
Hi @mrtuvn, thank you for your contribution! |
Description (*)
The main purpose move assign category ids operation to
_beforeToHtml
. Clean upgetLoadedProductCollection
like before version 2.3. Make sure we get predictable raw data collection product (any plugin from core or 3rd-party supplier can use this entry point for get product collection data from block list). Then before render data html to browser we set datacategory_id
to product if data availableHere is commit previous change
03a6f4d
baebe46
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
CC: @tzyganu
Contribution checklist (*)