pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] website, website_event, website_slides: add rel=canonical tag

Browse Source 
The canonical tag is important for SEO, indeed it prevents search engines from
indexing duplicate content.

Reasoning
=========

The choice has been made to create the canonical tag automatically depending on
the request path, ignoring the query string, and manually prefixing the
appropriate domain and language code.

Indeed creating it manually for each resource would create a lot of code and
potential mistakes.

It is more dangerous to do it the generic way, but after investigation it
appears that it is an acceptable trade-off since the vast majority of our routes
are well built and already ready for this:

- using query string only for minor features that do not change the main content
- having the models, the ids, the pager and other important features in the path

Override
========

It is still possible to override the default behavior by passing
`canonical_params` manually to the view or to the different methods.

This is done for `/event` because the only way to display Past Events is to add
`date=old`.

Languages
=========

Fix an issue where it was possible for a bot to be on the URL without language
code but to use a language that is not the default language.

Adapt hreflang, because it:
- must only be present on canonical pages
- must always lead to canonical pages
- should not be set if there is no alternate language

Misc
====

task-1958075
closes #12532

Inspired by OCA module `website_canonical_url` courtesy of Jairo Llopis.

closes odoo/odoo#35852

Signed-off-by: Jérémy Kersten (jke) <jke@openerp.com>


