From a676344ecc30dbe2eaee81d7ec82238c42596633 Mon Sep 17 00:00:00 2001
From: Benoit Socias <bso@odoo.com>
Date: Tue, 4 Jan 2022 10:11:43 +0000
Subject: [PATCH] [FIX] website{_*}: not truncate URLs in search results

website{_*}: website, website_blog, website_event, website_forum,
website_sale, website_slides

Since the generic search bar was introduced in [1] all text fields were
truncated in search results.
This caused problems for long URLs which were truncated as well, and
therefore could become invalid.

After this commit URL fields specify `'truncate': False` in their search
detail metadata, which informs the rendering to skip the text truncation
step for that field.
Also added previously missing controller-level tests of the
autocompletion.

Steps to reproduce:
- start odoo with website_forum and demo data
- go to the Help forum
- search for "configure" in the Help forum
- click on the auto-complete suggestion
- => redirected to a 404 page because the URL was shortened
To test the fix on other models, use a long enough name that causes the
problem. E.g.: "This product has such a long name its URL would have
been truncated without the fix contained in this branch".
Note that the problem did not occur on blogs because the URL does not
contain the name, but the same fix was applied for consistency.

[1]: https://github.com/odoo/odoo/commit/7559626c54e34b41e1549e28276a650accec6986

task-2727788

closes odoo/odoo#82621

X-original-commit: 045f741be35e62f5e3a636490c6c1d475b5d78eb
Signed-off-by: Romain Derie (rde) <rde@odoo.com>
---
 addons/website/controllers/main.py            |   2 +-
 addons/website/models/website_page.py         |   2 +-
 addons/website/tests/test_fuzzy.py            | 119 +++++++++++++++++-
 addons/website_blog/models/website_blog.py    |   4 +-
 addons/website_event/models/event_event.py    |   2 +-
 addons/website_forum/models/forum.py          |   4 +-
 .../models/product_public_category.py         |   2 +-
 .../website_sale/models/product_template.py   |   2 +-
 addons/website_slides/models/slide_channel.py |   2 +-
 addons/website_slides/models/slide_slide.py   |   4 +-
 10 files changed, 128 insertions(+), 15 deletions(-)

diff --git a/addons/website/controllers/main.py b/addons/website/controllers/main.py
index 4905bd42d9b..b42e0d61f35 100644
--- a/addons/website/controllers/main.py
+++ b/addons/website/controllers/main.py
@@ -417,7 +417,7 @@ class Website(Home):
                     continue
                 field_type = field_meta.get('type')
                 if field_type == 'text':
-                    if value:
+                    if value and field_meta.get('truncate', True):
                         value = shorten(value, max_nb_chars, placeholder='...')
                     if field_meta.get('match') and value and term:
                         pattern = '|'.join(map(re.escape, term.split()))
diff --git a/addons/website/models/website_page.py b/addons/website/models/website_page.py
index 98c198973da..dc2c8cb5a7c 100644
--- a/addons/website/models/website_page.py
+++ b/addons/website/models/website_page.py
@@ -265,7 +265,7 @@ class Page(models.Model):
         fetch_fields = ['id', 'name', 'url']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'url', 'type': 'text'},
+            'website_url': {'name': 'url', 'type': 'text', 'truncate': False},
         }
         if with_description:
             search_fields.append('arch_db')
diff --git a/addons/website/tests/test_fuzzy.py b/addons/website/tests/test_fuzzy.py
index 00e5aa0a1c5..bda44e95fe1 100644
--- a/addons/website/tests/test_fuzzy.py
+++ b/addons/website/tests/test_fuzzy.py
@@ -4,13 +4,16 @@
 import logging
 from lxml import etree
 import re
+from markupsafe import Markup
 
-from odoo.addons.website.tools import distance
+from odoo.addons.website.controllers.main import Website
+from odoo.addons.website.tools import distance, MockRequest
 import odoo.tests
 from odoo.tests.common import TransactionCase
 
 _logger = logging.getLogger(__name__)
 
+
 @odoo.tests.tagged('-at_install', 'post_install')
 class TestFuzzy(TransactionCase):
     def test_01_fuzzy_names(self):
@@ -36,7 +39,7 @@ class TestFuzzy(TransactionCase):
                 continue
             model = self.env[model_name]
             if 'description' not in fields and 'description' in model:
-                fields.append('description') # larger target dataset
+                fields.append('description')  # larger target dataset
             records = model.sudo().search_read([], fields, limit=100)
             for record in records:
                 for field, value in record.items():
@@ -51,6 +54,7 @@ class TestFuzzy(TransactionCase):
         website = self.env.ref('website.default_website')
 
         typos = {}
+
         def add_typo(expected, typo):
             if typo not in words:
                 typos.setdefault(typo, set()).add(expected)
@@ -67,7 +71,7 @@ class TestFuzzy(TransactionCase):
                 add_typo(search, search[:index - 1] + '!' + search[index:])
 
         words = list(words)
