From a13bf52dbf50b34ed28903d535b9b00594720b42 Mon Sep 17 00:00:00 2001 From: Ehtesham Date: Fri, 8 Jan 2016 17:26:33 +0500 Subject: [PATCH] [MA-1862] using NamespacedPageNumberPagination class for pagination --- lms/djangoapps/discussion_api/api.py | 17 +++-- lms/djangoapps/discussion_api/pagination.py | 73 +++++++++++++-------- lms/djangoapps/discussion_api/views.py | 50 +++++++------- openedx/core/lib/api/paginators.py | 16 ++++- 4 files changed, 93 insertions(+), 63 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 4b36b7f6aa..825d23f4ad 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -18,7 +18,6 @@ from courseware.courses import get_course_with_access from discussion_api.exceptions import ThreadNotFoundError, CommentNotFoundError, DiscussionDisabledError from discussion_api.forms import CommentActionsForm, ThreadActionsForm -from discussion_api.pagination import get_paginated_data from discussion_api.permissions import ( can_delete, get_editable_fields, @@ -38,6 +37,7 @@ from django_comment_common.signals import ( comment_deleted, ) from django_comment_client.utils import get_accessible_discussion_modules, is_commentable_cohorted +from lms.djangoapps.discussion_api.pagination import DiscussionAPIPagination from lms.lib.comment_client.comment import Comment from lms.lib.comment_client.thread import Thread from lms.lib.comment_client.utils import CommentClientRequestError @@ -341,9 +341,12 @@ def get_thread_list( raise PageNotFoundError("Page not found (No results on this page).") results = [ThreadSerializer(thread, context=context).data for thread in threads] - ret = get_paginated_data(request, results, page, num_pages) - ret["text_search_rewrite"] = text_search_rewrite - return ret + + paginator = DiscussionAPIPagination(request, result_page, num_pages) + return paginator.get_paginated_response({ + "results": results, + "text_search_rewrite": text_search_rewrite, + }) def get_comment_list(request, thread_id, endorsed, page, page_size): @@ -412,7 +415,8 @@ def get_comment_list(request, thread_id, endorsed, page, page_size): num_pages = (resp_total + page_size - 1) / page_size if resp_total else 1 results = [CommentSerializer(response, context=context).data for response in responses] - return get_paginated_data(request, results, page, num_pages) + paginator = DiscussionAPIPagination(request, page, num_pages) + return paginator.get_paginated_response(results) def _check_fields(allowed_fields, data, message): @@ -751,7 +755,8 @@ def get_response_comments(request, comment_id, page, page_size): comments_count = len(response_comments) num_pages = (comments_count + page_size - 1) / page_size if comments_count else 1 - return get_paginated_data(request, results, page, num_pages) + paginator = DiscussionAPIPagination(request, page, num_pages) + return paginator.get_paginated_response(results) except CommentClientRequestError: raise CommentNotFoundError("Comment not found") diff --git a/lms/djangoapps/discussion_api/pagination.py b/lms/djangoapps/discussion_api/pagination.py index e818c5da19..c355ee899d 100644 --- a/lms/djangoapps/discussion_api/pagination.py +++ b/lms/djangoapps/discussion_api/pagination.py @@ -2,6 +2,7 @@ Discussion API pagination support """ from rest_framework.utils.urls import replace_query_param +from openedx.core.lib.api.paginators import NamespacedPageNumberPagination class _Page(object): @@ -9,12 +10,11 @@ class _Page(object): Implements just enough of the django.core.paginator.Page interface to allow PaginationSerializer to work. """ - def __init__(self, object_list, page_num, num_pages): + def __init__(self, page_num, num_pages): """ Create a new page containing the given objects, with the given page number and number of pages """ - self.object_list = object_list self.page_num = page_num self.num_pages = num_pages @@ -35,33 +35,52 @@ class _Page(object): return self.page_num - 1 -def get_paginated_data(request, results, page_num, per_page): +class DiscussionAPIPagination(NamespacedPageNumberPagination): """ - Return a dict with the following values: - - next: The URL for the next page - previous: The URL for the previous page - results: The results on this page + Subclasses NamespacedPageNumberPagination to provide custom implementation of pagination metadata + by overriding it's methods """ - # Note: Previous versions of this function used Django Rest Framework's - # paginated serializer. With the upgrade to DRF 3.1, paginated serializers - # have been removed. We *could* use DRF's paginator classes, but there are - # some slight differences between how DRF does pagination and how we're doing - # pagination here. (For example, we respond with a next_url param even if - # there is only one result on the current page.) To maintain backwards - # compatability, we simulate the behavior that DRF used to provide. - page = _Page(results, page_num, per_page) - next_url, previous_url = None, None - base_url = request.build_absolute_uri() + def __init__(self, request, page_num, num_pages): + """ + Overrides parent constructor to take information from discussion api + essential for the parent method + """ + self.page = _Page(page_num, num_pages) + self.base_url = request.build_absolute_uri() + self.num_pages = num_pages - if page.has_next(): - next_url = replace_query_param(base_url, "page", page.next_page_number()) + super(DiscussionAPIPagination, self).__init__() - if page.has_previous(): - previous_url = replace_query_param(base_url, "page", page.previous_page_number()) + def get_result_count(self): + """ + A count can't be calculated with the information coming from the + Forum's api, so returning 0 for the parent method to work + """ + # TODO: return actual count + return 0 - return { - "next": next_url, - "previous": previous_url, - "results": results, - } + def get_num_pages(self): + """ + Returns total number of pages the response is divided into + """ + return self.num_pages + + def get_next_link(self): + """ + Returns absolute url of the next page if there's a next page available + otherwise returns None + """ + next_url = None + if self.page.has_next(): + next_url = replace_query_param(self.base_url, "page", self.page.next_page_number()) + return next_url + + def get_previous_link(self): + """ + Returns absolute url of the previous page if there's a previous page available + otherwise returns None + """ + previous_url = None + if self.page.has_previous(): + previous_url = replace_query_param(self.base_url, "page", self.page.previous_page_number()) + return previous_url diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 83ab5cac0f..f082211bd0 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -261,19 +261,17 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): form = ThreadListGetForm(request.GET) if not form.is_valid(): raise ValidationError(form.errors) - return Response( - get_thread_list( - request, - form.cleaned_data["course_id"], - form.cleaned_data["page"], - form.cleaned_data["page_size"], - form.cleaned_data["topic_id"], - form.cleaned_data["text_search"], - form.cleaned_data["following"], - form.cleaned_data["view"], - form.cleaned_data["order_by"], - form.cleaned_data["order_direction"], - ) + return get_thread_list( + request, + form.cleaned_data["course_id"], + form.cleaned_data["page"], + form.cleaned_data["page_size"], + form.cleaned_data["topic_id"], + form.cleaned_data["text_search"], + form.cleaned_data["following"], + form.cleaned_data["view"], + form.cleaned_data["order_by"], + form.cleaned_data["order_direction"], ) def retrieve(self, request, thread_id=None): @@ -443,14 +441,12 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet): form = CommentListGetForm(request.GET) if not form.is_valid(): raise ValidationError(form.errors) - return Response( - get_comment_list( - request, - form.cleaned_data["thread_id"], - form.cleaned_data["endorsed"], - form.cleaned_data["page"], - form.cleaned_data["page_size"] - ) + return get_comment_list( + request, + form.cleaned_data["thread_id"], + form.cleaned_data["endorsed"], + form.cleaned_data["page"], + form.cleaned_data["page_size"] ) def retrieve(self, request, comment_id=None): @@ -460,13 +456,11 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet): form = _PaginationForm(request.GET) if not form.is_valid(): raise ValidationError(form.errors) - return Response( - get_response_comments( - request, - comment_id, - form.cleaned_data["page"], - form.cleaned_data["page_size"] - ) + return get_response_comments( + request, + comment_id, + form.cleaned_data["page"], + form.cleaned_data["page_size"] ) def create(self, request): diff --git a/openedx/core/lib/api/paginators.py b/openedx/core/lib/api/paginators.py index 3fa9e0831c..ec33e78ff7 100644 --- a/openedx/core/lib/api/paginators.py +++ b/openedx/core/lib/api/paginators.py @@ -39,6 +39,18 @@ class NamespacedPageNumberPagination(pagination.PageNumberPagination): page_size_query_param = "page_size" + def get_result_count(self): + """ + Returns total number of results + """ + return self.page.paginator.count + + def get_num_pages(self): + """ + Returns total number of pages the results are divided into + """ + return self.page.paginator.num_pages + def get_paginated_response(self, data): """ Annotate the response with pagination information @@ -46,8 +58,8 @@ class NamespacedPageNumberPagination(pagination.PageNumberPagination): metadata = { 'next': self.get_next_link(), 'previous': self.get_previous_link(), - 'count': self.page.paginator.count, - 'num_pages': self.page.paginator.num_pages, + 'count': self.get_result_count(), + 'num_pages': self.get_num_pages(), } if isinstance(data, dict): if 'results' not in data: