From 0d0503a716f1e03b76d4e83c70c912f45ffd1bb2 Mon Sep 17 00:00:00 2001 From: Kyrylo Kireiev <90455454+KyryloKireiev@users.noreply.github.com> Date: Mon, 8 Apr 2024 13:03:33 +0300 Subject: [PATCH 1/8] feat: [AXM-200] Implement user's enrolments status API (#2530) * feat: [AXM-24] Update structure for course enrollments API (#2515) * feat: [AXM-24] Update structure for course enrollments API * style: [AXM-24] Improve code style * fix: [AXM-24] Fix student's latest enrollment filter * feat: [AXM-47] Add course_status field to primary object (#2517) * feat: [AXM-40] add courses progress to enrollment endpoint (#2519) * fix: workaround for staticcollection introduced in e40a01c * feat: [AXM-40] add courses progress to enrollment endpoint * refactor: [AXM-40] add caching to improve performance * refactor: [AXM-40] add progress only for primary course * refactor: [AXM-40] refactor enrollment caching optimization --------- Co-authored-by: Glib Glugovskiy * feat: [AXM-53] add assertions for primary course (#2522) * feat: [AXM-53] add assertions for primary course * test: [AXM-53] fix tests * style: [AXM-53] change future_assignment default value to None * refactor: [AXM-53] add some optimization for assignments collecting * feat: [AXM-200] Implement user's enrolments status API * style: [AXM-200] Improve code style * refactor: [AXM-200] Divide get method into smaller methods --------- Co-authored-by: NiedielnitsevIvan <81557788+NiedielnitsevIvan@users.noreply.github.com> Co-authored-by: Glib Glugovskiy --- lms/djangoapps/mobile_api/users/tests.py | 135 +++++++++++++++++++++++ lms/djangoapps/mobile_api/users/urls.py | 7 +- lms/djangoapps/mobile_api/users/views.py | 129 ++++++++++++++++++++++ 3 files changed, 269 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 65b1fba65ce3..25b2ab2b2aa8 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -9,6 +9,7 @@ import ddt import pytz +from completion.models import BlockCompletion from completion.test_utils import CompletionWaffleTestMixin, submit_completions_for_testing from django.conf import settings from django.db import transaction @@ -768,3 +769,137 @@ def test_discussion_tab_url(self, discussion_tab_enabled): assert isinstance(discussion_url, str) else: assert discussion_url is None + + +@ddt.ddt +class UserEnrollmentsStatus(MobileAPITestCase, MobileAuthUserTestMixin): + """ + Tests for /api/mobile/{api_version}/users//enrollments_status/ + """ + + REVERSE_INFO = {'name': 'user-enrollments-status', 'params': ['username', 'api_version']} + + def test_no_mobile_available_courses(self) -> None: + self.login() + courses = [CourseFactory.create(org="edx", mobile_available=False) for _ in range(3)] + for course in courses: + self.enroll(course.id) + + response = self.api_response(api_version=API_V1) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertListEqual(response.data, []) + + def test_no_enrollments(self) -> None: + self.login() + for _ in range(3): + CourseFactory.create(org="edx", mobile_available=True) + + response = self.api_response(api_version=API_V1) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertListEqual(response.data, []) + + def test_user_have_only_active_enrollments_and_no_completions(self) -> None: + self.login() + courses = [CourseFactory.create(org="edx", mobile_available=True) for _ in range(3)] + for course in courses: + self.enroll(course.id) + + response = self.api_response(api_version=API_V1) + + expected_response = [ + {'course_id': str(courses[0].course_id), 'course_name': courses[0].display_name, 'is_active': True}, + {'course_id': str(courses[1].course_id), 'course_name': courses[1].display_name, 'is_active': True}, + {'course_id': str(courses[2].course_id), 'course_name': courses[2].display_name, 'is_active': True}, + ] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertListEqual(response.data, expected_response) + + def test_user_have_active_and_inactive_enrollments_and_no_completions(self) -> None: + self.login() + courses = [CourseFactory.create(org="edx", mobile_available=True) for _ in range(3)] + for course in courses: + self.enroll(course.id) + old_course = CourseFactory.create(org="edx", mobile_available=True) + self.enroll(old_course.id) + old_enrollment = CourseEnrollment.objects.filter(user=self.user, course=old_course.course_id).first() + old_enrollment.created = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=31) + old_enrollment.save() + + response = self.api_response(api_version=API_V1) + + expected_response = [ + {'course_id': str(courses[0].course_id), 'course_name': courses[0].display_name, 'is_active': True}, + {'course_id': str(courses[1].course_id), 'course_name': courses[1].display_name, 'is_active': True}, + {'course_id': str(courses[2].course_id), 'course_name': courses[2].display_name, 'is_active': True}, + {'course_id': str(old_course.course_id), 'course_name': old_course.display_name, 'is_active': False} + ] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertListEqual(response.data, expected_response) + + @ddt.data( + (27, True), + (28, True), + (29, True), + (31, False), + (32, False), + ) + @ddt.unpack + def test_different_enrollment_dates(self, enrolled_days_ago: int, is_active_status: bool) -> None: + self.login() + course = CourseFactory.create(org="edx", mobile_available=True, run='1001') + self.enroll(course.id) + enrollment = CourseEnrollment.objects.filter(user=self.user, course=course.course_id).first() + enrollment.created = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=enrolled_days_ago) + enrollment.save() + + response = self.api_response(api_version=API_V1) + + expected_response = [ + {'course_id': str(course.course_id), 'course_name': course.display_name, 'is_active': is_active_status} + ] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertListEqual(response.data, expected_response) + + @ddt.data( + (27, True), + (28, True), + (29, True), + (31, False), + (32, False), + ) + @ddt.unpack + def test_different_completion_dates(self, completed_days_ago: int, is_active_status: bool) -> None: + self.login() + course = CourseFactory.create(org="edx", mobile_available=True, run='1010') + section = BlockFactory.create( + parent=course, + category='chapter', + ) + self.enroll(course.id) + enrollment = CourseEnrollment.objects.filter(user=self.user, course=course.course_id).first() + # make enrollment older 30 days ago + enrollment.created = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=50) + enrollment.save() + completion = BlockCompletion.objects.create( + user=self.user, + context_key=course.context_key, + block_type='course', + block_key=section.location, + completion=0.5, + ) + completion.created = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=completed_days_ago) + completion.save() + + response = self.api_response(api_version=API_V1) + + expected_response = [ + {'course_id': str(course.course_id), 'course_name': course.display_name, 'is_active': is_active_status} + ] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertListEqual(response.data, expected_response) diff --git a/lms/djangoapps/mobile_api/users/urls.py b/lms/djangoapps/mobile_api/users/urls.py index 266644246e88..874730d4d0f0 100644 --- a/lms/djangoapps/mobile_api/users/urls.py +++ b/lms/djangoapps/mobile_api/users/urls.py @@ -6,7 +6,7 @@ from django.conf import settings from django.urls import re_path -from .views import UserCourseEnrollmentsList, UserCourseStatus, UserDetail +from .views import UserCourseEnrollmentsList, UserCourseStatus, UserDetail, UserEnrollmentsStatus urlpatterns = [ re_path('^' + settings.USERNAME_PATTERN + '$', UserDetail.as_view(), name='user-detail'), @@ -17,5 +17,8 @@ ), re_path(f'^{settings.USERNAME_PATTERN}/course_status_info/{settings.COURSE_ID_PATTERN}', UserCourseStatus.as_view(), - name='user-course-status') + name='user-course-status'), + re_path(f'^{settings.USERNAME_PATTERN}/enrollments_status/', + UserEnrollmentsStatus.as_view(), + name='user-enrollments-status') ] diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 049678dcd7ba..2b6264263061 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -3,9 +3,13 @@ """ +import datetime import logging +from typing import Dict, List, Optional +import pytz from completion.exceptions import UnavailableCompletionData +from completion.models import BlockCompletion from completion.utilities import get_key_to_last_completed_block from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.contrib.auth.signals import user_logged_in @@ -410,3 +414,128 @@ def my_user_info(request, api_version): # updating it from the oauth2 related code is too complex user_logged_in.send(sender=User, user=request.user, request=request) return redirect("user-detail", api_version=api_version, username=request.user.username) + + +class UserCourseEnrollmentsV4Pagination(DefaultPagination): + """ + Pagination for `UserCourseEnrollments` API v4. + """ + page_size = 5 + max_page_size = 50 + + +@mobile_view(is_user=True) +class UserEnrollmentsStatus(views.APIView): + """ + **Use Case** + + Get information about user's enrolments status. + + Returns active enrolment status if user was enrolled for the course + less than 30 days ago or has progressed in the course in the last 30 days. + Otherwise, the registration is considered inactive. + + **Example Request** + + GET /api/mobile/{api_version}/users//enrollments_status/ + + **Response Values** + + If the request for information about the user's enrolments is successful, the + request returns an HTTP 200 "OK" response. + + The HTTP 200 response has the following values. + + * course_id (str): The course id associated with the user's enrollment. + * course_name (str): The course name associated with the user's enrollment. + * is_active (bool): User's course enrolment status. + + + The HTTP 200 response contains a list of dictionaries that contain info + about each user's enrolment status. + + **Example Response** + + ```json + [ + { + "course_id": "course-v1:a+a+a", + "course_name": "a", + "is_active": true + }, + { + "course_id": "course-v1:b+b+b", + "course_name": "b", + "is_active": true + }, + { + "course_id": "course-v1:c+c+c", + "course_name": "c", + "is_active": false + }, + ... + ] + ``` + """ + def get(self, request, *args, **kwargs) -> Response: + """ + Gets user's enrollments status. + """ + active_status_date = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=30) + username = kwargs.get('username') + course_ids_where_user_has_completions = self._get_course_ids_where_user_has_completions( + username, + active_status_date, + ) + enrollments_status = self._build_enrollments_status_dict( + username, + active_status_date, + course_ids_where_user_has_completions + ) + return Response(enrollments_status) + + def _build_enrollments_status_dict( + self, + username: str, + active_status_date: datetime, + course_ids: List[str], + ) -> List[Dict[str, bool]]: + """ + Builds list with dictionaries with user's enrolments statuses. + """ + user_enrollments = CourseEnrollment.objects.filter( + user__username=username, + is_active=True, + ) + mobile_available = [ + enrollment for enrollment in user_enrollments + if is_mobile_available_for_user(self.request.user, enrollment.course_overview) + ] + enrollments_status = [] + for user_enrollment in mobile_available: + course_id = str(user_enrollment.course_overview.id) + enrollments_status.append( + { + 'course_id': course_id, + 'course_name': user_enrollment.course_overview.display_name, + 'is_active': bool( + course_id in course_ids + or user_enrollment.created > active_status_date + ) + } + ) + return enrollments_status + + @staticmethod + def _get_course_ids_where_user_has_completions( + username: str, + active_status_date: datetime, + ) -> List[str]: + """ + Gets course ids where user has completions. + """ + user_completions_last_month = BlockCompletion.objects.filter( + user__username=username, + created__gte=active_status_date + ) + return [str(completion.block_key.course_key) for completion in user_completions_last_month] From bd8b35d0d84c42e8227bc7f0c7d3e0f8e8fae8c2 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Mon, 27 May 2024 14:42:16 +0300 Subject: [PATCH 2/8] fix: [AXM-549] Added missing import --- lms/djangoapps/mobile_api/users/tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 25b2ab2b2aa8..4ebda3ecd47f 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -19,6 +19,7 @@ from django.utils.timezone import now from milestones.tests.utils import MilestonesTestCaseMixin from opaque_keys.edx.keys import CourseKey +from rest_framework import status from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment From e474abda4fd00bebcd9f2a92a81b636dbe93b7fe Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Mon, 27 May 2024 18:14:12 +0300 Subject: [PATCH 3/8] style: [AXM-549] Remove unused import --- lms/djangoapps/mobile_api/users/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 2b6264263061..0be90f255cb2 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -5,7 +5,7 @@ import datetime import logging -from typing import Dict, List, Optional +from typing import Dict, List import pytz from completion.exceptions import UnavailableCompletionData From 1a7f55b01e273739802a06fcaa3b3bc05c84ca55 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Fri, 21 Jun 2024 18:30:47 +0300 Subject: [PATCH 4/8] refactor: [AXM-549] Refactor UserEnrollmentsStatus API --- lms/djangoapps/mobile_api/users/tests.py | 2 +- lms/djangoapps/mobile_api/users/views.py | 18 ++++-------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 4ebda3ecd47f..df7c4382971e 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -773,7 +773,7 @@ def test_discussion_tab_url(self, discussion_tab_enabled): @ddt.ddt -class UserEnrollmentsStatus(MobileAPITestCase, MobileAuthUserTestMixin): +class TestUserEnrollmentsStatus(MobileAPITestCase, MobileAuthUserTestMixin): """ Tests for /api/mobile/{api_version}/users//enrollments_status/ """ diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 0be90f255cb2..03149157811a 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -14,7 +14,7 @@ from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.contrib.auth.signals import user_logged_in from django.db import transaction -from django.shortcuts import redirect +from django.shortcuts import get_object_or_404, redirect from django.utils import dateparse from django.utils.decorators import method_decorator from opaque_keys import InvalidKeyError @@ -416,14 +416,6 @@ def my_user_info(request, api_version): return redirect("user-detail", api_version=api_version, username=request.user.username) -class UserCourseEnrollmentsV4Pagination(DefaultPagination): - """ - Pagination for `UserCourseEnrollments` API v4. - """ - page_size = 5 - max_page_size = 50 - - @mobile_view(is_user=True) class UserEnrollmentsStatus(views.APIView): """ @@ -503,13 +495,11 @@ def _build_enrollments_status_dict( """ Builds list with dictionaries with user's enrolments statuses. """ - user_enrollments = CourseEnrollment.objects.filter( - user__username=username, - is_active=True, - ) + user = get_object_or_404(User, username=username) + user_enrollments = CourseEnrollment.enrollments_for_user(user).select_related('course') mobile_available = [ enrollment for enrollment in user_enrollments - if is_mobile_available_for_user(self.request.user, enrollment.course_overview) + if is_mobile_available_for_user(user, enrollment.course_overview) ] enrollments_status = [] for user_enrollment in mobile_available: From 1eec8b80e43a0fd4c746aa53c5d04ba30be39088 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Tue, 8 Oct 2024 17:47:43 +0300 Subject: [PATCH 5/8] fix: [AXM-549] Use more efficient query --- lms/djangoapps/mobile_api/users/views.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 091eb054038a..d634b3072dab 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -641,11 +641,12 @@ def _get_course_ids_where_user_has_completions( """ Gets course ids where user has completions. """ - user_completions_last_month = BlockCompletion.objects.filter( + context_keys = BlockCompletion.objects.filter( user__username=username, created__gte=active_status_date - ) - return [str(completion.block_key.course_key) for completion in user_completions_last_month] + ).values_list('context_key', flat=True).distinct() + + return [str(context_key) for context_key in context_keys] class UserCourseEnrollmentsV4Pagination(DefaultPagination): From bf073fb68c69ea3b586ec5cd5a0f2c25068aa00e Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Thu, 17 Oct 2024 18:06:03 +0300 Subject: [PATCH 6/8] refactor: [AXM-549] Change enrollments status API field name --- lms/djangoapps/mobile_api/users/tests.py | 30 +++++++++++++++--------- lms/djangoapps/mobile_api/users/views.py | 10 ++++---- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 6a0ef0a72fdf..2cfefaa058d9 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -1422,9 +1422,9 @@ def test_user_have_only_active_enrollments_and_no_completions(self) -> None: response = self.api_response(api_version=API_V1) expected_response = [ - {'course_id': str(courses[0].course_id), 'course_name': courses[0].display_name, 'is_active': True}, - {'course_id': str(courses[1].course_id), 'course_name': courses[1].display_name, 'is_active': True}, - {'course_id': str(courses[2].course_id), 'course_name': courses[2].display_name, 'is_active': True}, + {'course_id': str(courses[0].course_id), 'course_name': courses[0].display_name, 'recently_active': True}, + {'course_id': str(courses[1].course_id), 'course_name': courses[1].display_name, 'recently_active': True}, + {'course_id': str(courses[2].course_id), 'course_name': courses[2].display_name, 'recently_active': True}, ] self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1444,10 +1444,10 @@ def test_user_have_active_and_inactive_enrollments_and_no_completions(self) -> N response = self.api_response(api_version=API_V1) expected_response = [ - {'course_id': str(courses[0].course_id), 'course_name': courses[0].display_name, 'is_active': True}, - {'course_id': str(courses[1].course_id), 'course_name': courses[1].display_name, 'is_active': True}, - {'course_id': str(courses[2].course_id), 'course_name': courses[2].display_name, 'is_active': True}, - {'course_id': str(old_course.course_id), 'course_name': old_course.display_name, 'is_active': False} + {'course_id': str(courses[0].course_id), 'course_name': courses[0].display_name, 'recently_active': True}, + {'course_id': str(courses[1].course_id), 'course_name': courses[1].display_name, 'recently_active': True}, + {'course_id': str(courses[2].course_id), 'course_name': courses[2].display_name, 'recently_active': True}, + {'course_id': str(old_course.course_id), 'course_name': old_course.display_name, 'recently_active': False} ] self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1461,7 +1461,7 @@ def test_user_have_active_and_inactive_enrollments_and_no_completions(self) -> N (32, False), ) @ddt.unpack - def test_different_enrollment_dates(self, enrolled_days_ago: int, is_active_status: bool) -> None: + def test_different_enrollment_dates(self, enrolled_days_ago: int, recently_active_status: bool) -> None: self.login() course = CourseFactory.create(org="edx", mobile_available=True, run='1001') self.enroll(course.id) @@ -1472,7 +1472,11 @@ def test_different_enrollment_dates(self, enrolled_days_ago: int, is_active_stat response = self.api_response(api_version=API_V1) expected_response = [ - {'course_id': str(course.course_id), 'course_name': course.display_name, 'is_active': is_active_status} + { + 'course_id': str(course.course_id), + 'course_name': course.display_name, + 'recently_active': recently_active_status + } ] self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1486,7 +1490,7 @@ def test_different_enrollment_dates(self, enrolled_days_ago: int, is_active_stat (32, False), ) @ddt.unpack - def test_different_completion_dates(self, completed_days_ago: int, is_active_status: bool) -> None: + def test_different_completion_dates(self, completed_days_ago: int, recently_active_status: bool) -> None: self.login() course = CourseFactory.create(org="edx", mobile_available=True, run='1010') section = BlockFactory.create( @@ -1511,7 +1515,11 @@ def test_different_completion_dates(self, completed_days_ago: int, is_active_sta response = self.api_response(api_version=API_V1) expected_response = [ - {'course_id': str(course.course_id), 'course_name': course.display_name, 'is_active': is_active_status} + { + 'course_id': str(course.course_id), + 'course_name': course.display_name, + 'recently_active': recently_active_status + } ] self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index d634b3072dab..b37a797403a0 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -557,7 +557,7 @@ class UserEnrollmentsStatus(views.APIView): * course_id (str): The course id associated with the user's enrollment. * course_name (str): The course name associated with the user's enrollment. - * is_active (bool): User's course enrolment status. + * recently_active (bool): User's course enrolment status. The HTTP 200 response contains a list of dictionaries that contain info @@ -570,17 +570,17 @@ class UserEnrollmentsStatus(views.APIView): { "course_id": "course-v1:a+a+a", "course_name": "a", - "is_active": true + "recently_active": true }, { "course_id": "course-v1:b+b+b", "course_name": "b", - "is_active": true + "recently_active": true }, { "course_id": "course-v1:c+c+c", "course_name": "c", - "is_active": false + "recently_active": false }, ... ] @@ -625,7 +625,7 @@ def _build_enrollments_status_dict( { 'course_id': course_id, 'course_name': user_enrollment.course_overview.display_name, - 'is_active': bool( + 'recently_active': bool( course_id in course_ids or user_enrollment.created > active_status_date ) From bc71be481148f61d67b7089796fe85becabba5d3 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Thu, 24 Oct 2024 13:15:36 +0300 Subject: [PATCH 7/8] feat: [AXM-549] Add query limit to User Enrollments --- lms/djangoapps/mobile_api/users/views.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index b37a797403a0..fad1218435d4 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -544,6 +544,9 @@ class UserEnrollmentsStatus(views.APIView): less than 30 days ago or has progressed in the course in the last 30 days. Otherwise, the registration is considered inactive. + USER_ENROLLMENTS_LIMIT - adds users enrollments query limit to + safe API from possible DDOS attacks. + **Example Request** GET /api/mobile/{api_version}/users//enrollments_status/ @@ -586,6 +589,9 @@ class UserEnrollmentsStatus(views.APIView): ] ``` """ + + USER_ENROLLMENTS_LIMIT = 500 + def get(self, request, *args, **kwargs) -> Response: """ Gets user's enrollments status. @@ -613,7 +619,12 @@ def _build_enrollments_status_dict( Builds list with dictionaries with user's enrolments statuses. """ user = get_object_or_404(User, username=username) - user_enrollments = CourseEnrollment.enrollments_for_user(user).select_related('course') + user_enrollments = ( + CourseEnrollment + .enrollments_for_user(user) + .select_related('course') + [:self.USER_ENROLLMENTS_LIMIT] + ) mobile_available = [ enrollment for enrollment in user_enrollments if is_mobile_available_for_user(user, enrollment.course_overview) From e868888f5422d2234ef06622d8553d64d5929040 Mon Sep 17 00:00:00 2001 From: KyryloKireiev Date: Wed, 30 Oct 2024 17:55:31 +0200 Subject: [PATCH 8/8] refactor: [AXM-549] Use course keys instead ids --- lms/djangoapps/mobile_api/users/views.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index fad1218435d4..c86f3add9d36 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -6,7 +6,7 @@ import datetime import logging from functools import cached_property -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Set import pytz from completion.exceptions import UnavailableCompletionData @@ -20,6 +20,7 @@ from django.utils.decorators import method_decorator from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.locator import CourseLocator from rest_framework import generics, views from rest_framework.decorators import api_view from rest_framework.permissions import SAFE_METHODS @@ -613,7 +614,7 @@ def _build_enrollments_status_dict( self, username: str, active_status_date: datetime, - course_ids: List[str], + course_ids: Set[CourseLocator], ) -> List[Dict[str, bool]]: """ Builds list with dictionaries with user's enrolments statuses. @@ -631,10 +632,10 @@ def _build_enrollments_status_dict( ] enrollments_status = [] for user_enrollment in mobile_available: - course_id = str(user_enrollment.course_overview.id) + course_id = user_enrollment.course_overview.id enrollments_status.append( { - 'course_id': course_id, + 'course_id': str(course_id), 'course_name': user_enrollment.course_overview.display_name, 'recently_active': bool( course_id in course_ids @@ -648,16 +649,16 @@ def _build_enrollments_status_dict( def _get_course_ids_where_user_has_completions( username: str, active_status_date: datetime, - ) -> List[str]: + ) -> Set[CourseLocator]: """ - Gets course ids where user has completions. + Gets course keys where user has completions. """ context_keys = BlockCompletion.objects.filter( user__username=username, created__gte=active_status_date ).values_list('context_key', flat=True).distinct() - return [str(context_key) for context_key in context_keys] + return set(context_keys) class UserCourseEnrollmentsV4Pagination(DefaultPagination):