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 <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Ryan Sun <[email protected]> Co-authored-by: Ryan Sun <[email protected]>
mage-os · Oct 30, 2024 · 7fc2e31 · 7fc2e31
1 parent 2f768ca
commit 7fc2e31
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 14 deletions.
diff --git a/app/code/Magento/Elasticsearch/Model/DataProvider/Base/Suggestions.php b/app/code/Magento/Elasticsearch/Model/DataProvider/Base/Suggestions.php
@@ -8,7 +8,9 @@
 use Elasticsearch\Common\Exceptions\BadRequest400Exception;
 use Exception;
 use Magento\AdvancedSearch\Model\SuggestedQueriesInterface;
+use Magento\Catalog\Model\ResourceModel\Product\Attribute\CollectionFactory;
 use Magento\Elasticsearch\Model\Adapter\FieldMapper\Product\FieldProviderInterface;
+use Magento\Elasticsearch\Model\Adapter\FieldMapperInterface;
 use Magento\Elasticsearch\Model\Config;
 use Magento\Elasticsearch\SearchAdapter\ConnectionManager;
 use Magento\Elasticsearch\SearchAdapter\SearchIndexNameResolver;
@@ -61,6 +63,16 @@ class Suggestions implements SuggestedQueriesInterface
      */
     private $fieldProvider;
 
+    /**
+     * @var CollectionFactory
+     */
+    private $productAttributeCollectionFactory;
+
+    /**
+     * @var FieldMapperInterface
+     */
+    private $fieldMapper;
+
     /**
      * @var LoggerInterface
      */
@@ -90,6 +102,8 @@ class Suggestions implements SuggestedQueriesInterface
      * @param FieldProviderInterface $fieldProvider
      * @param LoggerInterface|null $logger
      * @param GetSuggestionFrequencyInterface|null $getSuggestionFrequency
+     * @param CollectionFactory|null $productAttributeCollectionFactory
+     * @param FieldMapperInterface|null $fieldMapper
      * @param array $responseErrorExceptionList
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
@@ -103,6 +117,8 @@ public function __construct(
         FieldProviderInterface $fieldProvider,
         LoggerInterface $logger = null,
         ?GetSuggestionFrequencyInterface $getSuggestionFrequency = null,
+        ?CollectionFactory $productAttributeCollectionFactory = null,
+        ?FieldMapperInterface $fieldMapper = null,
         array $responseErrorExceptionList = []
     ) {
         $this->queryResultFactory = $queryResultFactory;
@@ -111,10 +127,13 @@ public function __construct(
         $this->config = $config;
         $this->searchIndexNameResolver = $searchIndexNameResolver;
         $this->storeManager = $storeManager;
-        $this->fieldProvider = $fieldProvider;
         $this->logger = $logger ?: ObjectManager::getInstance()->get(LoggerInterface::class);
         $this->getSuggestionFrequency = $getSuggestionFrequency ?:
             ObjectManager::getInstance()->get(GetSuggestionFrequencyInterface::class);
+        $this->productAttributeCollectionFactory = $productAttributeCollectionFactory ?:
+            ObjectManager::getInstance()->get(CollectionFactory::class);
+        $this->fieldMapper = $fieldMapper ?:
+            ObjectManager::getInstance()->get(FieldMapperInterface::class);
         $this->responseErrorExceptionList = array_merge($this->responseErrorExceptionList, $responseErrorExceptionList);
     }
 
@@ -281,11 +300,20 @@ private function addSuggestFields($searchQuery, $searchSuggestionsCount)
      */
     private function getSuggestFields()
     {
-        $fields = array_filter($this->fieldProvider->getFields(), function ($field) {
-            return (($field['type'] ?? null) === 'text') && (($field['index'] ?? null) !== false);
-        });
-
-        return array_keys($fields);
+        $productAttributes = $this->productAttributeCollectionFactory->create();
+        $productAttributes->addFieldToFilter(
+            'is_searchable',
+            1
+        );
+        $attributeCodes = $productAttributes->getColumnValues('attribute_code');
+        $fields = [];
+        foreach ($attributeCodes as $attributeCode) {
+            $fields[] = $this->fieldMapper->getFieldName(
+                $attributeCode,
+                ['type' => FieldMapperInterface::TYPE_QUERY]
+            );
+        }
+        return $fields;
     }
 
     /**

diff --git a/app/code/Magento/Elasticsearch7/Test/Unit/Model/DataProvider/Base/SuggestionsTest.php b/app/code/Magento/Elasticsearch7/Test/Unit/Model/DataProvider/Base/SuggestionsTest.php
@@ -19,6 +19,8 @@
 use Magento\Search\Model\QueryInterface;
 use Magento\Search\Model\QueryResult;
 use Magento\Search\Model\QueryResultFactory;
+use Magento\Catalog\Model\ResourceModel\Product\Attribute\CollectionFactory;
+use Magento\Catalog\Model\ResourceModel\Product\Attribute\Collection as ProductAttributeCollection;
 use Magento\Store\Api\Data\StoreInterface;
 use Magento\Store\Model\StoreManagerInterface as StoreManager;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -85,6 +87,11 @@ class SuggestionsTest extends TestCase
      */
     private $query;
 
+    /**
+     * @var CollectionFactory|MockObject
+     */
+    private $productAttributeCollectionFactory;
+
     /**
      * Set up test environment
      *
@@ -137,6 +144,11 @@ protected function setUp(): void
             ->disableOriginalConstructor()
             ->getMockForAbstractClass();
 
+        $this->productAttributeCollectionFactory = $this->getMockBuilder(CollectionFactory::class)
+            ->disableOriginalConstructor()
+            ->onlyMethods(['create'])
+            ->getMock();
+
         $objectManager = new ObjectManagerHelper($this);
 
         $this->model = $objectManager->getObject(
@@ -150,6 +162,7 @@ protected function setUp(): void
                 'storeManager' => $this->storeManager,
                 'fieldProvider' => $this->fieldProvider,
                 'logger' => $this->logger,
+                'productAttributeCollectionFactory' => $this->productAttributeCollectionFactory,
             ]
         );
     }
@@ -177,6 +190,9 @@ public function testGetItemsWithDisabledSearchSuggestion(): void
         $this->queryResultFactory->expects($this->never())
             ->method('create');
 
+        $this->productAttributeCollectionFactory->expects($this->never())
+            ->method('create');
+
         $this->assertEmpty($this->model->getItems($this->query));
     }
 
@@ -216,6 +232,21 @@ public function testGetItemsWithEnabledSearchSuggestion(): void
             ->method('create')
             ->willReturn($query);
 
+        $objectManager = new ObjectManagerHelper($this);
+        $productAttributeCollection = $objectManager->getCollectionMock(ProductAttributeCollection::class, []);
+
+        $productAttributeCollection->expects($this->once())
+            ->method('addFieldToFilter')
+            ->willReturnSelf();
+
+        $productAttributeCollection->expects($this->once())
+            ->method('getColumnValues')
+            ->willReturn([]);
+
+        $this->productAttributeCollectionFactory->expects($this->once())
+            ->method('create')
+            ->willReturn($productAttributeCollection);
+
         $this->assertEquals([$query], $this->model->getItems($this->query));
     }
 
@@ -243,6 +274,21 @@ public function testGetItemsException(): void
         $this->queryResultFactory->expects($this->never())
             ->method('create');
 
+        $objectManager = new ObjectManagerHelper($this);
+        $productAttributeCollection = $objectManager->getCollectionMock(ProductAttributeCollection::class, []);
+
+        $productAttributeCollection->expects($this->once())
+            ->method('addFieldToFilter')
+            ->willReturnSelf();
+
+        $productAttributeCollection->expects($this->once())
+            ->method('getColumnValues')
+            ->willReturn([]);
+
+        $this->productAttributeCollectionFactory->expects($this->once())
+            ->method('create')
+            ->willReturn($productAttributeCollection);
+
         $this->assertEmpty($this->model->getItems($this->query));
     }
 
@@ -287,10 +333,6 @@ private function prepareSearchQuery(): void
             ->method('getQueryText')
             ->willReturn('query');
 
-        $this->fieldProvider->expects($this->once())
-            ->method('getFields')
-            ->willReturn([]);
-
         $this->connectionManager->expects($this->once())
             ->method('getConnection')
             ->willReturn($this->client);

diff --git a/app/code/Magento/OpenSearch/Test/Unit/Model/DataProvider/Base/SuggestionsTest.php b/app/code/Magento/OpenSearch/Test/Unit/Model/DataProvider/Base/SuggestionsTest.php
@@ -17,6 +17,8 @@
 use Magento\OpenSearch\Model\SearchClient;
 use Magento\Search\Model\QueryInterface;
 use Magento\Search\Model\QueryResultFactory;
+use Magento\Catalog\Model\ResourceModel\Product\Attribute\CollectionFactory;
+use Magento\Catalog\Model\ResourceModel\Product\Attribute\Collection as ProductAttributeCollection;
 use Magento\Store\Api\Data\StoreInterface;
 use Magento\Store\Model\StoreManagerInterface as StoreManager;
 use OpenSearch\Common\Exceptions\BadRequest400Exception;
@@ -84,6 +86,11 @@ class SuggestionsTest extends TestCase
      */
     private $query;
 
+    /**
+     * @var CollectionFactory|MockObject
+     */
+    private $productAttributeCollectionFactory;
+
     /**
      * Set up test environment
      *
@@ -136,6 +143,11 @@ protected function setUp(): void
             ->disableOriginalConstructor()
             ->getMockForAbstractClass();
 
+        $this->productAttributeCollectionFactory = $this->getMockBuilder(CollectionFactory::class)
+            ->disableOriginalConstructor()
+            ->onlyMethods(['create'])
+            ->getMock();
+
         $objectManager = new ObjectManagerHelper($this);
 
         $this->model = $objectManager->getObject(
@@ -149,6 +161,7 @@ protected function setUp(): void
                 'storeManager' => $this->storeManager,
                 'fieldProvider' => $this->fieldProvider,
                 'logger' => $this->logger,
+                'productAttributeCollectionFactory' => $this->productAttributeCollectionFactory,
                 'responseErrorExceptionList' => ['opensearchBadRequest400' => BadRequest400Exception::class]
             ]
         );
@@ -175,6 +188,21 @@ public function testGetItemsException(): void
         $this->queryResultFactory->expects($this->never())
             ->method('create');
 
+        $objectManager = new ObjectManagerHelper($this);
+        $productAttributeCollection = $objectManager->getCollectionMock(ProductAttributeCollection::class, []);
+
+        $productAttributeCollection->expects($this->once())
+            ->method('addFieldToFilter')
+            ->willReturnSelf();
+
+        $productAttributeCollection->expects($this->once())
+            ->method('getColumnValues')
+            ->willReturn([]);
+
+        $this->productAttributeCollectionFactory->expects($this->once())
+            ->method('create')
+            ->willReturn($productAttributeCollection);
+
         $this->assertEmpty($this->model->getItems($this->query));
     }
 
@@ -219,10 +247,6 @@ private function prepareSearchQuery(): void
             ->method('getQueryText')
             ->willReturn('query');
 
-        $this->fieldProvider->expects($this->once())
-            ->method('getFields')
-            ->willReturn([]);
-
         $this->connectionManager->expects($this->once())
             ->method('getConnection')
             ->willReturn($this->client);