diff --git a/CHANGES.rst b/CHANGES.rst index 976b8382..3656afcc 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -16,11 +16,14 @@ In development flushed yet. (`#555`_) - Allow specifying a ``max_per_page`` limit for pagination, to avoid users specifying high values in the request args. (`#542`_) +- For ``paginate`` with ``error_out=False``, the minimum value for ``page`` is + 1 and ``per_page`` is 0. (`#558`_) .. _#542: https://github.com/mitsuhiko/flask-sqlalchemy/pull/542 .. _#551: https://github.com/mitsuhiko/flask-sqlalchemy/pull/551 .. _#555: https://github.com/mitsuhiko/flask-sqlalchemy/pull/555 .. _#556: https://github.com/mitsuhiko/flask-sqlalchemy/pull/556 +.. _#558: https://github.com/mitsuhiko/flask-sqlalchemy/pull/558 Version 2.3.0 diff --git a/flask_sqlalchemy/__init__.py b/flask_sqlalchemy/__init__.py index eff12623..27125f67 100644 --- a/flask_sqlalchemy/__init__.py +++ b/flask_sqlalchemy/__init__.py @@ -430,16 +430,20 @@ def first_or_404(self): def paginate(self, page=None, per_page=None, error_out=True, max_per_page=None): """Returns ``per_page`` items from page ``page``. - If no items are found and ``page`` is greater than 1, or if page is - less than 1, it aborts with 404. - This behavior can be disabled by passing ``error_out=False``. - If ``page`` or ``per_page`` are ``None``, they will be retrieved from - the request query. - If the values are not ints and ``error_out`` is ``True``, it aborts - with 404. - If there is no request or they aren't in the query, they default to 1 - and 20 respectively. + the request query. If ``max_per_page`` is specified, ``per_page`` will + be limited to that value. If there is no request or they aren't in the + query, they default to 1 and 20 respectively. + + When ``error_out`` is ``True`` (default), the following rules will + cause a 404 response: + + * No items are found and ``page`` is not 1. + * ``page`` is less than 1, or ``per_page`` is negative. + * ``page`` or ``per_page`` are not ints. + + When ``error_out`` is ``False``, ``page`` and ``per_page`` default to + 1 and 20 respectively. Returns a :class:`Pagination` object. """ @@ -472,8 +476,17 @@ def paginate(self, page=None, per_page=None, error_out=True, max_per_page=None): if max_per_page is not None: per_page = min(per_page, max_per_page) - if error_out and page < 1: - abort(404) + if page < 1: + if error_out: + abort(404) + else: + page = 1 + + if per_page < 0: + if error_out: + abort(404) + else: + per_page = 20 items = self.limit(per_page).offset((page - 1) * per_page).all() diff --git a/tests/test_pagination.py b/tests/test_pagination.py index 20d8ad1e..05621c71 100644 --- a/tests/test_pagination.py +++ b/tests/test_pagination.py @@ -1,3 +1,6 @@ +import pytest +from werkzeug.exceptions import NotFound + import flask_sqlalchemy as fsa @@ -49,3 +52,19 @@ def test_query_paginate_more_than_20(app, db, Todo): db.session.commit() assert len(Todo.query.paginate(max_per_page=10).items) == 10 + + +def test_paginate_min(app, db, Todo): + with app.app_context(): + db.session.add_all(Todo(str(x), '') for x in range(20)) + db.session.commit() + + assert Todo.query.paginate(error_out=False, page=-1).items[0].title == '0' + assert len(Todo.query.paginate(error_out=False, per_page=0).items) == 0 + assert len(Todo.query.paginate(error_out=False, per_page=-1).items) == 20 + + with pytest.raises(NotFound): + Todo.query.paginate(page=0) + + with pytest.raises(NotFound): + Todo.query.paginate(per_page=-1)