diff --git a/common/djangoapps/enrollment/api.py b/common/djangoapps/enrollment/api.py index 02055ccea5141bed524abd9675c73d8fab7eaefe..e5d1f3ef16419d5354894da088416f32ea272f34 100644 --- a/common/djangoapps/enrollment/api.py +++ b/common/djangoapps/enrollment/api.py @@ -225,7 +225,8 @@ def update_enrollment(user_id, course_id, mode=None, is_active=None): } """ - _validate_course_mode(course_id, mode) + if mode is not None: + _validate_course_mode(course_id, mode) enrollment = _data_api().update_course_enrollment(user_id, course_id, mode=mode, is_active=is_active) if enrollment is None: msg = u"Course Enrollment not found for user {user} in course {course}".format(user=user_id, course=course_id) diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index d3fff745e9e3ce85476051187d980ea299c32381..7e204f95e236b3d4ca15589857b98ccbbeab16e2 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -43,6 +43,7 @@ class EnrollmentTestMixin(object): email_opt_in=None, as_server=False, mode=CourseMode.HONOR, + is_active=None, ): """ Enroll in the course and verify the response's status code. If the expected status is 200, also validates @@ -61,6 +62,10 @@ class EnrollmentTestMixin(object): }, 'user': username } + + if is_active is not None: + data['is_active'] = is_active + if email_opt_in is not None: data['email_opt_in'] = email_opt_in @@ -72,14 +77,32 @@ class EnrollmentTestMixin(object): response = self.client.post(url, json.dumps(data), content_type='application/json', **extra) self.assertEqual(response.status_code, expected_status) - if expected_status in [status.HTTP_200_OK, status.HTTP_200_OK]: + if expected_status == status.HTTP_200_OK: data = json.loads(response.content) self.assertEqual(course_id, data['course_details']['course_id']) - self.assertEqual(mode, data['mode']) - self.assertTrue(data['is_active']) + + if mode is not None: + self.assertEqual(mode, data['mode']) + + if is_active is not None: + self.assertEqual(is_active, data['is_active']) + else: + self.assertTrue(data['is_active']) return response + def assert_enrollment_activation(self, expected_activation, expected_mode=CourseMode.VERIFIED): + """Change an enrollment's activation and verify its activation and mode are as expected.""" + self.assert_enrollment_status( + as_server=True, + mode=None, + is_active=expected_activation, + expected_status=status.HTTP_200_OK + ) + actual_mode, actual_activation = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + self.assertEqual(actual_activation, expected_activation) + self.assertEqual(actual_mode, expected_mode) + @override_settings(EDX_API_KEY="i am a key") @ddt.ddt @@ -503,6 +526,39 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase): self.assertTrue(is_active) self.assertEqual(course_mode, CourseMode.HONOR) + def test_deactivate_enrollment(self): + """With the right API key, deactivate (i.e., unenroll from) an existing enrollment.""" + # Create an honor and verified mode for a course. This allows an update. + for mode in [CourseMode.HONOR, CourseMode.VERIFIED]: + CourseModeFactory.create( + course_id=self.course.id, + mode_slug=mode, + mode_display_name=mode, + ) + + # Create a 'verified' enrollment + self.assert_enrollment_status(as_server=True, mode=CourseMode.VERIFIED) + + # Check that the enrollment is 'verified' and active. + self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) + course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + self.assertTrue(is_active) + self.assertEqual(course_mode, CourseMode.VERIFIED) + + # Verify that a non-Boolean enrollment status is treated as invalid. + self.assert_enrollment_status( + as_server=True, + mode=None, + is_active='foo', + expected_status=status.HTTP_400_BAD_REQUEST + ) + + # Verify that the enrollment has been deactivated, and that the mode is unchanged. + self.assert_enrollment_activation(False) + + # Verify that enrollment deactivation is idempotent. + self.assert_enrollment_activation(False) + def test_change_mode_from_user(self): """Users should not be able to alter the enrollment mode on an enrollment. """ # Create an honor and verified mode for a course. This allows an update. diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index 656fcdd7707133aca7c6ab802cb5a427dc7c39e6..c0e3a22e917be4bcc2605990e41dc515c26499e3 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -257,13 +257,16 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): * user: The user ID of the currently logged in user. Optional. You cannot use the command to enroll a different user. * mode: The Course Mode for the enrollment. Individual users cannot upgrade their enrollment mode from - 'honor'. Only server to server requests can enroll with other modes. Optional. + 'honor'. Only server-to-server requests can enroll with other modes. Optional. + + * is_active: A Boolean indicating whether the enrollment is active. Only server-to-server requests are + allowed to deactivate an enrollment. Optional. * course details: A collection that contains: * course_id: The unique identifier for the course. - * email_opt_in: A boolean indicating whether the user + * email_opt_in: A Boolean indicating whether the user wishes to opt into email from the organization running this course. Optional. **Response Values** @@ -313,9 +316,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): # cross-domain CSRF. @method_decorator(ensure_csrf_cookie_cross_domain) def get(self, request): - """ - Gets a list of all course enrollments for the currently logged in user. - """ + """Gets a list of all course enrollments for the currently logged in user.""" username = request.GET.get('user', request.user.username) if request.user.username != username and not self.has_api_key_permissions(request): # Return a 404 instead of a 403 (Unauthorized). If one user is looking up @@ -334,8 +335,10 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): ) def post(self, request): - """ - Enrolls the currently logged in user in a course. + """Enrolls the currently logged-in user in a course. + + Server-to-server calls may deactivate or modify the mode of existing enrollments. All other requests + go through `add_enrollment()`, which allows creation of new and reactivation of old enrollments. """ # Get the User, Course ID, and Mode from the request. username = request.DATA.get('user', request.user.username) @@ -407,22 +410,28 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): ) try: - # Check if the user is currently enrolled, and if it is the same as the current enrolled mode. We do not - # have to check if it is inactive or not, because if it is, we are still upgrading if the mode is different, - # and either path will re-activate the enrollment. - # - # Only server-to-server calls will currently be allowed to modify the mode for existing enrollments. All - # other requests will go through add_enrollment(), which will allow creating of new enrollments, and - # re-activating enrollments + is_active = request.DATA.get('is_active') + # Check if the requested activation status is None or a Boolean + if is_active is not None and not isinstance(is_active, bool): + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + 'message': (u"'{value}' is an invalid enrollment activation status.").format(value=is_active) + } + ) + enrollment = api.get_enrollment(username, unicode(course_id)) if has_api_key_permissions and enrollment and enrollment['mode'] != mode: - response = api.update_enrollment(username, unicode(course_id), mode=mode) + response = api.update_enrollment(username, unicode(course_id), mode=mode, is_active=is_active) else: + # Will reactivate inactive enrollments. response = api.add_enrollment(username, unicode(course_id), mode=mode) + email_opt_in = request.DATA.get('email_opt_in', None) if email_opt_in is not None: org = course_id.org update_email_opt_in(request.user, org, email_opt_in) + return Response(response) except CourseModeNotFoundError as error: return Response(