From 6e0d2b51ff5a00f24c32783be63ae4970b93ed77 Mon Sep 17 00:00:00 2001 From: Ben Sturmfels Date: Sun, 7 Aug 2016 21:48:52 +1000 Subject: [PATCH 1/2] Add Python 3 support in pagination. This issue was visible when attempting to view the home page of a MediaGoblin site with more than a single page worth of items, under Python 3. --- mediagoblin/tests/test_tools.py | 31 +++++++++++++++++++++++++++++++ mediagoblin/tools/pagination.py | 7 +++---- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/mediagoblin/tests/test_tools.py b/mediagoblin/tests/test_tools.py index 6d3dd47..30232e2 100644 --- a/mediagoblin/tests/test_tools.py +++ b/mediagoblin/tests/test_tools.py @@ -16,10 +16,16 @@ from __future__ import absolute_import, unicode_literals +try: + import mock +except ImportError: + import unittest.mock as mock + from werkzeug.wrappers import Request from werkzeug.test import EnvironBuilder from mediagoblin.tools.request import decode_request +from mediagoblin.tools.pagination import Pagination class TestDecodeRequest(object): """Test the decode_request function.""" @@ -59,3 +65,28 @@ class TestDecodeRequest(object): request.form = {'foo': 'bar'} data = decode_request(request) assert data['foo'] == 'bar' + + +class TestPagination(object): + def setup(self): + mock_cursor = mock.MagicMock() + mock_cursor.count.return_value = 1 + self.paginator = Pagination(1, mock_cursor) + + def test_creates_valid_page_url_from_explicit_base_url(self): + """Check that test_page_url_explicit runs. + + This is a regression test for a Python 2/3 compatibility fix. + + """ + url = self.paginator.get_page_url_explicit( + 'http://example.com', [], 1) + assert url == 'http://example.com?page=1' + + def test_iter_pages_handes_single_page(self): + """Check that iter_pages produces the expected result for single page. + + This is a regression test for a Python 2/3 compatibility fix. + + """ + assert list(self.paginator.iter_pages()) == [1] diff --git a/mediagoblin/tools/pagination.py b/mediagoblin/tools/pagination.py index a525caf..db5f69f 100644 --- a/mediagoblin/tools/pagination.py +++ b/mediagoblin/tools/pagination.py @@ -14,13 +14,12 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -import urllib import copy from math import ceil, floor from itertools import count from werkzeug.datastructures import MultiDict -from six.moves import zip +from six.moves import range, urllib, zip PAGINATION_DEFAULT_PER_PAGE = 30 @@ -86,7 +85,7 @@ class Pagination(object): def iter_pages(self, left_edge=2, left_current=2, right_current=5, right_edge=2): last = 0 - for num in xrange(1, self.pages + 1): + for num in range(1, self.pages + 1): if num <= left_edge or \ (num > self.page - left_current - 1 and \ num < self.page + right_current) or \ @@ -107,7 +106,7 @@ class Pagination(object): new_get_params['page'] = page_no return "%s?%s" % ( - base_url, urllib.urlencode(new_get_params)) + base_url, urllib.parse.urlencode(new_get_params)) def get_page_url(self, request, page_no): """ -- 1.9.1 From f9c47d68c63ef6b90ef7deca3c673a5babdb698c Mon Sep 17 00:00:00 2001 From: Ben Sturmfels Date: Thu, 15 Sep 2016 21:34:12 +1200 Subject: [PATCH 2/2] Extend Paginator tests to satisfy #55. --- mediagoblin/tests/test_tools.py | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/mediagoblin/tests/test_tools.py b/mediagoblin/tests/test_tools.py index 30232e2..5f91640 100644 --- a/mediagoblin/tests/test_tools.py +++ b/mediagoblin/tests/test_tools.py @@ -68,10 +68,11 @@ class TestDecodeRequest(object): class TestPagination(object): - def setup(self): + def _create_paginator(self, num_items, page, per_page): + """Create a Paginator with a mock database cursor.""" mock_cursor = mock.MagicMock() - mock_cursor.count.return_value = 1 - self.paginator = Pagination(1, mock_cursor) + mock_cursor.count.return_value = num_items + return Pagination(page, mock_cursor, per_page) def test_creates_valid_page_url_from_explicit_base_url(self): """Check that test_page_url_explicit runs. @@ -79,14 +80,39 @@ class TestPagination(object): This is a regression test for a Python 2/3 compatibility fix. """ - url = self.paginator.get_page_url_explicit( - 'http://example.com', [], 1) + paginator = self._create_paginator(num_items=1, page=1, per_page=30) + url = paginator.get_page_url_explicit('http://example.com', [], 1) assert url == 'http://example.com?page=1' - def test_iter_pages_handes_single_page(self): + def test_iter_pages_handles_single_page(self): """Check that iter_pages produces the expected result for single page. This is a regression test for a Python 2/3 compatibility fix. """ - assert list(self.paginator.iter_pages()) == [1] + paginator = self._create_paginator(num_items=1, page=1, per_page=30) + assert list(paginator.iter_pages()) == [1] + + def test_zero_items(self): + """Check that no items produces no pages.""" + paginator = self._create_paginator(num_items=0, page=1, per_page=30) + assert paginator.total_count == 0 + assert paginator.pages == 0 + + def test_single_item(self): + """Check that one item produces one page.""" + paginator = self._create_paginator(num_items=1, page=1, per_page=30) + assert paginator.total_count == 1 + assert paginator.pages == 1 + + def test_full_page(self): + """Check that a full page of items produces one page.""" + paginator = self._create_paginator(num_items=30, page=1, per_page=30) + assert paginator.total_count == 30 + assert paginator.pages == 1 + + def test_multiple_pages(self): + """Check that more than a full page produces two pages.""" + paginator = self._create_paginator(num_items=31, page=1, per_page=30) + assert paginator.total_count == 31 + assert paginator.pages == 2 -- 1.9.1