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/07/13 16:04:10 UTC
[allura] 02/02: [#8444] updated pagination_meta_tags with new args, added test and comments for querystring helper
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 29a826c211df894a41a5da7b5825c4d9881270f7
Author: Guillermo Cruz <gu...@slashdotmedia.com>
AuthorDate: Wed Jul 13 08:55:20 2022 -0600
[#8444] updated pagination_meta_tags with new args, added test and comments for querystring helper
---
Allura/allura/lib/helpers.py | 15 +++++++++++++--
Allura/allura/templates/jinja_master/lib.html | 17 +++++------------
Allura/allura/tests/test_helpers.py | 13 +++++++++++++
ForgeBlog/forgeblog/templates/blog/index.html | 4 +++-
.../templates/discussionforums/search.html | 2 +-
.../templates/discussionforums/thread.html | 2 +-
ForgeDiscussion/forgediscussion/templates/index.html | 2 +-
ForgeTracker/forgetracker/templates/tracker/index.html | 2 ++
.../forgetracker/templates/tracker/milestone.html | 3 +--
ForgeTracker/forgetracker/templates/tracker/search.html | 2 +-
ForgeWiki/forgewiki/templates/wiki/browse.html | 2 +-
ForgeWiki/forgewiki/templates/wiki/browse_tags.html | 2 +-
ForgeWiki/forgewiki/templates/wiki/page_history.html | 2 +-
ForgeWiki/forgewiki/templates/wiki/search.html | 2 +-
14 files changed, 45 insertions(+), 25 deletions(-)
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index 38b335747..9bd0eb57e 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -67,6 +67,7 @@ from allura.lib import exceptions as exc
from allura.lib import utils
import urllib.parse as urlparse
from urllib.parse import urlencode
+import math
from webob.multidict import MultiDict
# import to make available to templates, don't delete:
@@ -155,9 +156,17 @@ def make_safe_path_portion(ustr, relaxed=True):
def escape_json(data):
return json.dumps(data).replace('<', '\\u003C')
-def querystring(request, data):
+def querystring(request, url_params):
+ """
+ add/update/remove url parameters. When a value is set to None the key will
+ be removed from the final constructed url.
+
+ :param request: request object
+ :param url_params: dict with the params that should be updated/added/deleted.
+ :return: a full url with updated url parameters.
+ """
params = urlparse.parse_qs(request.query_string)
- params.update(data)
+ params.update(url_params)
for param in list(params.keys()):
if params[param] is None:
del params[param]
@@ -167,6 +176,8 @@ def querystring(request, data):
url = url_parts._replace(query=urlencode(params)).geturl()
return url
+def ceil(number):
+ return math.ceil(number)
def strip_bad_unicode(s):
"""
diff --git a/Allura/allura/templates/jinja_master/lib.html b/Allura/allura/templates/jinja_master/lib.html
index 568ee3503..027913e9c 100644
--- a/Allura/allura/templates/jinja_master/lib.html
+++ b/Allura/allura/templates/jinja_master/lib.html
@@ -889,20 +889,13 @@ This page is based on some examples from Greg Schueler, <a href="mailto:greg@var
{% 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 %}
+{% 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 -%}
+{% macro pagination_meta_tags(request, current_page=None, results_count=None, limit=None) %}
+ {%- if current_page > 0 -%}
<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))}}"/>
+ {% if results_count and current_page+1 < h.ceil(results_count/limit) -%}
+ <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/Allura/allura/tests/test_helpers.py b/Allura/allura/tests/test_helpers.py
index 3c1428586..650ba023f 100644
--- a/Allura/allura/tests/test_helpers.py
+++ b/Allura/allura/tests/test_helpers.py
@@ -472,6 +472,7 @@ class TestUrlOpen(TestCase):
def side_effect(url, timeout=None):
raise socket.timeout()
+
urlopen.side_effect = side_effect
self.assertRaises(socket.timeout, h.urlopen, 'myurl')
self.assertEqual(urlopen.call_count, 4)
@@ -483,6 +484,7 @@ class TestUrlOpen(TestCase):
def side_effect(url, timeout=None):
raise OSError(errno.ECONNRESET, 'Connection reset by peer')
+
urlopen.side_effect = side_effect
self.assertRaises(socket.error, h.urlopen, 'myurl')
self.assertEqual(urlopen.call_count, 4)
@@ -493,6 +495,7 @@ class TestUrlOpen(TestCase):
def side_effect(url, timeout=None):
raise HTTPError('url', 408, 'timeout', None, None)
+
urlopen.side_effect = side_effect
self.assertRaises(HTTPError, h.urlopen, 'myurl')
self.assertEqual(urlopen.call_count, 4)
@@ -503,6 +506,7 @@ class TestUrlOpen(TestCase):
def side_effect(url, timeout=None):
raise HTTPError('url', 404, 'timeout', None, None)
+
urlopen.side_effect = side_effect
self.assertRaises(HTTPError, h.urlopen, 'myurl')
self.assertEqual(urlopen.call_count, 1)
@@ -681,3 +685,12 @@ def test_hide_private_info():
def test_emojize():
assert_equals(h.emojize(':smile:'), '😄')
+
+
+def test_querystring():
+ req = Request.blank('/p/test/foobar?page=1&limit=10&count=100', remote_addr='127.0.0.1',
+ base_url='https://mysite.com/p/test/foobar')
+ assert_equals(h.querystring(req, dict(page=2, limit=5)),
+ 'https://mysite.com/p/test/foobar/p/test/foobar?page=2&limit=5&count=100')
+ assert_equals(h.querystring(req, dict(page=5, limit=2, count=None)),
+ 'https://mysite.com/p/test/foobar/p/test/foobar?page=5&limit=2')
diff --git a/ForgeBlog/forgeblog/templates/blog/index.html b/ForgeBlog/forgeblog/templates/blog/index.html
index b9e82d71c..8998b54d0 100644
--- a/ForgeBlog/forgeblog/templates/blog/index.html
+++ b/ForgeBlog/forgeblog/templates/blog/index.html
@@ -17,13 +17,15 @@
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}}{% endblock %}
{% block head %}
{% if count == 0 %}
<meta name="robots" content="noindex, follow"/>
{% endif %}
+ {{ lib.canonical_tag() }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
{% block header %}{{c.project.name}} / {{c.app.config.options.mount_label}}: Recent posts{% endblock %}
diff --git a/ForgeDiscussion/forgediscussion/templates/discussionforums/search.html b/ForgeDiscussion/forgediscussion/templates/discussionforums/search.html
index 19e0373c2..bd2568393 100644
--- a/ForgeDiscussion/forgediscussion/templates/discussionforums/search.html
+++ b/ForgeDiscussion/forgediscussion/templates/discussionforums/search.html
@@ -23,7 +23,7 @@
{% block head %}
<meta name="robots" content="noindex, follow">
{{ lib.canonical_tag() }}
- {{ lib.pagination_meta_tags(request, count) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% 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 9f1ed0530..c06d9676b 100644
--- a/ForgeDiscussion/forgediscussion/templates/discussionforums/thread.html
+++ b/ForgeDiscussion/forgediscussion/templates/discussionforums/thread.html
@@ -24,7 +24,7 @@
{% endblock %}
{% block head %}
{{ lib.canonical_tag() }}
- {{ lib.pagination_meta_tags(request, count) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
{% block header %}{{'subject' in thread and thread.subject or '(no subject)'}}{% endblock %}
{% block actions %}
diff --git a/ForgeDiscussion/forgediscussion/templates/index.html b/ForgeDiscussion/forgediscussion/templates/index.html
index 96529598d..8f199a05f 100644
--- a/ForgeDiscussion/forgediscussion/templates/index.html
+++ b/ForgeDiscussion/forgediscussion/templates/index.html
@@ -24,7 +24,7 @@
<meta name="robots" content="noindex, follow">
{% endif %}
{{ lib.canonical_tag() }}
- {{ lib.pagination_meta_tags(request, count) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
{% block actions %}
diff --git a/ForgeTracker/forgetracker/templates/tracker/index.html b/ForgeTracker/forgetracker/templates/tracker/index.html
index 92bda4c0a..d0ddfc337 100644
--- a/ForgeTracker/forgetracker/templates/tracker/index.html
+++ b/ForgeTracker/forgetracker/templates/tracker/index.html
@@ -26,8 +26,10 @@
<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, page, count, limit) }}
{% endblock %}
+
{% block header %}{{c.app.config.options.mount_label}}{% endblock %}
{% block actions %}
diff --git a/ForgeTracker/forgetracker/templates/tracker/milestone.html b/ForgeTracker/forgetracker/templates/tracker/milestone.html
index 1a65a4d51..35ecf8a3c 100644
--- a/ForgeTracker/forgetracker/templates/tracker/milestone.html
+++ b/ForgeTracker/forgetracker/templates/tracker/milestone.html
@@ -18,7 +18,6 @@
-#}
{% 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 %}
@@ -27,7 +26,7 @@
{% block head %}
{{ lib.canonical_tag() }}
- {{ lib.pagination_meta_tags(request, count) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
{% block actions %}
diff --git a/ForgeTracker/forgetracker/templates/tracker/search.html b/ForgeTracker/forgetracker/templates/tracker/search.html
index 62b1d2167..e492043e0 100644
--- a/ForgeTracker/forgetracker/templates/tracker/search.html
+++ b/ForgeTracker/forgetracker/templates/tracker/search.html
@@ -31,7 +31,7 @@
<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) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/browse.html b/ForgeWiki/forgewiki/templates/wiki/browse.html
index 8f2493a60..3a96aa719 100644
--- a/ForgeWiki/forgewiki/templates/wiki/browse.html
+++ b/ForgeWiki/forgewiki/templates/wiki/browse.html
@@ -23,7 +23,7 @@
{% block head %}
{{ super() }}
- {{ lib.pagination_meta_tags(request, count) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
{% block header %}Browse Pages{% endblock %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/browse_tags.html b/ForgeWiki/forgewiki/templates/wiki/browse_tags.html
index 98a76a216..7225526b6 100644
--- a/ForgeWiki/forgewiki/templates/wiki/browse_tags.html
+++ b/ForgeWiki/forgewiki/templates/wiki/browse_tags.html
@@ -22,7 +22,7 @@
{% block head %}
{{ super() }}
- {{ lib.pagination_meta_tags(request, count) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
{% block header %}Browse Labels{% endblock %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/page_history.html b/ForgeWiki/forgewiki/templates/wiki/page_history.html
index ccc41acdc..da255c5dd 100644
--- a/ForgeWiki/forgewiki/templates/wiki/page_history.html
+++ b/ForgeWiki/forgewiki/templates/wiki/page_history.html
@@ -25,7 +25,7 @@
{% block head %}
{{ super() }}
<meta name="robots" content="noindex,follow" />
- {{ lib.pagination_meta_tags(request, count) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
{% block header %}
diff --git a/ForgeWiki/forgewiki/templates/wiki/search.html b/ForgeWiki/forgewiki/templates/wiki/search.html
index 236308d5f..9faff6f2c 100644
--- a/ForgeWiki/forgewiki/templates/wiki/search.html
+++ b/ForgeWiki/forgewiki/templates/wiki/search.html
@@ -23,7 +23,7 @@
{% block head %}
{{ super() }}
<meta name="robots" content="noindex, follow">
- {{ lib.pagination_meta_tags(request, count) }}
+ {{ lib.pagination_meta_tags(request, page, count, limit) }}
{% endblock %}
{% block header %}Search {{c.app.config.options.mount_point}}: {{q}}{% endblock %}