From b203ec70fd7ffc4027380940157d1cf9c9e588ad Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Thu, 30 Jul 2020 12:45:32 +0100 Subject: [PATCH] Refs #25513 -- Adjusted admin pagination to be 1-indexed. --- .../contrib/admin/templatetags/admin_list.py | 20 ++++++++-------- django/contrib/admin/views/main.py | 6 ++--- docs/releases/3.2.txt | 6 +++++ tests/admin_changelist/tests.py | 24 +++++++++---------- tests/admin_views/tests.py | 2 +- 5 files changed, 32 insertions(+), 26 deletions(-) diff --git a/django/contrib/admin/templatetags/admin_list.py b/django/contrib/admin/templatetags/admin_list.py index 55565579ae..088b85991f 100644 --- a/django/contrib/admin/templatetags/admin_list.py +++ b/django/contrib/admin/templatetags/admin_list.py @@ -35,13 +35,13 @@ def paginator_number(cl, i): if i == DOT: return '… ' elif i == cl.page_num: - return format_html('{} ', i + 1) + return format_html('{} ', i) else: return format_html( '{} ', cl.get_query_string({PAGE_VAR: i}), - mark_safe(' class="end"' if i == cl.paginator.num_pages - 1 else ''), - i + 1, + mark_safe(' class="end"' if i == cl.paginator.num_pages else ''), + i, ) @@ -61,26 +61,26 @@ def pagination(cl): # If there are 10 or fewer pages, display links to every page. # Otherwise, do some fancy if paginator.num_pages <= 10: - page_range = range(paginator.num_pages) + page_range = range(1, paginator.num_pages + 1) else: # Insert "smart" pagination links, so that there are always ON_ENDS # links at either end of the list of pages, and there are always # ON_EACH_SIDE links at either end of the "current page" link. page_range = [] - if page_num > (ON_EACH_SIDE + ON_ENDS): + if page_num > (1 + ON_EACH_SIDE + ON_ENDS): page_range += [ - *range(0, ON_ENDS), DOT, + *range(1, ON_ENDS + 1), DOT, *range(page_num - ON_EACH_SIDE, page_num + 1), ] else: - page_range.extend(range(0, page_num + 1)) - if page_num < (paginator.num_pages - ON_EACH_SIDE - ON_ENDS - 1): + page_range.extend(range(1, page_num + 1)) + if page_num < (paginator.num_pages - ON_EACH_SIDE - ON_ENDS): page_range += [ *range(page_num + 1, page_num + ON_EACH_SIDE + 1), DOT, - *range(paginator.num_pages - ON_ENDS, paginator.num_pages) + *range(paginator.num_pages - ON_ENDS + 1, paginator.num_pages + 1) ] else: - page_range.extend(range(page_num + 1, paginator.num_pages)) + page_range.extend(range(page_num + 1, paginator.num_pages + 1)) need_show_all_link = cl.can_show_all and not cl.show_all and cl.multi_page return { diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 6df80e3627..fefed29933 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -77,9 +77,9 @@ class ChangeList: messages.error(request, ', '.join(error)) self.query = _search_form.cleaned_data.get(SEARCH_VAR) or '' try: - self.page_num = int(request.GET.get(PAGE_VAR, 0)) + self.page_num = int(request.GET.get(PAGE_VAR, 1)) except ValueError: - self.page_num = 0 + self.page_num = 1 self.show_all = ALL_VAR in request.GET self.is_popup = IS_POPUP_VAR in request.GET to_field = request.GET.get(TO_FIELD_VAR) @@ -247,7 +247,7 @@ class ChangeList: result_list = self.queryset._clone() else: try: - result_list = paginator.page(self.page_num + 1).object_list + result_list = paginator.page(self.page_num).object_list except InvalidPage: raise IncorrectLookupParameters diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index d1e21b2868..8908413cfe 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -380,6 +380,12 @@ backends. unique constraints (:attr:`.UniqueConstraint.include`), set ``DatabaseFeatures.supports_covering_indexes`` to ``True``. +:mod:`django.contrib.admin` +--------------------------- + +* Pagination links in the admin are now 1-indexed instead of 0-indexed, i.e. + the query string for the first page is ``?p=1`` instead of ``?p=0``. + :mod:`django.contrib.gis` ------------------------- diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py index 38143bf592..68319d69d4 100644 --- a/tests/admin_changelist/tests.py +++ b/tests/admin_changelist/tests.py @@ -259,7 +259,7 @@ class ChangeListTests(TestCase): Regression test for #14312: list_editable with pagination """ new_parent = Parent.objects.create(name='parent') - for i in range(200): + for i in range(1, 201): Child.objects.create(name='name %s' % i, parent=new_parent) request = self.factory.get('/child/', data={'p': -1}) # Anything outside range request.user = self.superuser @@ -274,7 +274,7 @@ class ChangeListTests(TestCase): def test_custom_paginator(self): new_parent = Parent.objects.create(name='parent') - for i in range(200): + for i in range(1, 201): Child.objects.create(name='name %s' % i, parent=new_parent) request = self.factory.get('/child/') @@ -576,7 +576,7 @@ class ChangeListTests(TestCase): use queryset set by modeladmin. """ parent = Parent.objects.create(name='anything') - for i in range(30): + for i in range(1, 31): Child.objects.create(name='name %s' % i, parent=parent) Child.objects.create(name='filtered %s' % i, parent=parent) @@ -652,7 +652,7 @@ class ChangeListTests(TestCase): def test_show_all(self): parent = Parent.objects.create(name='anything') - for i in range(30): + for i in range(1, 31): Child.objects.create(name='name %s' % i, parent=parent) Child.objects.create(name='filtered %s' % i, parent=parent) @@ -969,7 +969,7 @@ class ChangeListTests(TestCase): custom_site.register(UnorderedObject, UnorderedObjectAdmin) model_admin = UnorderedObjectAdmin(UnorderedObject, custom_site) counter = 0 if ascending else 51 - for page in range(0, 5): + for page in range(1, 6): request = self._mocked_authenticated_request('/unorderedobject/?p=%s' % page, superuser) response = model_admin.changelist_view(request) for result in response.context_data['cl'].result_list: @@ -1013,7 +1013,7 @@ class ChangeListTests(TestCase): custom_site.register(OrderedObject, OrderedObjectAdmin) model_admin = OrderedObjectAdmin(OrderedObject, custom_site) counter = 0 if ascending else 51 - for page in range(0, 5): + for page in range(1, 6): request = self._mocked_authenticated_request('/orderedobject/?p=%s' % page, superuser) response = model_admin.changelist_view(request) for result in response.context_data['cl'].result_list: @@ -1245,12 +1245,12 @@ class ChangeListTests(TestCase): per_page = cl.list_per_page = 10 for page_num, objects_count, expected_page_range in [ - (0, per_page, []), - (0, per_page * 2, list(range(2))), - (5, per_page * 11, list(range(11))), - (5, per_page * 12, [0, 1, 2, 3, 4, 5, 6, 7, 8, '.', 10, 11]), - (6, per_page * 12, [0, 1, '.', 3, 4, 5, 6, 7, 8, 9, 10, 11]), - (6, per_page * 13, [0, 1, '.', 3, 4, 5, 6, 7, 8, 9, '.', 11, 12]), + (1, per_page, []), + (1, per_page * 2, list(range(1, 3))), + (6, per_page * 11, list(range(1, 12))), + (6, per_page * 12, [1, 2, 3, 4, 5, 6, 7, 8, 9, '.', 11, 12]), + (7, per_page * 12, [1, 2, '.', 4, 5, 6, 7, 8, 9, 10, 11, 12]), + (7, per_page * 13, [1, 2, '.', 4, 5, 6, 7, 8, 9, 10, '.', 12, 13]), ]: # assuming we have exactly `objects_count` objects Group.objects.all().delete() diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index caa1d80164..e2c4605a20 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -3292,7 +3292,7 @@ class AdminViewListEditable(TestCase): self.assertContains(response, 'Unordered object #3') self.assertContains(response, 'Unordered object #2') self.assertNotContains(response, 'Unordered object #1') - response = self.client.get(reverse('admin:admin_views_unorderedobject_changelist') + '?p=1') + response = self.client.get(reverse('admin:admin_views_unorderedobject_changelist') + '?p=2') self.assertNotContains(response, 'Unordered object #3') self.assertNotContains(response, 'Unordered object #2') self.assertContains(response, 'Unordered object #1')