From 96cf3656c48f6c42714a70b4546bc42f7b904185 Mon Sep 17 00:00:00 2001 From: Gary Wilson Jr Date: Mon, 28 Jul 2008 05:30:35 +0000 Subject: [PATCH] Fixed #6997 -- Corrected `num_pages` calculation when one item is in the object list and `allow_empty_first_page=False`, thanks to framos for the report. Also, made Page's `start_index()` return 0 if there are no items in the object list (previously it was returning 1, but now it is consistent with `end_index()`). As an added bonus, I threw in quite a few pagination tests. git-svn-id: http://code.djangoproject.com/svn/django/trunk@8129 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/core/paginator.py | 14 +- .../pagination_regress/__init__.py | 0 .../pagination_regress/models.py | 1 + .../pagination_regress/tests.py | 157 ++++++++++++++++++ 4 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 tests/regressiontests/pagination_regress/__init__.py create mode 100644 tests/regressiontests/pagination_regress/models.py create mode 100644 tests/regressiontests/pagination_regress/tests.py diff --git a/django/core/paginator.py b/django/core/paginator.py index 366d2986ab..00725a6b74 100644 --- a/django/core/paginator.py +++ b/django/core/paginator.py @@ -1,3 +1,5 @@ +from math import ceil + class InvalidPage(Exception): pass @@ -55,13 +57,11 @@ class Paginator(object): def _get_num_pages(self): "Returns the total number of pages." if self._num_pages is None: - hits = self.count - 1 - self.orphans - if hits < 1: - hits = 0 - if hits == 0 and not self.allow_empty_first_page: + if self.count == 0 and not self.allow_empty_first_page: self._num_pages = 0 else: - self._num_pages = hits // self.per_page + 1 + hits = max(1, self.count - self.orphans) + self._num_pages = int(ceil(hits / float(self.per_page))) return self._num_pages num_pages = property(_get_num_pages) @@ -104,6 +104,9 @@ class Page(object): Returns the 1-based index of the first object on this page, relative to total objects in the paginator. """ + # Special case, return zero if no items. + if self.paginator.count == 0: + return 0 return (self.paginator.per_page * (self.number - 1)) + 1 def end_index(self): @@ -111,6 +114,7 @@ class Page(object): Returns the 1-based index of the last object on this page, relative to total objects found (hits). """ + # Special case for the last page because there can be orphans. if self.number == self.paginator.num_pages: return self.paginator.count return self.number * self.paginator.per_page diff --git a/tests/regressiontests/pagination_regress/__init__.py b/tests/regressiontests/pagination_regress/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regressiontests/pagination_regress/models.py b/tests/regressiontests/pagination_regress/models.py new file mode 100644 index 0000000000..cde172db68 --- /dev/null +++ b/tests/regressiontests/pagination_regress/models.py @@ -0,0 +1 @@ +# Models file for tests to run. diff --git a/tests/regressiontests/pagination_regress/tests.py b/tests/regressiontests/pagination_regress/tests.py new file mode 100644 index 0000000000..723d5712a7 --- /dev/null +++ b/tests/regressiontests/pagination_regress/tests.py @@ -0,0 +1,157 @@ +from unittest import TestCase + +from django.core.paginator import Paginator, EmptyPage + +class PaginatorTests(TestCase): + """ + Tests for the Paginator and Page classes. + """ + + def check_paginator(self, params, output): + """ + Helper method that instantiates a Paginator object from the passed + params and then checks that it's attributes match the passed output. + """ + count, num_pages, page_range = output + paginator = Paginator(*params) + self.check_attribute('count', paginator, count, params) + self.check_attribute('num_pages', paginator, num_pages, params) + self.check_attribute('page_range', paginator, page_range, params) + + def check_attribute(self, name, paginator, expected, params): + """ + Helper method to check a single attribute and gives a nice error + message upon test failure. + """ + got = getattr(paginator, name) + self.assertEqual(expected, got, + "For '%s', expected %s but got %s. Paginator parameters were: %s" + % (name, expected, got, params)) + + def test_paginator(self): + """ + Tests the paginator attributes using varying inputs. + """ + nine = [1, 2, 3, 4, 5, 6, 7, 8, 9] + ten = nine + [10] + eleven = ten + [11] + tests = ( + # Each item is two tuples: + # First tuple is Paginator parameters - object_list, per_page, + # orphans, and allow_empty_first_page. + # Second tuple is resulting Paginator attributes - count, + # num_pages, and page_range. + # Ten items, varying orphans, no empty first page. + ((ten, 4, 0, False), (10, 3, [1, 2, 3])), + ((ten, 4, 1, False), (10, 3, [1, 2, 3])), + ((ten, 4, 2, False), (10, 2, [1, 2])), + ((ten, 4, 5, False), (10, 2, [1, 2])), + ((ten, 4, 6, False), (10, 1, [1])), + # Ten items, varying orphans, allow empty first page. + ((ten, 4, 0, True), (10, 3, [1, 2, 3])), + ((ten, 4, 1, True), (10, 3, [1, 2, 3])), + ((ten, 4, 2, True), (10, 2, [1, 2])), + ((ten, 4, 5, True), (10, 2, [1, 2])), + ((ten, 4, 6, True), (10, 1, [1])), + # One item, varying orphans, no empty first page. + (([1], 4, 0, False), (1, 1, [1])), + (([1], 4, 1, False), (1, 1, [1])), + (([1], 4, 2, False), (1, 1, [1])), + # One item, varying orphans, allow empty first page. + (([1], 4, 0, True), (1, 1, [1])), + (([1], 4, 1, True), (1, 1, [1])), + (([1], 4, 2, True), (1, 1, [1])), + # Zero items, varying orphans, no empty first page. + (([], 4, 0, False), (0, 0, [])), + (([], 4, 1, False), (0, 0, [])), + (([], 4, 2, False), (0, 0, [])), + # Zero items, varying orphans, allow empty first page. + (([], 4, 0, True), (0, 1, [1])), + (([], 4, 1, True), (0, 1, [1])), + (([], 4, 2, True), (0, 1, [1])), + # Number if items one less than per_page. + (([], 1, 0, True), (0, 1, [1])), + (([], 1, 0, False), (0, 0, [])), + (([1], 2, 0, True), (1, 1, [1])), + ((nine, 10, 0, True), (9, 1, [1])), + # Number if items equal to per_page. + (([1], 1, 0, True), (1, 1, [1])), + (([1, 2], 2, 0, True), (2, 1, [1])), + ((ten, 10, 0, True), (10, 1, [1])), + # Number if items one more than per_page. + (([1, 2], 1, 0, True), (2, 2, [1, 2])), + (([1, 2, 3], 2, 0, True), (3, 2, [1, 2])), + ((eleven, 10, 0, True), (11, 2, [1, 2])), + # Number if items one more than per_page with one orphan. + (([1, 2], 1, 1, True), (2, 1, [1])), + (([1, 2, 3], 2, 1, True), (3, 1, [1])), + ((eleven, 10, 1, True), (11, 1, [1])), + ) + for params, output in tests: + self.check_paginator(params, output) + + def check_indexes(self, params, page_num, indexes): + """ + Helper method that instantiates a Paginator object from the passed + params and then checks that the start and end indexes of for the passed + page_num match those given as a 2-tuple in indexes. + """ + paginator = Paginator(*params) + if page_num == 'first': + page_num = 1 + elif page_num == 'last': + page_num = paginator.num_pages + page = paginator.page(page_num) + start, end = indexes + msg = ("For %s of page %s, expected %s but got %s." + " Paginator parameters were: %s") + self.assertEqual(start, page.start_index(), + msg % ('start index', page_num, start, page.start_index(), params)) + self.assertEqual(end, page.end_index(), + msg % ('end index', page_num, end, page.end_index(), params)) + + def test_page_indexes(self): + """ + Tests that paginator pages have the correct start and end indexes. + """ + ten = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + tests = ( + # Each item is three tuples: + # First tuple is Paginator parameters - object_list, per_page, + # orphans, and allow_empty_first_page. + # Second tuple is the start and end indexes of the first page. + # Third tuple is the start and end indexes of the last page. + # Ten items, varying per_page, no orphans. + ((ten, 1, 0, True), (1, 1), (10, 10)), + ((ten, 2, 0, True), (1, 2), (9, 10)), + ((ten, 3, 0, True), (1, 3), (10, 10)), + ((ten, 5, 0, True), (1, 5), (6, 10)), + # Ten items, varying per_page, with orphans. + ((ten, 1, 1, True), (1, 1), (9, 10)), + ((ten, 1, 2, True), (1, 1), (8, 10)), + ((ten, 3, 1, True), (1, 3), (7, 10)), + ((ten, 3, 2, True), (1, 3), (7, 10)), + ((ten, 3, 4, True), (1, 3), (4, 10)), + ((ten, 5, 1, True), (1, 5), (6, 10)), + ((ten, 5, 2, True), (1, 5), (6, 10)), + ((ten, 5, 5, True), (1, 10), (1, 10)), + # One item, varying orphans, no empty first page. + (([1], 4, 0, False), (1, 1), (1, 1)), + (([1], 4, 1, False), (1, 1), (1, 1)), + (([1], 4, 2, False), (1, 1), (1, 1)), + # One item, varying orphans, allow empty first page. + (([1], 4, 0, True), (1, 1), (1, 1)), + (([1], 4, 1, True), (1, 1), (1, 1)), + (([1], 4, 2, True), (1, 1), (1, 1)), + # Zero items, varying orphans, allow empty first page. + (([], 4, 0, True), (0, 0), (0, 0)), + (([], 4, 1, True), (0, 0), (0, 0)), + (([], 4, 2, True), (0, 0), (0, 0)), + ) + for params, first, last in tests: + self.check_indexes(params, 'first', first) + self.check_indexes(params, 'last', last) + # When no items and no empty first page, we should get EmptyPage error. + self.assertRaises(EmptyPage, self.check_indexes, ([], 4, 0, False), 1, None) + self.assertRaises(EmptyPage, self.check_indexes, ([], 4, 1, False), 1, None) + self.assertRaises(EmptyPage, self.check_indexes, ([], 4, 2, False), 1, None)