Skip to content

Issue 36096: Layered Navigation does not filter products properly #36102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: 2.4-develop
Choose a base branch
from
Open
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
73 changes: 66 additions & 7 deletions app/code/Magento/AdvancedSearch/Model/ResourceModel/Index.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\AdvancedSearch\Model\ResourceModel;

use Magento\Framework\Model\ResourceModel\Db\AbstractDb;
use Magento\Framework\Search\Request\IndexScopeResolverInterface;
use Magento\Store\Model\StoreManagerInterface;
use Magento\Framework\Model\ResourceModel\Db\Context;
use Magento\Framework\EntityManager\MetadataPool;
use Magento\Catalog\Api\Data\CategoryInterface;
use Magento\Catalog\Model\Indexer\Category\Product\AbstractAction;
use Magento\Catalog\Model\Indexer\Product\Price\DimensionCollectionFactory;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\EntityManager\MetadataPool;
use Magento\Framework\Model\ResourceModel\Db\AbstractDb;
use Magento\Framework\Model\ResourceModel\Db\Context;
use Magento\Framework\Search\Request\Dimension;
use Magento\Catalog\Model\Indexer\Category\Product\AbstractAction;
use Magento\Framework\Search\Request\IndexScopeResolverInterface;
use Magento\Framework\Search\Request\IndexScopeResolverInterface as TableResolver;
use Magento\Catalog\Model\Indexer\Product\Price\DimensionCollectionFactory;
use Magento\Store\Model\Indexer\WebsiteDimensionProvider;
use Magento\Store\Model\StoreManagerInterface;

