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 <[email protected]> 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(
