From 7bdd71fa2d5a9e7f26991c9a4fd2974ab393a2a4 Mon Sep 17 00:00:00 2001 From: Tim Heap Date: Thu, 9 Jul 2015 13:33:57 +1000 Subject: [PATCH 1/3] Refactor pagination into a helper Pagination is done almost exactly the same across all of Wagtail. Moving the repeated code in to one spot ensures mistakes are not made, and means that pagination is always done consistently. There are some locations where the pagination helper has not been used, as there is no request available or something similar. These instances are special enough that refactoring them does not make sense at this point. Conflicts: wagtail/wagtailadmin/views/chooser.py wagtail/wagtailadmin/views/pages.py wagtail/wagtaildocs/views/chooser.py wagtail/wagtaildocs/views/documents.py wagtail/wagtailforms/views.py wagtail/wagtailimages/views/chooser.py wagtail/wagtailimages/views/images.py wagtail/wagtailredirects/views.py wagtail/wagtailsearch/views/editorspicks.py wagtail/wagtailsnippets/views/chooser.py wagtail/wagtailsnippets/views/snippets.py wagtail/wagtailusers/views/groups.py wagtail/wagtailusers/views/users.py --- .../contrib/wagtailsearchpromotions/views.py | 12 ++----- wagtail/tests/demosite/models.py | 12 ++----- wagtail/utils/pagination.py | 15 ++++++++ .../wagtailadmin/tests/test_pages_views.py | 5 ++- wagtail/wagtailadmin/views/chooser.py | 11 ++---- wagtail/wagtailadmin/views/pages.py | 34 +++---------------- wagtail/wagtaildocs/views/chooser.py | 22 ++---------- wagtail/wagtaildocs/views/documents.py | 23 ++----------- wagtail/wagtailforms/views.py | 24 +++---------- wagtail/wagtailimages/views/chooser.py | 22 ++---------- wagtail/wagtailimages/views/images.py | 24 ++----------- wagtail/wagtailredirects/views.py | 12 ++----- wagtail/wagtailsearch/views/queries.py | 13 ++----- wagtail/wagtailsnippets/views/chooser.py | 12 ++----- wagtail/wagtailsnippets/views/snippets.py | 24 ++----------- wagtail/wagtailusers/views/groups.py | 12 ++----- wagtail/wagtailusers/views/users.py | 12 ++----- 17 files changed, 56 insertions(+), 233 deletions(-) create mode 100644 wagtail/utils/pagination.py diff --git a/wagtail/contrib/wagtailsearchpromotions/views.py b/wagtail/contrib/wagtailsearchpromotions/views.py index 61d5e111bc..d6a7f40e35 100644 --- a/wagtail/contrib/wagtailsearchpromotions/views.py +++ b/wagtail/contrib/wagtailsearchpromotions/views.py @@ -1,10 +1,10 @@ from django.shortcuts import render, redirect, get_object_or_404 from django.core.urlresolvers import reverse -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.utils.translation import ugettext as _ from django.views.decorators.vary import vary_on_headers +from wagtail.utils.pagination import paginate from wagtail.wagtailsearch import forms as search_forms from wagtail.wagtailsearch.models import Query from wagtail.wagtailadmin.forms import SearchForm @@ -18,7 +18,6 @@ from wagtail.contrib.wagtailsearchpromotions import forms @vary_on_headers('X-Requested-With') def index(request): is_searching = False - page = request.GET.get('p', 1) query_string = request.GET.get('q', "") queries = Query.objects.filter(editors_picks__isnull=False).distinct() @@ -28,14 +27,7 @@ def index(request): queries = queries.filter(query_string__icontains=query_string) is_searching = True - # Pagination - paginator = Paginator(queries, 20) - try: - queries = paginator.page(page) - except PageNotAnInteger: - queries = paginator.page(1) - except EmptyPage: - queries = paginator.page(paginator.num_pages) + paginator, queries = paginate(request, queries) if request.is_ajax(): return render(request, "wagtailsearchpromotions/results.html", { diff --git a/wagtail/tests/demosite/models.py b/wagtail/tests/demosite/models.py index 979cce4f35..b7697401c1 100644 --- a/wagtail/tests/demosite/models.py +++ b/wagtail/tests/demosite/models.py @@ -1,12 +1,12 @@ from datetime import date from django.db import models -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from modelcluster.fields import ParentalKey from modelcluster.contrib.taggit import ClusterTaggableManager from taggit.models import TaggedItemBase +from wagtail.utils.pagination import paginate from wagtail.wagtailcore.models import Page, Orderable from wagtail.wagtailcore.fields import RichTextField from wagtail.wagtailadmin.edit_handlers import FieldPanel, MultiFieldPanel, \ @@ -334,15 +334,7 @@ class BlogIndexPage(Page): if tag: entries = entries.filter(tags__name=tag) - # Pagination - page = request.GET.get('page') - paginator = Paginator(entries, 10) # Show 10 entries per page - try: - entries = paginator.page(page) - except PageNotAnInteger: - entries = paginator.page(1) - except EmptyPage: - entries = paginator.page(paginator.num_pages) + paginator, entries = paginate(request, entries, page_key='page', per_page=10) # Update template context context = super(BlogIndexPage, self).get_context(request) diff --git a/wagtail/utils/pagination.py b/wagtail/utils/pagination.py new file mode 100644 index 0000000000..56e8c6345f --- /dev/null +++ b/wagtail/utils/pagination.py @@ -0,0 +1,15 @@ +from django.core.paginator import Paginator, PageNotAnInteger, EmptyPage + + +def paginate(request, items, page_key='p', per_page=20): + page = request.GET.get(page_key, 1) + + paginator = Paginator(items, per_page) + try: + page = paginator.page(page) + except PageNotAnInteger: + page = paginator.page(1) + except EmptyPage: + page = paginator.page(paginator.num_pages) + + return paginator, page diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 3d7503353f..22c34bedfa 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -5,8 +5,7 @@ from django.test import TestCase from django.core.urlresolvers import reverse from django.contrib.auth import get_user_model from django.contrib.auth.models import Group, Permission -from django.core import mail -from django.core.paginator import Paginator +from django.core import mail, paginator from django.db.models.signals import pre_delete, post_delete from django.utils import timezone @@ -80,7 +79,7 @@ class TestPageExplorer(TestCase, WagtailTestUtils): self.assertEqual(response.context['ordering'], 'ord') # Pages must not be paginated - self.assertNotIsInstance(response.context['pages'], Paginator) + self.assertNotIsInstance(response.context['pages'], paginator.Page) def make_pages(self): for i in range(150): diff --git a/wagtail/wagtailadmin/views/chooser.py b/wagtail/wagtailadmin/views/chooser.py index 4fb782dcc4..ce64e1824c 100644 --- a/wagtail/wagtailadmin/views/chooser.py +++ b/wagtail/wagtailadmin/views/chooser.py @@ -1,8 +1,8 @@ -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.shortcuts import get_object_or_404, render from django.http import Http404 from django.utils.http import urlencode +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.modal_workflow import render_modal_workflow from wagtail.wagtailadmin.forms import SearchForm, ExternalLinkChooserForm, ExternalLinkChooserWithLinkTextForm, EmailLinkChooserForm, EmailLinkChooserWithLinkTextForm @@ -86,14 +86,7 @@ def browse(request, parent_page_id=None): # Pagination # We apply pagination first so we don't need to walk the entire list # in the block below - p = request.GET.get('p', 1) - paginator = Paginator(pages, 25) - try: - pages = paginator.page(p) - except PageNotAnInteger: - pages = paginator.page(1) - except EmptyPage: - pages = paginator.page(paginator.num_pages) + paginator, pages = paginate(request, pages, per_page=25) # Annotate each page with can_choose/can_decend flags for page in pages: diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 4f984738e3..7437ee72cd 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -2,7 +2,6 @@ from django.http import Http404, HttpResponse from django.shortcuts import render, redirect, get_object_or_404 from django.core.exceptions import ValidationError, PermissionDenied from django.contrib.contenttypes.models import ContentType -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.core.urlresolvers import reverse from django.utils import timezone from django.utils.translation import ugettext as _ @@ -11,6 +10,7 @@ from django.views.decorators.http import require_GET, require_POST from django.views.decorators.vary import vary_on_headers from django.db.models import Count +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.edit_handlers import TabbedInterface, ObjectList from wagtail.wagtailadmin.forms import SearchForm, CopyForm from wagtail.wagtailadmin.utils import send_notification @@ -49,14 +49,7 @@ def index(request, parent_page_id=None): ordering_no_minus = ordering[1:] pages = pages.order_by(ordering).annotate(null_position=Count(ordering_no_minus)).order_by('-null_position', ordering) - p = request.GET.get('p', 1) - paginator = Paginator(pages, 50) - try: - pages = paginator.page(p) - except PageNotAnInteger: - pages = paginator.page(1) - except EmptyPage: - pages = paginator.page(paginator.num_pages) + paginator, pages = paginate(request, pages, per_page=50) return render(request, 'wagtailadmin/pages/index.html', { 'parent_page': parent_page, @@ -93,8 +86,6 @@ def content_type_use(request, content_type_app_name, content_type_model_name): except ContentType.DoesNotExist: raise Http404 - p = request.GET.get("p", 1) - page_class = content_type.model_class() # page_class must be a Page type and not some other random model @@ -103,14 +94,7 @@ def content_type_use(request, content_type_app_name, content_type_model_name): pages = page_class.objects.all() - paginator = Paginator(pages, 10) - - try: - pages = paginator.page(p) - except PageNotAnInteger: - pages = paginator.page(1) - except EmptyPage: - pages = paginator.page(paginator.num_pages) + paginator, pages = paginate(request, pages, per_page=10) return render(request, 'wagtailadmin/pages/content_type_use.html', { 'pages': pages, @@ -690,18 +674,8 @@ def search(request): if form.is_valid(): q = form.cleaned_data['q'] - # page number - p = request.GET.get("p", 1) pages = Page.objects.all().prefetch_related('content_type').search(q, fields=['title']) - - # Pagination - paginator = Paginator(pages, 20) - try: - pages = paginator.page(p) - except PageNotAnInteger: - pages = paginator.page(1) - except EmptyPage: - pages = paginator.page(paginator.num_pages) + paginator, pages = paginate(request, pages) else: form = SearchForm() diff --git a/wagtail/wagtaildocs/views/chooser.py b/wagtail/wagtaildocs/views/chooser.py index 5d8ee150f6..421e7a7ec2 100644 --- a/wagtail/wagtaildocs/views/chooser.py +++ b/wagtail/wagtaildocs/views/chooser.py @@ -2,8 +2,8 @@ import json from django.core.urlresolvers import reverse from django.shortcuts import get_object_or_404, render -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.modal_workflow import render_modal_workflow from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailadmin.utils import permission_required @@ -48,15 +48,7 @@ def chooser(request): is_searching = False # Pagination - p = request.GET.get("p", 1) - paginator = Paginator(documents, 10) - - try: - documents = paginator.page(p) - except PageNotAnInteger: - documents = paginator.page(1) - except EmptyPage: - documents = paginator.page(paginator.num_pages) + paginator, documents = paginate(request, documents, per_page=10) return render(request, "wagtaildocs/chooser/results.html", { 'documents': documents, @@ -67,15 +59,7 @@ def chooser(request): searchform = SearchForm() documents = Document.objects.order_by('-created_at') - p = request.GET.get("p", 1) - paginator = Paginator(documents, 10) - - try: - documents = paginator.page(p) - except PageNotAnInteger: - documents = paginator.page(1) - except EmptyPage: - documents = paginator.page(paginator.num_pages) + paginator, documents = paginate(request, documents, per_page=10) return render_modal_workflow(request, 'wagtaildocs/chooser/chooser.html', 'wagtaildocs/chooser/chooser.js', { 'documents': documents, diff --git a/wagtail/wagtaildocs/views/documents.py b/wagtail/wagtaildocs/views/documents.py index 9cd6c1d636..81ad616a36 100644 --- a/wagtail/wagtaildocs/views/documents.py +++ b/wagtail/wagtaildocs/views/documents.py @@ -1,10 +1,10 @@ from django.shortcuts import render, redirect, get_object_or_404 -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.core.exceptions import PermissionDenied from django.utils.translation import ugettext as _ from django.views.decorators.vary import vary_on_headers from django.core.urlresolvers import reverse +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailadmin.utils import permission_required, any_permission_required from wagtail.wagtailsearch.backends import get_search_backends @@ -43,15 +43,7 @@ def index(request): form = SearchForm(placeholder=_("Search documents")) # Pagination - p = request.GET.get('p', 1) - paginator = Paginator(documents, 20) - - try: - documents = paginator.page(p) - except PageNotAnInteger: - documents = paginator.page(1) - except EmptyPage: - documents = paginator.page(paginator.num_pages) + paginator, documents = paginate(request, documents) # Create response if request.is_ajax(): @@ -170,16 +162,7 @@ def delete(request, document_id): def usage(request, document_id): doc = get_object_or_404(Document, id=document_id) - # Pagination - p = request.GET.get('p', 1) - paginator = Paginator(doc.get_usage(), 20) - - try: - used_by = paginator.page(p) - except PageNotAnInteger: - used_by = paginator.page(1) - except EmptyPage: - used_by = paginator.page(paginator.num_pages) + paginator, used_by = paginate(request, doc.get_usage()) return render(request, "wagtaildocs/documents/usage.html", { 'document': doc, diff --git a/wagtail/wagtailforms/views.py b/wagtail/wagtailforms/views.py index 70bcf209bf..0700f30c37 100644 --- a/wagtail/wagtailforms/views.py +++ b/wagtail/wagtailforms/views.py @@ -2,30 +2,22 @@ import datetime import csv -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.core.exceptions import PermissionDenied from django.http import HttpResponse from django.shortcuts import get_object_or_404, render, redirect from django.utils.encoding import smart_str from django.utils.translation import ugettext as _ + +from wagtail.utils.pagination import paginate from wagtail.wagtailcore.models import Page from wagtail.wagtailforms.models import FormSubmission, get_forms_for_user from wagtail.wagtailforms.forms import SelectDateForm from wagtail.wagtailadmin import messages def index(request): - p = request.GET.get("p", 1) - form_pages = get_forms_for_user(request.user) - paginator = Paginator(form_pages, 20) - - try: - form_pages = paginator.page(p) - except PageNotAnInteger: - form_pages = paginator.page(1) - except EmptyPage: - form_pages = paginator.page(paginator.num_pages) + paginator, form_pages = paginate(request, form_pages) return render(request, 'wagtailforms/index.html', { 'form_pages': form_pages, @@ -95,15 +87,7 @@ def list_submissions(request, page_id): writer.writerow(data_row) return response - p = request.GET.get('p', 1) - paginator = Paginator(submissions, 20) - - try: - submissions = paginator.page(p) - except PageNotAnInteger: - submissions = paginator.page(1) - except EmptyPage: - submissions = paginator.page(paginator.num_pages) + paginator, submissions = paginate(request, submissions) data_headings = [label for name, label in data_fields] data_rows = [] diff --git a/wagtail/wagtailimages/views/chooser.py b/wagtail/wagtailimages/views/chooser.py index f6524f9278..315c9e2933 100644 --- a/wagtail/wagtailimages/views/chooser.py +++ b/wagtail/wagtailimages/views/chooser.py @@ -2,8 +2,8 @@ import json from django.core.urlresolvers import reverse from django.shortcuts import get_object_or_404, render -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.modal_workflow import render_modal_workflow from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailadmin.utils import permission_required @@ -55,15 +55,7 @@ def chooser(request): is_searching = False # Pagination - page_number = request.GET.get("p", 1) - paginator = Paginator(images, 12) - - try: - images = paginator.page(page_number) - except PageNotAnInteger: - images = paginator.page(1) - except EmptyPage: - images = paginator.page(paginator.num_pages) + paginator, images = paginate(request, images, per_page=12) return render(request, "wagtailimages/chooser/results.html", { 'images': images, @@ -75,15 +67,7 @@ def chooser(request): searchform = SearchForm() images = Image.objects.order_by('-created_at') - p = request.GET.get("p", 1) - paginator = Paginator(images, 12) - - try: - images = paginator.page(p) - except PageNotAnInteger: - images = paginator.page(1) - except EmptyPage: - images = paginator.page(paginator.num_pages) + paginator, images = paginate(request, images, per_page=12) return render_modal_workflow(request, 'wagtailimages/chooser/chooser.html', 'wagtailimages/chooser/chooser.js', { 'images': images, diff --git a/wagtail/wagtailimages/views/images.py b/wagtail/wagtailimages/views/images.py index cbe409d04f..f4f93160d3 100644 --- a/wagtail/wagtailimages/views/images.py +++ b/wagtail/wagtailimages/views/images.py @@ -1,13 +1,13 @@ import os from django.shortcuts import render, redirect, get_object_or_404 -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.core.exceptions import PermissionDenied from django.utils.translation import ugettext as _ from django.views.decorators.vary import vary_on_headers from django.core.urlresolvers import reverse, NoReverseMatch from django.http import HttpResponse, JsonResponse +from wagtail.utils.pagination import paginate from wagtail.wagtailcore.models import Site from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailadmin import messages @@ -44,16 +44,7 @@ def index(request): else: form = SearchForm(placeholder=_("Search images")) - # Pagination - p = request.GET.get('p', 1) - paginator = Paginator(images, 20) - - try: - images = paginator.page(p) - except PageNotAnInteger: - images = paginator.page(1) - except EmptyPage: - images = paginator.page(paginator.num_pages) + paginator, images = paginate(request, images) # Create response if request.is_ajax(): @@ -253,16 +244,7 @@ def add(request): def usage(request, image_id): image = get_object_or_404(get_image_model(), id=image_id) - # Pagination - p = request.GET.get('p', 1) - paginator = Paginator(image.get_usage(), 20) - - try: - used_by = paginator.page(p) - except PageNotAnInteger: - used_by = paginator.page(1) - except EmptyPage: - used_by = paginator.page(paginator.num_pages) + paginator, used_by = paginate(request, image.get_usage()) return render(request, "wagtailimages/images/usage.html", { 'image': image, diff --git a/wagtail/wagtailredirects/views.py b/wagtail/wagtailredirects/views.py index c07a7a28aa..fccd2f2c30 100644 --- a/wagtail/wagtailredirects/views.py +++ b/wagtail/wagtailredirects/views.py @@ -1,14 +1,13 @@ from django.shortcuts import render, redirect, get_object_or_404 -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.utils.translation import ugettext as _ from django.views.decorators.vary import vary_on_headers from django.core.urlresolvers import reverse +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.edit_handlers import ObjectList from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailadmin.utils import permission_required, any_permission_required from wagtail.wagtailadmin import messages - from wagtail.wagtailredirects import models @@ -18,7 +17,6 @@ REDIRECT_EDIT_HANDLER = ObjectList(models.Redirect.content_panels).bind_to_model @any_permission_required('wagtailredirects.add_redirect', 'wagtailredirects.change_redirect', 'wagtailredirects.delete_redirect') @vary_on_headers('X-Requested-With') def index(request): - page = request.GET.get('p', 1) query_string = request.GET.get('q', "") ordering = request.GET.get('ordering', 'old_path') @@ -36,13 +34,7 @@ def index(request): redirects = redirects.order_by(ordering) # Pagination - paginator = Paginator(redirects, 20) - try: - redirects = paginator.page(page) - except PageNotAnInteger: - redirects = paginator.page(1) - except EmptyPage: - redirects = paginator.page(paginator.num_pages) + paginator, redirects = paginate(request, redirects) # Render template if request.is_ajax(): diff --git a/wagtail/wagtailsearch/views/queries.py b/wagtail/wagtailsearch/views/queries.py index 0baee0ae4d..1912059ac3 100644 --- a/wagtail/wagtailsearch/views/queries.py +++ b/wagtail/wagtailsearch/views/queries.py @@ -1,6 +1,6 @@ from django.shortcuts import render -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.modal_workflow import render_modal_workflow from wagtail.wagtailadmin.forms import SearchForm @@ -22,16 +22,7 @@ def chooser(request, get_results=False): else: searchform = SearchForm() - # Pagination - p = request.GET.get('p', 1) - - paginator = Paginator(queries, 10) - try: - queries = paginator.page(p) - except PageNotAnInteger: - queries = paginator.page(1) - except EmptyPage: - queries = paginator.page(paginator.num_pages) + paginator, queries = paginate(request, queries, per_page=10) # Render if get_results: diff --git a/wagtail/wagtailsnippets/views/chooser.py b/wagtail/wagtailsnippets/views/chooser.py index ac8dd6b264..fc9e3b762c 100644 --- a/wagtail/wagtailsnippets/views/chooser.py +++ b/wagtail/wagtailsnippets/views/chooser.py @@ -1,11 +1,11 @@ import json -from django.core.paginator import Paginator, PageNotAnInteger, EmptyPage from django.core.urlresolvers import reverse from django.shortcuts import render, get_object_or_404 from django.utils.six import text_type from django.utils.translation import ugettext as _ +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.modal_workflow import render_modal_workflow from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailsearch.index import class_is_indexed @@ -43,15 +43,7 @@ def choose(request, content_type_app_name, content_type_model_name): }) # Pagination - p = request.GET.get("p", 1) - paginator = Paginator(items, 25) - - try: - paginated_items = paginator.page(p) - except PageNotAnInteger: - paginated_items = paginator.page(1) - except EmptyPage: - paginated_items = paginator.page(paginator.num_pages) + paginator, paginated_items = paginate(request, items, per_page=25) # If paginating or searching, render "results.html" if request.GET.get('results', None) == 'true': diff --git a/wagtail/wagtailsnippets/views/snippets.py b/wagtail/wagtailsnippets/views/snippets.py index 8db9d82d95..43d77f4247 100644 --- a/wagtail/wagtailsnippets/views/snippets.py +++ b/wagtail/wagtailsnippets/views/snippets.py @@ -5,8 +5,8 @@ from django.utils.text import capfirst from django.contrib.contenttypes.models import ContentType from django.utils.translation import ugettext as _ from django.core.urlresolvers import reverse -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin.edit_handlers import ObjectList, extract_panel_definitions_from_model_class from wagtail.wagtailadmin.utils import permission_denied @@ -113,16 +113,7 @@ def list(request, content_type_app_name, content_type_model_name): 'snippet_type_name': snippet_type_name_plural }) - # Pagination - p = request.GET.get('p', 1) - paginator = Paginator(items, 20) - - try: - paginated_items = paginator.page(p) - except PageNotAnInteger: - paginated_items = paginator.page(1) - except EmptyPage: - paginated_items = paginator.page(paginator.num_pages) + paginator, paginated_items = paginate(request, items) # Template if request.is_ajax(): @@ -269,16 +260,7 @@ def usage(request, content_type_app_name, content_type_model_name, id): model = content_type.model_class() instance = get_object_or_404(model, id=id) - # Pagination - p = request.GET.get('p', 1) - paginator = Paginator(instance.get_usage(), 20) - - try: - used_by = paginator.page(p) - except PageNotAnInteger: - used_by = paginator.page(1) - except EmptyPage: - used_by = paginator.page(paginator.num_pages) + paginator, used_by = paginate(request, instance.get_usage()) return render(request, "wagtailsnippets/snippets/usage.html", { 'instance': instance, diff --git a/wagtail/wagtailusers/views/groups.py b/wagtail/wagtailusers/views/groups.py index b0cad20b02..e2eee979b7 100644 --- a/wagtail/wagtailusers/views/groups.py +++ b/wagtail/wagtailusers/views/groups.py @@ -1,10 +1,10 @@ from django.shortcuts import render, redirect, get_object_or_404 from django.contrib.auth.models import Group -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ from django.views.decorators.vary import vary_on_headers +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin import messages from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailadmin.utils import permission_required, any_permission_required @@ -15,7 +15,6 @@ from wagtail.wagtailusers.forms import GroupForm, GroupPagePermissionFormSet @vary_on_headers('X-Requested-With') def index(request): q = None - p = request.GET.get("p", 1) is_searching = False if 'q' in request.GET: @@ -42,14 +41,7 @@ def index(request): else: ordering = 'name' - paginator = Paginator(groups, 20) - - try: - groups = paginator.page(p) - except PageNotAnInteger: - groups = paginator.page(1) - except EmptyPage: - groups = paginator.page(paginator.num_pages) + paginator, groups = paginate(request, groups) if request.is_ajax(): return render(request, "wagtailusers/groups/results.html", { diff --git a/wagtail/wagtailusers/views/users.py b/wagtail/wagtailusers/views/users.py index 9b4ef7b70a..bfbd7513bd 100644 --- a/wagtail/wagtailusers/views/users.py +++ b/wagtail/wagtailusers/views/users.py @@ -1,11 +1,11 @@ from django.shortcuts import render, redirect, get_object_or_404 from django.contrib.auth import get_user_model -from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger from django.core.urlresolvers import reverse from django.db.models import Q from django.utils.translation import ugettext as _ from django.views.decorators.vary import vary_on_headers +from wagtail.utils.pagination import paginate from wagtail.wagtailadmin import messages from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailadmin.utils import permission_required, any_permission_required @@ -26,7 +26,6 @@ delete_user_perm = "{0}.delete_{1}".format(AUTH_USER_APP_LABEL, AUTH_USER_MODEL_ @vary_on_headers('X-Requested-With') def index(request): q = None - p = request.GET.get("p", 1) is_searching = False if 'q' in request.GET: @@ -56,14 +55,7 @@ def index(request): else: ordering = 'name' - paginator = Paginator(users, 20) - - try: - users = paginator.page(p) - except PageNotAnInteger: - users = paginator.page(1) - except EmptyPage: - users = paginator.page(paginator.num_pages) + paginator, users = paginate(request, users) if request.is_ajax(): return render(request, "wagtailusers/users/results.html", { From 9c01930d9328af33d089a5ae37678c130e593644 Mon Sep 17 00:00:00 2001 From: Tim Heap Date: Thu, 16 Jul 2015 12:16:51 +1000 Subject: [PATCH 2/3] Add {% pagination %} template tag It complements the `wagtail.utils.pagination.paginate` function. Views and templates now do not need to know anything about pagination. This supersedes pagination in templates that was done with `{% include "wagtailadmin/pages/listing/_pagination.html" %}`. A few instances of `{% include "wagtailadmin/shared/pagination_nav.html }` have also been converted, but I am not familiar enough with the Wagtail admin template layout and view structure to convert all uses of this. Conflicts: wagtail/wagtailadmin/templates/wagtailadmin/pages/search_results.html wagtail/wagtailadmin/templates/wagtailadmin/pages/usage_results.html wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py wagtail/wagtailadmin/views/chooser.py --- wagtail/utils/pagination.py | 4 +- .../wagtailadmin/chooser/_browse_results.html | 4 +- .../wagtailadmin/chooser/email_link.html | 4 +- .../wagtailadmin/chooser/external_link.html | 4 +- .../templates/wagtailadmin/pages/index.html | 6 +- .../pages/listing/_navigation_choose.html | 4 +- .../pages/listing/_pagination.html | 20 ++--- .../wagtailadmin/pages/search_results.html | 4 +- .../wagtailadmin/pages/usage_results.html | 4 +- .../templatetags/wagtailadmin_tags.py | 77 +++++++++++++++++++ wagtail/wagtailadmin/views/chooser.py | 11 --- wagtail/wagtailadmin/views/pages.py | 11 +-- .../queries/chooser/results.html | 4 +- wagtail/wagtailsearch/tests/test_queries.py | 1 - wagtail/wagtailsearch/views/queries.py | 2 - 15 files changed, 109 insertions(+), 51 deletions(-) diff --git a/wagtail/utils/pagination.py b/wagtail/utils/pagination.py index 56e8c6345f..5b1d8b2d88 100644 --- a/wagtail/utils/pagination.py +++ b/wagtail/utils/pagination.py @@ -1,7 +1,9 @@ from django.core.paginator import Paginator, PageNotAnInteger, EmptyPage -def paginate(request, items, page_key='p', per_page=20): +DEFAULT_PAGE_KEY = 'p' + +def paginate(request, items, page_key=DEFAULT_PAGE_KEY, per_page=20): page = request.GET.get(page_key, 1) paginator = Paginator(items, per_page) diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/chooser/_browse_results.html b/wagtail/wagtailadmin/templates/wagtailadmin/chooser/_browse_results.html index 0a54eb6c5f..4922be3694 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/chooser/_browse_results.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/chooser/_browse_results.html @@ -1,4 +1,4 @@ -{% load i18n %} +{% load i18n wagtailadmin_tags %}

{% trans "Explorer" %}

{% include "wagtailadmin/shared/breadcrumb.html" with page=parent_page choosing=1 %} @@ -7,5 +7,5 @@ {% include "wagtailadmin/pages/listing/_list_choose.html" with allow_navigation=1 orderable=0 pages=pages parent_page=parent_page %} {% url 'wagtailadmin_choose_page_child' parent_page.id as pagination_base_url %} - {% include "wagtailadmin/pages/listing/_pagination.html" with page=pages base_url=pagination_base_url query_params=querystring classnames="navigate-pages" only %} + {% paginate pages base_url=pagination_base_url classnames="navigate-pages" %} {% endif %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/chooser/email_link.html b/wagtail/wagtailadmin/templates/wagtailadmin/chooser/email_link.html index 0f454a2c2e..51960294ac 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/chooser/email_link.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/chooser/email_link.html @@ -1,11 +1,11 @@ -{% load i18n %} +{% load i18n wagtailadmin_tags %} {% trans "Add an email link" as email_str %} {% include "wagtailadmin/shared/header.html" with title=email_str %}
{% include 'wagtailadmin/chooser/_link_types.html' with current='email' %} -
+ {% csrf_token %}
    {% for field in form %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/chooser/external_link.html b/wagtail/wagtailadmin/templates/wagtailadmin/chooser/external_link.html index 352836c2f7..ec131d48ce 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/chooser/external_link.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/chooser/external_link.html @@ -1,11 +1,11 @@ -{% load i18n %} +{% load i18n wagtailadmin_tags %} {% trans "Add an external link" as add_str %} {% include "wagtailadmin/shared/header.html" with title=add_str %}
    {% include 'wagtailadmin/chooser/_link_types.html' with current='external' %} - + {% csrf_token %}
      {% for field in form %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/index.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/index.html index 7e1a2961bc..cdc5cdf851 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/index.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/index.html @@ -16,8 +16,10 @@ {% page_permissions parent_page as parent_page_perms %} {% include "wagtailadmin/pages/listing/_list_explore.html" with sortable=1 allow_navigation=1 full_width=1 show_ordering_column=1 parent_page=parent_page orderable=parent_page_perms.can_reorder_children %} - {% url 'wagtailadmin_explore' parent_page.id as pagination_base_url %} - {% include "wagtailadmin/pages/listing/_pagination.html" with page=pages base_url=pagination_base_url query_params=pagination_query_params only %} + {% if do_paginate %} + {% url 'wagtailadmin_explore' parent_page.id as pagination_base_url %} + {% paginate pages base_url=pagination_base_url %} + {% endif %} {% endblock %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/listing/_navigation_choose.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/listing/_navigation_choose.html index 62d8751bcb..803f8214b8 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/listing/_navigation_choose.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/listing/_navigation_choose.html @@ -1,4 +1,4 @@ -{% load i18n %} +{% load i18n wagtailadmin_tags %} {% comment %} Navigation controls for the page listing in 'choose' mode @@ -6,6 +6,6 @@ Navigation controls for the page listing in 'choose' mode {% if allow_navigation and page.can_descend %} - {% trans 'Explore' %} + {% trans 'Explore' %} {% endif %} diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/listing/_pagination.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/listing/_pagination.html index 3f99637bd8..5c9afa785b 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/listing/_pagination.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/listing/_pagination.html @@ -1,32 +1,22 @@ -{% load i18n %} +{% load i18n wagtailadmin_tags %} {% comment %} -Pagination include for page listings. - -Accepts the following parameters: - -page - a django.core.pagination Page object -base_url - a URL string that we can append ?p=page_number to -query_params - any additional query params to include in the link - (urlencoded but not html-escaped, and without the joining '&') -classnames - additional classnames to put on the element - -TODO: Port all existing uses of shared/_pagination_nav.html over to this new API (extending the API as necessary to cover special cases), so that this can become a replacement for shared/_pagination_nav.html. +Pagination for page listings. Used by the `{% paginate %}` template tag. {% endcomment %}