Tests fixed

common/djangoapps/student/models.py

@@ -38,13 +38,16 @@ from importlib import import_module
 
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xmodule.modulestore import Location
-from xmodule.modulestore.django import modulestore
+from opaque_keys import InvalidKeyError
 
 import lms.lib.comment_client as cc
 from util.query import use_read_replica_if_available
 from xmodule_django.models import CourseKeyField, NoneToEmptyManager
+from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.django import modulestore
 from opaque_keys.edx.keys import CourseKey
 from functools import total_ordering
+from courseware.access import has_access
 
 from certificates.models import GeneratedCertificate
 from course_modes.models import CourseMode
@@ -818,31 +821,23 @@ class CourseEnrollment(models.Model):
 
         # All the server-side checks for whether a user is allowed to enroll.
         try:
-            course_id = SlashSeparatedCourseKey.from_deprecated_string(request.POST.get("course_id"))
-        except InvalidKeyError:
-            log.warning(
-                "User {username} tried to {action} with invalid course id: {course_id}".format(
-                    username=user.username,
-                    action=action,
-                    course_id=request.POST.get("course_id")
-                )
-            )
-            raise CourseEnrollmentException
-        try:
-            course = modulestore().get_course(course_id)
+            course = modulestore().get_course(course_key)
         except ItemNotFoundError:
             log.warning(
                 "User {0} failed to enroll in non-existent course {1}".format(
                     user.username,
-                    course_id
+                    course_key.to_deprecated_string()
                 )
             )
             raise NonExistentCourseError
+        if course is None:
+            raise NonExistentCourseError
+
         if not has_access(user, 'enroll', course):
             log.warning(
                 "User {0} failed to enroll in course {1} because enrollment is closed".format(
                     user.username,
-                    course_id
+                    course_key.to_deprecated_string()
                 )
             )
             raise EnrollmentClosedError
@@ -850,15 +845,15 @@ class CourseEnrollment(models.Model):
             log.warning(
                 "User {0} failed to enroll in full course {1}".format(
                     user.username,
-                    course_id
+                    course_key.to_deprecated_string()
                 )
             )
             raise CourseFullError
-        if CourseEnrollment.is_enrolled(user, course_id):
+        if CourseEnrollment.is_enrolled(user, course_key):
             log.warning(
                 "User {0} attempted to enroll in {1}, but they were already enrolled".format(
                     user.username,
-                    course_id
+                    course_key.to_deprecated_string()
                 )
             )
             raise AlreadyEnrolledError

common/djangoapps/student/roles.py

@@ -6,7 +6,7 @@ adding users, removing users, and listing members
 from abc import ABCMeta, abstractmethod
 
 from django.contrib.auth.models import User
-from student.models import CourseAccessRole
+
 from xmodule_django.models import CourseKeyField
 
 
@@ -15,6 +15,7 @@ class RoleCache(object):
     A cache of the CourseAccessRoles held by a particular user
     """
     def __init__(self, user):
+        from student.models import CourseAccessRole
         self._roles = set(
             CourseAccessRole.objects.filter(user=user).all()
         )
@@ -127,6 +128,7 @@ class RoleBase(AccessRole):
         """
         # silently ignores anonymous and inactive users so that any that are
         # legit get updated.
+        from student.models import CourseAccessRole
         for user in users:
             if user.is_authenticated and user.is_active and not self.has_user(user):
                 entry = CourseAccessRole(user=user, role=self._role_name, course_id=self.course_key, org=self.org)
@@ -138,6 +140,7 @@ class RoleBase(AccessRole):
         """
         Remove the supplied django users from this role.
         """
+        from student.models import CourseAccessRole
         entries = CourseAccessRole.objects.filter(
             user__in=users, role=self._role_name, org=self.org, course_id=self.course_key
         )
@@ -174,6 +177,7 @@ class CourseRole(RoleBase):
 
     @classmethod
     def course_group_already_exists(self, course_key):
+        from student.models import CourseAccessRole
         return CourseAccessRole.objects.filter(org=course_key.org, course_id=course_key).exists()
 
 
@@ -267,6 +271,7 @@ class UserBasedRole(object):
         """
         Grant this object's user the object's role for the supplied courses
         """
