#33387: fix GraphQl products query layered navigation category child #33547
Conversation
|
Hi @korovitskyi. 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 |
There was a problem hiding this comment.
Hello @korovitskyi
Thank you for your contribution.
Could you please add some fixes?
Thank you in advance
| * @param array $categoryFilter | ||
| * @param Bucket[] $bucketList | ||
| * | ||
| * @return Aggregation |
There was a problem hiding this comment.
Hello @korovitskyi
Could you please merge tags to the common block?
Remove redundant line.
| * @param array $categoryFilter | |
| * @param Bucket[] $bucketList | |
| * | |
| * @return Aggregation | |
| * @param array $categoryFilter | |
| * @param Bucket[] $bucketList | |
| * @return Aggregation |
| if (!$searchableCategoryValue || empty($values)) { | ||
| return $this->aggregationFactory->create([self::BUCKETS => $bucketList]); | ||
| } | ||
|
|
There was a problem hiding this comment.
Please remove redundant line
|
|
||
| $resolvedCategoryBucket = $this->bucketFactory->create( | ||
| [ | ||
| 'name' => self::CATEGORY_BUCKET, | ||
| 'values' => $resolvedValues | ||
| ] | ||
| ); | ||
|
|
There was a problem hiding this comment.
Please remove redundant lines
| $resolvedCategoryBucket = $this->bucketFactory->create( | |
| [ | |
| 'name' => self::CATEGORY_BUCKET, | |
| 'values' => $resolvedValues | |
| ] | |
| ); | |
| $resolvedCategoryBucket = $this->bucketFactory->create( | |
| [ | |
| 'name' => self::CATEGORY_BUCKET, | |
| 'values' => $resolvedValues | |
| ] | |
| ); |
| * @param int $searchableCategoryId | ||
| * @param AggregationValueInterface[] $aggregationValues | ||
| * | ||
| * @return AggregationValueInterface[] | ||
| * | ||
| * @throws NoSuchEntityException |
There was a problem hiding this comment.
Hello @korovitskyi
Could you please merge tags to the common block?
Remove redundant lines.
| * @param int $searchableCategoryId | |
| * @param AggregationValueInterface[] $aggregationValues | |
| * | |
| * @return AggregationValueInterface[] | |
| * | |
| * @throws NoSuchEntityException | |
| * @param int $searchableCategoryId | |
| * @param AggregationValueInterface[] $aggregationValues | |
| * @return AggregationValueInterface[] | |
| * @throws NoSuchEntityException |
| */ | ||
| private function getValidCategories(int $searchableCategoryId, array $aggregationValues): array | ||
| { | ||
| $stareId = (int) $this->storeManager->getStore()->getId(); |
There was a problem hiding this comment.
Hello @korovitskyi
Perhaps this is typo ($stareId should be named $storeId)
| if (!in_array($childCategoryId, $validChildIdList) || | ||
| in_array($childCategoryId, $this->resolvedChildrenIds) | ||
| ) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Hello @korovitskyi
Could you please fix it according to PSR-12 requirements and examples https://www.php-fig.org/psr/psr-12/ (see p.5.1)
| if (!in_array($childCategoryId, $validChildIdList) || | |
| in_array($childCategoryId, $this->resolvedChildrenIds) | |
| ) { | |
| continue; | |
| } | |
| if ( | |
| !in_array($childCategoryId, $validChildIdList) | |
| ||. in_array($childCategoryId, $this->resolvedChildrenIds) | |
| ) { | |
| continue; | |
| } |
| } | ||
|
|
||
| /** | ||
| * Check is valid searchable category children. |
There was a problem hiding this comment.
Hello @korovitskyi
This method not only checks, but returns list of categories, right?
This summary can confuse.
Could you please fix it?
| @@ -84,6 +91,7 @@ public function __construct( | |||
| $this->fieldSelection = $fieldSelection; | |||
| $this->productsProvider = $productsProvider; | |||
| $this->searchCriteriaBuilder = $searchCriteriaBuilder; | |||
| $this->resolveCategoryAggregation = $resolveCategoryAggregation; | |||
There was a problem hiding this comment.
Hello @korovitskyi
These changes are backward incompatible.
Could you please fix it.
| * @param array $args | ||
| * @param ResolveInfo $info | ||
| * @param ContextInterface $context | ||
| * | ||
| * @return SearchResult | ||
| * | ||
| * @throws GraphQlInputException | ||
| */ |
There was a problem hiding this comment.
| * @param array $args | |
| * @param ResolveInfo $info | |
| * @param ContextInterface $context | |
| * | |
| * @return SearchResult | |
| * | |
| * @throws GraphQlInputException | |
| */ | |
| * @param array $args | |
| * @param ResolveInfo $info | |
| * @param ContextInterface $context | |
| * @return SearchResult | |
| * @throws GraphQlInputException | |
| */ |
|
Hi, @andrewbess . |
|
Hi @korovitskyi, Thanks for your contribution! As per your comment, you have implemented the review changes so moving it for review. |
|
Hi @korovitskyi, thank you for your contribution! |
Description (*)
It was investigated the category grid page -> category filters. There we check the category filter by current category ID and children.
In the same way, was added a point to
\Magento\CatalogGraphQl\Model\Resolver\Products\Query\Searchto modify the aggregation category by searchable category ID. As for me, it is the best point, as we must know if request queries have been filtered by category_id and know ID value(s).Reopen by #33520.
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Send request and the count of categories should be 4:
Contribution checklist (*)