-
Notifications
You must be signed in to change notification settings - Fork 9.4k
31253 default store breadcrumbs returned for products which in both root categories #31822
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
Conversation
Default store (Root category A) breadcrumbs are being returned for New Store (Root category B) when Product belongs to both root categories - fixed catalog_category_product index
…are being returned for New Store (Root category B) when Product belongs to both root categories - added test for not relevant store
Hi @sasha19957099. 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 |
@@ -379,13 +380,13 @@ private function addFilteringByChildProductsToSelect(Select $select, Store $stor | |||
[] | |||
)->joinLeft( | |||
['child_cpsd' => $this->getTable('catalog_product_entity_int')], | |||
'child_cpsd.' . $linkField . ' = '. 'relation_product_entity.' . $linkField | |||
'child_cpsd.' . $linkField . ' = ' . 'relation_product_entity.' . $linkField |
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.
is there a safer alternative to this?
just concatenating strings as parameters to a mysql query could lead to a sql injection
use Magento\TestFramework\Helper\Bootstrap; | ||
use Magento\TestFramework\Workaround\Override\Fixture\Resolver; | ||
|
||
//$objectManager = Bootstrap::getObjectManager(); |
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.
don't forget to remove debug code
@@ -171,7 +172,7 @@ protected function reindex() | |||
{ | |||
foreach ($this->storeManager->getStores() as $store) { | |||
if ($this->getPathFromCategoryId($store->getRootCategoryId())) { | |||
$this->currentStoreId = $store->getId(); | |||
$this->currentStore = $store; |
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.
better to move this to a method that ensures a strict type return
that way when getCurrentStore is used it can throw an error is currentStore is empty
…ned_for_products_which_in_both_root_categories
…n both root categories - fixed requested changes
@magento run all tests |
@magento run Functional Tests CE |
Hi @sasha19957099. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review. For more details, 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, join the Community Contributions Triage session to discuss the appropriate ticket. ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
…o 31253_default_store_breadcrumbs_returned_for_products_which_in_both_root_categories
@magento run all tests |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
@magento run all tests |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
The code review points seem to be addressed, moving it into Pending Review column Thanks! |
It seems that bot moved this PR from Pending Review to Changes Requested so moving back again to Pending Review. |
…ned_for_products_which_in_both_root_categories
@magento run all tests |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
@@ -0,0 +1,67 @@ | |||
<?php |
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.
Please use parametrized fixture format
@@ -0,0 +1,12 @@ | |||
<?php |
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.
Please use parametrized fixture format
Hi @sasha19957099 & @ishakhsuvarov Thanks for your contribution and collaboration. As I have the latest updates from our internal team, our internal team resolving the PR comments to deliver within a short period along with 2.4.5 release itself in the scope of internal ticket Any further updates will be posted here. Thanks |
We raised internal PR-https://github.com/magento-gl/magento2ce/pull/303 along with this PR commits and covering the automation tests and resolved PR comments |
Description (*)
Issue was related to the indexer catalog_category_product
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Magento 2.4.1
1.Two root categories exists in Admin-Categories
2.Two stores exists with different root categories for each of it
3.Each Stores should have atleast one store view.
4.Each Root category should have one sub-category.
5.Product should belong to both stores. i.e. Assign product to all root and sub categories.
6.Run query
{
"store": "engchildren"
}
query {
products(filter: { url_key: { eq: "gabrielle-micro-sleeve-top"} }){
items {
categories {
name
id
url_path
breadcrumbs {
category_id
category_name
category_level
}
}
}
}
}
Questions or comments
Contribution checklist (*)