Skip to content

Commit ab4b541

Browse files
committed
MC-42799: [Magento Cloud] Premiere Support - Layered Navigation setting for price range is not working with our custom attributes
- Removed caching of aggregation algorithm instance as data provider instance is tied to a specific field name. Previously data provider used "price" field to aggregate the data regardless of the bucket field name. Now bucket field name is passed to the data provider constructor to avoid BiC changes.
1 parent dfe7b89 commit ab4b541

File tree

11 files changed

+220
-41
lines changed

11 files changed

+220
-41
lines changed

app/code/Magento/CatalogSearch/Model/Layer/Filter/Decimal.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ protected function _getItemsData()
112112
$facets = $productCollection->getFacetedData($attribute->getAttributeCode());
113113

114114
$data = [];
115+
$lastFacet = array_key_last($facets);
115116
foreach ($facets as $key => $aggregation) {
116117
$count = $aggregation['count'];
117118
if (!$this->isOptionReducesResults($count, $productSize)) {
@@ -124,7 +125,7 @@ protected function _getItemsData()
124125
if ($to == '*') {
125126
$to = null;
126127
}
127-
$label = $this->renderRangeLabel(empty($from) ? 0 : $from, $to);
128+
$label = $this->renderRangeLabel(empty($from) ? 0 : $from, $lastFacet === $key ? null : $to);
128129
$value = $from . '-' . $to;
129130

130131
$data[] = [

app/code/Magento/CatalogSearch/Model/Search/RequestGenerator/Decimal.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@
1616
*/
1717
class Decimal implements GeneratorInterface
1818
{
19+
/**
20+
* Price attribute aggregation algorithm
21+
*/
22+
private const AGGREGATION_ALGORITHM_VARIABLE = 'price_dynamic_algorithm';
23+
24+
/**
25+
* Default decimal attribute aggregation algorithm
26+
*/
27+
private const DEFAULT_AGGREGATION_ALGORITHM = 'manual';
28+
1929
/**
2030
* @inheritdoc
2131
*/
@@ -39,7 +49,9 @@ public function getAggregationData(Attribute $attribute, $bucketName)
3949
'type' => BucketInterface::TYPE_DYNAMIC,
4050
'name' => $bucketName,
4151
'field' => $attribute->getAttributeCode(),
42-
'method' => 'manual',
52+
'method' => $attribute->getFrontendInput() === 'price'
53+
? '$' . self::AGGREGATION_ALGORITHM_VARIABLE . '$'
54+
: self::DEFAULT_AGGREGATION_ALGORITHM,
4355
'metric' => [['type' => 'count']],
4456
];
4557
}

app/code/Magento/Elasticsearch/SearchAdapter/Aggregation/Builder.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,12 @@ public function build(RequestInterface $request, array $queryResult)
7979
$aggregations = [];
8080
$buckets = $request->getAggregation();
8181

82-
$dataProvider = $this->dataProviderFactory->create(
83-
$this->dataProviderContainer[$request->getIndex()],
84-
$this->query
85-
);
8682
foreach ($buckets as $bucket) {
83+
$dataProvider = $this->dataProviderFactory->create(
84+
$this->dataProviderContainer[$request->getIndex()],
85+
$this->query,
86+
$bucket->getField()
87+
);
8788
$bucketAggregationBuilder = $this->aggregationContainer[$bucket->getType()];
8889
$aggregations[$bucket->getName()] = $bucketAggregationBuilder->build(
8990
$bucket,

app/code/Magento/Elasticsearch/SearchAdapter/Aggregation/DataProviderFactory.php

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,16 @@ public function __construct(ObjectManagerInterface $objectManager)
4848
* which will be recreated with its default configuration.
4949
*
5050
* @param DataProviderInterface $dataProvider
51-
* @param QueryContainer $query
51+
* @param QueryContainer|null $query
52+
* @param string|null $aggregationFieldName
5253
* @return DataProviderInterface
5354
* @throws \LogicException when the query is missing but it required according to the QueryAwareInterface
5455
*/
55-
public function create(DataProviderInterface $dataProvider, QueryContainer $query = null)
56-
{
56+
public function create(
57+
DataProviderInterface $dataProvider,
58+
QueryContainer $query = null,
59+
?string $aggregationFieldName = null
60+
) {
5761
$result = $dataProvider;
5862
if ($dataProvider instanceof QueryAwareInterface) {
5963
if (null === $query) {
@@ -64,7 +68,13 @@ public function create(DataProviderInterface $dataProvider, QueryContainer $quer
6468
}
6569

6670
$className = get_class($dataProvider);
67-
$result = $this->objectManager->create($className, ['queryContainer' => $query]);
71+
$result = $this->objectManager->create(
72+
$className,
73+
[
74+
'queryContainer' => $query,
75+
'aggregationFieldName' => $aggregationFieldName
76+
]
77+
);
6878
}
6979

7080
return $result;

app/code/Magento/Elasticsearch/SearchAdapter/Dynamic/DataProvider.php

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Magento\Elasticsearch\SearchAdapter\QueryAwareInterface;
99
use Magento\Elasticsearch\SearchAdapter\QueryContainer;
1010
use Magento\Framework\App\ObjectManager;
11+
use Magento\Framework\Search\Dynamic\EntityStorage;
1112
use Psr\Log\LoggerInterface;
1213

1314
/**
@@ -18,6 +19,11 @@
1819
*/
1920
class DataProvider implements \Magento\Framework\Search\Dynamic\DataProviderInterface, QueryAwareInterface
2021
{
22+
/**
23+
* Default field name used to aggregate data
24+
*/
25+
private const DEFAULT_AGGREGATION_FIELD = 'price';
26+
2127
/**
2228
* @var \Magento\Elasticsearch\SearchAdapter\ConnectionManager
2329
* @since 100.1.0
@@ -90,6 +96,11 @@ class DataProvider implements \Magento\Framework\Search\Dynamic\DataProviderInte
9096
*/
9197
private $logger;
9298

99+
/**
100+
* @var string
101+
*/
102+
private $aggregationFieldName;
103+
93104
/**
94105
* @param \Magento\Elasticsearch\SearchAdapter\ConnectionManager $connectionManager
95106
* @param \Magento\Elasticsearch\Model\Adapter\FieldMapperInterface $fieldMapper
@@ -102,6 +113,7 @@ class DataProvider implements \Magento\Framework\Search\Dynamic\DataProviderInte
102113
* @param \Magento\Framework\App\ScopeResolverInterface $scopeResolver
103114
* @param QueryContainer|null $queryContainer
104115
* @param LoggerInterface|null $logger
116+
* @param string|null $aggregationFieldName
105117
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
106118
*/
107119
public function __construct(
@@ -115,7 +127,8 @@ public function __construct(
115127
$indexerId,
116128
\Magento\Framework\App\ScopeResolverInterface $scopeResolver,
117129
QueryContainer $queryContainer = null,
118-
LoggerInterface $logger = null
130+
LoggerInterface $logger = null,
131+
?string $aggregationFieldName = null
119132
) {
120133
$this->connectionManager = $connectionManager;
121134
$this->fieldMapper = $fieldMapper;
@@ -128,6 +141,7 @@ public function __construct(
128141
$this->scopeResolver = $scopeResolver;
129142
$this->queryContainer = $queryContainer;
130143
$this->logger = $logger ?? ObjectManager::getInstance()->get(LoggerInterface::class);
144+
$this->aggregationFieldName = $aggregationFieldName ?? self::DEFAULT_AGGREGATION_FIELD;
131145
}
132146

133147
/**
@@ -143,7 +157,7 @@ public function getRange()
143157
* @inheritdoc
144158
* @since 100.1.0
145159
*/
146-
public function getAggregations(\Magento\Framework\Search\Dynamic\EntityStorage $entityStorage)
160+
public function getAggregations(EntityStorage $entityStorage)
147161
{
148162
$aggregations = [
149163
'count' => 0,
@@ -154,7 +168,7 @@ public function getAggregations(\Magento\Framework\Search\Dynamic\EntityStorage
154168

155169
$query = $this->getBasicSearchQuery($entityStorage);
156170

157-
$fieldName = $this->fieldMapper->getFieldName('price');
171+
$fieldName = $this->fieldMapper->getFieldName($this->aggregationFieldName);
158172
$query['body']['aggregations'] = [
159173
'prices' => [
160174
'extended_stats' => [
@@ -188,10 +202,10 @@ public function getAggregations(\Magento\Framework\Search\Dynamic\EntityStorage
188202
public function getInterval(
189203
\Magento\Framework\Search\Request\BucketInterface $bucket,
190204
array $dimensions,
191-
\Magento\Framework\Search\Dynamic\EntityStorage $entityStorage
205+
EntityStorage $entityStorage
192206
) {
193207
$entityIds = $entityStorage->getSource();
194-
$fieldName = $this->fieldMapper->getFieldName('price');
208+
$fieldName = $this->fieldMapper->getFieldName($this->aggregationFieldName);
195209
$dimension = current($dimensions);
196210
$storeId = $this->scopeResolver->getScope($dimension->getValue())->getId();
197211

@@ -212,7 +226,7 @@ public function getAggregation(
212226
\Magento\Framework\Search\Request\BucketInterface $bucket,
213227
array $dimensions,
214228
$range,
215-
\Magento\Framework\Search\Dynamic\EntityStorage $entityStorage
229+
EntityStorage $entityStorage
216230
) {
217231
$query = $this->getBasicSearchQuery($entityStorage);
218232

@@ -277,12 +291,12 @@ public function prepareData($range, array $dbRanges)
277291
* but for now it's a question of backward compatibility as this class may be used somewhere else
278292
* by extension developers and we can't guarantee that they'll pass a query into constructor.
279293
*
280-
* @param \Magento\Framework\Search\Dynamic\EntityStorage $entityStorage
294+
* @param EntityStorage $entityStorage
281295
* @param array $dimensions
282296
* @return array
283297
*/
284298
private function getBasicSearchQuery(
285-
\Magento\Framework\Search\Dynamic\EntityStorage $entityStorage,
299+
EntityStorage $entityStorage,
286300
array $dimensions = []
287301
) {
288302
if (null !== $this->queryContainer) {

dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchAggregationsTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,35 @@ function ($a) {
111111
$this->assertEquals($expectedOptions, $priceAggregation['options']);
112112
}
113113

114+
/**
115+
* @magentoApiDataFixture Magento/Catalog/_files/products_for_search_with_custom_price_attribute.php
116+
*/
117+
public function testAggregationCustomPriceAttribute()
118+
{
119+
$query = $this->getGraphQlQuery(
120+
'"search_product_1", "search_product_2", "search_product_3", "search_product_4" ,"search_product_5"'
121+
);
122+
$result = $this->graphQlQuery($query);
123+
124+
$this->assertArrayNotHasKey('errors', $result);
125+
$this->assertArrayHasKey('aggregations', $result['products']);
126+
127+
$priceAggregation = array_filter(
128+
$result['products']['aggregations'],
129+
function ($a) {
130+
return $a['attribute_code'] == 'product_price_attribute_bucket';
131+
}
132+
);
133+
$this->assertNotEmpty($priceAggregation);
134+
$priceAggregation = reset($priceAggregation);
135+
$this->assertEquals(2, $priceAggregation['count']);
136+
$expectedOptions = [
137+
['label' => '0_1000', 'value'=> '0_1000', 'count' => '3'],
138+
['label' => '1000_2000', 'value'=> '1000_2000', 'count' => '2']
139+
];
140+
$this->assertEquals($expectedOptions, $priceAggregation['options']);
141+
}
142+
114143
private function getGraphQlQuery(string $skus)
115144
{
116145
return <<<QUERY
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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+
use Magento\Catalog\Api\Data\ProductAttributeInterface;
9+
use Magento\Catalog\Api\ProductAttributeRepositoryInterface;
10+
use Magento\Catalog\Model\ResourceModel\Eav\AttributeFactory;
11+
use Magento\Catalog\Setup\CategorySetup;
12+
use Magento\TestFramework\Helper\Bootstrap;
13+
14+
$objectManager = Bootstrap::getObjectManager();
15+
$installer = $objectManager->create(CategorySetup::class);
16+
$attribute = $objectManager->create(AttributeFactory::class)->create();
17+
$attributeRepository = $objectManager->create(ProductAttributeRepositoryInterface::class);
18+
$entityType = $installer->getEntityTypeId(ProductAttributeInterface::ENTITY_TYPE_CODE);
19+
$attributeCode = 'product_price_attribute';
20+
if (!$attribute->loadByCode($entityType, $attributeCode)->getAttributeId()) {
21+
$attribute->setData(
22+
[
23+
'attribute_code' => $attributeCode,
24+
'entity_type_id' => $entityType,
25+
'is_global' => 0,
26+
'is_user_defined' => 1,
27+
'frontend_input' => 'price',
28+
'is_unique' => 0,
29+
'is_required' => 0,
30+
'is_searchable' => 1,
31+
'is_visible_in_advanced_search' => 0,
32+
'is_comparable' => 0,
33+
'is_filterable' => 1,
34+
'is_filterable_in_search' => 1,
35+
'is_used_for_promo_rules' => 0,
36+
'is_html_allowed_on_front' => 1,
37+
'is_visible_on_front' => 0,
38+
'used_in_product_listing' => 1,
39+
'used_for_sort_by' => 0,
40+
'frontend_label' => ['Product Price Attribute'],
41+
'backend_type' => 'decimal',
42+
]
43+
);
44+
$attribute->save();
45+
/* Assign attribute to attribute set */
46+
$installer->addAttributeToGroup(
47+
ProductAttributeInterface::ENTITY_TYPE_CODE,
48+
'Default',
49+
'General',
50+
$attribute->getId()
51+
);
52+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
/* Delete attribute with text_attribute code */
8+
$registry = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(\Magento\Framework\Registry::class);
9+
$registry->unregister('isSecureArea');
10+
$registry->register('isSecureArea', true);
11+
12+
/** @var $attribute \Magento\Catalog\Model\ResourceModel\Eav\Attribute */
13+
$attribute = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
14+
\Magento\Catalog\Model\ResourceModel\Eav\Attribute::class
15+
);
16+
$attribute->load('product_price_attribute', 'attribute_code');
17+
$attribute->delete();
18+
19+
$registry->unregister('isSecureArea');
20+
$registry->register('isSecureArea', false);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
use Magento\Catalog\Api\ProductRepositoryInterface;
8+
use Magento\TestFramework\Helper\Bootstrap;
9+
use Magento\TestFramework\Helper\CacheCleaner;
10+
use Magento\TestFramework\Workaround\Override\Fixture\Resolver;
11+
12+
Resolver::getInstance()->requireDataFixture('Magento/Catalog/_files/product_price_attribute.php');
13+
Resolver::getInstance()->requireDataFixture('Magento/Catalog/_files/products_for_search.php');
14+
15+
$objectManager = Bootstrap::getObjectManager();
16+
/** @var ProductRepositoryInterface $productRepository */
17+
$productRepository = $objectManager->get(ProductRepositoryInterface::class);
18+
$productSkus = [
19+
'search_product_1' => 55,
20+
'search_product_2' => 110,
21+
'search_product_3' => 515,
22+
'search_product_4' => 1020,
23+
'search_product_5' => 1225
24+
];
25+
foreach ($productSkus as $sku => $price) {
26+
$product = $productRepository->get($sku, true, null, true);
27+
$product->setProductPriceAttribute($price);
28+
$productRepository->save($product);
29+
}
30+
31+
CacheCleaner::cleanAll();
32+
/** @var \Magento\Indexer\Model\Indexer\Collection $indexerCollection */
33+
$indexerCollection = $objectManager->get(\Magento\Indexer\Model\Indexer\Collection::class);
34+
$indexerCollection->load();
35+
/** @var \Magento\Indexer\Model\Indexer $indexer */
36+
foreach ($indexerCollection->getItems() as $indexer) {
37+
$indexer->reindexAll();
38+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
use Magento\TestFramework\Workaround\Override\Fixture\Resolver;
8+
9+
Resolver::getInstance()->requireDataFixture('Magento/Catalog/_files/products_for_search_rollback.php');
10+
Resolver::getInstance()->requireDataFixture('Magento/Catalog/_files/product_price_attribute_rollback.php');

0 commit comments

Comments
 (0)