diff --git a/common/djangoapps/enrollment/api.py b/common/djangoapps/enrollment/api.py index e5511989bc..1290992663 100644 --- a/common/djangoapps/enrollment/api.py +++ b/common/djangoapps/enrollment/api.py @@ -6,50 +6,24 @@ course level, such as available course modes. from django.utils import importlib import logging from django.conf import settings +from enrollment import errors log = logging.getLogger(__name__) - -class CourseEnrollmentError(Exception): - """Generic Course Enrollment Error. - - Describes any error that may occur when reading or updating enrollment information for a student or a course. - - """ - def __init__(self, msg, data=None): - super(CourseEnrollmentError, self).__init__(msg) - # Corresponding information to help resolve the error. - self.data = data - - -class CourseModeNotFoundError(CourseEnrollmentError): - """The requested course mode could not be found.""" - pass - - -class EnrollmentNotFoundError(CourseEnrollmentError): - """The requested enrollment could not be found.""" - pass - - -class EnrollmentApiLoadError(CourseEnrollmentError): - """The data API could not be loaded.""" - pass - DEFAULT_DATA_API = 'enrollment.data' -def get_enrollments(student_id): - """Retrieves all the courses a student is enrolled in. +def get_enrollments(user_id): + """Retrieves all the courses a user is enrolled in. - Takes a student and retrieves all relative enrollments. Includes information regarding how the student is enrolled + Takes a user and retrieves all relative enrollments. Includes information regarding how the user is enrolled in the the course. Args: - student_id (str): The username of the student we want to retrieve course enrollment information for. + user_id (str): The username of the user we want to retrieve course enrollment information for. Returns: - A list of enrollment information for the given student. + A list of enrollment information for the given user. Examples: >>> get_enrollments("Bob") @@ -58,7 +32,7 @@ def get_enrollments(student_id): "created": "2014-10-20T20:18:00Z", "mode": "honor", "is_active": True, - "student": "Bob", + "user": "Bob", "course": { "course_id": "edX/DemoX/2014T2", "enrollment_end": 2014-12-20T20:18:00Z, @@ -81,7 +55,7 @@ def get_enrollments(student_id): "created": "2014-10-25T20:18:00Z", "mode": "verified", "is_active": True, - "student": "Bob", + "user": "Bob", "course": { "course_id": "edX/edX-Insider/2014T2", "enrollment_end": 2014-12-20T20:18:00Z, @@ -103,16 +77,16 @@ def get_enrollments(student_id): ] """ - return _data_api().get_course_enrollments(student_id) + return _data_api().get_course_enrollments(user_id) -def get_enrollment(student_id, course_id): - """Retrieves all enrollment information for the student in respect to a specific course. +def get_enrollment(user_id, course_id): + """Retrieves all enrollment information for the user in respect to a specific course. - Gets all the course enrollment information specific to a student in a course. + Gets all the course enrollment information specific to a user in a course. Args: - student_id (str): The student to get course enrollment information for. + user_id (str): The user to get course enrollment information for. course_id (str): The course to get enrollment information for. Returns: @@ -124,7 +98,7 @@ def get_enrollment(student_id, course_id): "created": "2014-10-20T20:18:00Z", "mode": "honor", "is_active": True, - "student": "Bob", + "user": "Bob", "course": { "course_id": "edX/DemoX/2014T2", "enrollment_end": 2014-12-20T20:18:00Z, @@ -145,17 +119,17 @@ def get_enrollment(student_id, course_id): } """ - return _data_api().get_course_enrollment(student_id, course_id) + return _data_api().get_course_enrollment(user_id, course_id) -def add_enrollment(student_id, course_id, mode='honor', is_active=True): - """Enrolls a student in a course. +def add_enrollment(user_id, course_id, mode='honor', is_active=True): + """Enrolls a user in a course. - Enrolls a student in a course. If the mode is not specified, this will default to 'honor'. + Enrolls a user in a course. If the mode is not specified, this will default to 'honor'. Args: - student_id (str): The student to enroll. - course_id (str): The course to enroll the student in. + user_id (str): The user to enroll. + course_id (str): The course to enroll the user in. mode (str): Optional argument for the type of enrollment to create. Ex. 'audit', 'honor', 'verified', 'professional'. If not specified, this defaults to 'honor'. is_active (boolean): Optional argument for making the new enrollment inactive. If not specified, is_active @@ -170,7 +144,7 @@ def add_enrollment(student_id, course_id, mode='honor', is_active=True): "created": "2014-10-20T20:18:00Z", "mode": "honor", "is_active": True, - "student": "Bob", + "user": "Bob", "course": { "course_id": "edX/DemoX/2014T2", "enrollment_end": 2014-12-20T20:18:00Z, @@ -191,66 +165,19 @@ def add_enrollment(student_id, course_id, mode='honor', is_active=True): } """ _validate_course_mode(course_id, mode) - return _data_api().update_course_enrollment(student_id, course_id, mode=mode, is_active=is_active) + return _data_api().create_course_enrollment(user_id, course_id, mode, is_active) -def deactivate_enrollment(student_id, course_id): - """Un-enrolls a student in a course - - Deactivate the enrollment of a student in a course. We will not remove the enrollment data, but simply flag it - as inactive. - - Args: - student_id (str): The student associated with the deactivated enrollment. - course_id (str): The course associated with the deactivated enrollment. - - Returns: - A serializable dictionary representing the deactivated course enrollment for the student. - - Example: - >>> deactivate_enrollment("Bob", "edX/DemoX/2014T2") - { - "created": "2014-10-20T20:18:00Z", - "mode": "honor", - "is_active": False, - "student": "Bob", - "course": { - "course_id": "edX/DemoX/2014T2", - "enrollment_end": 2014-12-20T20:18:00Z, - "course_modes": [ - { - "slug": "honor", - "name": "Honor Code Certificate", - "min_price": 0, - "suggested_prices": "", - "currency": "usd", - "expiration_datetime": null, - "description": null - } - ], - "enrollment_start": 2014-10-15T20:18:00Z, - "invite_only": False - } - } - """ - # Check to see if there is an enrollment. We do not want to create a deactivated enrollment. - if not _data_api().get_course_enrollment(student_id, course_id): - raise EnrollmentNotFoundError( - u"No enrollment was found for student {student} in course {course}" - .format(student=student_id, course=course_id) - ) - return _data_api().update_course_enrollment(student_id, course_id, is_active=False) - - -def update_enrollment(student_id, course_id, mode): +def update_enrollment(user_id, course_id, mode=None, is_active=None): """Updates the course mode for the enrolled user. - Update a course enrollment for the given student and course. + Update a course enrollment for the given user and course. Args: - student_id (str): The student associated with the updated enrollment. + user_id (str): The user associated with the updated enrollment. course_id (str): The course associated with the updated enrollment. mode (str): The new course mode for this enrollment. + is_active (bool): Sets whether the enrollment is active or not. Returns: A serializable dictionary representing the updated enrollment. @@ -261,7 +188,7 @@ def update_enrollment(student_id, course_id, mode): "created": "2014-10-20T20:18:00Z", "mode": "honor", "is_active": True, - "student": "Bob", + "user": "Bob", "course": { "course_id": "edX/DemoX/2014T2", "enrollment_end": 2014-12-20T20:18:00Z, @@ -283,7 +210,12 @@ def update_enrollment(student_id, course_id, mode): """ _validate_course_mode(course_id, mode) - return _data_api().update_course_enrollment(student_id, 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) + log.warn(msg) + raise errors.EnrollmentNotFoundError(msg) + return enrollment def get_course_enrollment_details(course_id): @@ -354,7 +286,7 @@ def _validate_course_mode(course_id, mode): available=", ".join(available_modes) ) log.warn(msg) - raise CourseModeNotFoundError(msg, course_enrollment_info) + raise errors.CourseModeNotFoundError(msg, course_enrollment_info) def _data_api(): @@ -371,4 +303,4 @@ def _data_api(): return importlib.import_module(api_path) except (ImportError, ValueError): log.exception(u"Could not load module at '{path}'".format(path=api_path)) - raise EnrollmentApiLoadError(api_path) + raise errors.EnrollmentApiLoadError(api_path) diff --git a/common/djangoapps/enrollment/data.py b/common/djangoapps/enrollment/data.py index fe1e07552f..3efbb70e46 100644 --- a/common/djangoapps/enrollment/data.py +++ b/common/djangoapps/enrollment/data.py @@ -7,37 +7,40 @@ import logging from django.contrib.auth.models import User from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.django import modulestore +from enrollment.errors import CourseNotFoundError, CourseEnrollmentClosedError, CourseEnrollmentFullError, \ + CourseEnrollmentExistsError, UserNotFoundError from enrollment.serializers import CourseEnrollmentSerializer, CourseField -from student.models import CourseEnrollment, NonExistentCourseError +from student.models import CourseEnrollment, NonExistentCourseError, CourseEnrollmentException, EnrollmentClosedError, \ + CourseFullError, AlreadyEnrolledError log = logging.getLogger(__name__) -def get_course_enrollments(student_id): - """Retrieve a list representing all aggregated data for a student's course enrollments. +def get_course_enrollments(user_id): + """Retrieve a list representing all aggregated data for a user's course enrollments. - Construct a representation of all course enrollment data for a specific student. + Construct a representation of all course enrollment data for a specific user. Args: - student_id (str): The name of the student to retrieve course enrollment information for. + user_id (str): The name of the user to retrieve course enrollment information for. Returns: - A serializable list of dictionaries of all aggregated enrollment data for a student. + A serializable list of dictionaries of all aggregated enrollment data for a user. """ qset = CourseEnrollment.objects.filter( - user__username=student_id, is_active=True + user__username=user_id, is_active=True ).order_by('created') return CourseEnrollmentSerializer(qset).data # pylint: disable=no-member -def get_course_enrollment(student_id, course_id): - """Retrieve an object representing all aggregated data for a student's course enrollment. +def get_course_enrollment(username, course_id): + """Retrieve an object representing all aggregated data for a user's course enrollment. - Get the course enrollment information for a specific student and course. + Get the course enrollment information for a specific user and course. Args: - student_id (str): The name of the student to retrieve course enrollment information for. + username (str): The name of the user to retrieve course enrollment information for. course_id (str): The course to retrieve course enrollment information for. Returns: @@ -47,22 +50,65 @@ def get_course_enrollment(student_id, course_id): course_key = CourseKey.from_string(course_id) try: enrollment = CourseEnrollment.objects.get( - user__username=student_id, course_id=course_key + user__username=username, course_id=course_key ) return CourseEnrollmentSerializer(enrollment).data # pylint: disable=no-member except CourseEnrollment.DoesNotExist: return None -def update_course_enrollment(student_id, course_id, mode=None, is_active=None): - """Modify a course enrollment for a student. +def create_course_enrollment(username, course_id, mode, is_active): + """Create a new course enrollment for the given user. + + Creates a new course enrollment for the specified user username. + + Args: + username (str): The name of the user to create a new course enrollment for. + course_id (str): The course to create the course enrollment for. + mode (str): (Optional) The mode for the new enrollment. + is_active (boolean): (Optional) Determines if the enrollment is active. + + Returns: + A serializable dictionary representing the new course enrollment. + + Raises: + CourseNotFoundError + CourseEnrollmentFullError + EnrollmentClosedError + CourseEnrollmentExistsError + + """ + course_key = CourseKey.from_string(course_id) + + try: + user = User.objects.get(username=username) + except User.DoesNotExist: + msg = u"Not user with username '{username}' found.".format(username=username) + log.warn(msg) + raise UserNotFoundError(msg) + + try: + enrollment = CourseEnrollment.enroll(user, course_key, check_access=True) + return _update_enrollment(enrollment, is_active=is_active, mode=mode) + except NonExistentCourseError as err: + raise CourseNotFoundError(err.message) + except EnrollmentClosedError as err: + raise CourseEnrollmentClosedError(err.message) + except CourseFullError as err: + raise CourseEnrollmentFullError(err.message) + except AlreadyEnrolledError as err: + raise CourseEnrollmentExistsError(err.message) + + +def update_course_enrollment(username, course_id, mode=None, is_active=None): + """Modify a course enrollment for a user. Allows updates to a specific course enrollment. Args: - student_id (str): The name of the student to retrieve course enrollment information for. + username (str): The name of the user to retrieve course enrollment information for. course_id (str): The course to retrieve course enrollment information for. - mode (str): (Optional) The mode for the new enrollment. + mode (str): (Optional) If specified, modify the mode for this enrollment. is_active (boolean): (Optional) Determines if the enrollment is active. Returns: @@ -70,12 +116,22 @@ def update_course_enrollment(student_id, course_id, mode=None, is_active=None): """ course_key = CourseKey.from_string(course_id) - student = User.objects.get(username=student_id) - if not CourseEnrollment.is_enrolled(student, course_key): - enrollment = CourseEnrollment.enroll(student, course_key, check_access=True) - else: - enrollment = CourseEnrollment.objects.get(user=student, course_id=course_key) + try: + user = User.objects.get(username=username) + except User.DoesNotExist: + msg = u"Not user with username '{username}' found.".format(username=username) + log.warn(msg) + raise UserNotFoundError(msg) + + try: + enrollment = CourseEnrollment.objects.get(user=user, course_id=course_key) + return _update_enrollment(enrollment, is_active=is_active, mode=mode) + except CourseEnrollment.DoesNotExist: + return None + + +def _update_enrollment(enrollment, is_active=None, mode=None): enrollment.update_enrollment(is_active=is_active, mode=mode) enrollment.save() return CourseEnrollmentSerializer(enrollment).data # pylint: disable=no-member @@ -92,13 +148,14 @@ def get_course_enrollment_info(course_id): Returns: A serializable dictionary representing the course's enrollment information. + Raises: + CourseNotFoundError + """ course_key = CourseKey.from_string(course_id) course = modulestore().get_course(course_key) if course is None: - log.warning( - u"Requested enrollment information for unknown course {course}" - .format(course=course_id) - ) - raise NonExistentCourseError + msg = u"Requested enrollment information for unknown course {course}".format(course=course_id) + log.warning(msg) + raise CourseNotFoundError(msg) return CourseField().to_native(course) diff --git a/common/djangoapps/enrollment/errors.py b/common/djangoapps/enrollment/errors.py new file mode 100644 index 0000000000..9ea5c41542 --- /dev/null +++ b/common/djangoapps/enrollment/errors.py @@ -0,0 +1,48 @@ +"""All Error Types pertaining to Enrollment.""" + + +class CourseEnrollmentError(Exception): + """Generic Course Enrollment Error. + + Describes any error that may occur when reading or updating enrollment information for a user or a course. + + """ + def __init__(self, msg, data=None): + super(CourseEnrollmentError, self).__init__(msg) + # Corresponding information to help resolve the error. + self.data = data + + +class CourseNotFoundError(CourseEnrollmentError): + pass + + +class UserNotFoundError(CourseEnrollmentError): + pass + + +class CourseEnrollmentClosedError(CourseEnrollmentError): + pass + + +class CourseEnrollmentFullError(CourseEnrollmentError): + pass + + +class CourseEnrollmentExistsError(CourseEnrollmentError): + pass + + +class CourseModeNotFoundError(CourseEnrollmentError): + """The requested course mode could not be found.""" + pass + + +class EnrollmentNotFoundError(CourseEnrollmentError): + """The requested enrollment could not be found.""" + pass + + +class EnrollmentApiLoadError(CourseEnrollmentError): + """The data API could not be loaded.""" + pass diff --git a/common/djangoapps/enrollment/serializers.py b/common/djangoapps/enrollment/serializers.py index e95df0943a..4cdc6faabc 100644 --- a/common/djangoapps/enrollment/serializers.py +++ b/common/djangoapps/enrollment/serializers.py @@ -53,8 +53,12 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): the Course Descriptor and course modes, to give a complete representation of course enrollment. """ - course = CourseField() - student = serializers.SerializerMethodField('get_username') + course_details = serializers.SerializerMethodField('get_course_details') + user = serializers.SerializerMethodField('get_username') + + def get_course_details(self, model): + field = CourseField() + return field.to_native(model.course) def get_username(self, model): """Retrieves the username from the associated model.""" @@ -62,7 +66,7 @@ class CourseEnrollmentSerializer(serializers.ModelSerializer): class Meta: # pylint: disable=missing-docstring model = CourseEnrollment - fields = ('created', 'mode', 'is_active', 'course', 'student') + fields = ('created', 'mode', 'is_active', 'course_details', 'user') lookup_field = 'username' diff --git a/common/djangoapps/enrollment/tests/fake_data_api.py b/common/djangoapps/enrollment/tests/fake_data_api.py index 80140aaec5..adee6dd320 100644 --- a/common/djangoapps/enrollment/tests/fake_data_api.py +++ b/common/djangoapps/enrollment/tests/fake_data_api.py @@ -31,14 +31,17 @@ def get_course_enrollment(student_id, course_id): return _get_fake_enrollment(student_id, course_id) +def create_course_enrollment(student_id, course_id, mode='honor', is_active=True): + """Stubbed out Enrollment creation request. """ + return add_enrollment(student_id, course_id, mode=mode, is_active=is_active) + + def update_course_enrollment(student_id, course_id, mode=None, is_active=None): """Stubbed out Enrollment data request.""" enrollment = _get_fake_enrollment(student_id, course_id) - if not enrollment: - enrollment = add_enrollment(student_id, course_id) - if mode is not None: + if enrollment and mode is not None: enrollment['mode'] = mode - if is_active is not None: + if enrollment and is_active is not None: enrollment['is_active'] = is_active return enrollment diff --git a/common/djangoapps/enrollment/tests/test_api.py b/common/djangoapps/enrollment/tests/test_api.py index fe996bd6dc..f6b80994b6 100644 --- a/common/djangoapps/enrollment/tests/test_api.py +++ b/common/djangoapps/enrollment/tests/test_api.py @@ -8,6 +8,7 @@ from django.test import TestCase from django.test.utils import override_settings from django.conf import settings from enrollment import api +from enrollment.errors import EnrollmentApiLoadError, EnrollmentNotFoundError, CourseModeNotFoundError from enrollment.tests import fake_data_api @@ -51,7 +52,7 @@ class EnrollmentTest(TestCase): get_result = api.get_enrollment(self.USERNAME, self.COURSE_ID) self.assertEquals(result, get_result) - @raises(api.CourseModeNotFoundError) + @raises(CourseModeNotFoundError) def test_prof_ed_enroll(self): # Add a fake course enrollment information to the fake data API fake_data_api.add_course(self.COURSE_ID, course_modes=['professional']) @@ -83,18 +84,18 @@ class EnrollmentTest(TestCase): self.assertEquals(result['mode'], mode) self.assertTrue(result['is_active']) - result = api.deactivate_enrollment(self.USERNAME, self.COURSE_ID) + result = api.update_enrollment(self.USERNAME, self.COURSE_ID, mode=mode, is_active=False) self.assertIsNotNone(result) self.assertEquals(result['student'], self.USERNAME) self.assertEquals(result['course']['course_id'], self.COURSE_ID) self.assertEquals(result['mode'], mode) self.assertFalse(result['is_active']) - @raises(api.EnrollmentNotFoundError) + @raises(EnrollmentNotFoundError) def test_unenroll_not_enrolled_in_course(self): # Add a fake course enrollment information to the fake data API fake_data_api.add_course(self.COURSE_ID, course_modes=['honor']) - api.deactivate_enrollment(self.USERNAME, self.COURSE_ID) + api.update_enrollment(self.USERNAME, self.COURSE_ID, mode='honor', is_active=False) @ddt.data( # Simple test of honor and verified. @@ -145,7 +146,7 @@ class EnrollmentTest(TestCase): self.assertEquals(3, len(result['course_modes'])) @override_settings(ENROLLMENT_DATA_API='foo.bar.biz.baz') - @raises(api.EnrollmentApiLoadError) + @raises(EnrollmentApiLoadError) def test_data_api_config_error(self): # Enroll in the course and verify the URL we get sent to api.add_enrollment(self.USERNAME, self.COURSE_ID, mode='audit') diff --git a/common/djangoapps/enrollment/tests/test_data.py b/common/djangoapps/enrollment/tests/test_data.py index e39b3ea74e..aba98599ce 100644 --- a/common/djangoapps/enrollment/tests/test_data.py +++ b/common/djangoapps/enrollment/tests/test_data.py @@ -3,6 +3,7 @@ Test the Data Aggregation Layer for Course Enrollments. """ import ddt +from mock import patch from nose.tools import raises import unittest @@ -12,8 +13,10 @@ from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, mixed_store_config ) from xmodule.modulestore.tests.factories import CourseFactory +from enrollment.errors import CourseNotFoundError, UserNotFoundError, CourseEnrollmentClosedError, \ + CourseEnrollmentFullError, CourseEnrollmentExistsError from student.tests.factories import UserFactory, CourseModeFactory -from student.models import CourseEnrollment, NonExistentCourseError +from student.models import CourseEnrollment, EnrollmentClosedError, CourseFullError, AlreadyEnrolledError from enrollment import data # Since we don't need any XML course fixtures, use a modulestore configuration @@ -54,12 +57,11 @@ class EnrollmentDataTest(ModuleStoreTestCase): def test_enroll(self, course_modes, enrollment_mode): # Create the course modes (if any) required for this test case self._create_course_modes(course_modes) - - enrollment = data.update_course_enrollment( + enrollment = data.create_course_enrollment( self.user.username, unicode(self.course.id), - mode=enrollment_mode, - is_active=True + enrollment_mode, + True ) self.assertTrue(CourseEnrollment.is_enrolled(self.user, self.course.id)) @@ -72,7 +74,7 @@ class EnrollmentDataTest(ModuleStoreTestCase): self.assertEqual(is_active, enrollment['is_active']) def test_unenroll(self): - # Enroll the student in the course + # Enroll the user in the course CourseEnrollment.enroll(self.user, self.course.id, mode="honor") enrollment = data.update_course_enrollment( @@ -119,9 +121,11 @@ class EnrollmentDataTest(ModuleStoreTestCase): for course in created_courses: self._create_course_modes(course_modes, course=course) # Create the original enrollment. - created_enrollments.append(data.update_course_enrollment( + created_enrollments.append(data.create_course_enrollment( self.user.username, unicode(course.id), + 'honor', + True )) # Compare the created enrollments with the results @@ -148,18 +152,18 @@ class EnrollmentDataTest(ModuleStoreTestCase): self.assertIsNone(result) # Create the original enrollment. - enrollment = data.update_course_enrollment( + enrollment = data.create_course_enrollment( self.user.username, unicode(self.course.id), - mode=enrollment_mode, - is_active=True + enrollment_mode, + True ) # Get the enrollment and compare it to the original. result = data.get_course_enrollment(self.user.username, unicode(self.course.id)) - self.assertEqual(self.user.username, result['student']) + self.assertEqual(self.user.username, result['user']) self.assertEqual(enrollment, result) - @raises(NonExistentCourseError) + @raises(CourseNotFoundError) def test_non_existent_course(self): data.get_course_enrollment_info("this/is/bananas") @@ -172,3 +176,37 @@ class EnrollmentDataTest(ModuleStoreTestCase): mode_slug=mode_slug, mode_display_name=mode_slug, ) + + @raises(UserNotFoundError) + def test_enrollment_for_non_existent_user(self): + data.create_course_enrollment("some_fake_user", unicode(self.course.id), 'honor', True) + + @raises(CourseNotFoundError) + def test_enrollment_for_non_existent_course(self): + data.create_course_enrollment(self.user.username, "some/fake/course", 'honor', True) + + @raises(CourseEnrollmentClosedError) + @patch.object(CourseEnrollment, "enroll") + def test_enrollment_for_closed_course(self, mock_enroll): + mock_enroll.side_effect = EnrollmentClosedError("Bad things happened") + data.create_course_enrollment(self.user.username, unicode(self.course.id), 'honor', True) + + @raises(CourseEnrollmentFullError) + @patch.object(CourseEnrollment, "enroll") + def test_enrollment_for_closed_course(self, mock_enroll): + mock_enroll.side_effect = CourseFullError("Bad things happened") + data.create_course_enrollment(self.user.username, unicode(self.course.id), 'honor', True) + + @raises(CourseEnrollmentExistsError) + @patch.object(CourseEnrollment, "enroll") + def test_enrollment_for_closed_course(self, mock_enroll): + mock_enroll.side_effect = AlreadyEnrolledError("Bad things happened") + data.create_course_enrollment(self.user.username, unicode(self.course.id), 'honor', True) + + @raises(UserNotFoundError) + def test_update_for_non_existent_user(self): + data.update_course_enrollment("some_fake_user", unicode(self.course.id), is_active=False) + + def test_update_for_non_existent_course(self): + enrollment = data.update_course_enrollment(self.user.username, "some/fake/course", is_active=False) + self.assertIsNone(enrollment) diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index 2f4b5fff9a..38a52883af 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -1,10 +1,11 @@ """ -Tests for student enrollment. +Tests for user enrollment. """ import ddt import json import unittest +from mock import patch from django.test.utils import override_settings from django.core.urlresolvers import reverse from rest_framework.test import APITestCase @@ -14,6 +15,8 @@ from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, mixed_store_config ) from xmodule.modulestore.tests.factories import CourseFactory +from enrollment import api +from enrollment.errors import CourseEnrollmentError from student.tests.factories import UserFactory, CourseModeFactory from student.models import CourseEnrollment @@ -27,7 +30,7 @@ MODULESTORE_CONFIG = mixed_store_config(settings.COMMON_TEST_DATA_ROOT, {}, incl @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class EnrollmentTest(ModuleStoreTestCase, APITestCase): """ - Test student enrollment, especially with different course modes. + Test user enrollment, especially with different course modes. """ USERNAME = "Bob" EMAIL = "bob@example.com" @@ -68,6 +71,23 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): self.assertTrue(is_active) self.assertEqual(course_mode, enrollment_mode) + def test_check_enrollment(self): + CourseModeFactory.create( + course_id=self.course.id, + mode_slug='honor', + mode_display_name='Honor', + ) + # Create an enrollment + self._create_enrollment() + resp = self.client.get( + reverse('courseenrollment', kwargs={"user": self.user.username, "course_id": unicode(self.course.id)}) + ) + 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.assertTrue(data['is_active']) + def test_enroll_prof_ed(self): # Create the prod ed mode. CourseModeFactory.create( @@ -77,51 +97,125 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase): ) # Enroll in the course, this will fail if the mode is not explicitly professional. - resp = self.client.post(reverse('courseenrollment', kwargs={'course_id': (unicode(self.course.id))})) - self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + resp = self._create_enrollment(expected_status=status.HTTP_400_BAD_REQUEST) # While the enrollment wrong is invalid, the response content should have # all the valid enrollment modes. data = json.loads(resp.content) - self.assertEqual(unicode(self.course.id), data['course_id']) - self.assertEqual(1, len(data['course_modes'])) - self.assertEqual('professional', data['course_modes'][0]['slug']) + self.assertEqual(unicode(self.course.id), data['course_details']['course_id']) + self.assertEqual(1, len(data['course_details']['course_modes'])) + self.assertEqual('professional', data['course_details']['course_modes'][0]['slug']) def test_user_not_authenticated(self): # Log out, so we're no longer authenticated self.client.logout() # Try to enroll, this should fail. - resp = self.client.post(reverse('courseenrollment', kwargs={'course_id': (unicode(self.course.id))})) - self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + self._create_enrollment(expected_status=status.HTTP_403_FORBIDDEN) def test_user_not_activated(self): - # Create a user account, but don't activate it + # Log out the default user, Bob. + self.client.logout() + + # Create a user account self.user = UserFactory.create( username="inactive", email="inactive@example.com", password=self.PASSWORD, - is_active=False + is_active=True ) # Log in with the unactivated account self.client.login(username="inactive", password=self.PASSWORD) + # Deactivate the user. Has to be done after login to get the user into the + # request and properly logged in. + self.user.is_active = False + self.user.save() + # Enrollment should succeed, even though we haven't authenticated. - resp = self.client.post(reverse('courseenrollment', kwargs={'course_id': (unicode(self.course.id))})) - self.assertEqual(resp.status_code, 200) + self._create_enrollment() + + def test_user_does_not_match_url(self): + # 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', + ) + self._create_enrollment(username='not_the_user', expected_status=status.HTTP_404_NOT_FOUND) + + def test_user_does_not_match_param_for_list(self): + CourseModeFactory.create( + course_id=self.course.id, + mode_slug='honor', + mode_display_name='Honor', + ) + resp = self.client.get(reverse('courseenrollments'), {"user": "not_the_user"}) + self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) + + def test_user_does_not_match_param(self): + CourseModeFactory.create( + course_id=self.course.id, + mode_slug='honor', + mode_display_name='Honor', + ) + resp = self.client.get( + reverse('courseenrollment', kwargs={"user": "not_the_user", "course_id": unicode(self.course.id)}) + ) + self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND) + + def test_get_course_details(self): + CourseModeFactory.create( + course_id=self.course.id, + mode_slug='honor', + mode_display_name='Honor', + ) + resp = self.client.get( + reverse('courseenrollmentdetails', kwargs={"course_id": unicode(self.course.id)}) + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + data = json.loads(resp.content) + self.assertEqual(unicode(self.course.id), data['course_id']) def test_with_invalid_course_id(self): - # Create an enrollment - resp = self.client.post(reverse('courseenrollment', kwargs={'course_id': 'entirely/fake/course'})) + self._create_enrollment(course_id='entirely/fake/course', expected_status=status.HTTP_400_BAD_REQUEST) + + def test_get_enrollment_details_bad_course(self): + resp = self.client.get( + reverse('courseenrollmentdetails', kwargs={"course_id": "some/fake/course"}) + ) self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) - def _create_enrollment(self): + @patch.object(api, "get_enrollment") + def test_get_enrollment_internal_error(self, mock_get_enrollment): + mock_get_enrollment.side_effect = CourseEnrollmentError("Something bad happened.") + resp = self.client.get( + reverse('courseenrollment', kwargs={"user": self.user.username, "course_id": unicode(self.course.id)}) + ) + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + + def _create_enrollment(self, course_id=None, username=None, expected_status=status.HTTP_200_OK): + course_id = unicode(self.course.id) if course_id is None else course_id + username = self.user.username if username is None else username """Enroll in the course and verify the URL we are sent to. """ - resp = self.client.post(reverse('courseenrollment', kwargs={'course_id': (unicode(self.course.id))})) - self.assertEqual(resp.status_code, status.HTTP_200_OK) - data = json.loads(resp.content) - self.assertEqual(unicode(self.course.id), data['course']['course_id']) - self.assertEqual('honor', data['mode']) - self.assertTrue(data['is_active']) + + resp = self.client.post( + reverse('courseenrollments'), + { + 'course_details': { + 'course_id': course_id + }, + 'user': username + }, + format='json' + ) + self.assertEqual(resp.status_code, expected_status) + + if expected_status == status.HTTP_200_OK: + data = json.loads(resp.content) + self.assertEqual(course_id, data['course_details']['course_id']) + self.assertEqual('honor', data['mode']) + self.assertTrue(data['is_active']) return resp diff --git a/common/djangoapps/enrollment/urls.py b/common/djangoapps/enrollment/urls.py index e3621a2e94..6cff23ce4a 100644 --- a/common/djangoapps/enrollment/urls.py +++ b/common/djangoapps/enrollment/urls.py @@ -5,17 +5,25 @@ URLs for the Enrollment API from django.conf import settings from django.conf.urls import patterns, url -from .views import get_course_enrollment, list_student_enrollments +from .views import ( + EnrollmentView, + EnrollmentListView, + EnrollmentCourseDetailView +) -urlpatterns = [] +USER_PATTERN = '(?P[\w.@+-]+)' -if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): - urlpatterns += patterns( - 'enrollment.views', - url(r'^student$', list_student_enrollments, name='courseenrollments'), - url( - r'^course/{course_key}$'.format(course_key=settings.COURSE_ID_PATTERN), - get_course_enrollment, - name='courseenrollment' - ), - ) +urlpatterns = patterns( + 'enrollment.views', + url( + r'^enrollment/{user},{course_key}$'.format(user=USER_PATTERN, course_key=settings.COURSE_ID_PATTERN), + EnrollmentView.as_view(), + name='courseenrollment' + ), + url(r'^enrollment$', EnrollmentListView.as_view(), name='courseenrollments'), + url( + r'^course/{course_key}$'.format(course_key=settings.COURSE_ID_PATTERN), + EnrollmentCourseDetailView.as_view(), + name='courseenrollmentdetails' + ), +) diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index 73d4ced3d6..7aaa245327 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -4,13 +4,13 @@ consist primarily of authentication, request validation, and serialization. """ from rest_framework import status -from rest_framework.authentication import OAuth2Authentication, SessionAuthentication -from rest_framework.decorators import api_view, authentication_classes, permission_classes, throttle_classes -from rest_framework.permissions import IsAuthenticated +from rest_framework.authentication import OAuth2Authentication +from rest_framework import permissions from rest_framework.response import Response from rest_framework.throttling import UserRateThrottle +from rest_framework.views import APIView from enrollment import api -from student.models import NonExistentCourseError, CourseEnrollmentException +from enrollment.errors import CourseNotFoundError, CourseEnrollmentError, CourseModeNotFoundError from util.authentication import SessionAuthenticationAllowInactiveUser @@ -20,58 +20,181 @@ class EnrollmentUserThrottle(UserRateThrottle): rate = '50/second' -@api_view(['GET']) -@authentication_classes((OAuth2Authentication, SessionAuthentication)) -@permission_classes((IsAuthenticated,)) -@throttle_classes([EnrollmentUserThrottle]) -def list_student_enrollments(request): - """List out all the enrollments for the current student +class EnrollmentView(APIView): + """ Enrollment API View for creating, updating, and viewing course enrollments. """ - Returns a JSON response with all the course enrollments for the current student. + authentication_classes = OAuth2Authentication, SessionAuthenticationAllowInactiveUser + permission_classes = permissions.IsAuthenticated, + throttle_classes = EnrollmentUserThrottle, - Args: - request (Request): The GET request for course enrollment listings. + def get(self, request, course_id=None, user=None): + """Create, read, or update enrollment information for a user. - Returns: - A JSON serialized representation of the student's course enrollments. + HTTP Endpoint for all CRUD operations for a user course enrollment. Allows creation, reading, and + updates of the current enrollment for a particular course. - """ - return Response(api.get_enrollments(request.user.username)) + Args: + request (Request): To get current course enrollment information, a GET request will return + information for the current user and the specified course. + course_id (str): URI element specifying the course location. Enrollment information will be + returned, created, or updated for this particular course. + user (str): The user username associated with this enrollment request. + + Return: + A JSON serialized representation of the course enrollment. + + """ + if request.user.username != user: + # 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) + try: + return Response(api.get_enrollment(user, course_id)) + except CourseEnrollmentError: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "message": ( + u"An error occurred while retrieving enrollments for user " + u"'{user}' in course '{course_id}'" + ).format(user=user, course_id=course_id) + } + ) -@api_view(['GET', 'POST']) -@authentication_classes((OAuth2Authentication, SessionAuthenticationAllowInactiveUser)) -@permission_classes((IsAuthenticated,)) -@throttle_classes([EnrollmentUserThrottle]) -def get_course_enrollment(request, course_id=None): - """Create, read, or update enrollment information for a student. +class EnrollmentCourseDetailView(APIView): + """ Enrollment API View for viewing course enrollment details. """ - HTTP Endpoint for all CRUD operations for a student course enrollment. Allows creation, reading, and - updates of the current enrollment for a particular course. + authentication_classes = [] + permission_classes = [] + throttle_classes = EnrollmentUserThrottle, - Args: - request (Request): To get current course enrollment information, a GET request will return - information for the current user and the specified course. A POST request will create a - new course enrollment for the current user. If 'mode' or 'deactivate' are found in the - POST parameters, the mode can be modified, or the enrollment can be deactivated. - course_id (str): URI element specifying the course location. Enrollment information will be - returned, created, or updated for this particular course. + def get(self, request, course_id=None): + """Read enrollment information for a particular course. - Return: - A JSON serialized representation of the course enrollment. If this is a new or modified enrollment, - the returned enrollment will reflect all changes. + HTTP Endpoint for retrieving course level enrollment information. - """ - try: - if course_id and request.method == 'POST': - return Response(api.add_enrollment(request.user.username, course_id)) - else: - return Response(api.get_enrollment(request.user.username, course_id)) - except api.CourseModeNotFoundError as error: - return Response(status=status.HTTP_400_BAD_REQUEST, data=error.data) - except NonExistentCourseError: - return Response(status=status.HTTP_400_BAD_REQUEST) - except api.EnrollmentNotFoundError: - return Response(status=status.HTTP_400_BAD_REQUEST) - except CourseEnrollmentException: - return Response(status=status.HTTP_400_BAD_REQUEST) + Args: + request (Request): To get current course enrollment information, a GET request will return + information for the specified course. + course_id (str): URI element specifying the course location. Enrollment information will be + returned. + + Return: + A JSON serialized representation of the course enrollment details. + + """ + try: + return Response(api.get_course_enrollment_details(course_id)) + except CourseNotFoundError: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "message": ( + u"No course found for course ID '{course_id}'" + ).format(course_id=course_id) + } + ) + + +class EnrollmentListView(APIView): + """ Enrollment API List View for viewing all course enrollments for a user. """ + + authentication_classes = OAuth2Authentication, SessionAuthenticationAllowInactiveUser + permission_classes = permissions.IsAuthenticated, + throttle_classes = EnrollmentUserThrottle, + + def get(self, request): + """List out all the enrollments for the current user + + Returns a JSON response with all the course enrollments for the current user. + + Args: + request (Request): The GET request for course enrollment listings. + user (str): Get all enrollments for the specified user's username. + + Returns: + A JSON serialized representation of the user's course enrollments. + + """ + user = request.GET.get('user', request.user.username) + if request.user.username != user: + # 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) + try: + return Response(api.get_enrollments(user)) + except CourseEnrollmentError: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "message": ( + u"An error occurred while retrieving enrollments for user '{user}'" + ).format(user=user) + } + ) + + def post(self, request): + """Create a new enrollment + + Creates a new enrollment based on the data posted. Currently all that can be specified is + the course id. All other attributes will be determined by the server, and cannot be updated + through this endpoint. + + By default, this will attempt to create the enrollment with course mode 'honor'. If the course + does not have an 'honor' course mode, it will fail as a bad request and list the available + course modes. + + Args: + request (Request): The POST request to create a new enrollment. POST DATA should contain + 'course_details' with an attribute 'course_id' to identify which course to enroll in. + 'user' may be specified as well, but must match the username of the authenticated user. + Ex. {'user': 'Bob', 'course_details': { 'course_id': 'edx/demo/T2014' } } + + Returns: + A JSON serialized representation of the user's new course enrollment. + + """ + user = request.DATA.get('user', request.user.username) + if not user: + user = request.user.username + if user != request.user.username: + # 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( + status=status.HTTP_400_BAD_REQUEST, + data={"message": u"Course ID must be specified to create a new enrollment."} + ) + course_id = request.DATA['course_details']['course_id'] + + try: + return Response(api.add_enrollment(user, course_id)) + except CourseModeNotFoundError as error: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "message": ( + u"The course mode '{mode}' is not available for course '{course_id}'." + ).format(mode="honor", course_id=course_id), + "course_details": error.data + }) + except CourseNotFoundError: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "message": u"No course '{course_id}' found for enrollment".format(course_id=course_id) + } + ) + except CourseEnrollmentError: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={ + "message": ( + u"An error occurred while creating the new course enrollment for user " + u"'{user}' in course '{course_id}'" + ).format(user=user, course_id=course_id) + } + ) diff --git a/lms/static/js/spec/student_account/enrollment_spec.js b/lms/static/js/spec/student_account/enrollment_spec.js index 05119f1414..712e468dbf 100644 --- a/lms/static/js/spec/student_account/enrollment_spec.js +++ b/lms/static/js/spec/student_account/enrollment_spec.js @@ -5,7 +5,7 @@ define(['js/common_helpers/ajax_helpers', 'js/student_account/enrollment'], describe( 'edx.student.account.EnrollmentInterface', function() { var COURSE_KEY = 'edX/DemoX/Fall', - ENROLL_URL = '/enrollment/v0/course/edX/DemoX/Fall', + ENROLL_URL = '/api/enrollment/v1/enrollment', FORWARD_URL = '/course_modes/choose/edX/DemoX/Fall/'; beforeEach(function() { @@ -21,7 +21,12 @@ define(['js/common_helpers/ajax_helpers', 'js/student_account/enrollment'], EnrollmentInterface.enroll( COURSE_KEY ); // Expect that the correct request was made to the server - AjaxHelpers.expectRequest( requests, 'POST', ENROLL_URL ); + AjaxHelpers.expectRequest( + requests, + 'POST', + ENROLL_URL, + '{"course_details":{"course_id":"edX/DemoX/Fall"}}' + ); // Simulate a successful response from the server AjaxHelpers.respondWithJson(requests, {}); diff --git a/lms/static/js/student_account/enrollment.js b/lms/static/js/student_account/enrollment.js index f16e78fcbf..7ec64f392b 100644 --- a/lms/static/js/student_account/enrollment.js +++ b/lms/static/js/student_account/enrollment.js @@ -9,7 +9,7 @@ var edx = edx || {}; edx.student.account.EnrollmentInterface = { urls: { - course: '/enrollment/v0/course/', + enrollment: '/api/enrollment/v1/enrollment', trackSelection: '/course_modes/choose/' }, @@ -23,10 +23,17 @@ var edx = edx || {}; * @param {string} courseKey Slash-separated course key. */ enroll: function( courseKey ) { + var data_obj = { + course_details: { + course_id: courseKey + } + }; + var data = JSON.stringify(data_obj); $.ajax({ - url: this.courseEnrollmentUrl( courseKey ), + url: this.urls.enrollment, type: 'POST', - data: {}, + contentType: 'application/json; charset=utf-8', + data: data, headers: this.headers, context: this }).always(function() { @@ -43,15 +50,6 @@ var edx = edx || {}; return this.urls.trackSelection + courseKey + '/'; }, - /** - * Construct a URL to enroll in a course. - * @param {string} courseKey Slash-separated course key. - * @return {string} The URL to enroll in a course. - */ - courseEnrollmentUrl: function( courseKey ) { - return this.urls.course + courseKey; - }, - /** * Redirect to a URL. Mainly useful for mocking out in tests. * @param {string} url The URL to redirect to. diff --git a/lms/urls.py b/lms/urls.py index 8a5e20a966..cff6855eac 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -74,7 +74,7 @@ urlpatterns = ('', # nopep8 url(r'^submit_feedback$', 'util.views.submit_feedback'), # Enrollment API RESTful endpoints - url(r'^enrollment/v0/', include('enrollment.urls')), + url(r'^api/enrollment/v1/', include('enrollment.urls')), )