Skip to content
This repository was archived by the owner on Dec 19, 2019. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions app/code/Magento/CatalogGraphQl/Model/Resolver/Products.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,27 +53,42 @@ class Products implements ResolverInterface
* @var Layer\DataProvider\Filters
*/
private $filtersDataProvider;

/**
* @var \Magento\Catalog\Model\Config
*/
private $catalogConfig;

/**
* @var \Magento\Store\Model\StoreManagerInterface
*/
private $storeManager;

/**
* @param Builder $searchCriteriaBuilder
* @param Search $searchQuery
* @param Filter $filterQuery
* @param ValueFactory $valueFactory
* @param \Magento\Catalog\Model\Config $catalogConfig
*/
public function __construct(
Builder $searchCriteriaBuilder,
Search $searchQuery,
Filter $filterQuery,
SearchFilter $searchFilter,
ValueFactory $valueFactory,
\Magento\CatalogGraphQl\Model\Resolver\Layer\DataProvider\Filters $filtersDataProvider
\Magento\CatalogGraphQl\Model\Resolver\Layer\DataProvider\Filters $filtersDataProvider,
\Magento\Catalog\Model\Config $catalogConfig,
\Magento\Store\Model\StoreManagerInterface $storeManager

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add constructor doc block type hint for StoreManagerInterface

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!

) {
$this->searchCriteriaBuilder = $searchCriteriaBuilder;
$this->searchQuery = $searchQuery;
$this->filterQuery = $filterQuery;
$this->searchFilter = $searchFilter;
$this->valueFactory = $valueFactory;
$this->filtersDataProvider = $filtersDataProvider;
$this->catalogConfig = $catalogConfig;
$this->storeManager = $storeManager;
}

/**
Expand Down Expand Up @@ -118,14 +133,28 @@ public function resolve(
);
}

$options = $this->catalogConfig->getAttributeUsedForSortByArray();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a separate resolver for sort_fields. There are 2 main reasons for that:

  1. Performance. In case sort_fields is not requested, all this logic will not be executed in case of separate resolver
  2. Following single responsibility and object decomposition simplifies maintenance and understanding

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roger, i will fix it, thanks for your feedback


$sortFields = [
'default' => $this->catalogConfig->getProductListDefaultSortBy($this->storeManager->getStore()->getId()),
'options' => []
];

$sortFields['options'][] = ['key' => 'position', 'label' => 'Position'];
foreach ($this->catalogConfig->getAttributesUsedForSortBy() as $attribute) {
$sortFields['options'][] = ['key' => $attribute->getAttributeCode(), 'label' => $attribute->getStoreLabel()];
}

$data = [
'total_count' => $searchResult->getTotalCount(),
'items' => $searchResult->getProductsSearchResult(),
'page_info' => [
'page_size' => $searchCriteria->getPageSize(),
'current_page' => $currentPage
'current_page' => $currentPage,
'sort_fields' => $sortFields,
],
'filters' => $this->filtersDataProvider->getData($layerType)
'filters' => $this->filtersDataProvider->getData($layerType),

];

$result = function () use ($data) {
Expand Down
11 changes: 11 additions & 0 deletions app/code/Magento/GraphQl/etc/schema.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,20 @@ input FilterTypeInput @doc(description: "FilterTypeInput specifies which action
type SearchResultPageInfo @doc(description: "SearchResultPageInfo provides navigation for the query response") {
page_size: Int @doc(description: "Specifies the maximum number of items to return")
current_page: Int @doc(description: "Specifies which page of results to return")
sort_fields: SortFields @doc(description: "An object that includes the default sort field and all available sort fields")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a single new resolver, just for sort_fields.

}

enum SortEnum @doc(description: "This enumeration indicates whether to return results in ascending or descending order") {
ASC
DESC
}

type SortField {
key: String @doc(description: "Attribute code of sort field")
label: String @doc(description: "Label of sort field")
}

type SortFields @doc(description: "SortFields contains a default value for sort fields and all available sort fields") {
default: String @doc(description: "Default value of sort fields")
options: [SortField] @doc(description: "Available sort fields")
}
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,15 @@ public function testQueryProductsInCurrentPageSortedByPriceASC()
{
page_size
current_page
sort_fields
{
default
options
{
key

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename key to value for consistency with naming in other places in Magento, for example \Magento\Catalog\Model\Category\Attribute\Source\Sortby::getAllOptions

label
}
}
}
}
}
Expand All @@ -530,6 +539,10 @@ public function testQueryProductsInCurrentPageSortedByPriceASC()
$this->assertProductItems($filteredChildProducts, $response);
$this->assertEquals(4, $response['products']['page_info']['page_size']);
$this->assertEquals(1, $response['products']['page_info']['current_page']);
$this->assertArrayHasKey('sort_fields', $response['products']['page_info']);
$this->assertArrayHasKey('options', $response['products']['page_info']['sort_fields']);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add assertion for options values, it is not validated at the moment.

$this->assertArrayHasKey('default', $response['products']['page_info']['sort_fields']);
$this->assertEquals('position', $response['products']['page_info']['sort_fields']['default']);
}

/**
Expand Down