Smile-SA / elasticsuite

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

GraphQL products filtering with category_uid using IN condition not supported #3231

Closed ilnytskyi closed 6 months ago

ilnytskyi commented 6 months ago

Preconditions

Magento Version : 2.4.5

ElasticSuite Version : 2.10.19

Environment :

Third party modules :

Steps to reproduce

  1. Filter products using graphql query with many or one category_uid
    query {
    products(filter: {
    category_uid: {
      in: ["MzAwMQ=="]
    }
    }) {
    total_count
    items {
      name
      sku
    }
    page_info {
      page_size
      total_pages
      current_page
      is_spellchecked
    }
    aggregations {
      attribute_code
      count
      frontend_input
      has_more
      options {
        count
        label
        value
      }
    }
    }
    }

Expected result

  1. See related products from given category

Actual result

  1. No results returned. Selection_20240326_007

interestingly it works fine when using eq filter

Selection_20240326_006

It looks like the bug is in here:

\Smile\ElasticsuiteVirtualCategory\Plugin\Search\RequestMapperPlugin::afterGetFilters

The plugin only checks if eq condition exists, and does not check other condition types

Selection_20240326_008

vahonc commented 6 months ago

Hello @ilnytskyi,

I was not able to reproduce your issue on my test environments with Magento EE 2.4.5 + ES (2.10.latest), Magento EE 2.4.5-p3 + ES 2.10..19, Magento EE 2.4.6 + ES (2.11.latest).

Screenshot from 2024-03-26 20-16-46

Screenshot from 2024-03-26 20-17-09

Screenshot from 2024-03-26 20-17-37

As you can see I see all related products from given category (Watches (default sample data)).

So, are you sure that there no any impact on your side?

Also what if you update the conditions here: https://github.com/Smile-SA/elasticsuite/blob/2.11.x/src/module-elasticsuite-virtual-category/Plugin/Search/RequestMapperPlugin.php#L106 like:

