From 8c9f6370b8528e421ebc260ec01debd24e4c35b6 Mon Sep 17 00:00:00 2001 From: Stephen Sanchez Date: Mon, 16 Mar 2015 19:14:27 +0000 Subject: [PATCH] Allow Enrollment API supports creating and upgrade course modes other than honor. Fixing tests, cleaning up POST handler to be easier to read. Wording fixed for unit test comments. --- .../djangoapps/enrollment/tests/test_views.py | 180 ++++++++++++++---- common/djangoapps/enrollment/views.py | 61 ++++-- 2 files changed, 193 insertions(+), 48 deletions(-) diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index 1fb4d71d9b..0756b125a9 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -13,6 +13,7 @@ from django.conf import settings from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from course_modes.models import CourseMode +from enrollment.views import EnrollmentUserThrottle from util.models import RateLimitConfiguration from util.testing import UrlResetMixin from enrollment import api @@ -40,9 +41,12 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): """ Create a course and user, then log in. """ super(EnrollmentTest, self).setUp() - rate_limit_config = RateLimitConfiguration.current() - rate_limit_config.enabled = False - rate_limit_config.save() + self.rate_limit_config = RateLimitConfiguration.current() + self.rate_limit_config.enabled = False + self.rate_limit_config.save() + + throttle = EnrollmentUserThrottle() + self.rate_limit, rate_duration = throttle.parse_rate(throttle.rate) self.course = CourseFactory.create() self.user = UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) @@ -52,12 +56,12 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): @ddt.data( # Default (no course modes in the database) # Expect that users are automatically enrolled as "honor". - ([], 'honor'), + ([], CourseMode.HONOR), # Audit / Verified / Honor # We should always go to the "choose your course" page. # We should also be enrolled as "honor" by default. - (['honor', 'verified', 'audit'], 'honor'), + ([CourseMode.HONOR, CourseMode.VERIFIED, CourseMode.AUDIT], CourseMode.HONOR), ) @ddt.unpack def test_enroll(self, course_modes, enrollment_mode): @@ -80,8 +84,8 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): def test_check_enrollment(self): CourseModeFactory.create( course_id=self.course.id, - mode_slug='honor', - mode_display_name='Honor', + mode_slug=CourseMode.HONOR, + mode_display_name=CourseMode.HONOR, ) # Create an enrollment self._create_enrollment() @@ -91,7 +95,7 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): self.assertEqual(resp.status_code, status.HTTP_200_OK) data = json.loads(resp.content) self.assertEqual(unicode(self.course.id), data['course_details']['course_id']) - self.assertEqual('honor', data['mode']) + self.assertEqual(CourseMode.HONOR, data['mode']) self.assertTrue(data['is_active']) @ddt.data( @@ -139,8 +143,8 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): def test_user_not_specified(self): CourseModeFactory.create( course_id=self.course.id, - mode_slug='honor', - mode_display_name='Honor', + mode_slug=CourseMode.HONOR, + mode_display_name=CourseMode.HONOR, ) # Create an enrollment self._create_enrollment() @@ -150,7 +154,7 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): self.assertEqual(resp.status_code, status.HTTP_200_OK) data = json.loads(resp.content) self.assertEqual(unicode(self.course.id), data['course_details']['course_id']) - self.assertEqual('honor', data['mode']) + self.assertEqual(CourseMode.HONOR, data['mode']) self.assertTrue(data['is_active']) def test_user_not_authenticated(self): @@ -187,8 +191,8 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): # Try to enroll a user that is not the authenticated user. CourseModeFactory.create( course_id=self.course.id, - mode_slug='honor', - mode_display_name='Honor', + mode_slug=CourseMode.HONOR, + mode_display_name=CourseMode.HONOR, ) self._create_enrollment(username=self.other_user.username, expected_status=status.HTTP_404_NOT_FOUND) # Verify that the server still has access to this endpoint. @@ -198,8 +202,8 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): def test_user_does_not_match_param_for_list(self): CourseModeFactory.create( course_id=self.course.id, - mode_slug='honor', - mode_display_name='Honor', + mode_slug=CourseMode.HONOR, + mode_display_name=CourseMode.HONOR, ) resp = self.client.get(reverse('courseenrollments'), {"user": self.other_user.username}) self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) @@ -213,8 +217,8 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): def test_user_does_not_match_param(self): CourseModeFactory.create( course_id=self.course.id, - mode_slug='honor', - mode_display_name='Honor', + mode_slug=CourseMode.HONOR, + mode_display_name=CourseMode.HONOR, ) resp = self.client.get( reverse('courseenrollment', kwargs={"user": self.other_user.username, "course_id": unicode(self.course.id)}) @@ -231,8 +235,8 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): def test_get_course_details(self): CourseModeFactory.create( course_id=self.course.id, - mode_slug='honor', - mode_display_name='Honor', + mode_slug=CourseMode.HONOR, + mode_display_name=CourseMode.HONOR, sku='123', ) resp = self.client.get( @@ -243,9 +247,9 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): data = json.loads(resp.content) self.assertEqual(unicode(self.course.id), data['course_id']) mode = data['course_modes'][0] - self.assertEqual(mode['slug'], 'honor') + self.assertEqual(mode['slug'], CourseMode.HONOR) self.assertEqual(mode['sku'], '123') - self.assertEqual(mode['name'], 'Honor') + self.assertEqual(mode['name'], CourseMode.HONOR) def test_with_invalid_course_id(self): self._create_enrollment(course_id='entirely/fake/course', expected_status=status.HTTP_400_BAD_REQUEST) @@ -266,7 +270,7 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): def test_enrollment_already_enrolled(self): response = self._create_enrollment() - repeat_response = self._create_enrollment() + repeat_response = self._create_enrollment(expected_status=status.HTTP_200_OK) self.assertEqual(json.loads(response.content), json.loads(repeat_response.content)) def test_get_enrollment_with_invalid_key(self): @@ -285,39 +289,143 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): def test_enrollment_throttle_for_user(self): """Make sure a user requests do not exceed the maximum number of requests""" - rate_limit_config = RateLimitConfiguration.current() - rate_limit_config.enabled = True - rate_limit_config.save() + self.rate_limit_config.enabled = True + self.rate_limit_config.save() CourseModeFactory.create( course_id=self.course.id, mode_slug=CourseMode.HONOR, mode_display_name=CourseMode.HONOR, ) - for attempt in range(0, 50): - expected_status = status.HTTP_429_TOO_MANY_REQUESTS if attempt >= 40 else status.HTTP_200_OK + for attempt in xrange(self.rate_limit + 10): + expected_status = status.HTTP_200_OK + if attempt == 0: + expected_status = status.HTTP_201_CREATED + elif attempt >= self.rate_limit: + expected_status = status.HTTP_429_TOO_MANY_REQUESTS self._create_enrollment(expected_status=expected_status) def test_enrollment_throttle_for_service(self): """Make sure a service can call the enrollment API as many times as needed. """ - rate_limit_config = RateLimitConfiguration.current() - rate_limit_config.enabled = True - rate_limit_config.save() + self.rate_limit_config.enabled = True + self.rate_limit_config.save() CourseModeFactory.create( course_id=self.course.id, mode_slug=CourseMode.HONOR, mode_display_name=CourseMode.HONOR, ) - for attempt in range(0, 50): - self._create_enrollment(as_server=True) + for attempt in xrange(self.rate_limit + 10): + expected_status = status.HTTP_201_CREATED if attempt == 0 else status.HTTP_200_OK + self._create_enrollment(as_server=True, expected_status=expected_status) - def _create_enrollment(self, course_id=None, username=None, expected_status=status.HTTP_200_OK, email_opt_in=None, as_server=False): + def test_create_enrollment_with_mode(self): + """With the right API key, create a new enrollment with a mode set other than the default.""" + # Create a professional ed course mode. + CourseModeFactory.create( + course_id=self.course.id, + mode_slug='professional', + mode_display_name='professional', + ) + + # Create an enrollment + self._create_enrollment(as_server=True, mode='professional') + + 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, 'professional') + + def test_update_enrollment_with_mode(self): + """With the right API key, update an existing enrollment with a new mode. """ + # 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 an enrollment + self._create_enrollment(as_server=True) + + # Check that the enrollment is honor. + 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.HONOR) + + # Check that the enrollment upgraded to verified. + self._create_enrollment(as_server=True, mode=CourseMode.VERIFIED, expected_status=status.HTTP_200_OK) + course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + self.assertTrue(is_active) + self.assertEqual(course_mode, CourseMode.VERIFIED) + + def test_downgrade_enrollment_with_mode(self): + """With the right API key, downgrade an existing enrollment with a new mode. """ + # 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._create_enrollment(as_server=True, mode=CourseMode.VERIFIED) + + # Check that the enrollment is verified. + 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) + + # Check that the enrollment downgraded to honor. + self._create_enrollment(as_server=True, mode=CourseMode.HONOR, expected_status=status.HTTP_200_OK) + course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + self.assertTrue(is_active) + self.assertEqual(course_mode, CourseMode.HONOR) + + 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. + for mode in [CourseMode.HONOR, CourseMode.VERIFIED]: + CourseModeFactory.create( + course_id=self.course.id, + mode_slug=mode, + mode_display_name=mode, + ) + + # Create an enrollment + self._create_enrollment() + + # Check that the enrollment is honor. + 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.HONOR) + + # Get a 403 response when trying to upgrade yourself. + self._create_enrollment(mode=CourseMode.VERIFIED, expected_status=status.HTTP_403_FORBIDDEN) + course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + self.assertTrue(is_active) + self.assertEqual(course_mode, CourseMode.HONOR) + + def _create_enrollment( + self, + course_id=None, + username=None, + expected_status=status.HTTP_201_CREATED, + email_opt_in=None, + as_server=False, + mode=CourseMode.HONOR, + ): """Enroll in the course and verify the URL we are sent to. """ course_id = unicode(self.course.id) if course_id is None else course_id username = self.user.username if username is None else username params = { + 'mode': mode, 'course_details': { 'course_id': course_id }, @@ -332,10 +440,10 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): self.assertEqual(resp.status_code, expected_status) - if expected_status == status.HTTP_200_OK: + if expected_status in [status.HTTP_201_CREATED, status.HTTP_200_OK]: data = json.loads(resp.content) self.assertEqual(course_id, data['course_details']['course_id']) - self.assertEqual('honor', data['mode']) + self.assertEqual(mode, data['mode']) self.assertTrue(data['is_active']) return resp @@ -391,7 +499,7 @@ class EnrollmentEmbargoTest(UrlResetMixin, ModuleStoreTestCase): }) response = self.client.post(url, data, content_type='application/json') - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) # Verify that we were enrolled self.assertEqual(len(self._get_enrollments()), 1) diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index 59057a413e..60d160dc1d 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -6,6 +6,7 @@ consist primarily of authentication, request validation, and serialization. from ipware.ip import get_ip from django.utils.decorators import method_decorator from opaque_keys import InvalidKeyError +from course_modes.models import CourseMode from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in from openedx.core.lib.api.permissions import ApiKeyHeaderPermission, ApiKeyHeaderPermissionIsAuthenticated from rest_framework import status @@ -215,20 +216,27 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): 2. Enroll the currently logged in user in a course. - Currently you can use this command only to enroll the user in "honor" mode. + Currently a user can use this command only to enroll the user in "honor" mode. If honor mode is not supported for the course, the request fails and returns the available modes. + A server-to-server call can be used by this command to enroll a user in other modes, such as "verified" + or "professional". If the mode is not supposed for the course, the request will fail and return the + available modes. + **Example Requests**: GET /api/enrollment/v1/enrollment - POST /api/enrollment/v1/enrollment{"course_details":{"course_id": "edX/DemoX/Demo_Course"}} + POST /api/enrollment/v1/enrollment{"mode": "honor", "course_details":{"course_id": "edX/DemoX/Demo_Course"}} **Post Parameters** * 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. + * course details: A collection that contains: * course_id: The unique identifier for the course. @@ -302,13 +310,8 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): """ Enrolls the currently logged in user in a course. """ + # Get the User, Course ID, and Mode from the request. user = request.DATA.get('user', request.user.username) - if not user: - user = request.user.username - if user != request.user.username and not self.has_api_key_permissions(request): - # Return a 404 instead of a 403 (Unauthorized). If one user is looking up - # other users, do not let them deduce the existence of an enrollment. - return Response(status=status.HTTP_404_NOT_FOUND) if 'course_details' not in request.DATA or 'course_id' not in request.DATA['course_details']: return Response( @@ -316,7 +319,6 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): data={"message": u"Course ID must be specified to create a new enrollment."} ) course_id = request.DATA['course_details']['course_id'] - try: course_id = CourseKey.from_string(course_id) except InvalidKeyError: @@ -327,11 +329,33 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): } ) + mode = request.DATA.get('mode', CourseMode.HONOR) + + has_api_key_permissions = self.has_api_key_permissions(request) + + # Check that the user specified is either the same user, or this is a server-to-server request. + if not user: + user = request.user.username + if user != request.user.username and not has_api_key_permissions: + # Return a 404 instead of a 403 (Unauthorized). If one user is looking up + # other users, do not let them deduce the existence of an enrollment. + return Response(status=status.HTTP_404_NOT_FOUND) + + if mode != CourseMode.HONOR and not has_api_key_permissions: + return Response( + status=status.HTTP_403_FORBIDDEN, + data={ + "message": u"User does not have permission to create enrollment with mode [{mode}].".format( + mode=mode + ) + } + ) + # Check whether any country access rules block the user from enrollment # We do this at the view level (rather than the Python API level) # because this check requires information about the HTTP request. redirect_url = embargo_api.redirect_if_blocked( - course_id, user=request.user, + course_id, user=user, ip_address=get_ip(request), url=request.path ) @@ -347,12 +371,25 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): ) try: - response = api.add_enrollment(user, unicode(course_id)) + # 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 + enrollment = api.get_enrollment(user, unicode(course_id)) + if has_api_key_permissions and enrollment and enrollment['mode'] != mode: + response = api.update_enrollment(user, unicode(course_id), mode=mode) + http_success_status = status.HTTP_200_OK + else: + response = api.add_enrollment(user, unicode(course_id), mode=mode) + http_success_status = status.HTTP_201_CREATED 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) + return Response(response, status=http_success_status) except CourseModeNotFoundError as error: return Response( status=status.HTTP_400_BAD_REQUEST,