+        from student.models import CourseAccessRole
         if self.user.is_authenticated and self.user.is_active:
             for course_key in course_keys:
                 entry = CourseAccessRole(user=self.user, role=self.role, course_id=course_key, org=course_key.org)
@@ -280,6 +285,7 @@ class UserBasedRole(object):
         """
         Remove the supplied courses from this user's configured role.
         """
+        from student.models import CourseAccessRole
         entries = CourseAccessRole.objects.filter(user=self.user, role=self.role, course_id__in=course_keys)
         entries.delete()
         if hasattr(self.user, '_roles'):
@@ -294,4 +300,5 @@ class UserBasedRole(object):
         * course_id
         * role (will be self.role--thus uninteresting)
         """
+        from student.models import CourseAccessRole
         return CourseAccessRole.objects.filter(role=self.role, user=self.user)

common/djangoapps/student/views.py

@@ -669,6 +669,18 @@ def change_enrollment(request, auto_register=False):
     if 'course_id' not in request.POST:
         return HttpResponseBadRequest(_("Course id not specified"))
 
+    try:
+        course_id = SlashSeparatedCourseKey.from_deprecated_string(request.POST.get("course_id"))
+    except InvalidKeyError:
+        log.warning(
+            "User {username} tried to {action} with invalid course id: {course_id}".format(
+                username=user.username,
+                action=action,
+                course_id=request.POST.get("course_id")
+            )
+        )
+        return HttpResponseBadRequest(_("Invalid course id"))
+
     # Ensure the user is authenticated
     if not user.is_authenticated():
         return HttpResponseForbidden()
@@ -721,7 +733,7 @@ def change_enrollment(request, auto_register=False):
                 # for no such model to exist, even though we've set the enrollment type
                 # to "honor".
                 try:
-                    CourseEnrollment.enroll(user, course.id, mode=current_mode.slug)
+                    CourseEnrollment.enroll(user, course_id, mode=current_mode.slug)
                 except Exception:
                     return HttpResponseBadRequest(_("Could not enroll"))
 
@@ -757,7 +769,7 @@ def change_enrollment(request, auto_register=False):
                 )
 
             try:
-                CourseEnrollment.enroll(user, course.id, mode=current_mode.slug)
+                CourseEnrollment.enroll(user, course_id, mode=current_mode.slug)
             except Exception:
                 return HttpResponseBadRequest(_("Could not enroll"))
 

common/djangoapps/third_party_auth/pipeline.py

@@ -399,3 +399,10 @@ def login_analytics(*args, **kwargs):
                 }
             }
         )
+
+@partial.partial
+def change_enrollment(*args, **kwargs):
+    try:
+        CourseEnrollment.enroll(user, 'foo')
+    except:
+        pass

common/djangoapps/third_party_auth/settings.py

@@ -110,6 +110,7 @@ def _set_global_settings(django_settings):
         'social.pipeline.social_auth.load_extra_data',
         'social.pipeline.user.user_details',
         'third_party_auth.pipeline.login_analytics',
+        'third_party_auth.pipeline.change_enrollment',
     )
 
     # We let the user specify their email address during signup.

lms/djangoapps/courseware/access.py

@@ -14,11 +14,9 @@ from xmodule.x_module import XModule
 
 from xblock.core import XBlock
 
-from student.models import CourseEnrollmentAllowed
 from external_auth.models import ExternalAuthMap
 from courseware.masquerade import is_masquerading_as_student
 from django.utils.timezone import UTC
-from student.models import CourseEnrollment
 from student.roles import (
     GlobalStaff, CourseStaffRole, CourseInstructorRole,
     OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole
@@ -125,6 +123,7 @@ def _has_access_course_desc(user, action, course):
         """
         Can this user access the forums in this course?
         """
+        from student.models import CourseEnrollment
         return (
             can_load() and
             (
@@ -149,6 +148,7 @@ def _has_access_course_desc(user, action, course):
         """
 
         # if using registration method to restrict (say shibboleth)
+        from student.models import CourseEnrollmentAllowed
         if settings.FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain:
             if user is not None and user.is_authenticated() and \
                 ExternalAuthMap.objects.filter(user=user, external_domain=course.enrollment_domain):