Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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
  • Loading branch information...
commit 96cf3656c48f6c42714a70b4546bc42f7b904185 1 parent 19c7db0
Gary Wilson Jr. gdub authored
14 django/core/paginator.py
View
@@ -1,3 +1,5 @@
+from math import ceil
+
class InvalidPage(Exception):
pass
@@ -55,13 +57,11 @@ def _get_count(self):
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 @@ def start_index(self):
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 @@ def end_index(self):
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
0  tests/regressiontests/pagination_regress/__init__.py
View
No changes.
1  tests/regressiontests/pagination_regress/models.py
View
@@ -0,0 +1 @@
+# Models file for tests to run.
157 tests/regressiontests/pagination_regress/tests.py
View
@@ -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)
Please sign in to comment.
Something went wrong with that request. Please try again.