[MA-1862] using NamespacedPageNumberPagination class for pagination

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")
 

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
+
+        super(DiscussionAPIPagination, self).__init__()
+
+    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
 
-    if page.has_next():
-        next_url = replace_query_param(base_url, "page", page.next_page_number())
+    def get_num_pages(self):
+        """
+        Returns total number of pages the response is divided into
+        """
+        return self.num_pages
 
-    if page.has_previous():
-        previous_url = replace_query_param(base_url, "page", page.previous_page_number())
+    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
 
-    return {
-        "next": next_url,
-        "previous": previous_url,
-        "results": results,
-    }
+    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

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):

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: