From ce0e745c7d2f498dfe95be8392a0db1dcba95072 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Wed, 21 Jun 2023 12:19:38 +0100 Subject: [PATCH] convert page listing view to a ListView --- .../templates/wagtailadmin/pages/index.html | 2 +- .../admin/tests/pages/test_explorer_view.py | 8 +- wagtail/admin/tests/tests.py | 6 +- wagtail/admin/views/pages/listing.py | 86 +++++++++++-------- 4 files changed, 52 insertions(+), 50 deletions(-) diff --git a/wagtail/admin/templates/wagtailadmin/pages/index.html b/wagtail/admin/templates/wagtailadmin/pages/index.html index a5bd4c1420..e5598a6558 100644 --- a/wagtail/admin/templates/wagtailadmin/pages/index.html +++ b/wagtail/admin/templates/wagtailadmin/pages/index.html @@ -16,7 +16,7 @@ {% if do_paginate %} {% url 'wagtailadmin_explore' parent_page.id as pagination_base_url %} - {% paginate pages base_url=pagination_base_url %} + {% paginate page_obj base_url=pagination_base_url %} {% endif %} {% trans "Select all pages in listing" as select_all_text %} {% include 'wagtailadmin/bulk_actions/footer.html' with select_all_obj_text=select_all_text app_label='wagtailcore' model_name='page' objects=pages %} diff --git a/wagtail/admin/tests/pages/test_explorer_view.py b/wagtail/admin/tests/pages/test_explorer_view.py index 7da729b9cd..65ba7119fb 100644 --- a/wagtail/admin/tests/pages/test_explorer_view.py +++ b/wagtail/admin/tests/pages/test_explorer_view.py @@ -63,11 +63,7 @@ class TestPageExplorer(WagtailTestUtils, TestCase): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, "wagtailadmin/pages/index.html") self.assertEqual(Page.objects.get(id=1), response.context["parent_page"]) - self.assertTrue( - response.context["pages"] - .paginator.object_list.filter(id=self.root_page.id) - .exists() - ) + self.assertIn(self.root_page, response.context["pages"]) def test_explore_root_shows_icon(self): response = self.client.get(reverse("wagtailadmin_explore_root")) @@ -217,7 +213,7 @@ class TestPageExplorer(WagtailTestUtils, TestCase): self.assertTemplateUsed(response, "wagtailadmin/pages/index.html") # Check that we got the correct page - self.assertEqual(response.context["pages"].number, 2) + self.assertEqual(response.context["page_obj"].number, 2) def test_pagination_invalid(self): self.make_pages() diff --git a/wagtail/admin/tests/tests.py b/wagtail/admin/tests/tests.py index 239fec0bf1..9a49e474f6 100644 --- a/wagtail/admin/tests/tests.py +++ b/wagtail/admin/tests/tests.py @@ -525,11 +525,7 @@ class TestAdminURLAppendSlash(WagtailTestUtils, TestCase): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, "wagtailadmin/pages/index.html") self.assertEqual(Page.objects.get(id=1), response.context["parent_page"]) - self.assertTrue( - response.context["pages"] - .paginator.object_list.filter(id=self.root_page.id) - .exists() - ) + self.assertIn(self.root_page, response.context["pages"]) class TestRemoveStaleContentTypes(TestCase): diff --git a/wagtail/admin/views/pages/listing.py b/wagtail/admin/views/pages/listing.py index acfe020a8c..c54b7d34ab 100644 --- a/wagtail/admin/views/pages/listing.py +++ b/wagtail/admin/views/pages/listing.py @@ -1,10 +1,8 @@ from django.conf import settings -from django.core.paginator import InvalidPage, Paginator from django.db.models import Count -from django.http import Http404 from django.shortcuts import get_object_or_404, redirect from django.urls import reverse -from django.views.generic import TemplateView +from django.views.generic import ListView from wagtail import hooks from wagtail.admin.ui.side_panels import PageSidePanels @@ -12,7 +10,7 @@ from wagtail.admin.views.generic.permissions import PermissionCheckedMixin from wagtail.permission_policies.pages import Page, PagePermissionPolicy -class IndexView(PermissionCheckedMixin, TemplateView): +class IndexView(PermissionCheckedMixin, ListView): template_name = "wagtailadmin/pages/index.html" permission_policy = PagePermissionPolicy() any_permission_required = { @@ -23,6 +21,9 @@ class IndexView(PermissionCheckedMixin, TemplateView): "lock", "unlock", } + context_object_name = "pages" + page_kwarg = "p" + paginate_by = 50 def get(self, request, parent_page_id=None): if parent_page_id: @@ -45,12 +46,7 @@ class IndexView(PermissionCheckedMixin, TemplateView): return super().get(request) - def get_context_data(self, **kwargs): - pages = self.parent_page.get_children().prefetch_related( - "content_type", "sites_rooted_here" - ) & self.permission_policy.explorable_instances(self.request.user) - - # Get page ordering + def get_ordering(self): ordering = self.request.GET.get("ordering", "-latest_revision_created_at") if ordering not in [ "title", @@ -65,10 +61,19 @@ class IndexView(PermissionCheckedMixin, TemplateView): ]: ordering = "-latest_revision_created_at" - if ordering == "ord": + return ordering + + def get_queryset(self): + pages = self.parent_page.get_children().prefetch_related( + "content_type", "sites_rooted_here" + ) & self.permission_policy.explorable_instances(self.request.user) + + self.ordering = self.get_ordering() + + if self.ordering == "ord": # preserve the native ordering from get_children() pass - elif ordering == "latest_revision_created_at": + elif self.ordering == "latest_revision_created_at": # order by oldest revision first. # Special case NULL entries - these should go at the top of the list. # Do this by annotating with Count('latest_revision_created_at'), @@ -76,18 +81,14 @@ class IndexView(PermissionCheckedMixin, TemplateView): pages = pages.annotate( null_position=Count("latest_revision_created_at") ).order_by("null_position", "latest_revision_created_at") - elif ordering == "-latest_revision_created_at": + elif self.ordering == "-latest_revision_created_at": # order by oldest revision first. # Special case NULL entries - these should go at the end of the list. pages = pages.annotate( null_position=Count("latest_revision_created_at") ).order_by("-null_position", "-latest_revision_created_at") else: - pages = pages.order_by(ordering) - - # Don't paginate if sorting by page order - all pages must be shown to - # allow drag-and-drop reordering - do_paginate = ordering != "ord" + pages = pages.order_by(self.ordering) # We want specific page instances, but do not need streamfield values here pages = pages.defer_streamfields().specific() @@ -102,15 +103,23 @@ class IndexView(PermissionCheckedMixin, TemplateView): pages = pages.annotate_site_root_state().annotate_approved_schedule() - # Pagination - if do_paginate: - paginator = Paginator(pages, per_page=50) - try: - pages = paginator.page(self.request.GET.get("p", 1)) - except InvalidPage: - raise Http404 + return pages - show_ordering_column = self.request.GET.get("ordering") == "ord" + def get_paginate_by(self, queryset): + if self.ordering == "ord": + # Don't paginate if sorting by page order - all pages must be shown to + # allow drag-and-drop reordering + return None + else: + return self.paginate_by + + def paginate_queryset(self, queryset, page_size): + return super().paginate_queryset(queryset, page_size) + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + + show_ordering_column = self.ordering == "ord" side_panels = PageSidePanels( self.request, @@ -123,18 +132,19 @@ class IndexView(PermissionCheckedMixin, TemplateView): comments_enabled=False, ) - context = { - "parent_page": self.parent_page, - "ordering": ordering, - "side_panels": side_panels, - "pages": pages, - "do_paginate": do_paginate, - "locale": None, - "translations": [], - "show_ordering_column": show_ordering_column, - "show_bulk_actions": not show_ordering_column, - "show_locale_labels": False, - } + context.update( + { + "parent_page": self.parent_page, + "ordering": self.ordering, + "side_panels": side_panels, + "do_paginate": context["is_paginated"], + "locale": None, + "translations": [], + "show_ordering_column": show_ordering_column, + "show_bulk_actions": not show_ordering_column, + "show_locale_labels": False, + } + ) if getattr(settings, "WAGTAIL_I18N_ENABLED", False): if not self.parent_page.is_root():