Smile-SA / elasticsuite

Smile ElasticSuite - Magento 2 merchandising and search engine built on ElasticSearch
https://elasticsuite.io
Open Software License 3.0
762 stars 341 forks source link

Product carousel shows blacklisted products and blacklisted products that have been removed from the category #2464

Open WFEugenioCelant opened 2 years ago

WFEugenioCelant commented 2 years ago

Description

By blacklisting a product in the category, it is still shown in FE carousels that retrieve the product using the category (did not test the other methods). Also, products blacklisted that are then removed from the category (via their own product pages in Backend) are still shown in FE carousels (always selected by category, didn't test other cases)

Preconditions

Magento Version : EE v.2.4.2-p1

ElasticSuite Version : tested on v.2.10.9 and v.2.10.9.1

Environment : both Developer and Production mode

Third party modules :

Show...
Amasty_Base
Amasty_Scroll
Amazon_Core
Amazon_Login
Amazon_Payment
Ampersand_DisableStockReservation
Anowave_Package
Anowave_Massactions
Dotdigitalgroup_Email
Dotdigitalgroup_Chat
Dotdigitalgroup_Enterprise
Dotdigitalgroup_Sms
Fooman_PrintOrderPdf
MSP_Common
MSP_CmsImportExport
Mageplaza_Core
Owebia_SharedPhpConfig
Owebia_AdvancedShipping
Smile_ElasticsuiteAdminNotification
Smile_ElasticsuiteCore
Smile_ElasticsuiteCatalog
Smile_ElasticsuiteCatalogGraphQl
Smile_ElasticsuiteCatalogRule
Smile_ElasticsuiteCatalogOptimizer
Smile_ElasticsuiteTracker
Smile_ElasticsuiteThesaurus
Smile_ElasticsuiteSwatches
Smile_ElasticsuiteIndices
Smile_ElasticsuiteAnalytics
Smile_ElasticsuiteVirtualCategory

Steps to reproduce

  1. Add a product to a category X from its Product Page in BE
  2. php bin/magento indexer:reindex
  3. Add carousel to any Page with the following settings (other settings are not influential):
    • Select Products By: Category
    • Category: the one where we added the product in step 1.
    • bug_elasticsuite_product_carousel_1
  4. In FE you can see the product both in the Category and in our carousel
  5. Go to the category in BE and blacklist the product (Catalog > Categories > categoryName > Products in Category > eye icon) bug_elasticsuite_product_carousel_2
  6. php bin/magento indexer:reindex
  7. In FE you can now still see the product in the carousel but it has disappeared in the category page (FIRST BUG)
  8. In BE remove the product form the category from the Product form
  9. php bin/magento indexer:reindex
  10. In FE you can still see the product in the carousel and it's still missing in the category (SECOND BUG)

Sorry for the (maybe too) redundant reindexes. Probably the catalogsearch_fulltext would have been enough.

Expected result

Actual result

Other than what (I think) is already clear, I can say that the table smile_virtualcategory_catalog_category_product_position still has a row for the added product, even after it has been removed from the category. Since the association is removed, shouldn't the row in the aformentioned table be removed too?

Thanks very much in advance

WFEugenioCelant commented 2 years ago

Hello,

wanted to update the issue since we've been debugging and testing it locally to check if there was any way to fix the issue. After a while I think we have a good understanding of the problem and where it resides. I am going to try and explain what we think is the problem and our "hacky" solution. Maybe an expect in the codebase can help us identify if the problem is indeed this or we are missing the mark.

We think the problem is that currently there is no management of blacklisted products for carousels and PageBuilder items in general. This is because we have not found any filters defined in etc/elasticsuite_search_request.xml specific for PageBuilder elements, but only generic one for category pages and search requests. Currently we see that the PageBuilder carousel calls the standard category filters when setup to filter on a specific Category.

This class bases its logic on an attribute $searchContext that is filled mainly with the category we are in. Since the PageBuilder is not a normal Category though, the $searchContext attribute is missing a vital information to remove blacklisted products.

Because of this in class Smile\ElasticsuiteVirtualCategory\Model\Rule\Condition\Product:getSearchQuery, this part of the function always returns null as $searchQuery since it does not have any Category to work off of:

if ($this->getAttribute() === 'category_ids') {
    $searchQuery = $this->getCategorySearchQuery($excludedCategories, $virtualCategoryRoot);
}

This is pretty much the whole explanation we have come up with. We have also developed a very basic (and probably bad, "hacky") solutionto the problem at hand. I will copy/paste it here but obviously it's not to be used and was just a proof of concept to check if assigning a value to the aformentioned category would sort any effects, which indeed does.

"Hacky" fix:

public function __construct(
    \Magento\Rule\Model\Condition\Context $context,
    \Magento\Backend\Helper\Data $backendData,
    \Magento\Eav\Model\Config $config,
    \Smile\ElasticsuiteCatalogRule\Model\Rule\Condition\Product\AttributeList $attributeList,
    \Smile\ElasticsuiteCatalogRule\Model\Rule\Condition\Product\QueryBuilder $queryBuilder,
    \Magento\Catalog\Model\ProductFactory $productFactory,
    \Magento\Catalog\Api\ProductRepositoryInterface $productRepository,
    \Magento\Catalog\Model\ResourceModel\Product $productResource,
    \Magento\Eav\Model\ResourceModel\Entity\Attribute\Set\Collection $attrSetCollection,
    \Magento\Framework\Locale\FormatInterface $localeFormat,
    SpecialAttributesProvider $specialAttributesProvider,
    \Smile\ElasticsuiteCore\Search\Request\Query\QueryFactory $queryFactory,
    \Magento\Catalog\Model\CategoryRepository $categoryRepository,
    \Smile\ElasticsuiteCore\Api\Search\ContextInterface $searchContext,
    array $data = []
) {
    parent::__construct(
        $context,
        $backendData,
        $config,
        $attributeList,
        $queryBuilder,
        $productFactory,
        $productRepository,
        $productResource,
        $attrSetCollection,
        $localeFormat,
        $specialAttributesProvider,
        $data
    );
    $this->queryFactory = $queryFactory;
    $this->categoryRepository = $categoryRepository;
    $this->searchContext = $searchContext;
    }

    ....

public function getSearchQuery($excludedCategories = [], $virtualCategoryRoot = null): ?QueryInterface
{
    $searchQuery = parent::getSearchQuery();

    if ($this->getAttribute() === 'category_ids') {
        $this->searchContext->setCurrentCategory($this->categoryRepository->get($this->getValueParsed()[0]));
        $searchQuery = $this->getCategorySearchQuery($excludedCategories, $virtualCategoryRoot);
    }

    return $searchQuery;
}

For a brief explaination: this just adds the $searchContext dependecy and sets the category by getting the current category from the value selected in the PageBuilder carousel.

Hoping this helps at least to better understand the problem at hand.

romainruaud commented 2 years ago

Hi @WFEugenioCelant

you are right and your explanation is the proper one.

Blacklisting a product only works in the context of a category page actually, mostly due to the fact that this is the only context where the $searchContext is injected with the current category.

A widget does not have a "current category", because it can have no category at all (if selecting by SKU), or as many as you want (if selecting several categories with Conditions).

I think we could do something at least for the "Category" mode of the widget creation, where the customer can only select one category.

But I'm not a huge fan of injecting this category as the current category of the search context : the widget can be on a category page before the content, or something like that, and changing the search context for the whole page rendering might cause a lot of trouble.

I think it would be better in this case to directly inject a clause in the query to exclude blacklisted products, as it's done by the legacy filter here : https://github.com/Smile-SA/elasticsuite/blob/2.10.x/src/module-elasticsuite-virtual-category/Model/Product/Search/Request/Container/Filter/CategoryBlacklist.php#L82

Regards