From ee4b92ca100ec1e50d5738a062c4624674a9a15d Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 12 Sep 2023 15:39:36 +0300 Subject: [PATCH 1/3] feat: [AXIM-6] Add DefaultPagination for UserCourseEnrollmentsList v3 --- lms/djangoapps/mobile_api/users/tests.py | 25 +++++++++++++++++++++++- lms/djangoapps/mobile_api/users/views.py | 16 +++++++++++++-- lms/djangoapps/mobile_api/utils.py | 1 + lms/urls.py | 2 +- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 9d4752ea7c..f3ec1d055d 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -34,7 +34,7 @@ from lms.djangoapps.mobile_api.testutils import ( MobileAuthUserTestMixin, MobileCourseAccessTestMixin ) -from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2 +from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3 from openedx.core.lib.courses import course_image_url from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience.tests.views.helpers import add_course_mode @@ -376,6 +376,29 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.assertDictEqual(response.data['configs'], expected_result) assert 'enrollments' in response.data + def test_pagination_enrollment(self): + """ + Test pagination for UserCourseEnrollmentsList view v3 + for 3rd version of this view we use DefaultPagination + + Test for /api/mobile/{api_version}/users//course_enrollments/ + api_version = v3 + """ + self.login() + # Create and enroll to 15 courses + courses = [CourseFactory.create(org="my_org", mobile_available=True) for _ in range(15)] + for course in courses: + self.enroll(course.id) + + response = self.api_response(api_version=API_V3) + assert response.status_code == 200 + assert response.data["enrollments"]["count"] == 15 + assert response.data["enrollments"]["num_pages"] == 2 + assert response.data["enrollments"]["current_page"] == 1 + assert len(response.data["enrollments"]["results"]) == 10 + assert "next" in response.data["enrollments"] + assert "previous" in response.data["enrollments"] + @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 45a3e61bb5..da7b90265e 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -21,6 +21,7 @@ from rest_framework.permissions import SAFE_METHODS from rest_framework.response import Response from xblock.fields import Scope from xblock.runtime import KeyValueStore +from edx_rest_framework_extensions.paginators import DefaultPagination from common.djangoapps.student.models import CourseEnrollment, User # lint-amnesty, pylint: disable=reimported from lms.djangoapps.courseware.access import is_mobile_available_for_user @@ -30,7 +31,7 @@ from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.block_render import get_block_for_descriptor from lms.djangoapps.courseware.views.index import save_positions_recursively_up from lms.djangoapps.mobile_api.models import MobileConfig -from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2 +from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3 from openedx.features.course_duration_limits.access import check_course_expired from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -372,7 +373,7 @@ class UserCourseEnrollmentsList(generics.ListAPIView): response = super().list(request, *args, **kwargs) api_version = self.kwargs.get('api_version') - if api_version == API_V2: + if api_version in (API_V2, API_V3): enrollment_data = { 'configs': MobileConfig.get_structured_configs(), 'enrollments': response.data @@ -381,6 +382,17 @@ class UserCourseEnrollmentsList(generics.ListAPIView): return response + # pylint: disable=attribute-defined-outside-init + @property + def paginator(self): + super().paginator # pylint: disable=expression-not-assigned + api_version = self.kwargs.get('api_version') + + if self._paginator is None and api_version == API_V3: + self._paginator = DefaultPagination() + + return self._paginator + @api_view(["GET"]) @mobile_view() diff --git a/lms/djangoapps/mobile_api/utils.py b/lms/djangoapps/mobile_api/utils.py index 5dcdf9e04c..73a0cfea08 100644 --- a/lms/djangoapps/mobile_api/utils.py +++ b/lms/djangoapps/mobile_api/utils.py @@ -5,6 +5,7 @@ Common utility methods for Mobile APIs. API_V05 = 'v0.5' API_V1 = 'v1' API_V2 = 'v2' +API_V3 = 'v3' def parsed_version(version): diff --git a/lms/urls.py b/lms/urls.py index ca6a240eab..a4ed0f538d 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -224,7 +224,7 @@ urlpatterns = [ if settings.FEATURES.get('ENABLE_MOBILE_REST_API'): urlpatterns += [ - re_path(r'^api/mobile/(?Pv(2|1|0.5))/', include('lms.djangoapps.mobile_api.urls')), + re_path(r'^api/mobile/(?Pv(3|2|1|0.5))/', include('lms.djangoapps.mobile_api.urls')), ] urlpatterns += [ From 95fcb124ddc40dbbb1353dc995ef0e3aa8330ac4 Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Mon, 20 Nov 2023 22:39:48 +0200 Subject: [PATCH 2/3] docs: add docstring for the paginator property override --- lms/djangoapps/mobile_api/users/views.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index da7b90265e..7a94e00945 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -385,6 +385,11 @@ class UserCourseEnrollmentsList(generics.ListAPIView): # pylint: disable=attribute-defined-outside-init @property def paginator(self): + """ + Overrides API View paginator property to dynamically determine pagination class + based on the provided api_version. Implements solutions from the discussion at + https://www.github.com/encode/django-rest-framework/issues/6397. + """ super().paginator # pylint: disable=expression-not-assigned api_version = self.kwargs.get('api_version') From 3da85a994ea1deb3a55217a62e3af0eebd99f86c Mon Sep 17 00:00:00 2001 From: Glib Glugovskiy Date: Mon, 20 Nov 2023 23:08:56 +0200 Subject: [PATCH 3/3] fix: remove trailing whitespace failing quality check --- lms/djangoapps/mobile_api/users/views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 7a94e00945..049678dcd7 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -386,8 +386,9 @@ class UserCourseEnrollmentsList(generics.ListAPIView): @property def paginator(self): """ - Overrides API View paginator property to dynamically determine pagination class - based on the provided api_version. Implements solutions from the discussion at + Override API View paginator property to dynamically determine pagination class + + Implements solutions from the discussion at https://www.github.com/encode/django-rest-framework/issues/6397. """ super().paginator # pylint: disable=expression-not-assigned