Skip to content

Commit bc822ca

Browse files
bubasumabubasuma
committed
ACP2E-531: [Magento Cloud] Filtering with multiple categories sorting by position not working as expected
1 parent 7e78c28 commit bc822ca

File tree

8 files changed

+503
-39
lines changed

8 files changed

+503
-39
lines changed

‎app/code/Magento/Catalog/Model/Indexer/Category/Product/AbstractAction.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,23 @@ abstract class AbstractAction
2929
/**
3030
* Chunk size
3131
*/
32-
const RANGE_CATEGORY_STEP = 500;
32+
public const RANGE_CATEGORY_STEP = 500;
3333

3434
/**
3535
* Chunk size for product
3636
*/
37-
const RANGE_PRODUCT_STEP = 1000000;
37+
public const RANGE_PRODUCT_STEP = 1000000;
3838

3939
/**
4040
* Catalog category index table name
4141
*/
42-
const MAIN_INDEX_TABLE = 'catalog_category_product_index';
42+
public const MAIN_INDEX_TABLE = 'catalog_category_product_index';
4343

4444
/**
4545
* Suffix for table to show it is temporary
4646
* @deprecated see getIndexTable
4747
*/
48-
const TEMPORARY_TABLE_SUFFIX = '_tmp';
48+
public const TEMPORARY_TABLE_SUFFIX = '_tmp';
4949

5050
/**
5151
* Cached non anchor categories select by store id
@@ -582,7 +582,7 @@ protected function createAnchorSelect(Store $store)
582582
'category_id' => 'cc.entity_id',
583583
'product_id' => 'ccp.product_id',
584584
'position' => new \Zend_Db_Expr(
585-
$this->connection->getIfNullSql('ccp2.position', 'ccp.position + 10000')
585+
$this->connection->getIfNullSql('ccp2.position', 'MIN(ccp.position) + 10000')
586586
),
587587
'is_parent' => new \Zend_Db_Expr('0'),
588588
'store_id' => new \Zend_Db_Expr($store->getId()),
@@ -823,7 +823,7 @@ protected function getAllProducts(Store $store)
823823
'category_id' => new \Zend_Db_Expr($store->getRootCategoryId()),
824824
'product_id' => 'cp.entity_id',
825825
'position' => new \Zend_Db_Expr(
826-
$this->connection->getCheckSql('ccp.product_id IS NOT NULL', 'ccp.position', '0')
826+
$this->connection->getCheckSql('ccp.product_id IS NOT NULL', 'MIN(ccp.position)', '10000')
827827
),
828828
'is_parent' => new \Zend_Db_Expr(
829829
$this->connection->getCheckSql('ccp.product_id IS NOT NULL', '1', '0')

‎app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public function build(array $args, bool $includeAggregation): SearchCriteriaInte
104104
$this->addDefaultSortOrder($searchCriteria, $args, $isSearch);
105105
}
106106

107-
$this->addEntityIdSort($searchCriteria, $args);
107+
$this->addEntityIdSort($searchCriteria);
108108
$this->addVisibilityFilter($searchCriteria, $isSearch, !empty($args['filter']));
109109

110110
$searchCriteria->setCurrentPage($args['currentPage']);
@@ -137,15 +137,22 @@ private function addVisibilityFilter(SearchCriteriaInterface $searchCriteria, bo
137137
* Add sort by Entity ID
138138
*
139139
* @param SearchCriteriaInterface $searchCriteria
140-
* @param array $args
141140
*/
142-
private function addEntityIdSort(SearchCriteriaInterface $searchCriteria, array $args): void
141+
private function addEntityIdSort(SearchCriteriaInterface $searchCriteria): void
143142
{
144-
$sortOrder = !empty($args['sort']) ? reset($args['sort']) : SortOrder::SORT_DESC;
145143
$sortOrderArray = $searchCriteria->getSortOrders();
144+
$sortDir = SortOrder::SORT_DESC;
145+
if (count($sortOrderArray) > 0) {
146+
$sortOrder = end($sortOrderArray);
147+
// in the case the last sort order is by position, sort IDs in descendent order
148+
$sortDir = $sortOrder->getField() === EavAttributeInterface::POSITION
149+
? SortOrder::SORT_DESC
150+
: $sortOrder->getDirection();
151+
}
152+
146153
$sortOrderArray[] = $this->sortOrderBuilder
147154
->setField('_id')
148-
->setDirection($sortOrder)
155+
->setDirection($sortDir)
149156
->create();
150157
$searchCriteria->setSortOrders($sortOrderArray);
151158
}

‎app/code/Magento/CatalogGraphQl/Model/Resolver/Products/SearchCriteria/CollectionProcessor/FilterProcessor/CategoryFilter.php

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Magento\Catalog\Model\CategoryFactory;
1111
use Magento\Catalog\Model\ResourceModel\Category as CategoryResourceModel;
1212
use Magento\Catalog\Model\ResourceModel\Product\Collection;
13+
use Magento\CatalogGraphQl\Model\ResourceModel\Product\Collection as GraphQLProductsCollection;
1314
use Magento\Framework\Api\Filter;
1415
use Magento\Framework\Api\SearchCriteria\CollectionProcessor\FilterProcessor\CustomFilterInterface;
1516
use Magento\Framework\Data\Collection\AbstractDb;
@@ -82,35 +83,66 @@ public function __construct(
8283
*/
8384
public function apply(Filter $filter, AbstractDb $collection)
8485
{
85-
$conditionType = $filter->getConditionType() ?: self::CONDITION_TYPE_IN;
86-
$value = $filter->getValue();
87-
if ($value && in_array($conditionType, self::CONDITION_TYPES)) {
88-
if ($conditionType === self::CONDITION_TYPE_EQ) {
89-
$category = $this->getCategory((int) $value);
90-
/** @var Collection $collection */
86+
if ($this->isApplicable($filter)) {
87+
/** @var Collection $collection */
88+
$conditionType = $filter->getConditionType() ?: self::CONDITION_TYPE_IN;
89+
$value = $filter->getValue();
90+
$ids = is_array($value) ? $value : explode(',', (string) $value);
91+
if (in_array($conditionType, [self::CONDITION_TYPE_EQ, self::CONDITION_TYPE_IN]) && count($ids) === 1) {
92+
$category = $this->getCategory((int) reset($ids));
9193
/** This filter adds ability to sort by position*/
9294
$collection->addCategoryFilter($category);
93-
} elseif (!$collection->getFlag('search_resut_applied')) {
94-
/** Prevent filtering duplication as the filter should be already applied to the search result */
95-
$values = is_array($value) ? $value : explode(',', (string) $value);
96-
$categoryIds = [];
97-
foreach ($values as $value) {
98-
$category = $this->getCategory((int) $value);
99-
$children = [];
100-
$childrenStr = $category->getIsAnchor() ? $category->getChildren(true) : '';
101-
if ($childrenStr) {
102-
$children = explode(',', $childrenStr);
103-
}
104-
array_push($categoryIds, $value, ...$children);
105-
}
106-
/** @var Collection $collection */
107-
$collection->addCategoriesFilter([$conditionType => array_map('intval', $categoryIds)]);
95+
} elseif ($conditionType === self::CONDITION_TYPE_IN && $collection instanceof GraphQLProductsCollection) {
96+
$collection->joinMinimalPosition($ids);
97+
}
98+
/** Prevent filtering duplication as the filter should be already applied to the search result */
99+
if (!$collection->getFlag('search_resut_applied')) {
100+
$collection->addCategoriesFilter(
101+
[
102+
$conditionType => array_map('intval', $this->getCategoryIds($ids))
103+
]
104+
);
108105
}
109106
}
110107

111108
return true;
112109
}
113110

111+
/**
112+
* Check whether the filter can be applied
113+
*
114+
* @param Filter $filter
115+
* @return bool
116+
*/
117+
private function isApplicable(Filter $filter): bool
118+
{
119+
/** @var Collection $collection */
120+
$conditionType = $filter->getConditionType() ?: self::CONDITION_TYPE_IN;
121+
122+
return $filter->getValue() && in_array($conditionType, self::CONDITION_TYPES);
123+
}
124+
125+
/**
126+
* Returns all children category IDs for anchor categories including the provided categories
127+
*
128+
* @param array $values
129+
* @return array
130+
*/
131+
private function getCategoryIds(array $values): array
132+
{
133+
$categoryIds = [];
134+
foreach ($values as $value) {
135+
$category = $this->getCategory((int) $value);
136+
$children = [];
137+
$childrenStr = $category->getIsAnchor() ? $category->getChildren(true) : '';
138+
if ($childrenStr) {
139+
$children = explode(',', $childrenStr);
140+
}
141+
array_push($categoryIds, $value, ...$children);
142+
}
143+
return $categoryIds;
144+
}
145+
114146
/**
115147
* Retrieve the category model by ID
116148
*

‎app/code/Magento/CatalogGraphQl/Model/ResourceModel/Product/Collection.php

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\CatalogGraphQl\Model\ResourceModel\Product;
9+
10+
use Magento\Catalog\Model\Indexer\Category\Product\TableMaintainer;
11+
use Magento\Catalog\Model\Indexer\Product\Flat\State;
12+
use Magento\Catalog\Model\Indexer\Product\Price\PriceTableResolver;
13+
use Magento\Catalog\Model\Product\Gallery\ReadHandler as GalleryReadHandler;
14+
use Magento\Catalog\Model\Product\OptionFactory;
15+
use Magento\Catalog\Model\ResourceModel\Category;
16+
use Magento\Catalog\Model\ResourceModel\Helper;
17+
use Magento\Catalog\Model\ResourceModel\Product\Collection\ProductLimitationFactory;
18+
use Magento\Catalog\Model\ResourceModel\Product\Gallery;
19+
use Magento\Catalog\Model\ResourceModel\Url;
20+
use Magento\CatalogUrlRewrite\Model\Storage\DbStorage;
21+
use Magento\Customer\Api\GroupManagementInterface;
22+
use Magento\Customer\Model\Session;
23+
use Magento\Eav\Model\Config;
24+
use Magento\Eav\Model\EntityFactory;
25+
use Magento\Framework\App\Config\ScopeConfigInterface;
26+
use Magento\Framework\App\ObjectManager;
27+
use Magento\Framework\App\ResourceConnection;
28+
use Magento\Framework\Data\Collection\Db\FetchStrategyInterface;
29+
use Magento\Framework\DB\Adapter\AdapterInterface;
30+
use Magento\Framework\DB\Select;
31+
use Magento\Framework\EntityManager\MetadataPool;
32+
use Magento\Framework\Event\ManagerInterface;
33+
use Magento\Framework\Indexer\DimensionFactory;
34+
use Magento\Framework\Module\Manager;
35+
use Magento\Framework\Stdlib\DateTime;
36+
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
37+
use Magento\Framework\Validator\UniversalFactory;
38+
use Magento\Store\Model\StoreManagerInterface;
39+
use Psr\Log\LoggerInterface;
40+
use Zend_Db_Expr;
41+
use Zend_Db_Select_Exception;
42+
43+
/**
44+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
45+
*/
46+
class Collection extends \Magento\Catalog\Model\ResourceModel\Product\Collection
47+
{
48+
/**
49+
* @var TableMaintainer
50+
*/
51+
private $tableMaintainer;
52+
53+
/**
54+
* @param \Magento\Framework\Data\Collection\EntityFactory $entityFactory
55+
* @param LoggerInterface $logger
56+
* @param FetchStrategyInterface $fetchStrategy
57+
* @param ManagerInterface $eventManager
58+
* @param Config $eavConfig
59+
* @param ResourceConnection $resource
60+
* @param EntityFactory $eavEntityFactory
61+
* @param Helper $resourceHelper
62+
* @param UniversalFactory $universalFactory
63+
* @param StoreManagerInterface $storeManager
64+
* @param Manager $moduleManager
65+
* @param State $catalogProductFlatState
66+
* @param ScopeConfigInterface $scopeConfig
67+
* @param OptionFactory $productOptionFactory
68+
* @param Url $catalogUrl
69+
* @param TimezoneInterface $localeDate
70+
* @param Session $customerSession
71+
* @param DateTime $dateTime
72+
* @param GroupManagementInterface $groupManagement
73+
* @param AdapterInterface|null $connection
74+
* @param ProductLimitationFactory|null $productLimitationFactory
75+
* @param MetadataPool|null $metadataPool
76+
* @param TableMaintainer|null $tableMaintainer
77+
* @param PriceTableResolver|null $priceTableResolver
78+
* @param DimensionFactory|null $dimensionFactory
79+
* @param Category|null $categoryResourceModel
80+
* @param DbStorage|null $urlFinder
81+
* @param GalleryReadHandler|null $productGalleryReadHandler
82+
* @param Gallery|null $mediaGalleryResource
83+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
84+
*/
85+
public function __construct(
86+
\Magento\Framework\Data\Collection\EntityFactory $entityFactory,
87+
LoggerInterface $logger,
88+
FetchStrategyInterface $fetchStrategy,
89+
ManagerInterface $eventManager,
90+
Config $eavConfig,
91+
ResourceConnection $resource,
92+
EntityFactory $eavEntityFactory,
93+
Helper $resourceHelper,
94+
UniversalFactory $universalFactory,
95+
StoreManagerInterface $storeManager,
96+
Manager $moduleManager,
97+
State $catalogProductFlatState,
98+
ScopeConfigInterface $scopeConfig,
99+
OptionFactory $productOptionFactory,
100+
Url $catalogUrl,
101+
TimezoneInterface $localeDate,
102+
Session $customerSession,
103+
DateTime $dateTime,
104+
GroupManagementInterface $groupManagement,
105+
AdapterInterface $connection = null,
106+
ProductLimitationFactory $productLimitationFactory = null,
107+
MetadataPool $metadataPool = null,
108+
TableMaintainer $tableMaintainer = null,
109+
PriceTableResolver $priceTableResolver = null,
110+
DimensionFactory $dimensionFactory = null,
111+
Category $categoryResourceModel = null,
112+
DbStorage $urlFinder = null,
113+
GalleryReadHandler $productGalleryReadHandler = null,
114+
Gallery $mediaGalleryResource = null
115+
) {
116+
parent::__construct(
117+
$entityFactory,
118+
$logger,
119+
$fetchStrategy,
120+
$eventManager,
121+
$eavConfig,
122+
$resource,
123+
$eavEntityFactory,
124+
$resourceHelper,
125+
$universalFactory,
126+
$storeManager,
127+
$moduleManager,
128+
$catalogProductFlatState,
129+
$scopeConfig,
130+
$productOptionFactory,
131+
$catalogUrl,
132+
$localeDate,
133+
$customerSession,
134+
$dateTime,
135+
$groupManagement,
136+
$connection,
137+
$productLimitationFactory,
138+
$metadataPool,
139+
$tableMaintainer,
140+
$priceTableResolver,
141+
$dimensionFactory,
142+
$categoryResourceModel,
143+
$urlFinder,
144+
$productGalleryReadHandler,
145+
$mediaGalleryResource
146+
);
147+
148+
$this->tableMaintainer = $tableMaintainer ?: ObjectManager::getInstance()
149+
->get(TableMaintainer::class);
150+
}
151+
152+
/**
153+
* Join minimal position to the select
154+
*
155+
* @param array $categoryIds
156+
* @return void
157+
* @throws Zend_Db_Select_Exception
158+
*/
159+
public function joinMinimalPosition(array $categoryIds): void
160+
{
161+
$this->_applyProductLimitations();
162+
$filters = $this->_productLimitationFilters;
163+
$positions = [];
164+
$connection = $this->getConnection();
165+
$select = $this->getSelect();
166+
167+
foreach ($categoryIds as $categoryId) {
168+
$table = 'cat_index_' . $categoryId;
169+
$conditions = [
170+
$table . '.product_id=e.entity_id',
171+
$connection->quoteInto(
172+
$table . '.store_id=?',
173+
$filters['store_id'],
174+
'int'
175+
),
176+
$connection->quoteInto(
177+
$table . '.category_id=?',
178+
$categoryId,
179+
'int'
180+
)
181+
];
182+
183+
$joinCond = implode(' AND ', $conditions);
184+
$fromPart = $select->getPart(Select::FROM);
185+
if (isset($fromPart[$table])) {
186+
$fromPart[$table]['joinCondition'] = $joinCond;
187+
$select->setPart(Select::FROM, $fromPart);
188+
} else {
189+
$select->joinLeft(
190+
[$table => $this->tableMaintainer->getMainTable($this->getStoreId())],
191+
$joinCond,
192+
[]
193+
);
194+
}
195+
$positions[] = $connection->getIfNullSql($table . '.position', '~0');
196+
}
197+
198+
$columns = $select->getPart(Select::COLUMNS);
199+
$columnIndex = false;
200+
$minPos = $connection->getLeastSql($positions);
201+
foreach ($columns as $index => [,, $columnAlias]) {
202+
if ($columnAlias === 'cat_index_position') {
203+
$columnIndex = $index;
204+
break;
205+
}
206+
}
207+
if ($columnIndex) {
208+
$columns[$columnIndex][1] = $minPos;
209+
$select->setPart(Select::COLUMNS, $columns);
210+
} else {
211+
$select->columns(['cat_index_position' => $minPos]);
212+
}
213+
$this->_joinFields['position'] = ['table' => '', 'field' => 'cat_index_position'];
214+
}
215+
}

0 commit comments

Comments
 (0)