elseif (isset($result['category.category_uid']) &&
   isset($result['category.category_uid']['eq']) &&
   isset($result['category.category_uid']['in']) &&
   !isset($result['category.category_id'])
) {

and in and line L108 like: $categoryUid[] = $this->uidEncoder->decode($result['category.category_uid']['in']);

Does it solve your issue and you will be able to see related products from your category?

BR, Vadym

ilnytskyi commented 6 months ago

@vahonc if I apply patch that processes all uids from IN array. The result works fine

vahonc commented 6 months ago

@ilnytskyi,

Could you test on your environment my updated code for the https://github.com/Smile-SA/elasticsuite/blob/2.11.x/src/module-elasticsuite-virtual-category/Plugin/Search/RequestMapperPlugin.php#L101-L118 lines:

if ($this->isEnabled($containerConfiguration)) {
            // Check if 'category.category_id' exists but 'category.category_uid' does not
            if (isset($result['category.category_id']) && !isset($result['category.category_uid'])) {
                $result[] = $this->getCategoriesQuery($result['category.category_id'], $storeId);
                unset($result['category.category_id']);
            }
            // Check if 'category.category_uid' exists with any condition type but 'category.category_id' does not
            elseif (isset($result['category.category_uid'])) {
                $conditionTypes = ['eq', 'in'];

                $allConditionsExist = true;
                foreach ($conditionTypes as $condition) {
                    if (!isset($result['category.category_uid'][$condition])) {
                        $allConditionsExist = false;
                        break;
                    }
                }

                if ($allConditionsExist && !isset($result['category.category_id'])) {
                    $categoryUid = [];
                    foreach ($conditionTypes as $condition) {
                        $categoryUid[] = $this->uidEncoder->decode($result['category.category_uid'][$condition]);
                    }
                    $result[] = $this->getCategoriesQuery($categoryUid, $storeId);
                    unset($result['category.category_uid']);
                }
            }
            // Check if both 'category.category_id' and 'category.category_uid' exist
            elseif (isset($result['category.category_id']) && isset($result['category.category_uid'])) {
                $result[] = $this->getCategoriesQuery($result['category.category_id'], $storeId);
                unset($result['category.category_id']);
                unset($result['category.category_uid']);
            }
        }

BR, Vadym

ilnytskyi commented 6 months ago

@vahonc AFAIK GraphQL allows only one condition at a time. eq cannot be used together with in

I managed to fix the issue with patch like this

Index: vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Plugin/Search/RequestMapperPlugin.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Plugin/Search/RequestMapperPlugin.php b/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Plugin/Search/RequestMapperPlugin.php
--- a/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Plugin/Search/RequestMapperPlugin.php  
+++ b/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Plugin/Search/RequestMapperPlugin.php  (date 1711450284097)
@@ -109,6 +109,16 @@

                 $result[] = $this->getCategoriesQuery($categoryUid, $storeId);
                 unset($result['category.category_uid']);
+                //TASKNUM patch hotfix
+            } elseif (isset($result['category.category_uid']) && isset($result['category.category_uid']['in']) &&
+                !isset($result['category.category_id'])) {
+                foreach ($result['category.category_uid']['in'] as $uid) {
+                    $categoryUid[] = $this->uidEncoder->decode($uid);
+                }
+
+                $result[] = $this->getCategoriesQuery($categoryUid, $storeId);
+                unset($result['category.category_uid']);
+                //TASKNUM END
             } elseif (isset($result['category.category_id']) && isset($result['category.category_uid'])) {
                 $result[] = $this->getCategoriesQuery($result['category.category_id'], $storeId);
romainruaud commented 6 months ago

As Vadym said initially, I'm not able to reproduce your issue either.

but the RequestMapper plugin is not managing properly the "IN" case, you're right.

By chance, I end up with the following query being built and sent to Elasticsearch, which is of course correct (syntaxically) :

                        {
                            "nested": {
                                "path": "category",
                                "score_mode": "none",
                                "query": {
                                    "terms": {
                                        "category.category_uid.untouched": [
                                            "OA==",
                                            "MTE="
                                        ],
                                        "boost": 1
                                    }
                                },
                                "boost": 1
                            }
                        }

But if I'm using category_id instead of category_uid, the plugin triggers accordingly and I got this :

                                            "should": [
                                                {
                                                    "nested": {
                                                        "path": "category",
                                                        "score_mode": "none",
                                                        "query": {
                                                            "bool": {
                                                                "must": [
                                                                    {
                                                                        "bool": {
                                                                            "must_not": [
                                                                                {
                                                                                    "term": {
                                                                                        "category.is_virtual": {
                                                                                            "value": true,
                                                                                            "boost": 1
                                                                                        }
                                                                                    }
                                                                                }
                                                                            ],
                                                                            "boost": 1
                                                                        }
                                                                    },
                                                                    {
                                                                        "terms": {
                                                                            "category.category_id": [
                                                                                "8"
                                                                            ],
                                                                            "boost": 1
                                                                        }
                                                                    }
                                                                ],
                                                                "must_not": [],
                                                                "should": [],
                                                                "boost": 1
                                                            }
                                                        },
                                                        "boost": 1
                                                    }
                                                },
                                                {
                                                    "nested": {
                                                        "path": "category",
                                                        "score_mode": "none",
                                                        "query": {
                                                            "bool": {
                                                                "must": [
                                                                    {
                                                                        "bool": {
                                                                            "must_not": [
                                                                                {
                                                                                    "term": {
                                                                                        "category.is_virtual": {
                                                                                            "value": true,
                                                                                            "boost": 1
                                                                                        }
                                                                                    }
                                                                                }
                                                                            ],
                                                                            "boost": 1
                                                                        }
                                                                    },
                                                                    {
                                                                        "terms": {
                                                                            "category.category_id": [
                                                                                "11"
                                                                            ],
                                                                            "boost": 1
                                                                        }
                                                                    }
                                                                ],
                                                                "must_not": [],
                                                                "should": [],
                                                                "boost": 1
                                                            }
                                                        },
                                                        "boost": 1
                                                    }
                                                }
                                            ],
                                            "minimum_should_match": 1,
                                            "boost": 1
                                        }
                                    }
                                ],
romainruaud commented 6 months ago

3241 contains a better proposal to fix this.

This will harmonize how the subqueries are generated, it should always behave the same for category_id / category_uid now.

I also move the decode part at the beginning of the function so that if category_uid param is here, it will be decoded first before being used.

Regards.