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

Virtual Category Rewrite not working on subcategories having same url_key #3055

Open MaximGns opened 1 year ago

MaximGns commented 1 year ago

Preconditions

Magento Version : 2.4.6-p1

ElasticSuite Version : 2.11.2

Environment : Both

Third party modules :

Steps to reproduce

  1. Create a virtual category
  2. Assign a specific attribute with a specific value as rule
  3. Create 2 normal categories with 2 sublevels example: dier/katten/droogvoer and dier/honden/droogvoer with the subcategories (droogvoer) having the same url_key
  4. Navigate in virtual category to both subcategories

Expected result

  1. Correct subcategory is shown based on url_path instead of url_key as it is not unique

    Actual result

  2. One of the categories will shown incorrect, as it loads the first category containing the url_key
  3. As we visit the category with path dier/honden/droogvoer the category dier/katten/droogvoer is shown

Applied following patch locally

--- a/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Controller/Router.php  2023-09-25 15:14:31
+++ b/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Controller/Router.php  2023-09-26 09:20:36
@@ -156,7 +156,10 @@
     private function getCategoryRewrite($identifier)
     {
         $chunks       = explode('/', $identifier);
-        $categoryPath = array_pop($chunks);
+        //START PATCH
+        array_shift($chunks);
+        $categoryPath = implode('/', $chunks);
+        //END PATCH
         $storeId      = $this->storeManager->getStore()->getId();

         return $this->urlModel->getCategoryRewrite($categoryPath, $storeId);
--- a/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Model/Url.php  2023-09-26 09:15:40
+++ b/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Model/Url.php  2023-09-26 09:23:59
@@ -209,7 +209,9 @@
     public function getCategoryRewrite($categoryPath, $storeId)
     {
         $categoryPath = str_replace($this->getCategoryUrlSuffix() ?? '', '', $categoryPath);
-        $category = $this->loadCategoryByUrlKey($categoryPath);
+        //START PATCH
+        $category = $this->loadCategoryByUrlKey($categoryPath, 'url_path');
+        //END PATCH
         $rewrite  = null;

         if ($category && $category->getId()) {
@@ -252,12 +254,12 @@
      * @return \Magento\Framework\DataObject
      * @throws \Magento\Framework\Exception\LocalizedException
      */
-    private function loadCategoryByUrlKey($requestPath)
+    private function loadCategoryByUrlKey($requestPath, string $filter = 'url_key')
     {
         $collection = $this->categoryCollectionFactory->create();

         $collection->setStoreId($this->storeManager->getStore()->getId())
-            ->addAttributeToFilter('url_key', ['eq' => $requestPath]);
+            ->addAttributeToFilter($filter, ['eq' => $requestPath]);

         return $collection->getFirstItem();
     }
nathanchick commented 8 months ago

@MaximGns

Thanks for sharing, I tested the patch and we didn't have success. But it did massively help.

The issue is down to the array_shift, this will work fine providing your Virtual Category is a top level category.

Maybe not the cleanest solution, but this works and solves the immediate problem. It has no noticeable impact on performance with categories a couple of levels deep.

diff --git a/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Controller/Router.php b/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Controller/Router.php
index c83fe462..461cacca 100644
--- a/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Controller/Router.php
+++ b/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Controller/Router.php
@@ -155,11 +155,24 @@ class Router implements RouterInterface
      */
     private function getCategoryRewrite($identifier)
     {
-        $chunks       = explode('/', $identifier);
-        $categoryPath = array_pop($chunks);
-        $storeId      = $this->storeManager->getStore()->getId();
+        $chunks         = explode('/', $identifier);
+        $chunk_count    = count($chunks);
+        $storeId        = $this->storeManager->getStore()->getId();
+        $url_rewrite    = null;
+
+
+        while (1 < $chunk_count) {
+            array_shift($chunks);
+            $categoryPath = implode('/', $chunks);
+
+            $url_rewrite = $this->urlModel->getCategoryRewrite($categoryPath, $storeId);
+            if($url_rewrite != null) {
+                break;
+            }
+            $chunk_count = count($chunks);
+        }

-        return $this->urlModel->getCategoryRewrite($categoryPath, $storeId);
+        return $url_rewrite;
     }

     /**

This has been applied along side:

diff --git a/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Model/Url.php b/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Model/Url.php
index be411501..e6112b3d 100644
--- a/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Model/Url.php
+++ b/vendor/smile/elasticsuite/src/module-elasticsuite-virtual-category/Model/Url.php
@@ -218,7 +218,9 @@ class Url
     public function getCategoryRewrite($categoryPath, $storeId)
     {
         $categoryPath = str_replace($this->getCategoryUrlSuffix() ?? '', '', $categoryPath);
-        $category = $this->loadCategoryByUrlKey($categoryPath);
+        //START PATCH
+        $category = $this->loadCategoryByUrlKey($categoryPath, 'url_path');
+        //END PATCH
         $rewrite  = null;

         if ($category && $category->getId()) {
@@ -261,12 +263,12 @@ class Url
      * @return \Magento\Framework\DataObject
      * @throws \Magento\Framework\Exception\LocalizedException
      */
-    private function loadCategoryByUrlKey($requestPath)
+    private function loadCategoryByUrlKey($requestPath, string $filter = 'url_key')
     {
         $collection = $this->categoryCollectionFactory->create();

         $collection->setStoreId($this->storeManager->getStore()->getId())
-            ->addAttributeToFilter('url_key', ['eq' => $requestPath]);
+            ->addAttributeToFilter($filter, ['eq' => $requestPath]);

         return $collection->getFirstItem();
     }
MaximGns commented 8 months ago

hi @nathanchick , indeed. We have only 1 virtual categorie and it is a top level category. Did not really further investigate or tested other use cases.

thomas-kl1 commented 5 months ago

Hello, I confirm this issue is easily reproductible. I'm going to check the given patch, and if I've time to work on it open a PR with a proper solution.

thomas-kl1 commented 2 months ago

BTW the first patch works fine for me!