[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>
This commit is contained in:
@@ -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()))
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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'}
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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')
|
||||
|
||||
Reference in New Issue
Block a user