From a32b1441b8d93d261252289ca443f9f23b0ed89a Mon Sep 17 00:00:00 2001 From: NiedielnitsevIvan <81557788+NiedielnitsevIvan@users.noreply.github.com> Date: Mon, 8 Apr 2024 15:21:33 +0300 Subject: [PATCH] feat: [AXM-33] create enrollments filtering by course completion statuses (#2532) * feat: [AXM-33] create enrollments filtering by course completion statuses * test: [AXM-33] add tests for filtrations * style: [AXM-33] fix pylint issues --- .../student/models/course_enrollment.py | 60 ++++++ lms/djangoapps/mobile_api/users/enums.py | 22 ++ .../mobile_api/users/serializers.py | 2 +- lms/djangoapps/mobile_api/users/tests.py | 196 ++++++++++++++++++ lms/djangoapps/mobile_api/users/views.py | 56 +++-- .../features/course_duration_limits/access.py | 4 +- 6 files changed, 325 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 318a3afd8316..74cb5723167d 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..2a072b082fff --- /dev/null +++ b/lms/djangoapps/mobile_api/users/enums.py @@ -0,0 +1,22 @@ +""" +Enums for mobile_api users app. +""" +from enum import Enum + + +class EnrollmentStatuses(Enum): + """ + Enum for enrollment statuses. + """ + + ALL = 'all' + IN_PROGRESS = 'in_progress' + COMPLETED = 'completed' + EXPIRED = 'expired' + + @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/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/tests.py b/lms/djangoapps/mobile_api/users/tests.py index bad320b9db05..31ff2a059ef9 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -37,6 +37,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 @@ -707,6 +708,201 @@ 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): diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index e862f3893dd7..b255ef72eda2 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -45,6 +45,7 @@ from .. import errors from ..decorators import mobile_course_access, mobile_view +from .enums import EnrollmentStatuses from .serializers import ( CourseEnrollmentSerializer, CourseEnrollmentSerializerModifiedForPrimary, @@ -359,13 +360,33 @@ def get_serializer_class(self): @cached_property def queryset(self): - return CourseEnrollment.objects.all().select_related('course', 'user').filter( - user__username=self.kwargs['username'], + """ + 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'] + + 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 = ( @@ -373,7 +394,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) @@ -404,26 +425,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