From cdb3e99f39d0b79918e9490c2affce92aeb94819 Mon Sep 17 00:00:00 2001 From: Bill DeRusha Date: Wed, 9 Dec 2015 13:21:44 -0500 Subject: [PATCH 1/2] Enrollment API determines best default enrollment if none passed --- common/djangoapps/enrollment/api.py | 33 ++++++++++++++++--- .../djangoapps/enrollment/tests/test_api.py | 30 +++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/enrollment/api.py b/common/djangoapps/enrollment/api.py index 1a0043eb0e..25047219f7 100644 --- a/common/djangoapps/enrollment/api.py +++ b/common/djangoapps/enrollment/api.py @@ -133,7 +133,7 @@ def get_enrollment(user_id, course_id): return _data_api().get_course_enrollment(user_id, course_id) -def add_enrollment(user_id, course_id, mode=CourseMode.DEFAULT_MODE_SLUG, is_active=True): +def add_enrollment(user_id, course_id, mode=None, is_active=True): """Enrolls a user in a course. Enrolls a user in a course. If the mode is not specified, this will default to `CourseMode.DEFAULT_MODE_SLUG`. @@ -180,6 +180,8 @@ def add_enrollment(user_id, course_id, mode=CourseMode.DEFAULT_MODE_SLUG, is_act } } """ + if mode is None: + mode = _default_course_mode(course_id) _validate_course_mode(course_id, mode, is_active=is_active) return _data_api().create_course_enrollment(user_id, course_id, mode, is_active) @@ -359,16 +361,37 @@ def get_enrollment_attributes(user_id, course_id): return _data_api().get_enrollment_attributes(user_id, course_id) +def _default_course_mode(course_id): + """Return the default enrollment for a course. + + Special case the default enrollment to return if nothing else is found. + + Arguments: + course_id (str): The course to check against for available course modes. + + Returns: + str + """ + course_enrollment_info = _data_api().get_course_enrollment_info(course_id, include_expired=False) + course_modes = course_enrollment_info["course_modes"] + available_modes = [m['slug'] for m in course_modes] + + if CourseMode.DEFAULT_MODE_SLUG in available_modes: + return CourseMode.DEFAULT_MODE_SLUG + elif 'audit' in available_modes: + return 'audit' + elif 'honor' in available_modes: + return 'honor' + + return CourseMode.DEFAULT_MODE_SLUG + + def _validate_course_mode(course_id, mode, is_active=None): """Checks to see if the specified course mode is valid for the course. If the requested course mode is not available for the course, raise an error with corresponding course enrollment information. - 'honor' is special cased. If there are no course modes configured, and the specified mode is - 'honor', return true, allowing the enrollment to be 'honor' even if the mode is not explicitly - set for the course. - Arguments: course_id (str): The course to check against for available course modes. mode (str): The slug for the course mode specified in the enrollment. diff --git a/common/djangoapps/enrollment/tests/test_api.py b/common/djangoapps/enrollment/tests/test_api.py index 4c11ea0fb0..3d41d24743 100644 --- a/common/djangoapps/enrollment/tests/test_api.py +++ b/common/djangoapps/enrollment/tests/test_api.py @@ -8,6 +8,8 @@ import unittest from django.test import TestCase from django.test.utils import override_settings from django.conf import settings + +from course_modes.models import CourseMode from enrollment import api from enrollment.errors import EnrollmentApiLoadError, EnrollmentNotFoundError, CourseModeNotFoundError from enrollment.tests import fake_data_api @@ -56,6 +58,34 @@ class EnrollmentTest(TestCase): get_result = api.get_enrollment(self.USERNAME, self.COURSE_ID) self.assertEquals(result, get_result) + @ddt.data( + ([CourseMode.DEFAULT_MODE_SLUG, 'verified', 'credit'], CourseMode.DEFAULT_MODE_SLUG), + (['audit', 'verified', 'credit'], 'audit'), + (['honor', 'verified', 'credit'], 'honor'), + ) + @ddt.unpack + def test_enroll_no_mode_success(self, course_modes, expected_mode): + # Add a fake course enrollment information to the fake data API + fake_data_api.add_course(self.COURSE_ID, course_modes=course_modes) + # Enroll in the course and verify the URL we get sent to + result = api.add_enrollment(self.USERNAME, self.COURSE_ID) + self.assertIsNotNone(result) + self.assertEquals(result['student'], self.USERNAME) + self.assertEquals(result['course']['course_id'], self.COURSE_ID) + self.assertEquals(result['mode'], expected_mode) + + @ddt.data( + ['professional'], + ['verified'], + ['verified', 'professional'], + ) + @raises(CourseModeNotFoundError) + def test_enroll_no_mode_error(self, course_modes): + # Add a fake course enrollment information to the fake data API + fake_data_api.add_course(self.COURSE_ID, course_modes=course_modes) + # Enroll in the course and verify that we raise CourseModeNotFoundError + api.add_enrollment(self.USERNAME, self.COURSE_ID) + @raises(CourseModeNotFoundError) def test_prof_ed_enroll(self): # Add a fake course enrollment information to the fake data API From 005e07cb95e4de1bfaf07be77497f069a0627531 Mon Sep 17 00:00:00 2001 From: Bill DeRusha Date: Wed, 9 Dec 2015 17:51:03 -0500 Subject: [PATCH 2/2] Remove default course mode setting in enrollment api --- common/djangoapps/enrollment/api.py | 7 ++++--- common/djangoapps/enrollment/tests/test_api.py | 17 +++++++++++------ common/djangoapps/enrollment/views.py | 4 ++-- common/djangoapps/student/models.py | 7 +++++-- lms/djangoapps/instructor/enrollment.py | 2 +- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/common/djangoapps/enrollment/api.py b/common/djangoapps/enrollment/api.py index 25047219f7..2e352a7581 100644 --- a/common/djangoapps/enrollment/api.py +++ b/common/djangoapps/enrollment/api.py @@ -7,6 +7,8 @@ import importlib import logging from django.conf import settings from django.core.cache import cache +from opaque_keys.edx.keys import CourseKey + from course_modes.models import CourseMode from enrollment import errors @@ -372,9 +374,8 @@ def _default_course_mode(course_id): Returns: str """ - course_enrollment_info = _data_api().get_course_enrollment_info(course_id, include_expired=False) - course_modes = course_enrollment_info["course_modes"] - available_modes = [m['slug'] for m in course_modes] + course_modes = CourseMode.modes_for_course(CourseKey.from_string(course_id)) + available_modes = [m.slug for m in course_modes] if CourseMode.DEFAULT_MODE_SLUG in available_modes: return CourseMode.DEFAULT_MODE_SLUG diff --git a/common/djangoapps/enrollment/tests/test_api.py b/common/djangoapps/enrollment/tests/test_api.py index 3d41d24743..95b441d157 100644 --- a/common/djangoapps/enrollment/tests/test_api.py +++ b/common/djangoapps/enrollment/tests/test_api.py @@ -1,6 +1,8 @@ """ Tests for student enrollment. """ +from mock import patch, Mock + import ddt from django.core.cache import cache from nose.tools import raises @@ -67,12 +69,15 @@ class EnrollmentTest(TestCase): def test_enroll_no_mode_success(self, course_modes, expected_mode): # Add a fake course enrollment information to the fake data API fake_data_api.add_course(self.COURSE_ID, course_modes=course_modes) - # Enroll in the course and verify the URL we get sent to - result = api.add_enrollment(self.USERNAME, self.COURSE_ID) - self.assertIsNotNone(result) - self.assertEquals(result['student'], self.USERNAME) - self.assertEquals(result['course']['course_id'], self.COURSE_ID) - self.assertEquals(result['mode'], expected_mode) + with patch('enrollment.api.CourseMode.modes_for_course') as mock_modes_for_course: + mock_course_modes = [Mock(slug=mode) for mode in course_modes] + mock_modes_for_course.return_value = mock_course_modes + # Enroll in the course and verify the URL we get sent to + result = api.add_enrollment(self.USERNAME, self.COURSE_ID) + self.assertIsNotNone(result) + self.assertEquals(result['student'], self.USERNAME) + self.assertEquals(result['course']['course_id'], self.COURSE_ID) + self.assertEquals(result['mode'], expected_mode) @ddt.data( ['professional'], diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index 110df7e906..69aea5996c 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -520,7 +520,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): } ) - mode = request.data.get('mode', CourseMode.DEFAULT_MODE_SLUG) + mode = request.data.get('mode') has_api_key_permissions = self.has_api_key_permissions(request) @@ -532,7 +532,7 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): # other users, do not let them deduce the existence of an enrollment. return Response(status=status.HTTP_404_NOT_FOUND) - if mode != CourseMode.DEFAULT_MODE_SLUG and not has_api_key_permissions: + if mode not in (CourseMode.AUDIT, CourseMode.HONOR, None) and not has_api_key_permissions: return Response( status=status.HTTP_403_FORBIDDEN, data={ diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index de736b7f9f..d3976288a4 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -47,6 +47,7 @@ from xmodule_django.models import CourseKeyField, NoneToEmptyManager from certificates.models import GeneratedCertificate from course_modes.models import CourseMode +from enrollment.api import _default_course_mode import lms.lib.comment_client as cc from openedx.core.djangoapps.commerce.utils import ecommerce_api_client, ECOMMERCE_DATE_FORMAT from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -1090,7 +1091,7 @@ class CourseEnrollment(models.Model): ) @classmethod - def enroll(cls, user, course_key, mode=CourseMode.DEFAULT_MODE_SLUG, check_access=False): + def enroll(cls, user, course_key, mode=None, check_access=False): """ Enroll a user in a course. This saves immediately. @@ -1124,6 +1125,8 @@ class CourseEnrollment(models.Model): Also emits relevant events for analytics purposes. """ + if mode is None: + mode = _default_course_mode(unicode(course_key)) # All the server-side checks for whether a user is allowed to enroll. try: course = CourseOverview.get_from_id(course_key) @@ -1165,7 +1168,7 @@ class CourseEnrollment(models.Model): return enrollment @classmethod - def enroll_by_email(cls, email, course_id, mode=CourseMode.DEFAULT_MODE_SLUG, ignore_errors=True): + def enroll_by_email(cls, email, course_id, mode=None, ignore_errors=True): """ Enroll a user in a course given their email. This saves immediately. diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index e134964fa2..31f0f3ff25 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -119,7 +119,7 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal if CourseMode.is_white_label(course_id): course_mode = CourseMode.DEFAULT_SHOPPINGCART_MODE_SLUG else: - course_mode = CourseMode.DEFAULT_MODE_SLUG + course_mode = None if previous_state.enrollment: course_mode = previous_state.mode