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:51 UTC
[allura] branch gc/8444 created (now ec66ea3c2)
This is an automated email from the ASF dual-hosted git repository.
gcruz pushed a change to branch gc/8444
in repository https://gitbox.apache.org/repos/asf/allura.git
at ec66ea3c2 [#8444] adde canonical tag and rel=next/prev where pagination is used to wiki, discussions and tickets
This branch includes the following new commits:
new ec66ea3c2 [#8444] adde canonical tag and rel=next/prev where pagination is used to wiki, discussions and tickets
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[allura] 01/01: [#8444] adde canonical tag and rel=next/prev where pagination is used to wiki, discussions and tickets
Posted by gc...@apache.org.
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(