From 19358bd6f64578d9ca477f72145b280e26d901a2 Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Thu, 30 May 2024 19:01:44 +0300 Subject: [PATCH 1/2] feat: Use default pagination for v2 lib endpoints This ensures `num_pages` is included in paginated response. --- .../tests/test_content_libraries.py | 4 +- .../djangoapps/content_libraries/views.py | 53 +++++++++++-------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 67933f296413..3ef025b6a527 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -137,7 +137,7 @@ def test_library_validation(self): } @skip("This endpoint shouldn't support num_blocks and has_unpublished_*.") - @patch("openedx.core.djangoapps.content_libraries.views.LibraryApiPagination.page_size", new=2) + @patch("openedx.core.djangoapps.content_libraries.views.LibraryRootView._DEFAULT_PAGE_SIZE", new=2) def test_list_library(self): """ Test the /libraries API and its pagination @@ -374,7 +374,7 @@ def test_library_blocks_studio_view(self): assert 'resources' in fragment assert 'Hello world!' in fragment['content'] - @patch("openedx.core.djangoapps.content_libraries.views.LibraryApiPagination.page_size", new=2) + @patch("openedx.core.djangoapps.content_libraries.views.LibraryBlocksView._DEFAULT_PAGE_SIZE", new=2) def test_list_library_blocks(self): """ Test the /libraries/{lib_key_str}/blocks API and its pagination diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index 38f3e7efd6a1..5657a534143d 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -90,11 +90,11 @@ from organizations.models import Organization from rest_framework import status from rest_framework.exceptions import NotFound, PermissionDenied, ValidationError -from rest_framework.pagination import PageNumberPagination +from rest_framework.generics import GenericAPIView from rest_framework.parsers import MultiPartParser from rest_framework.response import Response from rest_framework.views import APIView -from rest_framework.viewsets import ViewSet +from rest_framework.viewsets import GenericViewSet from openedx.core.djangoapps.content_libraries import api, permissions from openedx.core.djangoapps.content_libraries.serializers import ( @@ -153,13 +153,10 @@ def wrapped_fn(*args, **kwargs): return wrapped_fn -class LibraryApiPagination(PageNumberPagination): +class LibraryApiPaginationDocs: """ - Paginates over ContentLibraryMetadata objects. + API docs for query params related to paginating ContentLibraryMetadata objects. """ - page_size = 50 - page_size_query_param = 'page_size' - apidoc_params = [ apidocs.query_parameter( 'pagination', @@ -181,14 +178,16 @@ class LibraryApiPagination(PageNumberPagination): @method_decorator(non_atomic_requests, name="dispatch") @view_auth_classes() -class LibraryRootView(APIView): +class LibraryRootView(GenericAPIView): """ Views to list, search for, and create content libraries. """ + _DEFAULT_PAGE_SIZE = 50 + @apidocs.schema( parameters=[ - *LibraryApiPagination.apidoc_params, + *LibraryApiPaginationDocs.apidoc_params, apidocs.query_parameter( 'org', str, @@ -211,21 +210,23 @@ def get(self, request): library_type = serializer.validated_data['type'] text_search = serializer.validated_data['text_search'] - paginator = LibraryApiPagination() + # Set default page size to 50 + self.pagination_class.page_size = self._DEFAULT_PAGE_SIZE + queryset = api.get_libraries_for_user( request.user, org=org, library_type=library_type, text_search=text_search, ) - paginated_qs = paginator.paginate_queryset(queryset, request) + paginated_qs = self.paginate_queryset(queryset) result = api.get_metadata(paginated_qs) serializer = ContentLibraryMetadataSerializer(result, many=True) # Verify `pagination` param to maintain compatibility with older # non pagination-aware clients if request.GET.get('pagination', 'false').lower() == 'true': - return paginator.get_paginated_response(serializer.data) + return self.get_paginated_response(serializer.data) return Response(serializer.data) def post(self, request): @@ -506,13 +507,16 @@ def delete(self, request, lib_key_str): # pylint: disable=unused-argument @method_decorator(non_atomic_requests, name="dispatch") @view_auth_classes() -class LibraryBlocksView(APIView): +class LibraryBlocksView(GenericAPIView): """ Views to work with XBlocks in a specific content library. """ + + _DEFAULT_PAGE_SIZE = 50 + @apidocs.schema( parameters=[ - *LibraryApiPagination.apidoc_params, + *LibraryApiPaginationDocs.apidoc_params, apidocs.query_parameter( 'text_search', str, @@ -538,13 +542,15 @@ def get(self, request, lib_key_str): api.require_permission_for_library_key(key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY) components = api.get_library_components(key, text_search=text_search, block_types=block_types) - paginator = LibraryApiPagination() + # Set default page size to 50 + self.pagination_class.page_size = self._DEFAULT_PAGE_SIZE + paginated_xblock_metadata = [ api.LibraryXBlockMetadata.from_component(key, component) - for component in paginator.paginate_queryset(components, request) + for component in self.paginate_queryset(components) ] serializer = LibraryXBlockMetadataSerializer(paginated_xblock_metadata, many=True) - return paginator.get_paginated_response(serializer.data) + return self.get_paginated_response(serializer.data) @convert_exceptions def post(self, request, lib_key_str): @@ -742,11 +748,13 @@ def delete(self, request, usage_key_str, file_path): @method_decorator(non_atomic_requests, name="dispatch") @view_auth_classes() -class LibraryImportTaskViewSet(ViewSet): +class LibraryImportTaskViewSet(GenericViewSet): """ Import blocks from Courseware through modulestore. """ + _DEFAULT_PAGE_SIZE = 50 + @convert_exceptions def list(self, request, lib_key_str): """ @@ -760,9 +768,12 @@ def list(self, request, lib_key_str): ) queryset = api.ContentLibrary.objects.get_by_key(library_key).import_tasks result = ContentLibraryBlockImportTaskSerializer(queryset, many=True).data - paginator = LibraryApiPagination() - return paginator.get_paginated_response( - paginator.paginate_queryset(result, request) + + # Set default page size to 50 + self.pagination_class.page_size = self._DEFAULT_PAGE_SIZE + + return self.get_paginated_response( + self.paginate_queryset(result) ) @convert_exceptions From 53c8a39800b86d0ef28b4e1d9dc756126a7eab2c Mon Sep 17 00:00:00 2001 From: Yusuf Musleh Date: Thu, 6 Jun 2024 17:15:28 +0300 Subject: [PATCH 2/2] refactor: Remove default pagesize override for lib --- .../tests/test_content_libraries.py | 4 ++-- .../core/djangoapps/content_libraries/views.py | 15 --------------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 3ef025b6a527..06588c13f99d 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -137,7 +137,7 @@ def test_library_validation(self): } @skip("This endpoint shouldn't support num_blocks and has_unpublished_*.") - @patch("openedx.core.djangoapps.content_libraries.views.LibraryRootView._DEFAULT_PAGE_SIZE", new=2) + @patch("openedx.core.djangoapps.content_libraries.views.LibraryRootView.pagination_class.page_size", new=2) def test_list_library(self): """ Test the /libraries API and its pagination @@ -374,7 +374,7 @@ def test_library_blocks_studio_view(self): assert 'resources' in fragment assert 'Hello world!' in fragment['content'] - @patch("openedx.core.djangoapps.content_libraries.views.LibraryBlocksView._DEFAULT_PAGE_SIZE", new=2) + @patch("openedx.core.djangoapps.content_libraries.views.LibraryBlocksView.pagination_class.page_size", new=2) def test_list_library_blocks(self): """ Test the /libraries/{lib_key_str}/blocks API and its pagination diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index 5657a534143d..78bc5ac205d7 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -183,8 +183,6 @@ class LibraryRootView(GenericAPIView): Views to list, search for, and create content libraries. """ - _DEFAULT_PAGE_SIZE = 50 - @apidocs.schema( parameters=[ *LibraryApiPaginationDocs.apidoc_params, @@ -210,9 +208,6 @@ def get(self, request): library_type = serializer.validated_data['type'] text_search = serializer.validated_data['text_search'] - # Set default page size to 50 - self.pagination_class.page_size = self._DEFAULT_PAGE_SIZE - queryset = api.get_libraries_for_user( request.user, org=org, @@ -512,8 +507,6 @@ class LibraryBlocksView(GenericAPIView): Views to work with XBlocks in a specific content library. """ - _DEFAULT_PAGE_SIZE = 50 - @apidocs.schema( parameters=[ *LibraryApiPaginationDocs.apidoc_params, @@ -542,9 +535,6 @@ def get(self, request, lib_key_str): api.require_permission_for_library_key(key, request.user, permissions.CAN_VIEW_THIS_CONTENT_LIBRARY) components = api.get_library_components(key, text_search=text_search, block_types=block_types) - # Set default page size to 50 - self.pagination_class.page_size = self._DEFAULT_PAGE_SIZE - paginated_xblock_metadata = [ api.LibraryXBlockMetadata.from_component(key, component) for component in self.paginate_queryset(components) @@ -753,8 +743,6 @@ class LibraryImportTaskViewSet(GenericViewSet): Import blocks from Courseware through modulestore. """ - _DEFAULT_PAGE_SIZE = 50 - @convert_exceptions def list(self, request, lib_key_str): """ @@ -769,9 +757,6 @@ def list(self, request, lib_key_str): queryset = api.ContentLibrary.objects.get_by_key(library_key).import_tasks result = ContentLibraryBlockImportTaskSerializer(queryset, many=True).data - # Set default page size to 50 - self.pagination_class.page_size = self._DEFAULT_PAGE_SIZE - return self.get_paginated_response( self.paginate_queryset(result) )