Merge pull request #5807 from magento-mpi/MC-31375

[MPI]MC-31375: Catalog graphQl - position sort doesn't work properly
magento · Jun 16, 2020 · dd63719 · dd63719
2 parents 8159f43 + 20a49e3
commit dd63719
Show file tree

Hide file tree

Showing 7 changed files with 211 additions and 39 deletions.
diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Product/Collection.php b/app/code/Magento/Catalog/Model/ResourceModel/Product/Collection.php
@@ -1718,7 +1718,10 @@ public function addAttributeToSort($attribute, $dir = self::SORT_ORDER_ASC)
             // optimize if using cat index
             $filters = $this->_productLimitationFilters;
             if (isset($filters['category_id']) || isset($filters['visibility'])) {
-                $this->getSelect()->order('cat_index.position ' . $dir);
+                $this->getSelect()->order([
+                    'cat_index.position ' . $dir,
+                    'e.entity_id ' . \Magento\Framework\DB\Select::SQL_DESC
+                ]);
             } else {
                 $this->getSelect()->order('e.entity_id ' . $dir);
             }

diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php
@@ -7,16 +7,18 @@
 
 namespace Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider;
 
+use Magento\Catalog\Api\Data\ProductSearchResultsInterfaceFactory;
+use Magento\Catalog\Model\ResourceModel\Product\Collection;
+use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory;
 use Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider\Product\CollectionPostProcessor;
+use Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider\Product\CollectionProcessorInterface;
+use Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider\ProductSearch\ProductCollectionSearchCriteriaBuilder;
 use Magento\CatalogSearch\Model\ResourceModel\Fulltext\Collection\SearchResultApplierFactory;
 use Magento\CatalogSearch\Model\ResourceModel\Fulltext\Collection\SearchResultApplierInterface;
 use Magento\Framework\Api\Search\SearchResultInterface;
 use Magento\Framework\Api\SearchCriteriaInterface;
-use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory;
-use Magento\Catalog\Model\ResourceModel\Product\Collection;
-use Magento\Catalog\Api\Data\ProductSearchResultsInterfaceFactory;
 use Magento\Framework\Api\SearchResultsInterface;
-use Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider\Product\CollectionProcessorInterface;
+use Magento\Framework\App\ObjectManager;
 
 /**
  * Product field data provider for product search, used for GraphQL resolver processing.
@@ -48,25 +50,34 @@ class ProductSearch
      */
     private $searchResultApplierFactory;
 
+    /**
+     * @var ProductCollectionSearchCriteriaBuilder
+     */
+    private $searchCriteriaBuilder;
+
     /**
      * @param CollectionFactory $collectionFactory
      * @param ProductSearchResultsInterfaceFactory $searchResultsFactory
      * @param CollectionProcessorInterface $collectionPreProcessor
      * @param CollectionPostProcessor $collectionPostProcessor
      * @param SearchResultApplierFactory $searchResultsApplierFactory
+     * @param ProductCollectionSearchCriteriaBuilder|null $searchCriteriaBuilder
      */
     public function __construct(
         CollectionFactory $collectionFactory,
         ProductSearchResultsInterfaceFactory $searchResultsFactory,
         CollectionProcessorInterface $collectionPreProcessor,
         CollectionPostProcessor $collectionPostProcessor,
-        SearchResultApplierFactory $searchResultsApplierFactory
+        SearchResultApplierFactory $searchResultsApplierFactory,
+        ?ProductCollectionSearchCriteriaBuilder $searchCriteriaBuilder = null
     ) {
         $this->collectionFactory = $collectionFactory;
         $this->searchResultsFactory = $searchResultsFactory;
         $this->collectionPreProcessor = $collectionPreProcessor;
         $this->collectionPostProcessor = $collectionPostProcessor;
         $this->searchResultApplierFactory = $searchResultsApplierFactory;
+        $this->searchCriteriaBuilder = $searchCriteriaBuilder
+            ?? ObjectManager::getInstance()->get(ProductCollectionSearchCriteriaBuilder::class);
     }
 
     /**
@@ -85,15 +96,21 @@ public function getList(
         /** @var Collection $collection */
         $collection = $this->collectionFactory->create();
 
-        //Join search results
-        $this->getSearchResultsApplier($searchResult, $collection, $this->getSortOrderArray($searchCriteria))->apply();
+        //Create a copy of search criteria without filters to preserve the results from search
+        $searchCriteriaForCollection = $this->searchCriteriaBuilder->build($searchCriteria);
+        //Apply CatalogSearch results from search and join table
+        $this->getSearchResultsApplier(
+            $searchResult,
+            $collection,
+            $this->getSortOrderArray($searchCriteriaForCollection)
+        )->apply();
 
-        $this->collectionPreProcessor->process($collection, $searchCriteria, $attributes);
+        $this->collectionPreProcessor->process($collection, $searchCriteriaForCollection, $attributes);
         $collection->load();
         $this->collectionPostProcessor->process($collection, $attributes);
 
         $searchResults = $this->searchResultsFactory->create();
-        $searchResults->setSearchCriteria($searchCriteria);
+        $searchResults->setSearchCriteria($searchCriteriaForCollection);
         $searchResults->setItems($collection->getItems());
         $searchResults->setTotalCount($searchResult->getTotalCount());
         return $searchResults;

diff --git a/...l/Resolver/Products/DataProvider/ProductSearch/ProductCollectionSearchCriteriaBuilder.php b/...l/Resolver/Products/DataProvider/ProductSearch/ProductCollectionSearchCriteriaBuilder.php
@@ -0,0 +1,77 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider\ProductSearch;
+
+use Magento\Catalog\Model\CategoryProductLink;
+use Magento\Framework\Api\FilterBuilder;
+use Magento\Framework\Api\Search\FilterGroupBuilder;
+use Magento\Framework\Api\Search\SearchCriteriaInterfaceFactory;
+use Magento\Framework\Api\SearchCriteriaInterface;
+
+/**
+ * Builds a search criteria intended for the product collection based on search criteria used on SearchAPI
+ */
+class ProductCollectionSearchCriteriaBuilder
+{
+    /** @var SearchCriteriaInterfaceFactory */
+    private $searchCriteriaFactory;
+
+    /** @var FilterBuilder */
+    private $filterBuilder;
+
+    /** @var FilterGroupBuilder */
+    private $filterGroupBuilder;
+
+    /**
+     * @param SearchCriteriaInterfaceFactory $searchCriteriaFactory
+     * @param FilterBuilder $filterBuilder
+     * @param FilterGroupBuilder $filterGroupBuilder
+     */
+    public function __construct(
+        SearchCriteriaInterfaceFactory $searchCriteriaFactory,
+        FilterBuilder $filterBuilder,
+        FilterGroupBuilder $filterGroupBuilder
+    ) {
+        $this->searchCriteriaFactory = $searchCriteriaFactory;
+        $this->filterBuilder = $filterBuilder;
+        $this->filterGroupBuilder = $filterGroupBuilder;
+    }
+
+    /**
+     * Build searchCriteria from search for product collection
+     *
+     * @param SearchCriteriaInterface $searchCriteria
+     * @return SearchCriteriaInterface
+     */
+    public function build(SearchCriteriaInterface $searchCriteria): SearchCriteriaInterface
+    {
+        //Create a copy of search criteria without filters to preserve the results from search
+        $searchCriteriaForCollection = $this->searchCriteriaFactory->create()
+            ->setSortOrders($searchCriteria->getSortOrders())
+            ->setPageSize($searchCriteria->getPageSize())
+            ->setCurrentPage($searchCriteria->getCurrentPage());
+
+        //Add category id to enable sorting by position
+        foreach ($searchCriteria->getFilterGroups() as $filterGroup) {
+            foreach ($filterGroup->getFilters() as $filter) {
+                if ($filter->getField() == CategoryProductLink::KEY_CATEGORY_ID) {
+                    $categoryFilter = $this->filterBuilder
+                        ->setField($filter->getField())
+                        ->setValue($filter->getValue())
+                        ->setConditionType($filter->getConditionType())
+                        ->create();
+
+                    $this->filterGroupBuilder->addFilter($categoryFilter);
+                    $categoryGroup = $this->filterGroupBuilder->create();
+                    $searchCriteriaForCollection->setFilterGroups([$categoryGroup]);
+                }
+            }
+        }
+        return $searchCriteriaForCollection;
+    }
+}
diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php
@@ -9,10 +9,10 @@
 
 use Magento\CatalogGraphQl\DataProvider\Product\SearchCriteriaBuilder;
 use Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider\ProductSearch;
-use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
-use Magento\Framework\Api\Search\SearchCriteriaInterface;
 use Magento\CatalogGraphQl\Model\Resolver\Products\SearchResult;
 use Magento\CatalogGraphQl\Model\Resolver\Products\SearchResultFactory;
+use Magento\Framework\Api\Search\SearchCriteriaInterface;
+use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
 use Magento\Search\Api\SearchInterface;
 use Magento\Framework\Api\Search\SearchCriteriaInterfaceFactory;
 use Magento\Search\Model\Search\PageSizeProvider;
@@ -101,28 +101,18 @@ public function getResult(
 
         $realPageSize = $searchCriteria->getPageSize();
         $realCurrentPage = $searchCriteria->getCurrentPage();
-        // Current page must be set to 0 and page size to max for search to grab all ID's as temporary workaround
+        //Because of limitations of sort and pagination on search API we will query all IDS
         $pageSize = $this->pageSizeProvider->getMaxPageSize();
         $searchCriteria->setPageSize($pageSize);
         $searchCriteria->setCurrentPage(0);
         $itemsResults = $this->search->search($searchCriteria);
 
-        //Create copy of search criteria without conditions (conditions will be applied by joining search result)
-        $searchCriteriaCopy = $this->searchCriteriaFactory->create()
-            ->setSortOrders($searchCriteria->getSortOrders())
-            ->setPageSize($realPageSize)
-            ->setCurrentPage($realCurrentPage);
-
-        $searchResults = $this->productsProvider->getList($searchCriteriaCopy, $itemsResults, $queryFields);
-
-        //possible division by 0
-        if ($realPageSize) {
-            $maxPages = (int)ceil($searchResults->getTotalCount() / $realPageSize);
-        } else {
-            $maxPages = 0;
-        }
+        //Address limitations of sort and pagination on search API apply original pagination from GQL query
         $searchCriteria->setPageSize($realPageSize);
         $searchCriteria->setCurrentPage($realCurrentPage);
+        $searchResults = $this->productsProvider->getList($searchCriteria, $itemsResults, $queryFields);
+
+        $totalPages = $realPageSize ? ((int)ceil($searchResults->getTotalCount() / $realPageSize)) : 0;
 
         $productArray = [];
         /** @var \Magento\Catalog\Model\Product $product */
@@ -138,7 +128,7 @@ public function getResult(
                 'searchAggregation' => $itemsResults->getAggregations(),
                 'pageSize' => $realPageSize,
                 'currentPage' => $realCurrentPage,
-                'totalPages' => $maxPages,
+                'totalPages' => $totalPages,
             ]
         );
     }

diff --git a/...l/Resolver/Products/SearchCriteria/CollectionProcessor/FilterProcessor/CategoryFilter.php b/...l/Resolver/Products/SearchCriteria/CollectionProcessor/FilterProcessor/CategoryFilter.php
@@ -61,6 +61,7 @@ public function apply(Filter $filter, AbstractDb $collection)
             $category = $this->categoryFactory->create();
             $this->categoryResourceModel->load($category, $categoryId);
             $categoryProducts[$categoryId] = $category->getProductCollection()->getAllIds();
+            $collection->addCategoryFilter($category);
         }
 
         $categoryProductIds = array_unique(array_merge(...$categoryProducts));

diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php
@@ -186,8 +186,8 @@ public function testQueryChildCategoriesWithProducts()
         //Check base category products
         $expectedBaseCategoryProducts = [
             ['sku' => 'simple', 'name' => 'Simple Product'],
-            ['sku' => '12345', 'name' => 'Simple Product Two'],
-            ['sku' => 'simple-4', 'name' => 'Simple Product Three']
+            ['sku' => 'simple-4', 'name' => 'Simple Product Three'],
+            ['sku' => '12345', 'name' => 'Simple Product Two']
         ];
         $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts);
         //Check base category children
@@ -202,8 +202,8 @@ public function testQueryChildCategoriesWithProducts()
         $this->assertEquals('Category 1.1', $firstChildCategory['name']);
         $this->assertEquals('Category 1.1 description.', $firstChildCategory['description']);
         $firstChildCategoryExpectedProducts = [
-            ['sku' => 'simple', 'name' => 'Simple Product'],
             ['sku' => '12345', 'name' => 'Simple Product Two'],
+            ['sku' => 'simple', 'name' => 'Simple Product'],
         ];
         $this->assertCategoryProducts($firstChildCategory, $firstChildCategoryExpectedProducts);
         $firstChildCategoryChildren = [['name' =>'Category 1.1.1']];
@@ -213,8 +213,8 @@ public function testQueryChildCategoriesWithProducts()
         $this->assertEquals('Category 1.2', $secondChildCategory['name']);
         $this->assertEquals('Its a description of Test Category 1.2', $secondChildCategory['description']);
         $firstChildCategoryExpectedProducts = [
+            ['sku' => 'simple-4', 'name' => 'Simple Product Three'],
             ['sku' => 'simple', 'name' => 'Simple Product'],
-            ['sku' => 'simple-4', 'name' => 'Simple Product Three']
         ];
         $this->assertCategoryProducts($secondChildCategory, $firstChildCategoryExpectedProducts);
         $firstChildCategoryChildren = [];
@@ -277,8 +277,8 @@ public function testQueryCategoryWithDisabledChildren()
         //Check base category products
         $expectedBaseCategoryProducts = [
             ['sku' => 'simple', 'name' => 'Simple Product'],
-            ['sku' => '12345', 'name' => 'Simple Product Two'],
-            ['sku' => 'simple-4', 'name' => 'Simple Product Three']
+            ['sku' => 'simple-4', 'name' => 'Simple Product Three'],
+            ['sku' => '12345', 'name' => 'Simple Product Two']
         ];
         $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts);
         //Check base category children
@@ -293,8 +293,8 @@ public function testQueryCategoryWithDisabledChildren()
         $this->assertEquals('Its a description of Test Category 1.2', $firstChildCategory['description']);
 
         $firstChildCategoryExpectedProducts = [
-            ['sku' => 'simple', 'name' => 'Simple Product'],
-            ['sku' => 'simple-4', 'name' => 'Simple Product Three']
+            ['sku' => 'simple-4', 'name' => 'Simple Product Three'],
+            ['sku' => 'simple', 'name' => 'Simple Product']
         ];
         $this->assertCategoryProducts($firstChildCategory, $firstChildCategoryExpectedProducts);
         $firstChildCategoryChildren = [];