-        words.sort() # guarantee results stability
+        words.sort()  # guarantee results stability
         mismatch_count = 0
         for search, expected in typos.items():
             fuzzy_guess = website._search_find_fuzzy_term({}, search, word_list=words)
@@ -97,3 +101,112 @@ class TestFuzzy(TransactionCase):
         self.assertEqual(distance("", "warranty", 10), 8)
         self.assertEqual(distance("warranty", "", 10), 8)
         self.assertEqual(distance("", "", 10), 0)
+
+
+@odoo.tests.tagged('-at_install', 'post_install')
+class TestAutoComplete(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.website = cls.env['website'].browse(1)
+        cls.WebsiteController = Website()
+        cls.options = {
+            'displayDescription': True,
+        }
+        cls.expectedParts = {
+            'name': True,
+            'description': True,
+            'website_url': True,
+        }
+        texts = [
+            "This page only matches few",
+            "This page matches both few and many",
+            "This page only matches many",
+            "Many results contain this page",
+            "How many times does the word many appear",
+            "Will there be many results",
+            "Welcome to our many friends",
+            "This should only be approximately matched",
+        ]
+        for text in texts:
+            cls._create_page(text, text, f"/{text.lower().replace(' ', '-')}")
+
+    @classmethod
+    def _create_page(cls, name, content, url):
+        cls.env['website.page'].create({
+            'name': name,
+            'type': 'qweb',
+            'arch': f'<div>{content}</div>',
+            'url': url,
+            'is_published': True,
+        })
+
+    def _autocomplete(self, term):
+        """ Calls the autocomplete for a given term and performs general checks """
+        with MockRequest(self.env, website=self.website):
+            suggestions = self.WebsiteController.autocomplete(
+                search_type="pages", term=term, max_nb_chars=50, options=self.options,
+            )
+            if suggestions['results_count']:
+                self.assertDictEqual(self.expectedParts, suggestions['parts'],
+                                     f"Parts should contain {self.expectedParts.keys()}")
+            for result in suggestions['results']:
+                self.assertEqual("fa-file-o", result['_fa'], "Expect an fa icon")
+                for field in suggestions['parts'].keys():
+                    value = result[field]
+                    if value:
+                        self.assertTrue(
+                            isinstance(value, Markup),
+                            f"All fields should be wrapped in Markup: found {type(value)}: '{value}' in {field}"
+                        )
+            return suggestions
+
+    def _check_highlight(self, term, value):
+        """ Verifies if a term is highlighted in a value """
+        self.assertTrue(f'<span class="text-primary">{term}</span>' in value.lower(),
+                        "Term must be highlighted")
+
+    def test_01_few_results(self):
+        """ Tests an autocomplete with exact match and less than the maximum number of results """
+        suggestions = self._autocomplete("few")
+        self.assertEqual(2, suggestions['results_count'], "Text data contains two pages with 'few'")
+        self.assertEqual(2, len(suggestions['results']), "All results must be present")
+        self.assertFalse(suggestions['fuzzy_search'], "Expects an exact match")
+        for result in suggestions['results']:
+            self._check_highlight("few", result['name'])
+            self._check_highlight("few", result['description'])
+
+    def test_02_many_results(self):
+        """ Tests an autocomplete with exact match and more than the maximum number of results """
+        suggestions = self._autocomplete("many")
+        self.assertEqual(6, suggestions['results_count'], "Test data contains six pages with 'many'")
+        self.assertEqual(5, len(suggestions['results']), "Results must be limited to 5")
+        self.assertFalse(suggestions['fuzzy_search'], "Expects an exact match")
+        for result in suggestions['results']:
+            self._check_highlight("many", result['name'])
+            self._check_highlight("many", result['description'])
+
+    def test_03_no_result(self):
+        """ Tests an autocomplete without matching results """
+        suggestions = self._autocomplete("nothing")
+        self.assertEqual(0, suggestions['results_count'], "Text data contains no page with 'nothing'")
+        self.assertEqual(0, len(suggestions['results']), "No result must be present")
+
+    def test_04_fuzzy_results(self):
+        """ Tests an autocomplete with fuzzy matching results """
+        suggestions = self._autocomplete("appoximtly")
+        self.assertEqual("approximately", suggestions['fuzzy_search'], "")
+        self.assertEqual(1, suggestions['results_count'], "Text data contains one page with 'approximately'")
+        self.assertEqual(1, len(suggestions['results']), "Single result must be present")
+        for result in suggestions['results']:
+            self._check_highlight("approximately", result['name'])
+            self._check_highlight("approximately", result['description'])
+
+    def test_05_long_url(self):
+        """ Ensures that long URL do not get truncated """
+        url = "/this-url-is-so-long-it-would-be-truncated-without-the-fix"
+        self._create_page("Too long", "Way too long URL", url)
+        suggestions = self._autocomplete("long url")
+        self.assertEqual(1, suggestions['results_count'], "Text data contains one page with 'long url'")
+        self.assertEqual(1, len(suggestions['results']), "Single result must be present")
+        self.assertEqual(url, suggestions['results'][0]['website_url'], 'URL must not be truncated')
diff --git a/addons/website_blog/models/website_blog.py b/addons/website_blog/models/website_blog.py
index 2794514552b..99bc7e116f0 100644
--- a/addons/website_blog/models/website_blog.py
+++ b/addons/website_blog/models/website_blog.py
@@ -99,7 +99,7 @@ class Blog(models.Model):
         fetch_fields = ['id', 'name']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'url', 'type': 'text'},
+            'website_url': {'name': 'url', 'type': 'text', 'truncate': False},
         }
         if with_description:
             search_fields.append('subtitle')
@@ -336,7 +336,7 @@ class BlogPost(models.Model):
         fetch_fields = ['name', 'website_url']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'website_url', 'type': 'text'},
+            'website_url': {'name': 'website_url', 'type': 'text', 'truncate': False},
         }
         if with_description:
             search_fields.append('content')
diff --git a/addons/website_event/models/event_event.py b/addons/website_event/models/event_event.py
index 41570117f58..9cfa57d2fab 100644
--- a/addons/website_event/models/event_event.py
+++ b/addons/website_event/models/event_event.py
@@ -522,7 +522,7 @@ class Event(models.Model):
         fetch_fields = ['name', 'website_url']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'website_url', 'type': 'text'},
+            'website_url': {'name': 'website_url', 'type': 'text', 'truncate': False},
         }
         if with_description:
             search_fields.append('subtitle')
diff --git a/addons/website_forum/models/forum.py b/addons/website_forum/models/forum.py
index 5c519646883..baee75d70c5 100644
--- a/addons/website_forum/models/forum.py
+++ b/addons/website_forum/models/forum.py
@@ -275,7 +275,7 @@ class Forum(models.Model):
         fetch_fields = ['id', 'name']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'website_url', 'type': 'text'},
+            'website_url': {'name': 'website_url', 'type': 'text', 'truncate': False},
         }
         if with_description:
             search_fields.append('description')
@@ -985,7 +985,7 @@ class Post(models.Model):
         fetch_fields = ['id', 'name']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'website_url', 'type': 'text'},
+            'website_url': {'name': 'website_url', 'type': 'text', 'truncate': False},
         }
 
         domain = website.website_domain()
diff --git a/addons/website_sale/models/product_public_category.py b/addons/website_sale/models/product_public_category.py
index 403de6e36cb..b22c141c6e6 100644
--- a/addons/website_sale/models/product_public_category.py
+++ b/addons/website_sale/models/product_public_category.py
@@ -57,7 +57,7 @@ class ProductPublicCategory(models.Model):
         fetch_fields = ['id', 'name']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'url', 'type': 'text'},
+            'website_url': {'name': 'url', 'type': 'text', 'truncate': False},
         }
         if with_description:
             search_fields.append('website_description')
diff --git a/addons/website_sale/models/product_template.py b/addons/website_sale/models/product_template.py
index 067d7b87c2c..5c9de19c184 100644
--- a/addons/website_sale/models/product_template.py
+++ b/addons/website_sale/models/product_template.py
@@ -366,7 +366,7 @@ class ProductTemplate(models.Model):
         fetch_fields = ['id', 'name', 'website_url']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'website_url', 'type': 'text'},
+            'website_url': {'name': 'website_url', 'type': 'text', 'truncate': False},
         }
         if with_image:
             mapping['image_url'] = {'name': 'image_url', 'type': 'html'}
diff --git a/addons/website_slides/models/slide_channel.py b/addons/website_slides/models/slide_channel.py
index 80d4cf6fca5..21b09fac862 100644
--- a/addons/website_slides/models/slide_channel.py
+++ b/addons/website_slides/models/slide_channel.py
@@ -873,7 +873,7 @@ class Channel(models.Model):
         fetch_fields = ['name', 'website_url']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'website_url', 'type': 'text'},
+            'website_url': {'name': 'website_url', 'type': 'text', 'truncate': False},
         }
         if with_description:
             search_fields.append('description_short')
diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py
index a62953d6090..631945e371d 100644
--- a/addons/website_slides/models/slide_slide.py
+++ b/addons/website_slides/models/slide_slide.py
@@ -1234,9 +1234,9 @@ class Slide(models.Model):
         fetch_fields = ['id', 'name']
         mapping = {
             'name': {'name': 'name', 'type': 'text', 'match': True},
-            'website_url': {'name': 'url', 'type': 'text'},
+            'website_url': {'name': 'url', 'type': 'text', 'truncate': False},
             'extra_link': {'name': 'course', 'type': 'text'},
-            'extra_link_url': {'name': 'course_url', 'type': 'text'},
+            'extra_link_url': {'name': 'course_url', 'type': 'text', 'truncate': False},
         }
         if with_description:
             search_fields.append('description')