Skip to content

Commit 7fc2e31

Browse files
rhoerrryansunxlryanisn
authored
Fix search suggestions (#110)
* Bugfix: autocomplete suggests nonsearchable terms (#102) * added additional check to ensure autocomplete suggests only searchable terms --------- Co-authored-by: Ryan Sun <ryansun@isnweb.com> * Update unit test for autocomplete suggests fix (#104) * added additional check to ensure autocomplete suggests only searchable terms * updated unitest for search suggest fix * added type casting for isSuggestible * reverted suggestible flag and fixed getSuggestFields method * updated suggest contructor params to avoid bic * updated unit test with new dependency productAttributeCollectionFactory for Suggestions * add fieldProvider back to Suggestions constructor --------- Co-authored-by: Ryan Sun <ryansun@isnweb.com> --------- Co-authored-by: Ryan Sun <ryansun81@gmail.com> Co-authored-by: Ryan Sun <ryansun@isnweb.com>
1 parent 2f768ca commit 7fc2e31

File tree

3 files changed

+108
-14
lines changed

3 files changed

+108
-14
lines changed

app/code/Magento/Elasticsearch/Model/DataProvider/Base/Suggestions.php

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
use Elasticsearch\Common\Exceptions\BadRequest400Exception;
99
use Exception;
1010
use Magento\AdvancedSearch\Model\SuggestedQueriesInterface;
11+
use Magento\Catalog\Model\ResourceModel\Product\Attribute\CollectionFactory;
1112
use Magento\Elasticsearch\Model\Adapter\FieldMapper\Product\FieldProviderInterface;
13+
use Magento\Elasticsearch\Model\Adapter\FieldMapperInterface;
1214
use Magento\Elasticsearch\Model\Config;
1315
use Magento\Elasticsearch\SearchAdapter\ConnectionManager;
1416
use Magento\Elasticsearch\SearchAdapter\SearchIndexNameResolver;
@@ -61,6 +63,16 @@ class Suggestions implements SuggestedQueriesInterface
6163
*/
6264
private $fieldProvider;
6365

66+
/**
67+
* @var CollectionFactory
68+
*/
69+
private $productAttributeCollectionFactory;
70+
71+
/**
72+
* @var FieldMapperInterface
73+
*/
74+
private $fieldMapper;
75+
6476
/**
6577
* @var LoggerInterface
6678
*/
@@ -90,6 +102,8 @@ class Suggestions implements SuggestedQueriesInterface
90102
* @param FieldProviderInterface $fieldProvider
91103
* @param LoggerInterface|null $logger
92104
* @param GetSuggestionFrequencyInterface|null $getSuggestionFrequency
105+
* @param CollectionFactory|null $productAttributeCollectionFactory
106+
* @param FieldMapperInterface|null $fieldMapper
93107
* @param array $responseErrorExceptionList
94108
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
95109
*/
@@ -103,6 +117,8 @@ public function __construct(
103117
FieldProviderInterface $fieldProvider,
104118
LoggerInterface $logger = null,
105119
?GetSuggestionFrequencyInterface $getSuggestionFrequency = null,
120+
?CollectionFactory $productAttributeCollectionFactory = null,
121+
?FieldMapperInterface $fieldMapper = null,
106122
array $responseErrorExceptionList = []
107123
) {
108124
$this->queryResultFactory = $queryResultFactory;
@@ -111,10 +127,13 @@ public function __construct(
111127
$this->config = $config;
112128
$this->searchIndexNameResolver = $searchIndexNameResolver;
113129
$this->storeManager = $storeManager;
114-
$this->fieldProvider = $fieldProvider;
115130
$this->logger = $logger ?: ObjectManager::getInstance()->get(LoggerInterface::class);
116131
$this->getSuggestionFrequency = $getSuggestionFrequency ?:
117132
ObjectManager::getInstance()->get(GetSuggestionFrequencyInterface::class);
133+
$this->productAttributeCollectionFactory = $productAttributeCollectionFactory ?:
134+
ObjectManager::getInstance()->get(CollectionFactory::class);
135+
$this->fieldMapper = $fieldMapper ?:
136+
ObjectManager::getInstance()->get(FieldMapperInterface::class);
118137
$this->responseErrorExceptionList = array_merge($this->responseErrorExceptionList, $responseErrorExceptionList);
119138
}
120139

@@ -281,11 +300,20 @@ private function addSuggestFields($searchQuery, $searchSuggestionsCount)
281300
*/
282301
private function getSuggestFields()
283302
{
284-
$fields = array_filter($this->fieldProvider->getFields(), function ($field) {
285-
return (($field['type'] ?? null) === 'text') && (($field['index'] ?? null) !== false);
286-
});
287-
288-
return array_keys($fields);
303+
$productAttributes = $this->productAttributeCollectionFactory->create();
304+
$productAttributes->addFieldToFilter(
305+
'is_searchable',
306+
1
307+
);
308+
$attributeCodes = $productAttributes->getColumnValues('attribute_code');
309+
$fields = [];
310+
foreach ($attributeCodes as $attributeCode) {
311+
$fields[] = $this->fieldMapper->getFieldName(
312+
$attributeCode,
313+
['type' => FieldMapperInterface::TYPE_QUERY]
314+
);
315+
}
316+
return $fields;
289317
}
290318

291319
/**

app/code/Magento/Elasticsearch7/Test/Unit/Model/DataProvider/Base/SuggestionsTest.php

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
use Magento\Search\Model\QueryInterface;
2020
use Magento\Search\Model\QueryResult;
2121
use Magento\Search\Model\QueryResultFactory;
22+
use Magento\Catalog\Model\ResourceModel\Product\Attribute\CollectionFactory;
23+
use Magento\Catalog\Model\ResourceModel\Product\Attribute\Collection as ProductAttributeCollection;
2224
use Magento\Store\Api\Data\StoreInterface;
2325
use Magento\Store\Model\StoreManagerInterface as StoreManager;
2426
use PHPUnit\Framework\MockObject\MockObject;
@@ -85,6 +87,11 @@ class SuggestionsTest extends TestCase
8587
*/
8688
private $query;
8789

90+
/**
91+
* @var CollectionFactory|MockObject
92+
*/
93+
private $productAttributeCollectionFactory;
94+
8895
/**
8996
* Set up test environment
9097
*
@@ -137,6 +144,11 @@ protected function setUp(): void
137144
->disableOriginalConstructor()
138145
->getMockForAbstractClass();
139146

147+
$this->productAttributeCollectionFactory = $this->getMockBuilder(CollectionFactory::class)
148+
->disableOriginalConstructor()
149+
->onlyMethods(['create'])
150+
->getMock();
151+
140152
$objectManager = new ObjectManagerHelper($this);
141153

142154
$this->model = $objectManager->getObject(
@@ -150,6 +162,7 @@ protected function setUp(): void
150162
'storeManager' => $this->storeManager,
151163
'fieldProvider' => $this->fieldProvider,
152164
'logger' => $this->logger,
165+
'productAttributeCollectionFactory' => $this->productAttributeCollectionFactory,
153166
]
154167
);
155168
}
@@ -177,6 +190,9 @@ public function testGetItemsWithDisabledSearchSuggestion(): void
177190
$this->queryResultFactory->expects($this->never())
178191
->method('create');
179192

193+
$this->productAttributeCollectionFactory->expects($this->never())
194+
->method('create');
195+
180196
$this->assertEmpty($this->model->getItems($this->query));
181197
}
182198

@@ -216,6 +232,21 @@ public function testGetItemsWithEnabledSearchSuggestion(): void
216232
->method('create')
217233
->willReturn($query);
218234

235+
$objectManager = new ObjectManagerHelper($this);
236+
$productAttributeCollection = $objectManager->getCollectionMock(ProductAttributeCollection::class, []);
237+
238+
$productAttributeCollection->expects($this->once())
239+
->method('addFieldToFilter')
240+
->willReturnSelf();
241+
242+
$productAttributeCollection->expects($this->once())
243+
->method('getColumnValues')
244+
->willReturn([]);
245+
246+
$this->productAttributeCollectionFactory->expects($this->once())
247+
->method('create')
248+
->willReturn($productAttributeCollection);
249+
219250
$this->assertEquals([$query], $this->model->getItems($this->query));
220251
}
221252

@@ -243,6 +274,21 @@ public function testGetItemsException(): void
243274
$this->queryResultFactory->expects($this->never())
244275
->method('create');
245276

277+
$objectManager = new ObjectManagerHelper($this);
278+
$productAttributeCollection = $objectManager->getCollectionMock(ProductAttributeCollection::class, []);
279+
280+
$productAttributeCollection->expects($this->once())
281+
->method('addFieldToFilter')
282+
->willReturnSelf();
283+
284+
$productAttributeCollection->expects($this->once())
285+
->method('getColumnValues')
286+
->willReturn([]);
287+
288+
$this->productAttributeCollectionFactory->expects($this->once())
289+
->method('create')
290+
->willReturn($productAttributeCollection);
291+
246292
$this->assertEmpty($this->model->getItems($this->query));
247293
}
248294

@@ -287,10 +333,6 @@ private function prepareSearchQuery(): void
287333
->method('getQueryText')
288334
->willReturn('query');
289335

290-
$this->fieldProvider->expects($this->once())
291-
->method('getFields')
292-
->willReturn([]);
293-
294336
$this->connectionManager->expects($this->once())
295337
->method('getConnection')
296338
->willReturn($this->client);

app/code/Magento/OpenSearch/Test/Unit/Model/DataProvider/Base/SuggestionsTest.php

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
use Magento\OpenSearch\Model\SearchClient;
1818
use Magento\Search\Model\QueryInterface;
1919
use Magento\Search\Model\QueryResultFactory;
20+
use Magento\Catalog\Model\ResourceModel\Product\Attribute\CollectionFactory;
21+
use Magento\Catalog\Model\ResourceModel\Product\Attribute\Collection as ProductAttributeCollection;
2022
use Magento\Store\Api\Data\StoreInterface;
2123
use Magento\Store\Model\StoreManagerInterface as StoreManager;
2224
use OpenSearch\Common\Exceptions\BadRequest400Exception;
@@ -84,6 +86,11 @@ class SuggestionsTest extends TestCase
8486
*/
8587
private $query;
8688

89+
/**
90+
* @var CollectionFactory|MockObject
91+
*/
92+
private $productAttributeCollectionFactory;
93+
8794
/**
8895
* Set up test environment
8996
*
@@ -136,6 +143,11 @@ protected function setUp(): void
136143
->disableOriginalConstructor()
137144
->getMockForAbstractClass();
138145

146+
$this->productAttributeCollectionFactory = $this->getMockBuilder(CollectionFactory::class)
147+
->disableOriginalConstructor()
148+
->onlyMethods(['create'])
149+
->getMock();
150+
139151
$objectManager = new ObjectManagerHelper($this);
140152

141153
$this->model = $objectManager->getObject(
@@ -149,6 +161,7 @@ protected function setUp(): void
149161
'storeManager' => $this->storeManager,
150162
'fieldProvider' => $this->fieldProvider,
151163
'logger' => $this->logger,
164+
'productAttributeCollectionFactory' => $this->productAttributeCollectionFactory,
152165
'responseErrorExceptionList' => ['opensearchBadRequest400' => BadRequest400Exception::class]
153166
]
154167
);
@@ -175,6 +188,21 @@ public function testGetItemsException(): void
175188
$this->queryResultFactory->expects($this->never())
176189
->method('create');
177190

191+
$objectManager = new ObjectManagerHelper($this);
192+
$productAttributeCollection = $objectManager->getCollectionMock(ProductAttributeCollection::class, []);
193+
194+
$productAttributeCollection->expects($this->once())
195+
->method('addFieldToFilter')
196+
->willReturnSelf();
197+
198+
$productAttributeCollection->expects($this->once())
199+
->method('getColumnValues')
200+
->willReturn([]);
201+
202+
$this->productAttributeCollectionFactory->expects($this->once())
203+
->method('create')
204+
->willReturn($productAttributeCollection);
205+
178206
$this->assertEmpty($this->model->getItems($this->query));
179207
}
180208

@@ -219,10 +247,6 @@ private function prepareSearchQuery(): void
219247
->method('getQueryText')
220248
->willReturn('query');
221249

222-
$this->fieldProvider->expects($this->once())
223-
->method('getFields')
224-
->willReturn([]);
225-
226250
$this->connectionManager->expects($this->once())
227251
->method('getConnection')
228252
->willReturn($this->client);

0 commit comments

Comments
 (0)