/**
* @api
Expand Down Expand Up @@ -150,6 +152,63 @@ public function getPriceIndexData($productIds, $storeId)
return $priceProductsIndexData[$websiteId];
}

/**
* Return array of inventory data products
*
* @param null|array $productIds
* @return array
* @since 100.1.0
*/
protected function _getCatalogProductInventoryData($productIds = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be private function
_getCatalogProductInventoryData change to getCatalogProductInventoryData as Magento standard code.

{
$connection = $this->getConnection();
$catalogProductIndexInventorySelect = [];

foreach ($this->dimensionCollectionFactory->create() as $dimensions) {
if (!isset($dimensions[WebsiteDimensionProvider::DIMENSION_NAME]) ||
$this->websiteId === null ||
$dimensions[WebsiteDimensionProvider::DIMENSION_NAME]->getValue() === $this->websiteId) {
$select = $connection->select()->from(
$this->tableResolver->resolve('cataloginventory_stock_status', $dimensions),
['product_id', 'website_id', 'stock_status']
);
if ($productIds) {
$select->where('product_id IN (?)', $productIds);
}
$catalogProductIndexInventorySelect[] = $select;
}
}

$catalogProductIndexInventoryUnionSelect = $connection->select()->union($catalogProductIndexInventorySelect);

$result = [];
foreach ($connection->fetchAll($catalogProductIndexInventoryUnionSelect) as $row) {
$result[$row['website_id']][$row['product_id']] = (int) $row['stock_status'];
}

return $result;
}

/**
* Retrieve inventory data for product
*
* @param null|array $productIds
* @param int $websiteId
* @return array
*/
public function getInventoryIndexData($productIds, $websiteId = null)
Copy link
Contributor

Choose a reason for hiding this comment

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

introduce the public function make the Semantic test failed. @sidolov Can you give us some advice about that ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Backward capability don't allow introduce new public methods to class marked as @api and should be replaced by new class

{
$this->websiteId = $websiteId;
$inventoryProductsIndexData = $this->_getCatalogProductInventoryData($productIds);
$this->websiteId = null;

if (!isset($inventoryProductsIndexData[(int) $websiteId])) {
return [];
}

return $inventoryProductsIndexData[(int) $websiteId];
}

/**
* Prepare system index data for products.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Elasticsearch\Model\ResourceModel\Fulltext\Collection;

use Magento\Catalog\Api\Data\ProductInterface;
use Magento\CatalogInventory\Model\StockStatusApplierInterface;
use Magento\CatalogInventory\Model\ResourceModel\StockStatusFilterInterface;
use Magento\CatalogInventory\Model\StockStatusApplierInterface;
use Magento\CatalogSearch\Model\ResourceModel\Fulltext\Collection\SearchResultApplierInterface;
use Magento\Framework\Api\Search\SearchResultInterface;
use Magento\Framework\App\Config\ScopeConfigInterface;
Expand Down Expand Up @@ -105,14 +105,12 @@ public function apply()
return;
}

$ids = $this->getProductIdsBySaleability();

if (count($ids) == 0) {
$items = $this->sliceItems($this->searchResult->getItems(), $this->size, $this->currentPage);
foreach ($items as $item) {
$ids[] = (int)$item->getId();
}
$ids = [];
$items = $this->sliceItems($this->searchResult->getItems(), $this->size, $this->currentPage);
foreach ($items as $item) {
$ids[] = (int)$item->getId();
}

$orderList = implode(',', $ids);
$this->collection->getSelect()
->where('e.entity_id IN (?)', $ids)
Expand Down Expand Up @@ -160,127 +158,4 @@ private function getOffset(int $pageNumber, int $pageSize): int
{
return ($pageNumber - 1) * $pageSize;
}
/**
* Fetch filtered product ids sorted by the saleability and other applied sort orders
*
* @return array
*/
private function getProductIdsBySaleability(): array
{
$ids = [];

if (!$this->hasShowOutOfStockStatus()) {
return $ids;
}

if ($this->collection->getFlag('has_stock_status_filter')
|| $this->collection->getFlag('has_category_filter')) {
$categoryId = null;
$searchCriteria = $this->searchResult->getSearchCriteria();
foreach ($searchCriteria->getFilterGroups() as $filterGroup) {
foreach ($filterGroup->getFilters() as $filter) {
if ($filter->getField() === 'category_ids') {
$categoryId = $filter->getValue();
break 2;
}
}
}

if ($categoryId) {
$resultSet = $this->categoryProductByCustomSortOrder($categoryId);
foreach ($resultSet as $item) {
$ids[] = (int)$item['entity_id'];
}
}
}

return $ids;
}

/**
* Fetch product resultset by custom sort orders
*
* @param int $categoryId
* @return array
* @throws \Magento\Framework\Exception\LocalizedException
* @throws \Exception
*/
private function categoryProductByCustomSortOrder(int $categoryId): array
{
$storeId = $this->collection->getStoreId();
$searchCriteria = $this->searchResult->getSearchCriteria();
$sortOrders = $searchCriteria->getSortOrders() ?? [];
$sortOrders = array_merge(['is_salable' => \Magento\Framework\DB\Select::SQL_DESC], $sortOrders);

$connection = $this->collection->getConnection();
$query = clone $connection->select()
->reset(\Magento\Framework\DB\Select::ORDER)
->reset(\Magento\Framework\DB\Select::LIMIT_COUNT)
->reset(\Magento\Framework\DB\Select::LIMIT_OFFSET)
->reset(\Magento\Framework\DB\Select::COLUMNS);
$query->from(
['e' => $this->collection->getTable('catalog_product_entity')],
['e.entity_id']
);
$this->stockStatusApplier->setSearchResultApplier(true);
$query = $this->stockStatusFilter->execute($query, 'e', 'stockItem');
$query->join(
['cat_index' => $this->collection->getTable('catalog_category_product_index_store' . $storeId)],
'cat_index.product_id = e.entity_id'
. ' AND cat_index.category_id = ' . $categoryId
. ' AND cat_index.store_id = ' . $storeId,
['cat_index.position']
);
foreach ($sortOrders as $field => $dir) {
if ($field === 'name') {
$entityTypeId = $this->collection->getEntity()->getTypeId();
$entityMetadata = $this->metadataPool->getMetadata(ProductInterface::class);
$linkField = $entityMetadata->getLinkField();
$query->joinLeft(
['product_var' => $this->collection->getTable('catalog_product_entity_varchar')],
"product_var.{$linkField} = e.{$linkField} AND product_var.attribute_id =
(SELECT attribute_id FROM eav_attribute WHERE entity_type_id={$entityTypeId}
AND attribute_code='name')",
['product_var.value AS name']
);
} elseif ($field === 'price') {
$query->joinLeft(
['price_index' => $this->collection->getTable('catalog_product_index_price')],
'price_index.entity_id = e.entity_id'
. ' AND price_index.customer_group_id = 0'
. ' AND price_index.website_id = (Select website_id FROM store WHERE store_id = '
. $storeId . ')',
['price_index.max_price AS price']
);
}
$columnFilters = [];
$columnsParts = $query->getPart('columns');
foreach ($columnsParts as $columns) {
$columnFilters[] = $columns[2] ?? $columns[1];
}
if (in_array($field, $columnFilters, true)) {
$query->order(new \Zend_Db_Expr("{$field} {$dir}"));
}
}

$query->limit(
$searchCriteria->getPageSize(),
$searchCriteria->getCurrentPage() * $searchCriteria->getPageSize()
);

return $connection->fetchAssoc($query) ?? [];
}

/**
* Returns if display out of stock status set or not in catalog inventory
*
* @return bool
*/
private function hasShowOutOfStockStatus(): bool
{
return (bool) $this->scopeConfig->getValue(
\Magento\CatalogInventory\Model\Configuration::XML_PATH_SHOW_OUT_OF_STOCK,
\Magento\Store\Model\ScopeInterface::SCOPE_STORE
);
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Elasticsearch\SearchAdapter\Query\Builder;

use Magento\Elasticsearch\Model\Adapter\FieldMapper\Product\AttributeProvider;
use Magento\Elasticsearch\Model\Adapter\FieldMapper\Product\FieldProvider\FieldName\ResolverInterface
as FieldNameResolver;
use Magento\Elasticsearch\Model\Adapter\FieldMapperInterface;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\Search\RequestInterface;

/**
Expand Down Expand Up @@ -41,6 +44,11 @@ class Sort
*/
private $fieldNameResolver;

/**
* @var ScopeConfigInterface
*/
private $scopeConfig;

/**
* @var array
*/
Expand All @@ -54,17 +62,20 @@ class Sort
/**
* @param AttributeProvider $attributeAdapterProvider
* @param FieldNameResolver $fieldNameResolver
* @param ScopeConfigInterface $scopeConfig
* @param array $skippedFields
* @param array $map
*/
public function __construct(
AttributeProvider $attributeAdapterProvider,
FieldNameResolver $fieldNameResolver,
ScopeConfigInterface $scopeConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

ScopeConfigInterface $scopeConfig should be moved to the end of the contruct because of Backward compatible.

ScopeConfigInterface  $scopeConfig = null

and:

$this->scopeConfig = $scopeConfig ?: ObjectManager::getInstance()->get(ScopeConfigInterface::class);

array $skippedFields = [],
array $map = []
) {
$this->attributeAdapterProvider = $attributeAdapterProvider;
$this->fieldNameResolver = $fieldNameResolver;
$this->scopeConfig = $scopeConfig;
$this->skippedFields = array_merge(self::DEFAULT_SKIPPED_FIELDS, $skippedFields);
$this->map = array_merge(self::DEFAULT_MAP, $map);
}
Expand All @@ -80,7 +91,8 @@ public function __construct(
*/
public function getSort(RequestInterface $request)
{
$sorts = [];
$sorts = $this->getSortBySaleability();

/**
* Temporary solution for an existing interface of a fulltext search request in Backward compatibility purposes.
* Scope to split Search request interface on two different 'Search' and 'Fulltext Search' contains in MC-16461.
Expand Down Expand Up @@ -124,4 +136,39 @@ public function getSort(RequestInterface $request)

return $sorts;
}

/**
* Prepare sort by saleability.
*
* @return array
*/
public function getSortBySaleability()
Copy link
Contributor

Choose a reason for hiding this comment

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

introduce the public function. @sidolov Can you give us some advice about that ?

{
$sorts = [];

if ($this->hasShowOutOfStockStatus()) {
$attribute = $this->attributeAdapterProvider->getByAttributeCode('is_salable');
Copy link
Contributor

Choose a reason for hiding this comment

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

Should declare the constant for the is_salable

$fieldName = $this->fieldNameResolver->getFieldName($attribute);
$sorts[] = [
$fieldName => [
'order' => 'desc'
Copy link
Contributor

Choose a reason for hiding this comment

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

desc should be changed to constant in the Magento framework

]
];
}

return $sorts;
}

/**
* Returns if display out of stock status set or not in catalog inventory
*
* @return bool
*/
private function hasShowOutOfStockStatus(): bool
{
return (bool) $this->scopeConfig->getValue(
\Magento\CatalogInventory\Model\Configuration::XML_PATH_SHOW_OUT_OF_STOCK,
\Magento\Store\Model\ScopeInterface::SCOPE_STORE
);
}
}
1 change: 1 addition & 0 deletions app/code/Magento/Elasticsearch/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@
<argument name="fieldsProviders" xsi:type="array">
<item name="categories" xsi:type="object">Magento\Elasticsearch\Elasticsearch5\Model\Adapter\BatchDataMapper\CategoryFieldsProviderProxy</item>
<item name="prices" xsi:type="object">Magento\Elasticsearch\Model\Adapter\BatchDataMapper\PriceFieldsProvider</item>
<item name="inventory" xsi:type="object">Magento\Elasticsearch\Model\Adapter\BatchDataMapper\InventoryFieldsProvider</item>
</argument>
</arguments>
</virtualType>
Expand Down