Co-authored-by: Jairo Llopis <jairo.llopis@tecnativa.com>
Co-authored-by: Sébastien Theys <seb@odoo.com>
This commit is contained in:
Sébastien Theys
2019-09-03 12:11:35 +00:00
co-authored by Jairo Llopis
parent b89e0f6484
commit 1781041f13
7 changed files with 154 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/http_routing/models/ir_http.py
+16 -8
View File
@@ -418,14 +418,16 @@ class IrHttp(models.AbstractModel):
nearest_lang = not func and cls.get_nearest_lang(request.env['res.lang']._lang_get_code(path[1]))
url_lang = nearest_lang and path[1]
# if lang in url but not the displayed or default language --> change or remove
# or no lang in url, and lang to dispay not the default language --> add lang
# and not a POST request
# and not a bot or bot but default lang in url
if ((url_lang and (url_lang != request.lang.url_code or url_lang == default_lg_id.url_code))
or (not url_lang and request.is_frontend_multilang and request.lang != default_lg_id)
and request.httprequest.method != 'POST') \
and (not is_a_bot or (url_lang and url_lang == default_lg_id.url_code)):
# The default lang should never be in the URL, and a wrong lang
# should never be in the URL.
wrong_url_lang = url_lang and (url_lang != request.lang.url_code or url_lang == default_lg_id.url_code)
# The lang is missing from the URL if multi lang is enabled for
# the route and the current lang is not the default lang.
# POST requests are excluded from this condition.
missing_url_lang = not url_lang and request.is_frontend_multilang and request.lang != default_lg_id and request.httprequest.method != 'POST'
# Bots should never be redirected when the lang is missing
# because it is the only way for them to index the default lang.
if wrong_url_lang or (missing_url_lang and not is_a_bot):
if url_lang:
path.pop(1)
if request.lang != default_lg_id:
@@ -440,6 +442,12 @@ class IrHttp(models.AbstractModel):
path.pop(1)
routing_error = None
return cls.reroute('/'.join(path) or '/')
elif missing_url_lang and is_a_bot:
# Ensure that if the URL without lang is not redirected, the
# current lang is indeed the default lang, because it is the
# lang that bots should index in that case.
request.lang = default_lg_id
request.context = dict(request.context, lang=default_lg_id.code)
if request.lang == default_lg_id:
context = dict(request.context)
addons/website/models/website.py
+79 -23
View File
@@ -7,7 +7,9 @@ import logging
import hashlib
import re
from werkzeug import urls
from werkzeug.datastructures import OrderedMultiDict
from werkzeug.exceptions import NotFound
from odoo import api, fields, models, tools
@@ -437,37 +439,42 @@ class Website(models.Model):
# Languages
# ----------------------------------------------------------
def get_alternate_languages(self, req=None):
def _get_alternate_languages(self, canonical_params):
self.ensure_one()
if not self._is_canonical_url(canonical_params=canonical_params):
# no hreflang on non-canonical pages
return []
languages = self.language_ids
if len(languages) <= 1:
# no hreflang if no alternate language
return []
langs = []
if req is None:
req = request.httprequest
default = self.get_current_website().default_lang_id.url_code
shorts = []
def get_url_localized(router, lang):
arguments = dict(request.endpoint_arguments)
for key, val in list(arguments.items()):
if isinstance(val, models.BaseModel):
arguments[key] = val.with_context(lang=lang)
return router.build(request.endpoint, arguments)
router = request.httprequest.app.get_db_router(request.db).bind('')
for lg in self.language_ids:
lg_path = ('/' + lg.url_code) if lg.url_code != default else ''
for lg in languages:
lg_codes = lg.code.split('_')
shorts.append(lg_codes[0])
uri = get_url_localized(router, lg.url_code) if request.endpoint else request.httprequest.path
if req.query_string:
uri += u'?' + req.query_string.decode('utf-8')
lang = {
short = lg_codes[0]
shorts.append(short)
langs.append({
'hreflang': ('-'.join(lg_codes)).lower(),
'short': lg_codes[0],
'href': req.url_root[0:-1] + lg_path + uri,
}
langs.append(lang)
'short': short,
'href': self._get_canonical_url_localized(lang=lg, canonical_params=canonical_params),
})
# if there is only one region for a language, use only the language code
for lang in langs:
if shorts.count(lang['short']) == 1:
lang['hreflang'] = lang['short']
# add the default
langs.append({
'hreflang': 'x-default',
'href': self._get_canonical_url_localized(lang=self.default_lang_id, canonical_params=canonical_params),
})
return langs
# ----------------------------------------------------------
@@ -827,6 +834,55 @@ class Website(models.Model):
self.ensure_one()
return self._get_http_domain() or super(BaseModel, self).get_base_url()
def _get_canonical_url_localized(self, lang, canonical_params):
"""Returns the canonical URL for the current request with translatable
elements appropriately translated in `lang`.
If `request.endpoint` is not true, returns the current `path` instead.
`url_quote_plus` is applied on the returned path.
"""
self.ensure_one()
if request.endpoint:
router = request.httprequest.app.get_db_router(request.db).bind('')
arguments = dict(request.endpoint_arguments)
for key, val in list(arguments.items()):
if isinstance(val, models.BaseModel):
if val.env.context.get('lang') != lang.url_code:
arguments[key] = val.with_context(lang=lang.url_code)
path = router.build(request.endpoint, arguments)
else:
# The build method returns a quoted URL so convert in this case for consistency.
path = urls.url_quote_plus(request.httprequest.path, safe='/')
lang_path = ('/' + lang.url_code) if lang != self.default_lang_id else ''
canonical_query_string = '?%s' % urls.url_encode(canonical_params) if canonical_params else ''
return self.get_base_url() + lang_path + path + canonical_query_string
def _get_canonical_url(self, canonical_params):
"""Returns the canonical URL for the current request."""
self.ensure_one()
return self._get_canonical_url_localized(lang=request.lang, canonical_params=canonical_params)
def _is_canonical_url(self, canonical_params):
"""Returns whether the current request URL is canonical."""
self.ensure_one()
# Compare OrderedMultiDict because the order is important, there must be
# only one canonical and not params permutations.
params = request.httprequest.args
canonical_params = canonical_params or OrderedMultiDict()
if params != canonical_params:
return False
# Compare URL at the first rerouting iteration (if available) because
# it's the one with the language in the path.
# It is important to also test the domain of the current URL.
current_url = request.httprequest.url_root[:-1] + (hasattr(request, 'rerouting') and request.rerouting[0] or request.httprequest.path)
canonical_url = self._get_canonical_url_localized(lang=request.lang, canonical_params=None)
# A request path with quotable characters (such as ",") is never
# canonical because request.httprequest.base_url is always unquoted,
# and canonical url is always quoted, so it is never possible to tell
# if the current URL is indeed canonical or not.
return current_url == canonical_url
class BaseModel(models.AbstractModel):
_inherit = 'base'
addons/website/tests/test_base_url.py
+38 -13
View File
@@ -1,22 +1,40 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
import odoo
from lxml.html import document_fromstring
import odoo.tests
@odoo.tests.tagged('-at_install', 'post_install')
class TestBaseUrl(odoo.tests.HttpCase):
def test_base_url(self):
ICP = self.env['ir.config_parameter']
Website = self.env['website']
class TestUrlCommon(odoo.tests.HttpCase):
def setUp(self):
super(TestUrlCommon, self).setUp()
self.domain = 'http://' + odoo.tests.HOST
self.website = self.env['website'].create({
'name': 'test base url',
'domain': self.domain,
})
lang_fr = self.env.ref('base.lang_fr')
lang_fr.write({'active': True})
self.website.language_ids = self.env.ref('base.lang_en') + lang_fr
self.website.default_lang_id = self.env.ref('base.lang_en')
def _assertCanonical(self, url, canonical_url):
res = self.url_open(url)
canonical_link = document_fromstring(res.content).xpath("/html/head/link[@rel='canonical']")
self.assertEqual(len(canonical_link), 1)
self.assertEqual(canonical_link[0].attrib["href"], canonical_url)
@odoo.tests.tagged('-at_install', 'post_install')
class TestBaseUrl(TestUrlCommon):
def test_01_base_url(self):
ICP = self.env['ir.config_parameter']
icp_base_url = ICP.sudo().get_param('web.base.url')
domain = 'https://www.domain.jke'
website = Website.create({'name': 'test base url', 'domain': domain})
# Test URL is correct for the website itself when the domain is set
self.assertEqual(website.get_base_url(), domain)
self.assertEqual(self.website.get_base_url(), self.domain)
# Test URL is correct for a model without website_id
without_website_id = self.env['ir.attachment'].create({'name': 'test base url'})
@@ -30,12 +48,19 @@ class TestBaseUrl(odoo.tests.HttpCase):
self.assertEqual(with_website_id.get_base_url(), icp_base_url)
# ...when the website is correctly set
with_website_id.website_id = website
self.assertEqual(with_website_id.get_base_url(), domain)
with_website_id.website_id = self.website
self.assertEqual(with_website_id.get_base_url(), self.domain)
# ...when the set website doesn't have a domain
website.domain = False
self.website.domain = False
self.assertEqual(with_website_id.get_base_url(), icp_base_url)
# Test URL is correct for the website itself when no domain is set
self.assertEqual(website.get_base_url(), icp_base_url)
self.assertEqual(self.website.get_base_url(), icp_base_url)
def test_02_canonical_url(self):
self._assertCanonical('/', self.domain + '/')
self._assertCanonical('/?debug=1', self.domain + '/')
self._assertCanonical('/a-page', self.domain + '/a-page')
self._assertCanonical('/en_US', self.domain + '/')
self._assertCanonical('/fr_FR', self.domain + '/fr/')
addons/website/views/website_templates.xml
+3 -1
View File
@@ -228,10 +228,12 @@
</t>
<t t-if="request and request.is_frontend_multilang and website">
<t t-foreach="website.get_alternate_languages(request.httprequest)" t-as="lg">
<t t-set="alternate_languages" t-value="website._get_alternate_languages(canonical_params=canonical_params)"/>
<t t-foreach="alternate_languages" t-as="lg">
<link rel="alternate" t-att-hreflang="lg['hreflang']" t-att-href="lg['href']"/>
</t>
</t>
<link t-if="request and website" rel="canonical" t-att-href="website._get_canonical_url(canonical_params=canonical_params)"/>
</xpath>
<xpath expr="//head/t[@t-js='false'][last()]" position="after">
addons/website_event/controllers/main.py
+8 -3
View File
@@ -3,7 +3,7 @@
import babel.dates
import re
import werkzeug
import json
from werkzeug.datastructures import OrderedMultiDict
from datetime import datetime, timedelta
from dateutil.relativedelta import relativedelta
@@ -28,6 +28,7 @@ class WebsiteEventController(http.Controller):
searches.setdefault('type', 'all')
searches.setdefault('country', 'all')
website = request.website
def sdn(date):
return fields.Datetime.to_string(date.replace(hour=23, minute=59, second=59))
@@ -63,7 +64,7 @@ class WebsiteEventController(http.Controller):
]
# search domains
domain_search = {'website_specific': request.website.website_domain()}
domain_search = {'website_specific': website.website_domain()}
current_date = None
current_type = None
current_country = None
@@ -110,7 +111,7 @@ class WebsiteEventController(http.Controller):
step = 10 # Number of events per page
event_count = Event.search_count(dom_without("none"))
pager = request.website.pager(
pager = website.pager(
url="/event",
url_args={'date': searches.get('date'), 'type': searches.get('type'), 'country': searches.get('country')},
total=event_count,
@@ -139,6 +140,10 @@ class WebsiteEventController(http.Controller):
'search_path': "?%s" % werkzeug.url_encode(searches),
}
if searches['date'] == 'old':
# the only way to display this content is to set date=old so it must be canonical
values['canonical_params'] = OrderedMultiDict([('date', 'old')])
return request.render("website_event.index", values)
@http.route(['''/event/<model("event.event", "[('website_id', 'in', (False, current_website_id))]"):event>/page/<path:page>'''], type='http', auth="public", website=True, sitemap=False)
addons/website_event/tests/test_event_website.py
+9
View File
@@ -2,6 +2,8 @@ from datetime import datetime, timedelta
from odoo import fields
from odoo.addons.event.tests.common import TestEventCommon
from odoo.addons.website.tests.test_base_url import TestUrlCommon
import odoo.tests
class TestEventWebsiteHelper(TestEventCommon):
@@ -35,3 +37,10 @@ class TestEventWebsite(TestEventWebsiteHelper):
self.assertFalse(self.event_0.menu_id)
self.event_0.website_menu = True
self._assert_website_menus(self.event_0)
@odoo.tests.tagged('-at_install', 'post_install')
class TestUrlCanonical(TestUrlCommon):
def test_01_canonical_url(self):
self._assertCanonical('/event?date=all', self.domain + '/event')
self._assertCanonical('/event?date=old', self.domain + '/event?date=old')
addons/website_slides/views/website_slides_templates_homepage.xml
+1 -1
View File
@@ -238,7 +238,7 @@
t-esc="tag_group.name"/>
<div class="dropdown-menu" t-att-id="'navToogleTagGroup%s' % tag_group.id">
<t t-foreach="tag_group.tag_ids" t-as="tag">
<a t-att-class="'dropdown-item %s' % ('active' if tag in search_tags else '')"
<a rel="nofollow" t-att-class="'dropdown-item %s' % ('active' if tag in search_tags else '')"
t-att-href="'/slides/all?%s' % keep_query('*', **{'channel_tag_group_id_%s' % tag_group.id: tag.id if tag not in search_tags else False})"
t-esc="tag.name"/>
</t>