From 3833a5bb8a9174e5fb09dac59a964eff24b6065e Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 14 Jan 2015 16:51:26 +0000 Subject: Include pagination control in browsable API --- rest_framework/pagination.py | 90 +++++++++++++++++++++- rest_framework/renderers.py | 1 + .../static/rest_framework/css/bootstrap-tweaks.css | 4 - rest_framework/templates/rest_framework/base.html | 9 +++ .../rest_framework/pagination/numbers.html | 27 +++++++ rest_framework/templatetags/rest_framework.py | 17 ++++ 6 files changed, 143 insertions(+), 5 deletions(-) create mode 100644 rest_framework/templates/rest_framework/pagination/numbers.html (limited to 'rest_framework') diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index b9d48796..bd343c0d 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -3,14 +3,18 @@ Pagination serializers determine the structure of the output that should be used for paginated responses. """ from __future__ import unicode_literals +from collections import namedtuple from django.core.paginator import InvalidPage, Paginator as DjangoPaginator +from django.template import Context, loader from django.utils import six from django.utils.translation import ugettext as _ from rest_framework.compat import OrderedDict from rest_framework.exceptions import NotFound from rest_framework.response import Response from rest_framework.settings import api_settings -from rest_framework.templatetags.rest_framework import replace_query_param +from rest_framework.templatetags.rest_framework import ( + replace_query_param, remove_query_param +) def _strict_positive_int(integer_string, cutoff=None): @@ -35,6 +39,49 @@ def _get_count(queryset): return len(queryset) +def _get_displayed_page_numbers(current, final): + """ + This utility function determines a list of page numbers to display. + This gives us a nice contextually relevant set of page numbers. + + For example: + current=14, final=16 -> [1, None, 13, 14, 15, 16] + """ + assert current >= 1 + assert final >= current + + # We always include the first two pages, last two pages, and + # two pages either side of the current page. + included = set(( + 1, + current - 1, current, current + 1, + final + )) + + # If the break would only exclude a single page number then we + # may as well include the page number instead of the break. + if current == 4: + included.add(2) + if current == final - 3: + included.add(final - 1) + + # Now sort the page numbers and drop anything outside the limits. + included = [ + idx for idx in sorted(list(included)) + if idx > 0 and idx <= final + ] + + # Finally insert any `...` breaks + if current > 4: + included.insert(1, None) + if current < final - 3: + included.insert(len(included) - 1, None) + return included + + +PageLink = namedtuple('PageLink', ['url', 'number', 'is_active', 'is_break']) + + class BasePagination(object): def paginate_queryset(self, queryset, request, view): raise NotImplemented('paginate_queryset() must be implemented.') @@ -66,6 +113,8 @@ class PageNumberPagination(BasePagination): # Only relevant if 'paginate_by_param' has also been set. max_paginate_by = api_settings.MAX_PAGINATE_BY + template = 'rest_framework/pagination/numbers.html' + def paginate_queryset(self, queryset, request, view): """ Paginate a queryset if required, either returning a @@ -104,6 +153,8 @@ class PageNumberPagination(BasePagination): ) raise NotFound(msg) + # Indicate that the browsable API should display pagination controls. + self.mark_as_used = True self.request = request return self.page @@ -139,8 +190,45 @@ class PageNumberPagination(BasePagination): return None url = self.request.build_absolute_uri() page_number = self.page.previous_page_number() + if page_number == 1: + return remove_query_param(url, self.page_query_param) return replace_query_param(url, self.page_query_param, page_number) + def to_html(self): + current = self.page.number + final = self.page.paginator.num_pages + + page_links = [] + base_url = self.request.build_absolute_uri() + for page_number in _get_displayed_page_numbers(current, final): + if page_number is None: + page_link = PageLink( + url=None, + number=None, + is_active=False, + is_break=True + ) + else: + if page_number == 1: + url = remove_query_param(base_url, self.page_query_param) + else: + url = replace_query_param(url, self.page_query_param, page_number) + page_link = PageLink( + url=url, + number=page_number, + is_active=(page_number == current), + is_break=False + ) + page_links.append(page_link) + + template = loader.get_template(self.template) + context = Context({ + 'previous_url': self.get_previous_link(), + 'next_url': self.get_next_link(), + 'page_links': page_links + }) + return template.render(context) + class LimitOffsetPagination(BasePagination): """ diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index c4de30db..4c002b16 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -592,6 +592,7 @@ class BrowsableAPIRenderer(BaseRenderer): 'description': self.get_description(view), 'name': self.get_name(view), 'version': VERSION, + 'pager': getattr(view, 'pager', None), 'breadcrumblist': self.get_breadcrumbs(request), 'allowed_methods': view.allowed_methods, 'available_formats': [renderer_cls.format for renderer_cls in view.renderer_classes], diff --git a/rest_framework/static/rest_framework/css/bootstrap-tweaks.css b/rest_framework/static/rest_framework/css/bootstrap-tweaks.css index 36c7be48..d4a7d31a 100644 --- a/rest_framework/static/rest_framework/css/bootstrap-tweaks.css +++ b/rest_framework/static/rest_framework/css/bootstrap-tweaks.css @@ -185,10 +185,6 @@ body a:hover { color: #c20000; } -#content a span { - text-decoration: underline; - } - .request-info { clear:both; } diff --git a/rest_framework/templates/rest_framework/base.html b/rest_framework/templates/rest_framework/base.html index e9668193..e0030981 100644 --- a/rest_framework/templates/rest_framework/base.html +++ b/rest_framework/templates/rest_framework/base.html @@ -119,9 +119,18 @@ +
{% block description %} {{ description }} {% endblock %} +
+ + {% if pager.mark_as_used %} + + {% endif %} +
{{ request.method }} {{ request.get_full_path }}
diff --git a/rest_framework/templates/rest_framework/pagination/numbers.html b/rest_framework/templates/rest_framework/pagination/numbers.html new file mode 100644 index 00000000..04045810 --- /dev/null +++ b/rest_framework/templates/rest_framework/pagination/numbers.html @@ -0,0 +1,27 @@ + diff --git a/rest_framework/templatetags/rest_framework.py b/rest_framework/templatetags/rest_framework.py index 69e03af4..bf159d8b 100644 --- a/rest_framework/templatetags/rest_framework.py +++ b/rest_framework/templatetags/rest_framework.py @@ -26,6 +26,23 @@ def replace_query_param(url, key, val): return urlparse.urlunsplit((scheme, netloc, path, query, fragment)) +def remove_query_param(url, key): + """ + Given a URL and a key/val pair, set or replace an item in the query + parameters of the URL, and return the new URL. + """ + (scheme, netloc, path, query, fragment) = urlparse.urlsplit(url) + query_dict = QueryDict(query).copy() + query_dict.pop(key, None) + query = query_dict.urlencode() + return urlparse.urlunsplit((scheme, netloc, path, query, fragment)) + + +@register.simple_tag +def get_pagination_html(pager): + return pager.to_html() + + # Regex for adding classes to html snippets class_re = re.compile(r'(?<=class=["\'])(.*)(?=["\'])') -- cgit v1.2.3 From 313aa727e3c44016e531a7af75051fc6e6d7cb96 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 14 Jan 2015 17:46:41 +0000 Subject: Tweaks --- rest_framework/pagination.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index bd343c0d..69d0f77d 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -46,6 +46,12 @@ def _get_displayed_page_numbers(current, final): For example: current=14, final=16 -> [1, None, 13, 14, 15, 16] + + This implementation gives one page to each side of the cursor, + for an implementation which gives two pages to each side of the cursor, + which is a copy of how GitHub treat pagination in their issue lists, see: + + https://gist.github.com/tomchristie/321140cebb1c4a558b15 """ assert current >= 1 assert final >= current @@ -60,10 +66,12 @@ def _get_displayed_page_numbers(current, final): # If the break would only exclude a single page number then we # may as well include the page number instead of the break. - if current == 4: + if current <= 4: included.add(2) - if current == final - 3: + included.add(3) + if current >= final - 3: included.add(final - 1) + included.add(final - 2) # Now sort the page numbers and drop anything outside the limits. included = [ -- cgit v1.2.3 From d76e83dd78627a0cf4bcd4b28a7710fb678d8d4e Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 15 Jan 2015 16:52:07 +0000 Subject: Tweaks, and add pagination controls for offset/limit. --- rest_framework/generics.py | 16 +-- rest_framework/pagination.py | 126 ++++++++++++++++----- rest_framework/renderers.py | 7 +- .../static/rest_framework/css/bootstrap-tweaks.css | 7 ++ rest_framework/templates/rest_framework/base.html | 4 +- 5 files changed, 119 insertions(+), 41 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/generics.py b/rest_framework/generics.py index cdf6ece0..4cc4c64d 100644 --- a/rest_framework/generics.py +++ b/rest_framework/generics.py @@ -150,21 +150,21 @@ class GenericAPIView(views.APIView): return queryset @property - def pager(self): - if not hasattr(self, '_pager'): + def paginator(self): + if not hasattr(self, '_paginator'): if self.pagination_class is None: - self._pager = None + self._paginator = None else: - self._pager = self.pagination_class() - return self._pager + self._paginator = self.pagination_class() + return self._paginator def paginate_queryset(self, queryset): - if self.pager is None: + if self.paginator is None: return queryset - return self.pager.paginate_queryset(queryset, self.request, view=self) + return self.paginator.paginate_queryset(queryset, self.request, view=self) def get_paginated_response(self, data): - return self.pager.get_paginated_response(data) + return self.paginator.get_paginated_response(data) # Concrete view classes that provide method handlers diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 69d0f77d..2b78f1f7 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -29,6 +29,15 @@ def _strict_positive_int(integer_string, cutoff=None): return ret +def _divide_with_ceil(a, b): + """ + Returns 'a' divded by 'b', with any remainder rounded up. + """ + if a % b: + return (a / b) + 1 + return a / b + + def _get_count(queryset): """ Determine an object count, supporting either querysets or regular lists. @@ -48,14 +57,21 @@ def _get_displayed_page_numbers(current, final): current=14, final=16 -> [1, None, 13, 14, 15, 16] This implementation gives one page to each side of the cursor, - for an implementation which gives two pages to each side of the cursor, - which is a copy of how GitHub treat pagination in their issue lists, see: + or two pages to the side when the cursor is at the edge, then + ensures that any breaks between non-continous page numbers never + remove only a single page. + + For an alernativative implementation which gives two pages to each side of + the cursor, eg. as in GitHub issue list pagination, see: https://gist.github.com/tomchristie/321140cebb1c4a558b15 """ assert current >= 1 assert final >= current + if final <= 5: + return range(1, final + 1) + # We always include the first two pages, last two pages, and # two pages either side of the current page. included = set(( @@ -87,16 +103,46 @@ def _get_displayed_page_numbers(current, final): return included +def _get_page_links(page_numbers, current, url_func): + """ + Given a list of page numbers and `None` page breaks, + return a list of `PageLink` objects. + """ + page_links = [] + for page_number in page_numbers: + if page_number is None: + page_link = PageLink( + url=None, + number=None, + is_active=False, + is_break=True + ) + else: + page_link = PageLink( + url=url_func(page_number), + number=page_number, + is_active=(page_number == current), + is_break=False + ) + page_links.append(page_link) + return page_links + + PageLink = namedtuple('PageLink', ['url', 'number', 'is_active', 'is_break']) class BasePagination(object): + display_page_controls = False + def paginate_queryset(self, queryset, request, view): raise NotImplemented('paginate_queryset() must be implemented.') def get_paginated_response(self, data): raise NotImplemented('get_paginated_response() must be implemented.') + def to_html(self): + raise NotImplemented('to_html() must be implemented to display page controls.') + class PageNumberPagination(BasePagination): """ @@ -161,8 +207,9 @@ class PageNumberPagination(BasePagination): ) raise NotFound(msg) - # Indicate that the browsable API should display pagination controls. - self.mark_as_used = True + if paginator.count > 1: + # The browsable API should display pagination controls. + self.display_page_controls = True self.request = request return self.page @@ -203,31 +250,17 @@ class PageNumberPagination(BasePagination): return replace_query_param(url, self.page_query_param, page_number) def to_html(self): - current = self.page.number - final = self.page.paginator.num_pages - - page_links = [] base_url = self.request.build_absolute_uri() - for page_number in _get_displayed_page_numbers(current, final): - if page_number is None: - page_link = PageLink( - url=None, - number=None, - is_active=False, - is_break=True - ) + def page_number_to_url(page_number): + if page_number == 1: + return remove_query_param(base_url, self.page_query_param) else: - if page_number == 1: - url = remove_query_param(base_url, self.page_query_param) - else: - url = replace_query_param(url, self.page_query_param, page_number) - page_link = PageLink( - url=url, - number=page_number, - is_active=(page_number == current), - is_break=False - ) - page_links.append(page_link) + return replace_query_param(base_url, self.page_query_param, page_number) + + current = self.page.number + final = self.page.paginator.num_pages + page_numbers = _get_displayed_page_numbers(current, final) + page_links = _get_page_links(page_numbers, current, page_number_to_url) template = loader.get_template(self.template) context = Context({ @@ -250,11 +283,15 @@ class LimitOffsetPagination(BasePagination): offset_query_param = 'offset' max_limit = None + template = 'rest_framework/pagination/numbers.html' + def paginate_queryset(self, queryset, request, view): self.limit = self.get_limit(request) self.offset = self.get_offset(request) self.count = _get_count(queryset) self.request = request + if self.count > self.limit: + self.display_page_controls = True return queryset[self.offset:self.offset + self.limit] def get_paginated_response(self, data): @@ -285,16 +322,45 @@ class LimitOffsetPagination(BasePagination): except (KeyError, ValueError): return 0 - def get_next_link(self, page): + def get_next_link(self): if self.offset + self.limit >= self.count: return None + url = self.request.build_absolute_uri() offset = self.offset + self.limit return replace_query_param(url, self.offset_query_param, offset) - def get_previous_link(self, page): - if self.offset - self.limit < 0: + def get_previous_link(self): + if self.offset <= 0: return None + url = self.request.build_absolute_uri() + + if self.offset - self.limit <= 0: + return remove_query_param(url, self.offset_query_param) + offset = self.offset - self.limit return replace_query_param(url, self.offset_query_param, offset) + + def to_html(self): + base_url = self.request.build_absolute_uri() + current = _divide_with_ceil(self.offset, self.limit) + 1 + final = _divide_with_ceil(self.count, self.limit) + + def page_number_to_url(page_number): + if page_number == 1: + return remove_query_param(base_url, self.offset_query_param) + else: + offset = self.offset + ((page_number - current) * self.limit) + return replace_query_param(base_url, self.offset_query_param, offset) + + page_numbers = _get_displayed_page_numbers(current, final) + page_links = _get_page_links(page_numbers, current, page_number_to_url) + + template = loader.get_template(self.template) + context = Context({ + 'previous_url': self.get_previous_link(), + 'next_url': self.get_next_link(), + 'page_links': page_links + }) + return template.render(context) \ No newline at end of file diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 4c002b16..4c46b049 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -584,6 +584,11 @@ class BrowsableAPIRenderer(BaseRenderer): renderer_content_type += ' ;%s' % renderer.charset response_headers['Content-Type'] = renderer_content_type + if hasattr(view, 'paginator') and view.paginator.display_page_controls: + paginator = view.paginator + else: + paginator = None + context = { 'content': self.get_content(renderer, data, accepted_media_type, renderer_context), 'view': view, @@ -592,7 +597,7 @@ class BrowsableAPIRenderer(BaseRenderer): 'description': self.get_description(view), 'name': self.get_name(view), 'version': VERSION, - 'pager': getattr(view, 'pager', None), + 'paginator': paginator, 'breadcrumblist': self.get_breadcrumbs(request), 'allowed_methods': view.allowed_methods, 'available_formats': [renderer_cls.format for renderer_cls in view.renderer_classes], diff --git a/rest_framework/static/rest_framework/css/bootstrap-tweaks.css b/rest_framework/static/rest_framework/css/bootstrap-tweaks.css index d4a7d31a..15b42178 100644 --- a/rest_framework/static/rest_framework/css/bootstrap-tweaks.css +++ b/rest_framework/static/rest_framework/css/bootstrap-tweaks.css @@ -60,6 +60,13 @@ a single block in the template. color: #C20000; } +.pagination>.disabled>a, +.pagination>.disabled>a:hover, +.pagination>.disabled>a:focus { + cursor: default; + pointer-events: none; +} + /*=== dabapps bootstrap styles ====*/ html { diff --git a/rest_framework/templates/rest_framework/base.html b/rest_framework/templates/rest_framework/base.html index e0030981..877387f2 100644 --- a/rest_framework/templates/rest_framework/base.html +++ b/rest_framework/templates/rest_framework/base.html @@ -125,9 +125,9 @@ {% endblock %} - {% if pager.mark_as_used %} + {% if paginator %} {% endif %} -- cgit v1.2.3 From 68dfa369b5ca877643b41c8df7c5fc0c786a9f08 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 15 Jan 2015 16:55:04 +0000 Subject: Flake 8 fixes --- rest_framework/pagination.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'rest_framework') diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 2b78f1f7..61b8e07a 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -251,6 +251,7 @@ class PageNumberPagination(BasePagination): def to_html(self): base_url = self.request.build_absolute_uri() + def page_number_to_url(page_number): if page_number == 1: return remove_query_param(base_url, self.page_query_param) @@ -363,4 +364,4 @@ class LimitOffsetPagination(BasePagination): 'next_url': self.get_next_link(), 'page_links': page_links }) - return template.render(context) \ No newline at end of file + return template.render(context) -- cgit v1.2.3 From 53edd37df5aa0ac29dbe7824db2e33da1d901f98 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 15 Jan 2015 21:07:05 +0000 Subject: Tests for LimitOffsetPagination --- rest_framework/pagination.py | 57 +++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 30 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 61b8e07a..0dac5683 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -44,7 +44,7 @@ def _get_count(queryset): """ try: return queryset.count() - except AttributeError: + except (AttributeError, TypeError): return len(queryset) @@ -111,12 +111,7 @@ def _get_page_links(page_numbers, current, url_func): page_links = [] for page_number in page_numbers: if page_number is None: - page_link = PageLink( - url=None, - number=None, - is_active=False, - is_break=True - ) + page_link = PAGE_BREAK else: page_link = PageLink( url=url_func(page_number), @@ -130,11 +125,13 @@ def _get_page_links(page_numbers, current, url_func): PageLink = namedtuple('PageLink', ['url', 'number', 'is_active', 'is_break']) +PAGE_BREAK = PageLink(url=None, number=None, is_active=False, is_break=True) + class BasePagination(object): display_page_controls = False - def paginate_queryset(self, queryset, request, view): + def paginate_queryset(self, queryset, request, view=None): raise NotImplemented('paginate_queryset() must be implemented.') def get_paginated_response(self, data): @@ -167,9 +164,11 @@ class PageNumberPagination(BasePagination): # Only relevant if 'paginate_by_param' has also been set. max_paginate_by = api_settings.MAX_PAGINATE_BY + last_page_strings = ('last',) + template = 'rest_framework/pagination/numbers.html' - def paginate_queryset(self, queryset, request, view): + def paginate_queryset(self, queryset, request, view=None): """ Paginate a queryset if required, either returning a page object, or `None` if pagination is not configured for this view. @@ -186,18 +185,9 @@ class PageNumberPagination(BasePagination): return None paginator = DjangoPaginator(queryset, page_size) - page_string = request.query_params.get(self.page_query_param, 1) - try: - page_number = paginator.validate_number(page_string) - except InvalidPage: - if page_string == 'last': - page_number = paginator.num_pages - else: - msg = _( - 'Choose a valid page number. Page numbers must be a ' - 'whole number, or must be the string "last".' - ) - raise NotFound(msg) + page_number = request.query_params.get(self.page_query_param, 1) + if page_number in self.last_page_strings: + page_number = paginator.num_pages try: self.page = paginator.page(page_number) @@ -210,6 +200,7 @@ class PageNumberPagination(BasePagination): if paginator.count > 1: # The browsable API should display pagination controls. self.display_page_controls = True + self.request = request return self.page @@ -249,7 +240,7 @@ class PageNumberPagination(BasePagination): return remove_query_param(url, self.page_query_param) return replace_query_param(url, self.page_query_param, page_number) - def to_html(self): + def get_html_context(self): base_url = self.request.build_absolute_uri() def page_number_to_url(page_number): @@ -263,12 +254,15 @@ class PageNumberPagination(BasePagination): page_numbers = _get_displayed_page_numbers(current, final) page_links = _get_page_links(page_numbers, current, page_number_to_url) - template = loader.get_template(self.template) - context = Context({ + return { 'previous_url': self.get_previous_link(), 'next_url': self.get_next_link(), 'page_links': page_links - }) + } + + def to_html(self): + template = loader.get_template(self.template) + context = Context(self.get_html_context()) return template.render(context) @@ -286,7 +280,7 @@ class LimitOffsetPagination(BasePagination): template = 'rest_framework/pagination/numbers.html' - def paginate_queryset(self, queryset, request, view): + def paginate_queryset(self, queryset, request, view=None): self.limit = self.get_limit(request) self.offset = self.get_offset(request) self.count = _get_count(queryset) @@ -343,7 +337,7 @@ class LimitOffsetPagination(BasePagination): offset = self.offset - self.limit return replace_query_param(url, self.offset_query_param, offset) - def to_html(self): + def get_html_context(self): base_url = self.request.build_absolute_uri() current = _divide_with_ceil(self.offset, self.limit) + 1 final = _divide_with_ceil(self.count, self.limit) @@ -358,10 +352,13 @@ class LimitOffsetPagination(BasePagination): page_numbers = _get_displayed_page_numbers(current, final) page_links = _get_page_links(page_numbers, current, page_number_to_url) - template = loader.get_template(self.template) - context = Context({ + return { 'previous_url': self.get_previous_link(), 'next_url': self.get_next_link(), 'page_links': page_links - }) + } + + def to_html(self): + template = loader.get_template(self.template) + context = Context(self.get_html_context()) return template.render(context) -- cgit v1.2.3 From 8b0f25aa0a91cb7b56f9ce4dde4330fe5daaad9b Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 16 Jan 2015 16:55:46 +0000 Subject: More pagination tests & cleanup --- rest_framework/generics.py | 12 +++++++++++- rest_framework/pagination.py | 31 +++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 9 deletions(-) (limited to 'rest_framework') diff --git a/rest_framework/generics.py b/rest_framework/generics.py index 4cc4c64d..61dcb84a 100644 --- a/rest_framework/generics.py +++ b/rest_framework/generics.py @@ -151,6 +151,9 @@ class GenericAPIView(views.APIView): @property def paginator(self): + """ + The paginator instance associated with the view, or `None`. + """ if not hasattr(self, '_paginator'): if self.pagination_class is None: self._paginator = None @@ -159,11 +162,18 @@ class GenericAPIView(views.APIView): return self._paginator def paginate_queryset(self, queryset): + """ + Return a single page of results, or `None` if pagination is disabled. + """ if self.paginator is None: - return queryset + return None return self.paginator.paginate_queryset(queryset, self.request, view=self) def get_paginated_response(self, data): + """ + Return a paginated style `Response` object for the given output data. + """ + assert self.paginator is not None return self.paginator.get_paginated_response(data) diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index 0dac5683..c5a364f0 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -131,13 +131,13 @@ PAGE_BREAK = PageLink(url=None, number=None, is_active=False, is_break=True) class BasePagination(object): display_page_controls = False - def paginate_queryset(self, queryset, request, view=None): + def paginate_queryset(self, queryset, request, view=None): # pragma: no cover raise NotImplemented('paginate_queryset() must be implemented.') - def get_paginated_response(self, data): + def get_paginated_response(self, data): # pragma: no cover raise NotImplemented('get_paginated_response() must be implemented.') - def to_html(self): + def to_html(self): # pragma: no cover raise NotImplemented('to_html() must be implemented to display page controls.') @@ -168,10 +168,11 @@ class PageNumberPagination(BasePagination): template = 'rest_framework/pagination/numbers.html' - def paginate_queryset(self, queryset, request, view=None): + def _handle_backwards_compat(self, view): """ - Paginate a queryset if required, either returning a - page object, or `None` if pagination is not configured for this view. + Prior to version 3.1, pagination was handled in the view, and the + attributes were set there. The attributes should now be set on + the pagination class, but the old style is still pending deprecation. """ for attr in ( 'paginate_by', 'page_query_param', @@ -180,6 +181,13 @@ class PageNumberPagination(BasePagination): if hasattr(view, attr): setattr(self, attr, getattr(view, attr)) + def paginate_queryset(self, queryset, request, view=None): + """ + Paginate a queryset if required, either returning a + page object, or `None` if pagination is not configured for this view. + """ + self._handle_backwards_compat(view) + page_size = self.get_page_size(request) if not page_size: return None @@ -277,7 +285,6 @@ class LimitOffsetPagination(BasePagination): limit_query_param = 'limit' offset_query_param = 'offset' max_limit = None - template = 'rest_framework/pagination/numbers.html' def paginate_queryset(self, queryset, request, view=None): @@ -340,7 +347,15 @@ class LimitOffsetPagination(BasePagination): def get_html_context(self): base_url = self.request.build_absolute_uri() current = _divide_with_ceil(self.offset, self.limit) + 1 - final = _divide_with_ceil(self.count, self.limit) + # The number of pages is a little bit fiddly. + # We need to sum both the number of pages from current offset to end + # plus the number of pages up to the current offset. + # When offset is not strictly divisible by the limit then we may + # end up introducing an extra page as an artifact. + final = ( + _divide_with_ceil(self.count - self.offset, self.limit) + + _divide_with_ceil(self.offset, self.limit) + ) def page_number_to_url(page_number): if page_number == 1: -- cgit v1.2.3 From 86d2774cf30351fd4174e97501532056ed0d8f95 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 16 Jan 2015 20:30:46 +0000 Subject: Fix compat issues --- rest_framework/pagination.py | 8 +++---- rest_framework/templatetags/rest_framework.py | 34 +++------------------------ rest_framework/utils/urls.py | 25 ++++++++++++++++++++ 3 files changed, 32 insertions(+), 35 deletions(-) create mode 100644 rest_framework/utils/urls.py (limited to 'rest_framework') diff --git a/rest_framework/pagination.py b/rest_framework/pagination.py index c5a364f0..1b7524c6 100644 --- a/rest_framework/pagination.py +++ b/rest_framework/pagination.py @@ -12,7 +12,7 @@ from rest_framework.compat import OrderedDict from rest_framework.exceptions import NotFound from rest_framework.response import Response from rest_framework.settings import api_settings -from rest_framework.templatetags.rest_framework import ( +from rest_framework.utils.urls import ( replace_query_param, remove_query_param ) @@ -34,8 +34,8 @@ def _divide_with_ceil(a, b): Returns 'a' divded by 'b', with any remainder rounded up. """ if a % b: - return (a / b) + 1 - return a / b + return (a // b) + 1 + return a // b def _get_count(queryset): @@ -70,7 +70,7 @@ def _get_displayed_page_numbers(current, final): assert final >= current if final <= 5: - return range(1, final + 1) + return list(range(1, final + 1)) # We always include the first two pages, last two pages, and # two pages either side of the current page. diff --git a/rest_framework/templatetags/rest_framework.py b/rest_framework/templatetags/rest_framework.py index bf159d8b..a969836f 100644 --- a/rest_framework/templatetags/rest_framework.py +++ b/rest_framework/templatetags/rest_framework.py @@ -1,41 +1,19 @@ from __future__ import unicode_literals, absolute_import from django import template from django.core.urlresolvers import reverse, NoReverseMatch -from django.http import QueryDict from django.utils import six -from django.utils.six.moves.urllib import parse as urlparse from django.utils.encoding import iri_to_uri, force_text from django.utils.html import escape from django.utils.safestring import SafeData, mark_safe from django.utils.html import smart_urlquote from rest_framework.renderers import HTMLFormRenderer +from rest_framework.utils.urls import replace_query_param import re register = template.Library() - -def replace_query_param(url, key, val): - """ - Given a URL and a key/val pair, set or replace an item in the query - parameters of the URL, and return the new URL. - """ - (scheme, netloc, path, query, fragment) = urlparse.urlsplit(url) - query_dict = QueryDict(query).copy() - query_dict[key] = val - query = query_dict.urlencode() - return urlparse.urlunsplit((scheme, netloc, path, query, fragment)) - - -def remove_query_param(url, key): - """ - Given a URL and a key/val pair, set or replace an item in the query - parameters of the URL, and return the new URL. - """ - (scheme, netloc, path, query, fragment) = urlparse.urlsplit(url) - query_dict = QueryDict(query).copy() - query_dict.pop(key, None) - query = query_dict.urlencode() - return urlparse.urlunsplit((scheme, netloc, path, query, fragment)) +# Regex for adding classes to html snippets +class_re = re.compile(r'(?<=class=["\'])(.*)(?=["\'])') @register.simple_tag @@ -43,12 +21,6 @@ def get_pagination_html(pager): return pager.to_html() -# Regex for adding classes to html snippets -class_re = re.compile(r'(?<=class=["\'])(.*)(?=["\'])') - - -# And the template tags themselves... - @register.simple_tag def render_field(field, style=None): style = style or {} diff --git a/rest_framework/utils/urls.py b/rest_framework/utils/urls.py new file mode 100644 index 00000000..880ef9ed --- /dev/null +++ b/rest_framework/utils/urls.py @@ -0,0 +1,25 @@ +from django.utils.six.moves.urllib import parse as urlparse + + +def replace_query_param(url, key, val): + """ + Given a URL and a key/val pair, set or replace an item in the query + parameters of the URL, and return the new URL. + """ + (scheme, netloc, path, query, fragment) = urlparse.urlsplit(url) + query_dict = urlparse.parse_qs(query) + query_dict[key] = [val] + query = urlparse.urlencode(sorted(list(query_dict.items())), doseq=True) + return urlparse.urlunsplit((scheme, netloc, path, query, fragment)) + + +def remove_query_param(url, key): + """ + Given a URL and a key/val pair, remove an item in the query + parameters of the URL, and return the new URL. + """ + (scheme, netloc, path, query, fragment) = urlparse.urlsplit(url) + query_dict = urlparse.parse_qs(query) + query_dict.pop(key, None) + query = urlparse.urlencode(sorted(list(query_dict.items())), doseq=True) + return urlparse.urlunsplit((scheme, netloc, path, query, fragment)) -- cgit v1.2.3