diff --git a/common/djangoapps/entitlements/rest_api/v1/tests/test_views.py b/common/djangoapps/entitlements/rest_api/v1/tests/test_views.py index 3ba57fe7a2..4e9275182d 100644 --- a/common/djangoapps/entitlements/rest_api/v1/tests/test_views.py +++ b/common/djangoapps/entitlements/rest_api/v1/tests/test_views.py @@ -13,10 +13,12 @@ from django.urls import reverse from django.utils.timezone import now from mock import patch from opaque_keys.edx.locator import CourseKey +from pytz import UTC from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory from lms.djangoapps.courseware.models import DynamicUpgradeDeadlineConfiguration +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory @@ -967,6 +969,44 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase): assert CourseEnrollment.is_enrolled(self.user, self.course.id) assert course_entitlement.enrollment_course_run is not None + @patch("entitlements.rest_api.v1.views.get_course_runs_for_course") + def test_enrollment_closed_upgrade_open(self, mock_get_course_runs): + """ + Test that user can still select a session while course enrollment + is closed and upgrade deadline is in future. + """ + course_entitlement = CourseEntitlementFactory.create(user=self.user, mode=CourseMode.VERIFIED) + mock_get_course_runs.return_value = self.return_values + + # Setup enrollment period to be in the past + utc_now = datetime.now(UTC) + self.course.enrollment_start = utc_now - timedelta(days=15) + self.course.enrollment_end = utc_now - timedelta(days=1) + self.course = self.update_course(self.course, self.user.id) + CourseOverview.update_select_courses([self.course.id], force_update=True) + + assert CourseEnrollment.is_enrollment_closed(self.user, self.course) + assert not CourseEnrollment.is_enrolled(self.user, self.course.id) + + url = reverse( + self.ENTITLEMENTS_ENROLLMENT_NAMESPACE, + args=[str(course_entitlement.uuid)] + ) + + data = { + 'course_run_id': str(self.course.id) + } + response = self.client.post( + url, + data=json.dumps(data), + content_type='application/json', + ) + course_entitlement.refresh_from_db() + + assert response.status_code == 201 + assert CourseEnrollment.is_enrolled(self.user, self.course.id) + assert course_entitlement.enrollment_course_run is not None + @patch("entitlements.rest_api.v1.views.get_course_runs_for_course") def test_user_already_enrolled_in_unpaid_mode(self, mock_get_course_runs): course_entitlement = CourseEntitlementFactory.create(user=self.user, mode=CourseMode.VERIFIED) diff --git a/common/djangoapps/entitlements/rest_api/v1/views.py b/common/djangoapps/entitlements/rest_api/v1/views.py index 63b27718b2..81e02f9e86 100644 --- a/common/djangoapps/entitlements/rest_api/v1/views.py +++ b/common/djangoapps/entitlements/rest_api/v1/views.py @@ -337,17 +337,20 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): Returns a response object is there is an error or exception, None otherwise """ try: + unexpired_paid_modes = [mode.slug for mode in CourseMode.paid_modes_for_course(course_run_key)] + can_upgrade = unexpired_paid_modes and entitlement.mode in unexpired_paid_modes enrollment = CourseEnrollment.enroll( user=user, course_key=course_run_key, mode=entitlement.mode, - check_access=True + check_access=True, + can_upgrade=can_upgrade ) except AlreadyEnrolledError: enrollment = CourseEnrollment.get_enrollment(user, course_run_key) if enrollment.mode == entitlement.mode: entitlement.set_enrollment(enrollment) - elif enrollment.mode not in [mode.slug for mode in CourseMode.paid_modes_for_course(course_run_key)]: + elif enrollment.mode not in unexpired_paid_modes: enrollment.update_enrollment(mode=entitlement.mode) entitlement.set_enrollment(enrollment) # Else the User is already enrolled in another paid Mode and we should diff --git a/common/djangoapps/entitlements/tests/test_utils.py b/common/djangoapps/entitlements/tests/test_utils.py index 84d62374d1..0cd598b33f 100644 --- a/common/djangoapps/entitlements/tests/test_utils.py +++ b/common/djangoapps/entitlements/tests/test_utils.py @@ -102,18 +102,6 @@ class TestCourseRunFulfillableForEntitlement(ModuleStoreTestCase): assert not is_course_run_entitlement_fulfillable(course_overview.id, entitlement) - def test_course_run_not_fulfillable_enroll_period_ended(self): - course_overview = self.create_course( - start_from_now=-3, - end_from_now=2, - enrollment_start_from_now=-2, - enrollment_end_from_now=-1 - ) - - entitlement = CourseEntitlementFactory.create(mode=CourseMode.VERIFIED) - - assert not is_course_run_entitlement_fulfillable(course_overview.id, entitlement) - def test_course_run_not_fulfillable_enrollment_start_in_future(self): course_overview = self.create_course( start_from_now=-3, @@ -140,7 +128,7 @@ class TestCourseRunFulfillableForEntitlement(ModuleStoreTestCase): assert is_course_run_entitlement_fulfillable(course_overview.id, entitlement) - def test_course_run_fulfillable_enrollment_ended_upgrade_open(self): + def test_course_run_fulfillable_enrollment_ended_already_enrolled(self): course_overview = self.create_course( start_from_now=-3, end_from_now=2, @@ -166,3 +154,15 @@ class TestCourseRunFulfillableForEntitlement(ModuleStoreTestCase): entitlement = CourseEntitlementFactory.create(mode=CourseMode.VERIFIED) assert not is_course_run_entitlement_fulfillable(course_overview.id, entitlement) + + def test_course_run_fulfillable_enrollment_ended_upgrade_open(self): + course_overview = self.create_course( + start_from_now=-3, + end_from_now=2, + enrollment_start_from_now=-2, + enrollment_end_from_now=-1, + ) + + entitlement = CourseEntitlementFactory.create(mode=CourseMode.VERIFIED) + + assert is_course_run_entitlement_fulfillable(course_overview.id, entitlement) diff --git a/common/djangoapps/entitlements/utils.py b/common/djangoapps/entitlements/utils.py index 0e8a933b80..a16bfc52ba 100644 --- a/common/djangoapps/entitlements/utils.py +++ b/common/djangoapps/entitlements/utils.py @@ -48,13 +48,11 @@ def is_course_run_entitlement_fulfillable( run_end = course_overview.end is_running = run_start and (not run_end or (run_end and (run_end > compare_date))) - # Verify that the course run can currently be enrolled + # Verify that the course run can currently be enrolled, explicitly + # not checking for enrollment end date becasue course run is still + # fulfillable if enrollment has ended but VUD is in future enrollment_start = course_overview.enrollment_start - enrollment_end = course_overview.enrollment_end - can_enroll = ( - (not enrollment_start or enrollment_start < compare_date) - and (not enrollment_end or enrollment_end > compare_date) - ) + can_enroll = not enrollment_start or enrollment_start < compare_date # Is the user already enrolled in the Course Run is_enrolled = CourseEnrollment.is_enrolled(entitlement.user, course_run_key) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index c0e74ae4d1..01f72682ce 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1417,7 +1417,7 @@ class CourseEnrollment(models.Model): ) @classmethod - def enroll(cls, user, course_key, mode=None, check_access=False): + def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=False): """ Enroll a user in a course. This saves immediately. @@ -1441,6 +1441,11 @@ class CourseEnrollment(models.Model): code with non-standard flows (ex. beta tester invitations), but for any standard enrollment flow you probably want this to be True. + `can_upgrade`: if course is upgradeable, alow learners to enroll even + if enrollment is closed. This is a special case for entitlements + while selecting a session. The default is set to False to avoid + breaking the orignal course enroll code. + Exceptions that can be raised: NonExistentCourseError, EnrollmentClosedError, CourseFullError, AlreadyEnrolledError. All these are subclasses of CourseEnrollmentException if you want to catch all of @@ -1464,7 +1469,7 @@ class CourseEnrollment(models.Model): raise NonExistentCourseError if check_access: - if cls.is_enrollment_closed(user, course): + if cls.is_enrollment_closed(user, course) and not can_upgrade: log.warning( u"User %s failed to enroll in course %s because enrollment is closed", user.username,