Skip to content

Commit

Permalink
Fixed #17159 -- Validated returned number of next|previous_page_number
Browse files Browse the repository at this point in the history
Thanks mehta.apurva at gmail.com for the report and the initial patch
and neaf for the complete patch.
  • Loading branch information
claudep committed Jun 9, 2012
1 parent ef906b1 commit fc40a65
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 19 deletions.
4 changes: 2 additions & 2 deletions django/core/paginator.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,10 @@ def has_other_pages(self):
return self.has_previous() or self.has_next()

def next_page_number(self):
return self.number + 1
return self.paginator.validate_number(self.number + 1)

def previous_page_number(self):
return self.number - 1
return self.paginator.validate_number(self.number - 1)

def start_index(self):
"""
Expand Down
10 changes: 10 additions & 0 deletions docs/releases/1.5.txt
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,16 @@ Unicode parameters (``password``, ``salt`` or ``encoded``). If any of the
hashing methods need byte strings, you can use the
:func:`~django.utils.encoding.smart_str` utility to encode the strings.

Validation of previous_page_number and next_page_number
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

When using :doc:`object pagination </topics/pagination>`,
the ``previous_page_number()`` and ``next_page_number()`` methods of the
:class:`~django.core.paginator.Page` object did not check if the returned
number was inside the existing page range.
It does check it now and raises an :exc:`InvalidPage` exception when the number
is either too low or too high.

Features deprecated in 1.5
==========================

Expand Down
14 changes: 10 additions & 4 deletions docs/topics/pagination.txt
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,19 @@ Methods

.. method:: Page.next_page_number()

Returns the next page number. Note that this is "dumb" and will return the
next page number regardless of whether a subsequent page exists.
Returns the next page number.

.. versionchanged:: 1.5

Raises :exc:`InvalidPage` if next page doesn't exist.

.. method:: Page.previous_page_number()

Returns the previous page number. Note that this is "dumb" and will return
the previous page number regardless of whether a previous page exists.
Returns the previous page number.

.. versionchanged:: 1.5

Raises :exc:`InvalidPage` if previous page doesn't exist.

.. method:: Page.start_index()

Expand Down
26 changes: 13 additions & 13 deletions tests/modeltests/pagination/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def test_first_page(self):
self.assertFalse(p.has_previous())
self.assertTrue(p.has_other_pages())
self.assertEqual(2, p.next_page_number())
self.assertEqual(0, p.previous_page_number())
self.assertRaises(InvalidPage, p.previous_page_number)
self.assertEqual(1, p.start_index())
self.assertEqual(5, p.end_index())

Expand All @@ -63,7 +63,7 @@ def test_last_page(self):
self.assertFalse(p.has_next())
self.assertTrue(p.has_previous())
self.assertTrue(p.has_other_pages())
self.assertEqual(3, p.next_page_number())
self.assertRaises(InvalidPage, p.next_page_number)
self.assertEqual(1, p.previous_page_number())
self.assertEqual(6, p.start_index())
self.assertEqual(9, p.end_index())
Expand Down Expand Up @@ -104,20 +104,20 @@ def test_orphans(self):

def test_paginate_list(self):
# Paginators work with regular lists/tuples, too -- not just with QuerySets.
paginator = Paginator([1, 2, 3, 4, 5, 6, 7, 8, 9], 5)
paginator = Paginator([1, 2, 3, 4, 5, 6, 7, 8, 9], 3)
self.assertEqual(9, paginator.count)
self.assertEqual(2, paginator.num_pages)
self.assertEqual([1, 2], paginator.page_range)
p = paginator.page(1)
self.assertEqual("<Page 1 of 2>", unicode(p))
self.assertEqual([1, 2, 3, 4, 5], p.object_list)
self.assertEqual(3, paginator.num_pages)
self.assertEqual([1, 2, 3], paginator.page_range)
p = paginator.page(2)
self.assertEqual("<Page 2 of 3>", unicode(p))
self.assertEqual([4, 5, 6], p.object_list)
self.assertTrue(p.has_next())
self.assertFalse(p.has_previous())
self.assertTrue(p.has_previous())
self.assertTrue(p.has_other_pages())
self.assertEqual(2, p.next_page_number())
self.assertEqual(0, p.previous_page_number())
self.assertEqual(1, p.start_index())
self.assertEqual(5, p.end_index())
self.assertEqual(3, p.next_page_number())
self.assertEqual(1, p.previous_page_number())
self.assertEqual(4, p.start_index())
self.assertEqual(6, p.end_index())

def test_paginate_misc_classes(self):
# Paginator can be passed other objects with a count() method.
Expand Down

0 comments on commit fc40a65

Please sign in to comment.