Fixes Images / documents pagination (#2700)

Refactored pagination_nav template to allow empty linkurl in which case
url is created replacing page in QUERY_STRING.

CHANGELOG.txt

@@ -30,6 +30,7 @@ Changelog
  * Fix: `InlinePanel` now accepts a `classname` parameter as per the documentation (emg36, Matt Westcott)
  * Fix: Disabled use of escape key to revert content of rich text fields, which could cause accidental data loss (Matt Westcott)
  * Fix: Setting `USE_THOUSAND_SEPARATOR = True` no longer breaks the rendering of numbers in JS code for InlinePanel (Mattias Loverot, Matt Westcott)
+ * Fix: Images / documents pagination now preserves GET parameters (Bojan Mihelac)
 
 
 1.6.3 (30.09.2016)

docs/releases/1.7.rst

@@ -65,6 +65,7 @@ Bug fixes
  * ``InlinePanel`` now accepts a ``classname`` parameter as per the documentation (emg36, Matt Westcott)
  * Disabled use of escape key to revert content of rich text fields, which could cause accidental data loss (Matt Westcott)
  * Setting ``USE_THOUSAND_SEPARATOR = True`` no longer breaks the rendering of numbers in JS code for InlinePanel (Mattias Loverot, Matt Westcott)
+ * Images / documents pagination now preserves GET parameters (Bojan Mihelac)
 
 
 Upgrade considerations

wagtail/utils/pagination.py

@@ -1,6 +1,9 @@
 from __future__ import absolute_import, unicode_literals
 
 from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
+from django.utils.http import urlencode
+from django.utils.six.moves.urllib.parse import parse_qs
+
 
 DEFAULT_PAGE_KEY = 'p'
 
@@ -17,3 +20,20 @@ def paginate(request, items, page_key=DEFAULT_PAGE_KEY, per_page=20):
         page = paginator.page(paginator.num_pages)
 
     return paginator, page
+
+
+def replace_page_in_query(query, page_number, page_key=DEFAULT_PAGE_KEY):
+    """
+    Replaces ``page_key`` from query string with ``page_number``.
+
+    >>> replace_page_in_query("p=1&key=value", 2)
+    'p=2&key=value'
+    >>> replace_page_in_query("p=1&key=value", None)
+    'key=value'
+    """
+    getvars = parse_qs(query)
+    if page_number is None:
+        getvars.pop(page_key, None)
+    else:
+        getvars[page_key] = page_number
+    return urlencode(getvars, True)

wagtail/wagtailadmin/templates/wagtailadmin/shared/pagination_nav.html

@@ -1,4 +1,5 @@
 {% load i18n %}
+{% load wagtailadmin_tags %}
 {% if not is_ajax %}
     {% comment %}
         HACK: This template expects to be passed a 'linkurl' parameter, containing a URL name
@@ -8,9 +9,15 @@
         of the form "?q=123", implicitly preserving the current URL path.
         Using the {% url ... as ... %} form of the tag ensures that this fails silently,
         rather than throwing a NoReverseMatch exception.
+
+        If 'linkurl' is not passed, it will instead preserve the current URL and parameters,
+        just replacing the 'p' parameter.
     {% endcomment %}
-    {% url linkurl as url_to_use %}
+    {% if linkurl %}
+      {% url linkurl as url_to_use %}
+    {% endif %}
 {% endif %}
+
 <div class="pagination">
     <p>{% blocktrans with page_num=items.number total_pages=items.paginator.num_pages %}Page {{ page_num }} of {{ total_pages }}.{% endblocktrans %}</p>
     <ul>
@@ -20,6 +27,8 @@
                     <a href="#" data-page="{{ items.previous_page_number }}" class="icon icon-arrow-left">{% trans 'Previous' %}</a>
                 {% elif is_searching %}
                     <a href="{{ url_to_use }}?q={{ query_string|urlencode }}&amp;p={{ items.previous_page_number }}" class="icon icon-arrow-left">{% trans 'Previous' %}</a>
+                {% elif not linkurl %}
+                    <a href="?{% replace_page_param request.META.QUERY_STRING items.previous_page_number %}" class="icon icon-arrow-left">{% trans 'Previous' %}</a>
                 {% else %}
                     <a href="{{ url_to_use }}?p={{ items.previous_page_number }}&amp;ordering={{ ordering }}" class="icon icon-arrow-left">{% trans 'Previous' %}</a>
                 {% endif %}
@@ -31,6 +40,8 @@
                     <a href="#" data-page="{{ items.next_page_number }}" class="icon icon-arrow-right-after">{% trans 'Next' %}</a>
                 {% elif is_searching %}
                     <a href="{{ url_to_use }}?q={{ query_string|urlencode }}&amp;p={{ items.next_page_number }}" class="icon icon-arrow-right-after">{% trans 'Next' %}</a>
+                {% elif not linkurl %}
+                    <a href="?{% replace_page_param request.META.QUERY_STRING items.next_page_number %}" class="icon icon-arrow-right-after">{% trans 'Next' %}</a>
                 {% else %}
                     <a href="{{ url_to_use }}?p={{ items.next_page_number }}&amp;ordering={{ ordering }}" class="icon icon-arrow-right-after">{% trans 'Next' %}</a>
                 {% endif %}

wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py

@@ -2,6 +2,7 @@ from __future__ import absolute_import, unicode_literals
 
 import itertools
 
+
 import django
 from django import template
 from django.conf import settings
@@ -10,7 +11,7 @@ from django.contrib.messages.constants import DEFAULT_TAGS as MESSAGE_TAGS
 from django.template.defaultfilters import stringfilter
 from django.utils.safestring import mark_safe
 
-from wagtail.utils.pagination import DEFAULT_PAGE_KEY
+from wagtail.utils.pagination import DEFAULT_PAGE_KEY, replace_page_in_query
 from wagtail.wagtailadmin.menu import admin_menu
 from wagtail.wagtailadmin.navigation import get_navigation_menu_items
 from wagtail.wagtailadmin.search import admin_search_areas
@@ -329,3 +330,11 @@ def message_tags(message):
         return level_tag
     else:
         return ''
+
+
+@register.simple_tag
+def replace_page_param(query, page_number, page_key='p'):
+    """
+    Replaces ``page_key`` from query string with ``page_number``.
+    """
+    return replace_page_in_query(query, page_number, page_key)

wagtail/wagtaildocs/templates/wagtaildocs/documents/results.html

@@ -14,7 +14,7 @@
 
     {% include "wagtaildocs/documents/list.html" %}
 
-    {% include "wagtailadmin/shared/pagination_nav.html" with items=documents is_searching=is_searching linkurl="wagtaildocs:index" %}
+    {% include "wagtailadmin/shared/pagination_nav.html" with items=documents is_searching=is_searching %}
 {% else %}
     {% if is_searching %}
          <h2>{% blocktrans %}Sorry, no documents match "<em>{{ query_string }}</em>"{% endblocktrans %}</h2>

wagtail/wagtailimages/templates/wagtailimages/images/results.html

@@ -26,7 +26,7 @@
         {% endfor %}
     </ul>
 
-    {% include "wagtailadmin/shared/pagination_nav.html" with items=images is_searching=is_searching query_string=query_string linkurl="wagtailimages:index" %}
+    {% include "wagtailadmin/shared/pagination_nav.html" with items=images is_searching=is_searching query_string=query_string %}
 
 {% else %}
     {% if is_searching %}