diff options
Diffstat (limited to 'mediagoblin')
| -rw-r--r-- | mediagoblin/db/models.py | 15 | ||||
| -rw-r--r-- | mediagoblin/static/css/base.css | 6 | ||||
| -rw-r--r-- | mediagoblin/tests/test_tools.py | 57 | ||||
| -rw-r--r-- | mediagoblin/tools/pagination.py | 7 |
4 files changed, 73 insertions, 12 deletions
diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 6448de36..f4644b9f 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -998,10 +998,17 @@ class Comment(Base): return self.comment().get_actor # noqa def __getattr__(self, attr): + if attr.startswith('_'): + # if attr starts with '_', then it's probably some internal + # sqlalchemy variable. Since __getattr__ is called when + # non-existing attributes are being accessed, we should not try to + # fetch it from self.comment() + raise AttributeError try: + _log.debug('Old attr is being accessed: {0}'.format(attr)) return getattr(self.comment(), attr) # noqa except Exception as e: - print(e) + _log.error(e) raise class TextComment(Base, TextCommentMixin, CommentingMixin): @@ -1155,11 +1162,11 @@ class Collection(Base, CollectionMixin, CommentingMixin): def get_collection_items(self, ascending=False): #TODO, is this still needed with self.collection_items being available? - order_col = MediaEntry.created + order_col = CollectionItem.position if not ascending: order_col = desc(order_col) - return CollectionItem.query.join(MediaEntry).filter( - CollectionItem.collection==self.id).order_by(order_col) + return CollectionItem.query.filter_by( + collection=self.id).order_by(order_col) def __repr__(self): safe_title = self.title.encode('ascii', 'replace') diff --git a/mediagoblin/static/css/base.css b/mediagoblin/static/css/base.css index 7852cae9..1628232b 100644 --- a/mediagoblin/static/css/base.css +++ b/mediagoblin/static/css/base.css @@ -457,11 +457,9 @@ text-align: center; } .form_field_label { - margin-bottom: 4px; -} - -.form_field_label { font-size:1.125em; + margin-bottom: 0; + padding: 10px 0; } .form_field_description { diff --git a/mediagoblin/tests/test_tools.py b/mediagoblin/tests/test_tools.py index 6d3dd475..5f916400 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,54 @@ class TestDecodeRequest(object): request.form = {'foo': 'bar'} data = decode_request(request) assert data['foo'] == 'bar' + + +class TestPagination(object): + 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 = 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. + + This is a regression test for a Python 2/3 compatibility fix. + + """ + 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_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. + + """ + 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 diff --git a/mediagoblin/tools/pagination.py b/mediagoblin/tools/pagination.py index a525caf7..db5f69fb 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 <http://www.gnu.org/licenses/>. -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): """ |