There are a couple of pages where the clickable list of pages would include missing or duplicate pages.
Write a test that ensures: - you always have a link to the next/prev numbered page - there are no duplicate page numbers Fiddle with the pagination algorithm to get it to pass - required tweaking a display parameter and a couple of comparison operators, so all pretty minor. Now, if there are 10 pages, the displayed page numbers for a given page are as follows: Page # | Displayed page #s --------------------------- 1 | [] [1, 2, 3, 4] [9, 10] 2 | [] [1, 2, 3, 4] [9, 10] 3 | [] [1, 2, 3, 4] [9, 10] 4 | [1, 2] [3, 4, 5] [9, 10] 5 | [1, 2] [4, 5, 6] [9, 10] 6 | [1, 2] [5, 6, 7] [9, 10] 7 | [1, 2] [6, 7, 8] [9, 10] 8 | [1, 2] [7, 8, 9, 10] [] 9 | [1, 2] [7, 8, 9, 10] [] 10 | [1, 2] [7, 8, 9, 10] [] Closes: #102 Signed-off-by: Daniel Axtens <d...@axtens.net> --- patchwork/paginator.py | 6 +++--- patchwork/tests/test_paginator.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/patchwork/paginator.py b/patchwork/paginator.py index e4cf556ebd96..359ec86793b7 100644 --- a/patchwork/paginator.py +++ b/patchwork/paginator.py @@ -28,7 +28,7 @@ from patchwork.compat import is_authenticated DEFAULT_ITEMS_PER_PAGE = 100 LONG_PAGE_THRESHOLD = 30 LEADING_PAGE_RANGE_DISPLAYED = 4 -TRAILING_PAGE_RANGE_DISPLAYED = 2 +TRAILING_PAGE_RANGE_DISPLAYED = 4 LEADING_PAGE_RANGE = 4 TRAILING_PAGE_RANGE = 2 NUM_PAGES_OUTSIDE_RANGE = 2 @@ -69,12 +69,12 @@ class Paginator(paginator.Paginator): if pages <= LEADING_PAGE_RANGE_DISPLAYED: adjacent_start = 1 adjacent_end = pages + 1 - elif page_no <= LEADING_PAGE_RANGE: + elif page_no < LEADING_PAGE_RANGE: adjacent_start = 1 adjacent_end = LEADING_PAGE_RANGE_DISPLAYED + 1 self.leading_set = [n + pages for n in range(0, -NUM_PAGES_OUTSIDE_RANGE, -1)] - elif page_no > pages - TRAILING_PAGE_RANGE: + elif page_no >= pages - TRAILING_PAGE_RANGE: adjacent_start = pages - TRAILING_PAGE_RANGE_DISPLAYED + 1 adjacent_end = pages + 1 self.trailing_set = [n + 1 for n in diff --git a/patchwork/tests/test_paginator.py b/patchwork/tests/test_paginator.py index 2291eb058fe0..b2191bbb92d1 100644 --- a/patchwork/tests/test_paginator.py +++ b/patchwork/tests/test_paginator.py @@ -66,6 +66,44 @@ class PaginatorTest(TestCase): self.assertEqual(response.context['page'].object_list[0].id, self.patches[-page].id) + def test_navigation(self): + self.client.login(username=self.user.username, + password=self.user.username) + for page_num in range(1, 11): + response = self._get_patches({'page': page_num}) + + page = response.context['page'] + leading = page.paginator.leading_set + adjacent = page.paginator.adjacent_set + trailing = page.paginator.trailing_set + + # if there is a prev page, it should be: + if page.has_previous(): + self.assertEqual(page.previous_page_number(), + page_num - 1) + # ... either in the adjacent set or in the trailing set + if adjacent is not None: + self.assertIn(page_num - 1, adjacent) + else: + self.assertIn(page_num - 1, trailing) + + # if there is a next page, it should be: + if page.has_next(): + self.assertEqual(page.next_page_number(), + page_num + 1) + # ... either in the adjacent set or in the leading set + if adjacent is not None: + self.assertIn(page_num + 1, adjacent) + else: + self.assertIn(page_num + 1, leading) + + # no page number should appear more than once + for x in adjacent: + self.assertNotIn(x, leading) + self.assertNotIn(x, trailing) + for x in leading: + self.assertNotIn(x, trailing) + def test_page_invalid(self): self.client.login(username=self.user.username, password=self.user.username) -- 2.14.1 _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork