From a87e8a30995344fa1ac437dee47219892a054bc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Wed, 3 Apr 2024 09:27:29 +0300 Subject: [PATCH 1/3] feat: [AXM-33] create enrollments filtering by course completion statuses --- .../student/models/course_enrollment.py | 60 +++++++++++++++++++ lms/djangoapps/mobile_api/users/enums.py | 19 ++++++ .../mobile_api/users/serializers.py | 2 +- lms/djangoapps/mobile_api/users/views.py | 52 ++++++++++++---- .../features/course_duration_limits/access.py | 4 +- 5 files changed, 122 insertions(+), 15 deletions(-) create mode 100644 lms/djangoapps/mobile_api/users/enums.py diff --git a/common/djangoapps/student/models/course_enrollment.py b/common/djangoapps/student/models/course_enrollment.py index d89a8d967a00..589db78af558 100644 --- a/common/djangoapps/student/models/course_enrollment.py +++ b/common/djangoapps/student/models/course_enrollment.py @@ -129,11 +129,71 @@ class UnenrollmentNotAllowed(CourseEnrollmentException): pass +class CourseEnrollmentQuerySet(models.QuerySet): + """ + Custom queryset for CourseEnrollment with Table-level filter methods. + """ + + def active(self): + """ + Returns a queryset of CourseEnrollment objects for courses that are currently active. + """ + return self.filter(is_active=True) + + def without_certificates(self, user_username): + """ + Returns a queryset of CourseEnrollment objects for courses that do not have a certificate. + """ + from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel + course_ids_with_certificates = GeneratedCertificate.objects.filter( + user__username=user_username + ).values_list('course_id', flat=True) + return self.exclude(course_id__in=course_ids_with_certificates) + + def with_certificates(self, user_username): + """ + Returns a queryset of CourseEnrollment objects for courses that have a certificate. + """ + from lms.djangoapps.certificates.models import GeneratedCertificate # pylint: disable=import-outside-toplevel + course_ids_with_certificates = GeneratedCertificate.objects.filter( + user__username=user_username + ).values_list('course_id', flat=True) + return self.filter(course_id__in=course_ids_with_certificates) + + def in_progress(self, user_username, time_zone=UTC): + """ + Returns a queryset of CourseEnrollment objects for courses that are currently in progress. + """ + now = datetime.now(time_zone) + return self.active().without_certificates(user_username).filter( + Q(course__start__lte=now, course__end__gte=now) + | Q(course__start__isnull=True, course__end__isnull=True) + | Q(course__start__isnull=True, course__end__gte=now) + | Q(course__start__lte=now, course__end__isnull=True), + ) + + def completed(self, user_username): + """ + Returns a queryset of CourseEnrollment objects for courses that have been completed. + """ + return self.active().with_certificates(user_username) + + def expired(self, user_username, time_zone=UTC): + """ + Returns a queryset of CourseEnrollment objects for courses that have expired. + """ + now = datetime.now(time_zone) + return self.active().without_certificates(user_username).filter(course__end__lt=now) + + class CourseEnrollmentManager(models.Manager): """ Custom manager for CourseEnrollment with Table-level filter methods. """ + def get_queryset(self): + return CourseEnrollmentQuerySet(self.model, using=self._db) + def is_small_course(self, course_id): """ Returns false if the number of enrollments are one greater than 'max_enrollments' else true diff --git a/lms/djangoapps/mobile_api/users/enums.py b/lms/djangoapps/mobile_api/users/enums.py new file mode 100644 index 000000000000..463572de71e3 --- /dev/null +++ b/lms/djangoapps/mobile_api/users/enums.py @@ -0,0 +1,19 @@ +from enum import Enum +from django.utils.functional import classproperty + + +class EnrollmentStatuses(Enum): + """ + + """ + + ALL = 'all' + IN_PROGRESS = 'in_progress' + COMPLETED = 'completed' + EXPIRED = 'expired' + + # values = [ALL, IN_PROGRESS, COMPLETED, EXPIRED] + + @classproperty + def values(cls): + return [e.value for e in cls] diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 0a17dbb5a82f..82eb3edee278 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -110,7 +110,7 @@ def get_audit_access_expires(self, model): """ Returns expiration date for a course audit expiration, if any or null """ - return get_user_course_expiration_date(model.user, model.course) + return get_user_course_expiration_date(model.user, model.course, model) def get_certificate(self, model): """Returns the information about the user's certificate in the course.""" diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 2c5e7736b288..b436eb0001a1 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -4,6 +4,7 @@ import logging +from datetime import datetime from functools import cached_property from typing import List, Optional @@ -42,6 +43,7 @@ from .. import errors from ..decorators import mobile_course_access, mobile_view +from .enums import EnrollmentStatuses from .serializers import ( CourseEnrollmentSerializer, CourseEnrollmentSerializerModifiedForPrimary, @@ -356,13 +358,28 @@ def get_serializer_class(self): @cached_property def queryset(self): - return CourseEnrollment.objects.all().select_related('course', 'user').filter( - user__username=self.kwargs['username'], + api_version = self.kwargs.get('api_version') + status = self.request.GET.get('status') + username = self.kwargs['username'] + + queryset = CourseEnrollment.objects.all().select_related('course', 'user').filter( + user__username=username, is_active=True ).order_by('-created') + if api_version == API_V4 and status in EnrollmentStatuses.values: + if status == EnrollmentStatuses.IN_PROGRESS.value: + queryset = queryset.in_progress(user_username=username, time_zone=self.user_timezone) + elif status == EnrollmentStatuses.COMPLETED.value: + queryset = queryset.completed(user_username=username) + elif status == EnrollmentStatuses.EXPIRED.value: + queryset = queryset.expired(user_username=username, time_zone=self.user_timezone) + + return queryset + def get_queryset(self): api_version = self.kwargs.get('api_version') + status = self.request.GET.get('status') mobile_available = self.get_mobile_available_enrollments() not_duration_limited = ( @@ -370,7 +387,7 @@ def get_queryset(self): if check_course_expired(self.request.user, enrollment.course) == ACCESS_GRANTED ) - if api_version == API_V4: + if api_version == API_V4 and status not in EnrollmentStatuses.values: primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() if primary_enrollment_obj: mobile_available.remove(primary_enrollment_obj) @@ -401,26 +418,37 @@ def get_mobile_available_enrollments(self) -> List[Optional[CourseEnrollment]]: def list(self, request, *args, **kwargs): response = super().list(request, *args, **kwargs) api_version = self.kwargs.get('api_version') + status = self.request.GET.get('status') if api_version in (API_V2, API_V3, API_V4): enrollment_data = { 'configs': MobileConfig.get_structured_configs(), - 'user_timezone': str(get_user_timezone_or_last_seen_timezone_or_utc(self.get_user())), + 'user_timezone': str(self.user_timezone), 'enrollments': response.data } - if api_version == API_V4: - primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() - if primary_enrollment_obj: - serializer = CourseEnrollmentSerializerModifiedForPrimary( - primary_enrollment_obj, - context=self.get_serializer_context(), - ) - enrollment_data.update({'primary': serializer.data}) + if api_version == API_V4 and status not in EnrollmentStatuses.values: + if status in EnrollmentStatuses.values: + enrollment_data.update({'primary': None}) + else: + primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() + if primary_enrollment_obj: + serializer = CourseEnrollmentSerializerModifiedForPrimary( + primary_enrollment_obj, + context=self.get_serializer_context(), + ) + enrollment_data.update({'primary': serializer.data}) return Response(enrollment_data) return response + @cached_property + def user_timezone(self): + """ + Get the user's timezone. + """ + return get_user_timezone_or_last_seen_timezone_or_utc(self.get_user()) + def get_user(self) -> User: """ Get user object by username. diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index ff817a315054..08a94702a5e0 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -68,7 +68,7 @@ def get_user_course_duration(user, course): return get_expected_duration(course.id) -def get_user_course_expiration_date(user, course): +def get_user_course_expiration_date(user, course, enrollment=None): """ Return expiration date for given user course pair. Return None if the course does not expire. @@ -81,7 +81,7 @@ def get_user_course_expiration_date(user, course): if access_duration is None: return None - enrollment = CourseEnrollment.get_enrollment(user, course.id) + enrollment = CourseEnrollment.get_enrollment(user, course.id) if not enrollment else enrollment if enrollment is None or enrollment.mode != CourseMode.AUDIT: return None From 5f27bbef3ffcbfa29b20a282aa523fd3e32b583f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Fri, 5 Apr 2024 18:37:57 +0300 Subject: [PATCH 2/3] test: [AXM-33] add tests for filtrations --- lms/djangoapps/mobile_api/users/tests.py | 197 +++++++++++++++++++++++ 1 file changed, 197 insertions(+) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index f1a9798c8108..8efe0e46390e 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -36,6 +36,7 @@ MobileAuthUserTestMixin, MobileCourseAccessTestMixin ) +from lms.djangoapps.mobile_api.users.enums import EnrollmentStatuses from lms.djangoapps.mobile_api.utils import API_V1, API_V05, API_V2, API_V3, API_V4 from openedx.core.lib.courses import course_image_url from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration @@ -706,6 +707,202 @@ def test_course_status_in_primary_obj_when_student_have_progress( self.assertEqual(response.data['primary']['course_status'], expected_course_status) get_last_completed_block_mock.assert_called_once_with(self.user, course.id) + @patch('lms.djangoapps.mobile_api.users.serializers.cache.set', return_value=None) + def test_user_enrollment_api_v4_in_progress_status(self, cache_mock: MagicMock): + """ + Testing + """ + self.login() + old_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + actual_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=self.NEXT_WEEK + ) + infinite_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=None + ) + + self.enroll(old_course.id) + self.enroll(actual_course.id) + self.enroll(infinite_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.IN_PROGRESS.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 2) + self.assertEqual(enrollments['results'][1]['course']['id'], str(actual_course.id)) + self.assertEqual(enrollments['results'][0]['course']['id'], str(infinite_course.id)) + self.assertNotIn('primary', response.data) + + def test_user_enrollment_api_v4_completed_status(self): + """ + Testing + """ + self.login() + old_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + actual_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=self.NEXT_WEEK + ) + infinite_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=None + ) + GeneratedCertificateFactory.create( + user=self.user, + course_id=infinite_course.id, + status=CertificateStatuses.downloadable, + mode='verified', + ) + + self.enroll(old_course.id) + self.enroll(actual_course.id) + self.enroll(infinite_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.COMPLETED.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 1) + self.assertEqual(enrollments['results'][0]['course']['id'], str(infinite_course.id)) + self.assertNotIn('primary', response.data) + + def test_user_enrollment_api_v4_expired_status(self): + """ + Testing + """ + self.login() + old_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + actual_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=self.NEXT_WEEK + ) + infinite_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=None + ) + self.enroll(old_course.id) + self.enroll(actual_course.id) + self.enroll(infinite_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.EXPIRED.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 1) + self.assertEqual(enrollments['results'][0]['course']['id'], str(old_course.id)) + self.assertNotIn('primary', response.data) + + + def test_user_enrollment_api_v4_expired_course_with_certificate(self): + """ + Testing that the API returns a course with + an expiration date in the past if the user has a certificate for this course. + """ + self.login() + expired_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + expired_course_with_cert = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + GeneratedCertificateFactory.create( + user=self.user, + course_id=expired_course_with_cert.id, + status=CertificateStatuses.downloadable, + mode='verified', + ) + + self.enroll(expired_course_with_cert.id) + self.enroll(expired_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.COMPLETED.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 1) + self.assertEqual(enrollments['results'][0]['course']['id'], str(expired_course_with_cert.id)) + self.assertNotIn('primary', response.data) + + def test_user_enrollment_api_v4_status_all(self): + """ + Testing + """ + self.login() + old_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.THREE_YEARS_AGO, + end=self.LAST_WEEK + ) + actual_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=self.NEXT_WEEK + ) + infinite_course = CourseFactory.create( + org="edx", + mobile_available=True, + start=self.LAST_WEEK, + end=None + ) + GeneratedCertificateFactory.create( + user=self.user, + course_id=infinite_course.id, + status=CertificateStatuses.downloadable, + mode='verified', + ) + + self.enroll(old_course.id) + self.enroll(actual_course.id) + self.enroll(infinite_course.id) + + response = self.api_response(api_version=API_V4, data={'status': EnrollmentStatuses.ALL.value}) + enrollments = response.data['enrollments'] + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(enrollments['count'], 3) + self.assertEqual(enrollments['results'][0]['course']['id'], str(infinite_course.id)) + self.assertEqual(enrollments['results'][1]['course']['id'], str(actual_course.id)) + self.assertEqual(enrollments['results'][2]['course']['id'], str(old_course.id)) + self.assertNotIn('primary', response.data) + @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestUserEnrollmentCertificates(UrlResetMixin, MobileAPITestCase, MilestonesTestCaseMixin): From 80b2f9c1a93e15389ed02a9f28c09ae07060c401 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=86=D0=B2=D0=B0=D0=BD=20=D0=9D=D1=94=D0=B4=D1=94=D0=BB?= =?UTF-8?q?=D1=8C=D0=BD=D1=96=D1=86=D0=B5=D0=B2?= Date: Mon, 8 Apr 2024 13:29:32 +0300 Subject: [PATCH 3/3] style: [AXM-33] fix pylint issues --- lms/djangoapps/mobile_api/users/enums.py | 13 ++++++++----- lms/djangoapps/mobile_api/users/tests.py | 1 - lms/djangoapps/mobile_api/users/views.py | 14 +++++++++----- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lms/djangoapps/mobile_api/users/enums.py b/lms/djangoapps/mobile_api/users/enums.py index 463572de71e3..2a072b082fff 100644 --- a/lms/djangoapps/mobile_api/users/enums.py +++ b/lms/djangoapps/mobile_api/users/enums.py @@ -1,10 +1,12 @@ +""" +Enums for mobile_api users app. +""" from enum import Enum -from django.utils.functional import classproperty class EnrollmentStatuses(Enum): """ - + Enum for enrollment statuses. """ ALL = 'all' @@ -12,8 +14,9 @@ class EnrollmentStatuses(Enum): COMPLETED = 'completed' EXPIRED = 'expired' - # values = [ALL, IN_PROGRESS, COMPLETED, EXPIRED] - - @classproperty + @classmethod def values(cls): + """ + Returns string representation of all enum values. + """ return [e.value for e in cls] diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 8efe0e46390e..5b842851db0b 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -822,7 +822,6 @@ def test_user_enrollment_api_v4_expired_status(self): self.assertEqual(enrollments['results'][0]['course']['id'], str(old_course.id)) self.assertNotIn('primary', response.data) - def test_user_enrollment_api_v4_expired_course_with_certificate(self): """ Testing that the API returns a course with diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index b436eb0001a1..4d2ddf2aad7a 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -4,7 +4,6 @@ import logging -from datetime import datetime from functools import cached_property from typing import List, Optional @@ -358,6 +357,11 @@ def get_serializer_class(self): @cached_property def queryset(self): + """ + Find and return the list of course enrollments for the user. + + In v4 added filtering by statuses. + """ api_version = self.kwargs.get('api_version') status = self.request.GET.get('status') username = self.kwargs['username'] @@ -367,7 +371,7 @@ def queryset(self): is_active=True ).order_by('-created') - if api_version == API_V4 and status in EnrollmentStatuses.values: + if api_version == API_V4 and status in EnrollmentStatuses.values(): if status == EnrollmentStatuses.IN_PROGRESS.value: queryset = queryset.in_progress(user_username=username, time_zone=self.user_timezone) elif status == EnrollmentStatuses.COMPLETED.value: @@ -387,7 +391,7 @@ def get_queryset(self): if check_course_expired(self.request.user, enrollment.course) == ACCESS_GRANTED ) - if api_version == API_V4 and status not in EnrollmentStatuses.values: + if api_version == API_V4 and status not in EnrollmentStatuses.values(): primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress() if primary_enrollment_obj: mobile_available.remove(primary_enrollment_obj) @@ -426,8 +430,8 @@ def list(self, request, *args, **kwargs): 'user_timezone': str(self.user_timezone), 'enrollments': response.data } - if api_version == API_V4 and status not in EnrollmentStatuses.values: - if status in EnrollmentStatuses.values: + if api_version == API_V4 and status not in EnrollmentStatuses.values(): + if status in EnrollmentStatuses.values(): enrollment_data.update({'primary': None}) else: primary_enrollment_obj = self.get_primary_enrollment_by_latest_enrollment_or_progress()