You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by gc...@apache.org on 2022/06/29 19:40:52 UTC
[allura] 01/01: [#8444] adde canonical tag and rel=next/prev where pagination is used to wiki, discussions and tickets
This is an automated email from the ASF dual-hosted git repository.
gcruz pushed a commit to branch gc/8444
in repository https://gitbox.apache.org/repos/asf/allura.git
commit ec66ea3c29e8b83a3c2ef07854b9ac5f0adfba98
Author: Guillermo Cruz <gu...@slashdotmedia.com>
AuthorDate: Wed Jun 29 13:40:34 2022 -0600
[#8444] adde canonical tag and rel=next/prev where pagination is used to wiki, discussions and tickets
---
Allura/allura/lib/helpers.py | 15 +++++++++++++++
Allura/allura/templates/jinja_master/lib.html | 21 +++++++++++++++++++++
.../templates/discussionforums/search.html | 4 +++-
.../templates/discussionforums/thread.html | 10 ++++++----
.../forgediscussion/templates/index.html | 4 +++-
.../forgetracker/templates/tracker/index.html | 1 +
.../forgetracker/templates/tracker/milestone.html | 7 ++++++-
.../forgetracker/templates/tracker/search.html | 3 +++
.../forgetracker/tests/functional/test_root.py | 21 +++++++++++++++++++++
ForgeWiki/forgewiki/templates/wiki/browse.html | 7 ++++++-
ForgeWiki/forgewiki/templates/wiki/browse_tags.html | 7 ++++++-
ForgeWiki/forgewiki/templates/wiki/master.html | 7 +++++++
.../forgewiki/templates/wiki/page_history.html | 4 +++-
ForgeWiki/forgewiki/templates/wiki/page_view.html | 7 ++++---
ForgeWiki/forgewiki/templates/wiki/search.html | 2 ++
ForgeWiki/forgewiki/tests/functional/test_root.py | 7 +++++++
16 files changed, 114 insertions(+), 13 deletions(-)
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index c6ec0e973..38b335747 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -65,6 +65,9 @@ from webob.exc import HTTPUnauthorized
from allura.lib import exceptions as exc
from allura.lib import utils
+import urllib.parse as urlparse
+from urllib.parse import urlencode
+from webob.multidict import MultiDict
# import to make available to templates, don't delete:
from .security import has_access, is_allowed_by_role, is_site_admin
@@ -152,6 +155,18 @@ def make_safe_path_portion(ustr, relaxed=True):
def escape_json(data):
return json.dumps(data).replace('<', '\\u003C')
+def querystring(request, data):
+ params = urlparse.parse_qs(request.query_string)
+ params.update(data)
+ for param in list(params.keys()):
+ if params[param] is None:
+ del params[param]
+ # flatten dict values
+ params = {k: v[0] if isinstance(v, list) else v for k, v in params.items()}
+ url_parts = urlparse.urlparse(request.url)
+ url = url_parts._replace(query=urlencode(params)).geturl()
+ return url
+
def strip_bad_unicode(s):
"""
diff --git a/Allura/allura/templates/jinja_master/lib.html b/Allura/allura/templates/jinja_master/lib.html
index 250682e49..8d6741b7c 100644
--- a/Allura/allura/templates/jinja_master/lib.html
+++ b/Allura/allura/templates/jinja_master/lib.html
@@ -885,3 +885,24 @@ This page is based on some examples from Greg Schueler, <a href="mailto:greg@var
{%- do g.register_forge_js('js/create-react-class.min.js', location=location) %}
{%- do g.register_forge_js('js/prop-types.min.js', location=location) %}
{%- endmacro %}
+
+{% macro canonical_tag() %}
+{% set page= '?page=' ~ request.GET['page'] if 'page=' in request.query_string and request.GET['page']|int > 0 else '' %}
+<link rel="canonical" href="{{ request.host_url ~ request.path }}{{ page }}"/>
+{% endmacro %}
+
+{% macro pagination_meta_tags(request, last_page=None) %}
+{% if 'page=' in request.query_string and request.GET['page']|int > 0 and last_page %}
+ {% set current_page = request.GET['page']|int %}
+ {%- if current_page > 1 -%}
+ <link rel="prev" href="{{ h.querystring(request, dict(page=current_page-1,limit=None)) }}"/>
+ {% elif current_page == 1 %}
+ <link rel="prev" href="{{ request.path_qs.split('?')[0] }}"/>
+ {% endif %}
+
+ {% if current_page+1 < last_page -%}
+ <link rel="next" href="{{ h.querystring(request, dict(page=current_page+1,limit=None))}}"/>
+ {% endif %}
+{%- endif %}
+
+{% endmacro %}
\ No newline at end of file
diff --git a/ForgeDiscussion/forgediscussion/templates/discussionforums/search.html b/ForgeDiscussion/forgediscussion/templates/discussionforums/search.html
index 85de03713..19e0373c2 100644
--- a/ForgeDiscussion/forgediscussion/templates/discussionforums/search.html
+++ b/ForgeDiscussion/forgediscussion/templates/discussionforums/search.html
@@ -17,11 +17,13 @@
under the License.
-#}
{% extends g.theme.master %}
-
+{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
{% block title %}{{c.project.name}} / {{c.app.config.options.mount_label}} / Search{% endblock %}
{% block head %}
<meta name="robots" content="noindex, follow">
+ {{ lib.canonical_tag() }}
+ {{ lib.pagination_meta_tags(request, count) }}
{% endblock %}
{% block header %}Search {{c.app.config.options.mount_point}}: {{q}}{% endblock %}
diff --git a/ForgeDiscussion/forgediscussion/templates/discussionforums/thread.html b/ForgeDiscussion/forgediscussion/templates/discussionforums/thread.html
index cbf05173d..9f1ed0530 100644
--- a/ForgeDiscussion/forgediscussion/templates/discussionforums/thread.html
+++ b/ForgeDiscussion/forgediscussion/templates/discussionforums/thread.html
@@ -17,14 +17,16 @@
under the License.
-#}
{% extends g.theme.master %}
-
+{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
{% block title %}
- {{c.project.name}} / {{c.app.config.options.mount_label}} /
+ {{c.project.name}} / {{c.app.config.options.mount_label}} /
{{thread.subject and '%s: %s' % (thread.discussion.name, (thread.subject or '(no subject)')) or thread.discussion.name}}
{% endblock %}
-
+{% block head %}
+ {{ lib.canonical_tag() }}
+ {{ lib.pagination_meta_tags(request, count) }}
+{% endblock %}
{% block header %}{{'subject' in thread and thread.subject or '(no subject)'}}{% endblock %}
-
{% block actions %}
{% if show_moderate and h.has_access(thread, 'moderate')() %}
{{ g.icons['edit'].render(id='mod_thread_link') }}
diff --git a/ForgeDiscussion/forgediscussion/templates/index.html b/ForgeDiscussion/forgediscussion/templates/index.html
index 57af42022..96529598d 100644
--- a/ForgeDiscussion/forgediscussion/templates/index.html
+++ b/ForgeDiscussion/forgediscussion/templates/index.html
@@ -17,12 +17,14 @@
under the License.
-#}
{% extends 'allura:templates/discussion/index.html' %}
-
+{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
{% block head %}
{{ super() }}
{% if not threads|length %}
<meta name="robots" content="noindex, follow">
{% endif %}
+ {{ lib.canonical_tag() }}
+ {{ lib.pagination_meta_tags(request, count) }}
{% endblock %}
{% block actions %}
diff --git a/ForgeTracker/forgetracker/templates/tracker/index.html b/ForgeTracker/forgetracker/templates/tracker/index.html
index c4c3cb189..92bda4c0a 100644
--- a/ForgeTracker/forgetracker/templates/tracker/index.html
+++ b/ForgeTracker/forgetracker/templates/tracker/index.html
@@ -25,6 +25,7 @@
{% block head %}
<link rel="alternate" type="application/rss+xml" title="RSS" href="feed.rss"/>
<link rel="alternate" type="application/atom+xml" title="Atom" href="feed.atom"/>
+ {{ lib.canonical_tag() }}
{% endblock %}
{% block header %}{{c.app.config.options.mount_label}}{% endblock %}
diff --git a/ForgeTracker/forgetracker/templates/tracker/milestone.html b/ForgeTracker/forgetracker/templates/tracker/milestone.html
index bb695f122..1a65a4d51 100644
--- a/ForgeTracker/forgetracker/templates/tracker/milestone.html
+++ b/ForgeTracker/forgetracker/templates/tracker/milestone.html
@@ -18,13 +18,18 @@
-#}
{% extends g.theme.master %}
{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
-
+{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
{% do g.register_app_css('css/tracker.css') %}
{% block title %}{{c.project.name}} / {{c.app.config.options.mount_label}} / {{field.label}} {{milestone.name}}{% endblock %}
{% block header %}{{field.label}} {{milestone.name}}{% endblock %}
+{% block head %}
+ {{ lib.canonical_tag() }}
+ {{ lib.pagination_meta_tags(request, count) }}
+{% endblock %}
+
{% block actions %}
{{ lib.maximize_content_button() }}
{% if allow_edit %}
diff --git a/ForgeTracker/forgetracker/templates/tracker/search.html b/ForgeTracker/forgetracker/templates/tracker/search.html
index 03c86c914..62b1d2167 100644
--- a/ForgeTracker/forgetracker/templates/tracker/search.html
+++ b/ForgeTracker/forgetracker/templates/tracker/search.html
@@ -30,6 +30,9 @@
{% endif %}
<link rel="alternate" type="application/rss+xml" title="RSS" href="feed.rss"/>
<link rel="alternate" type="application/atom+xml" title="Atom" href="feed.atom"/>
+ {{ lib.canonical_tag() }}
+ {{ lib.pagination_meta_tags(request, count) }}
+
{% endblock %}
{% block actions %}
diff --git a/ForgeTracker/forgetracker/tests/functional/test_root.py b/ForgeTracker/forgetracker/tests/functional/test_root.py
index a5c8b23b3..c42237a67 100644
--- a/ForgeTracker/forgetracker/tests/functional/test_root.py
+++ b/ForgeTracker/forgetracker/tests/functional/test_root.py
@@ -1380,6 +1380,7 @@ class TestFunctionalController(TrackerTestController):
M.MonQTask.run_ready()
ThreadLocalORMSession.flush_all()
response = self.app.get('/p/test/bugs/search/?q=test&limit=2')
+ response.mustcontain('canonical')
response.mustcontain('results of 3')
response.mustcontain('test second ticket')
next_page_link = response.html.select('.page_list a')[0]
@@ -1390,6 +1391,26 @@ class TestFunctionalController(TrackerTestController):
# 'filter' is special kwarg, don't let it cause problems
r = self.app.get('/p/test/bugs/search/?q=test&filter=blah')
+ def test_search_canonical(self):
+ self.new_ticket(summary='test first ticket')
+ self.new_ticket(summary='test second ticket')
+ self.new_ticket(summary='test third ticket')
+ self.new_ticket(summary='test fourth ticket')
+ self.new_ticket(summary='test fifth ticket')
+ self.new_ticket(summary='test sixth ticket')
+ ThreadLocalORMSession.flush_all()
+ M.MonQTask.run_ready()
+ ThreadLocalORMSession.flush_all()
+ response = self.app.get('/p/test/bugs/search/?q=test&limit=2')
+ canonical = response.html.select_one('link[rel=canonical]')
+ assert ('limit=2' not in canonical['href'])
+ response = self.app.get('/p/test/bugs/search/?q=test&limit=2&page=3')
+ next = response.html.select_one('link[rel=next]')
+ assert ('page=4' in next['href'])
+ prev = response.html.select_one('link[rel=prev]')
+ assert ('page=2' in prev['href'])
+
+
def test_search_with_strange_chars(self):
r = self.app.get('/p/test/bugs/search/?' +
urlencode({'q': 'tést'}))
diff --git a/ForgeWiki/forgewiki/templates/wiki/browse.html b/ForgeWiki/forgewiki/templates/wiki/browse.html
index dcc49059f..8f2493a60 100644
--- a/ForgeWiki/forgewiki/templates/wiki/browse.html
+++ b/ForgeWiki/forgewiki/templates/wiki/browse.html
@@ -18,9 +18,14 @@
-#}
{% extends 'forgewiki:templates/wiki/master.html' %}
{% from 'allura:templates/jinja_master/lib.html' import abbr_date with context %}
-
+{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
{% block title %}{{c.project.name}} / {{c.app.config.options.mount_label}} / Browse Pages{% endblock %}
+{% block head %}
+ {{ super() }}
+ {{ lib.pagination_meta_tags(request, count) }}
+{% endblock %}
+
{% block header %}Browse Pages{% endblock %}
{% block wiki_content %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/browse_tags.html b/ForgeWiki/forgewiki/templates/wiki/browse_tags.html
index e464a39ba..98a76a216 100644
--- a/ForgeWiki/forgewiki/templates/wiki/browse_tags.html
+++ b/ForgeWiki/forgewiki/templates/wiki/browse_tags.html
@@ -17,9 +17,14 @@
under the License.
-#}
{% extends 'forgewiki:templates/wiki/master.html' %}
-
+{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
{% block title %}{{c.project.name}} / {{c.app.config.options.mount_label}} / Browse Labels{% endblock %}
+{% block head %}
+ {{ super() }}
+ {{ lib.pagination_meta_tags(request, count) }}
+{% endblock %}
+
{% block header %}Browse Labels{% endblock %}
{% block wiki_content %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/master.html b/ForgeWiki/forgewiki/templates/wiki/master.html
index e2f561ea8..37d237a59 100644
--- a/ForgeWiki/forgewiki/templates/wiki/master.html
+++ b/ForgeWiki/forgewiki/templates/wiki/master.html
@@ -17,7 +17,14 @@
under the License.
-#}
{% extends g.theme.master %}
+{% import g.theme.jinja_macros as theme_macros with context %}
{% do g.register_app_css('css/wiki.css', compress=False) %}
+{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
+
+{% block head %}
+ {{ lib.canonical_tag() }}
+{% endblock %}
+
{% block edit_box %}
{% if show_meta %}{% block wiki_meta %}{% endblock %}{% endif %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/page_history.html b/ForgeWiki/forgewiki/templates/wiki/page_history.html
index 4bc43b11a..ccc41acdc 100644
--- a/ForgeWiki/forgewiki/templates/wiki/page_history.html
+++ b/ForgeWiki/forgewiki/templates/wiki/page_history.html
@@ -19,11 +19,13 @@
{% extends 'forgewiki:templates/wiki/master.html' %}
{% from 'allura:templates/jinja_master/lib.html' import abbr_date with context %}
{% import 'allura:templates/jinja_master/dialog_macros.html' as dialog_macros with context %}
-
+{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
{% block title %}{{c.project.name}} / {{c.app.config.options.mount_label}} / {{title}}{% endblock %}
{% block head %}
+ {{ super() }}
<meta name="robots" content="noindex,follow" />
+ {{ lib.pagination_meta_tags(request, count) }}
{% endblock %}
{% block header %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/page_view.html b/ForgeWiki/forgewiki/templates/wiki/page_view.html
index e26d9e149..07b1249b4 100644
--- a/ForgeWiki/forgewiki/templates/wiki/page_view.html
+++ b/ForgeWiki/forgewiki/templates/wiki/page_view.html
@@ -19,6 +19,7 @@
{% extends 'forgewiki:templates/wiki/master.html' %}
{% do g.register_forge_css('css/forge/hilite.css', compress=False) %}
{% import 'allura:templates/jinja_master/lib.html' as lib with context %}
+{% import g.theme.jinja_macros as theme_macros with context %}
{% set base_url = page.url().split('?')[0] %}
@@ -34,14 +35,14 @@
{% endblock %}
{% block head %}
- {% if noindex %}
+ {{ super() }}
+ {%- if noindex -%}
<meta name="robots" content="noindex, follow">
- {% endif %}
+ {%- endif -%}
<link rel="alternate" type="application/rss+xml" title="Page RSS" href="feed.rss"/>
<link rel="alternate" type="application/atom+xml" title="Page Atom" href="feed.atom"/>
<link rel="alternate" type="application/rss+xml" title="Wiki RSS" href="../feed.rss"/>
<link rel="alternate" type="application/atom+xml" title="Wiki Atom" href="../feed.atom"/>
-<link rel="canonical" href="{{ h.absurl(base_url) }}">
{% endblock %}
{% block body_css_class %} {{super()}} wiki-{{(page.title).replace(' ','_')}}{% endblock %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/search.html b/ForgeWiki/forgewiki/templates/wiki/search.html
index c13f1171b..236308d5f 100644
--- a/ForgeWiki/forgewiki/templates/wiki/search.html
+++ b/ForgeWiki/forgewiki/templates/wiki/search.html
@@ -21,7 +21,9 @@
{% block title %}{{c.project.name}} / {{c.app.config.options.mount_label}} / Search{% endblock %}
{% block head %}
+ {{ super() }}
<meta name="robots" content="noindex, follow">
+ {{ lib.pagination_meta_tags(request, count) }}
{% endblock %}
{% block header %}Search {{c.app.config.options.mount_point}}: {{q}}{% endblock %}
diff --git a/ForgeWiki/forgewiki/tests/functional/test_root.py b/ForgeWiki/forgewiki/tests/functional/test_root.py
index 698e0feb3..cf79c1748 100644
--- a/ForgeWiki/forgewiki/tests/functional/test_root.py
+++ b/ForgeWiki/forgewiki/tests/functional/test_root.py
@@ -478,6 +478,13 @@ class TestRootController(TestController):
r = self.app.get('/wiki/browse_tags/?page=3')
assert '<td>label77</td>' in r
assert '<td>label99</td>' in r
+ r.mustcontain('canonical')
+ canonical = r.html.select_one('link[rel=canonical]')
+ assert 'browse_tags' in canonical['href']
+ next = r.html.select_one('link[rel=next]')
+ assert('page=4' in next['href'])
+ prev = r.html.select_one('link[rel=prev]')
+ assert('page=2' in prev['href'])
def test_new_attachment(self):
self.app.post(