diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 89e81d65292a018612f972bd54ab68fa62753d47..056b5141801c78a99d435ad91b2f9d9e6fc635e9 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -589,6 +589,22 @@ class LoginFailures(models.Model): return +class CourseEnrollmentException(Exception): + pass + +class NonExistentCourseError(CourseEnrollmentException): + pass + +class EnrollmentClosedError(CourseEnrollmentException): + pass + +class CourseFullError(CourseEnrollmentException): + pass + +class AlreadyEnrolledError(CourseEnrollmentException): + pass + + class CourseEnrollment(models.Model): """ Represents a Student's Enrollment record for a single Course. You should @@ -795,10 +811,59 @@ class CourseEnrollment(models.Model): until we have these mapped out. It is expected that this method is called from a method which has already - verified the user authentication and access. + verified the user authentication. Also emits relevant events for analytics purposes. """ + + # 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) + except ItemNotFoundError: + log.warning( + "User {0} failed to enroll in non-existent course {1}".format( + user.username, + course_id + ) + ) + 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 + ) + ) + raise EnrollmentClosedError + if CourseEnrollment.is_course_full(course): + log.warning( + "User {0} failed to enroll in full course {1}".format( + user.username, + course_id + ) + ) + raise CourseFullError + if CourseEnrollment.is_enrolled(user, course_id): + log.warning( + "User {0} attempted to enroll in {1}, but they were already enrolled".format( + user.username, + course_id + ) + ) + raise AlreadyEnrolledError + + # User is allowed to enroll if they've reached this point. enrollment = cls.get_or_create_enrollment(user, course_key) enrollment.update_enrollment(is_active=True, mode=mode) return enrollment diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 093f2f5360563658d80a317c980d8f50a3c65388..c43ae54f6f05d88254ab59a6b0a5c4d0097d9bb5 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -661,20 +661,19 @@ def change_enrollment(request, auto_register=False): Response """ + # Get the user user = request.user + # Ensure we received a course_id action = request.POST.get("enrollment_action") 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() + # Sets the auto_register flag, if that's desired # TODO (ECOM-16): Remove this once the auto-registration A/B test completes # If a user is in the experimental condition (auto-registration enabled), # immediately set a session flag so they stay in the experimental condition. @@ -686,6 +685,7 @@ def change_enrollment(request, auto_register=False): if request.session.get('auto_register') and not auto_register: auto_register = True + # Don't execute auto-register for the set of courses excluded from auto-registration # TODO (ECOM-16): Remove this once the auto-registration A/B test completes # We've agreed to exclude certain courses from the A/B test. If we find ourselves # registering for one of these courses, immediately switch to the control. @@ -694,34 +694,7 @@ def change_enrollment(request, auto_register=False): if 'auto_register' in request.session: del request.session['auto_register'] - if not user.is_authenticated(): - return HttpResponseForbidden() - if action == "enroll": - # Make sure the course exists - # We don't do this check on unenroll, or a bad course id can't be unenrolled from - try: - course = modulestore().get_course(course_id) - except ItemNotFoundError: - log.warning("User {0} tried to enroll in non-existent course {1}" - .format(user.username, course_id)) - return HttpResponseBadRequest(_("Course id is invalid")) - - if not has_access(user, 'enroll', course): - return HttpResponseBadRequest(_("Enrollment is closed")) - - # see if we have already filled up all allowed enrollments - is_course_full = CourseEnrollment.is_course_full(course) - - if is_course_full: - return HttpResponseBadRequest(_("Course is full")) - - # check to see if user is currently enrolled in that course - if CourseEnrollment.is_enrolled(user, course_id): - return HttpResponseBadRequest( - _("Student is already enrolled") - ) - # We use this flag to determine which condition of an AB-test # for auto-registration we're currently in. # (We have two URLs that both point to this view, but vary the @@ -747,7 +720,10 @@ def change_enrollment(request, auto_register=False): # by its slug. If they do, it's possible (based on the state of the database) # for no such model to exist, even though we've set the enrollment type # to "honor". - CourseEnrollment.enroll(user, course.id) + try: + CourseEnrollment.enroll(user, course.id, mode=current_mode.slug) + except Exception: + return HttpResponseBadRequest(_("Could not enroll")) # If we have more than one course mode or professional ed is enabled, # then send the user to the choose your track page. @@ -780,7 +756,10 @@ def change_enrollment(request, auto_register=False): reverse("course_modes_choose", kwargs={'course_id': unicode(course_id)}) ) - CourseEnrollment.enroll(user, course.id, mode=current_mode.slug) + try: + CourseEnrollment.enroll(user, course.id, mode=current_mode.slug) + except Exception: + return HttpResponseBadRequest(_("Could not enroll")) return HttpResponse()