Modify course registration flow for new login and registration pages

Now that we are using separate pages for login and registration rather than
modals, clicking the registration button for a course should direct an
unauthenticated user to the registration page, and the user should be enrolled
in the course upon successful registration. Likewise, an unauthenticated user
attempting to unenroll from a course should be directed to the login page and
subsequently unenrolled from the course upon successful login. The enrollment
change service also now uses HTTP status codes rather than JSON to communicate
status to the front end.

common/djangoapps/student/views.py

@@ -20,7 +20,7 @@ from django.core.mail import send_mail
 from django.core.urlresolvers import reverse
 from django.core.validators import validate_email, validate_slug, ValidationError
 from django.db import IntegrityError
-from django.http import HttpResponse, HttpResponseRedirect, Http404
+from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotAllowed, HttpResponseRedirect, Http404
 from django.shortcuts import redirect
 from django_future.csrf import ensure_csrf_cookie, csrf_exempt
 from django.utils.http import cookie_date
@@ -216,14 +216,21 @@ def signin_user(request):
     """
     This view will display the non-modal login form
     """
-    context = {}
+    context = {
+        'course_id': request.GET.get('course_id'),
+        'enrollment_action': request.GET.get('enrollment_action')
+    }
     return render_to_response('login.html', context)
 
+
 def register_user(request):
     """
     This view will display the non-modal registration form
     """
-    context = {}
+    context = {
+        'course_id': request.GET.get('course_id'),
+        'enrollment_action': request.GET.get('enrollment_action')
+    }
     return render_to_response('register.html', context)
 
 
@@ -290,35 +297,47 @@ def try_change_enrollment(request):
     """
     if 'enrollment_action' in request.POST:
         try:
-            enrollment_output = change_enrollment(request)
+            enrollment_response = change_enrollment(request)
             # There isn't really a way to display the results to the user, so we just log it
             # We expect the enrollment to be a success, and will show up on the dashboard anyway
-            log.info("Attempted to automatically enroll after login. Results: {0}".format(enrollment_output))
+            log.info(
+                "Attempted to automatically enroll after login. Response code: {0}; response body: {1}".format(
+                    enrollment_response.status_code,
+                    enrollment_response.content
+                )
+            )
         except Exception, e:
             log.exception("Exception automatically enrolling after login: {0}".format(str(e)))
 
 
-@login_required
-def change_enrollment_view(request):
-    """Delegate to change_enrollment to actually do the work."""
-    return HttpResponse(json.dumps(change_enrollment(request)))
-
-
-
 def change_enrollment(request):
+    """
+    Modify the enrollment status for the logged-in user.
+
+    The request parameter must be a POST request (other methods return 405)
+    that specifies course_id and enrollment_action parameters. If course_id or
+    enrollment_action is not specified, if course_id is not valid, if
+    enrollment_action is something other than "enroll" or "unenroll", if
+    enrollment_action is "enroll" and enrollment is closed for the course, or
+    if enrollment_action is "unenroll" and the user is not enrolled in the
+    course, a 400 error will be returned. If the user is not logged in, 403
+    will be returned; it is important that only this case return 403 so the
+    front end can redirect the user to a registration or login page when this
+    happens. This function should only be called from an AJAX request or
+    as a post-login/registration helper, so the error messages in the responses
+    should never actually be user-visible.
+    """
     if request.method != "POST":
-        raise Http404
+        return HttpResponseNotAllowed(["POST"])
 
     user = request.user
     if not user.is_authenticated():
-        raise Http404
+        return HttpResponseForbidden()
 
-    action = request.POST.get("enrollment_action", "")
-
-    course_id = request.POST.get("course_id", None)
+    action = request.POST.get("enrollment_action")
+    course_id = request.POST.get("course_id")
     if course_id is None:
-        return HttpResponse(json.dumps({'success': False,
-                                        'error': 'There was an error receiving the course id.'}))
+        return HttpResponseBadRequest("Course id not specified")
 
     if action == "enroll":
         # Make sure the course exists
@@ -328,12 +347,10 @@ def change_enrollment(request):
         except ItemNotFoundError:
             log.warning("User {0} tried to enroll in non-existent course {1}"
                       .format(user.username, course_id))
-            return {'success': False, 'error': 'The course requested does not exist.'}
+            return HttpResponseBadRequest("Course id is invalid")
 
         if not has_access(user, course, 'enroll'):
-            return {'success': False,
-                    'error': 'enrollment in {} not allowed at this time'
-                    .format(course.display_name_with_default)}
+            return HttpResponseBadRequest("Enrollment is closed")
 
         org, course_num, run = course_id.split("/")
         statsd.increment("common.student.enrollment",
@@ -347,7 +364,7 @@ def change_enrollment(request):
             # If we've already created this enrollment in a separate transaction,
             # then just continue
             pass
-        return {'success': True}
+        return HttpResponse()
 
     elif action == "unenroll":
         try:
@@ -360,13 +377,11 @@ def change_enrollment(request):
                                   "course:{0}".format(course_num),
                                   "run:{0}".format(run)])
 
-            return {'success': True}
+            return HttpResponse()
         except CourseEnrollment.DoesNotExist:
-            return {'success': False, 'error': 'You are not enrolled for this course.'}
+            return HttpResponseBadRequest("You are not enrolled in this course")
     else:
-        return {'success': False, 'error': 'Invalid enrollment_action.'}
-
-    return {'success': False, 'error': 'We weren\'t able to unenroll you. Please try again.'}
+        return HttpResponseBadRequest("Enrollment action is invalid")
 
 
 @ensure_csrf_cookie

lms/djangoapps/courseware/tests/tests.py

@@ -220,25 +220,20 @@ class LoginEnrollmentTestCase(TestCase):
         # Now make sure that the user is now actually activated
         self.assertTrue(get_user(email).is_active)
 
-    def _enroll(self, course):
-        """Post to the enrollment view, and return the parsed json response"""
+    def try_enroll(self, course):
+        """Try to enroll.  Return bool success instead of asserting it."""
         resp = self.client.post('/change_enrollment', {
             'enrollment_action': 'enroll',
             'course_id': course.id,
         })
-        return parse_json(resp)
-
-    def try_enroll(self, course):
-        """Try to enroll.  Return bool success instead of asserting it."""
-        data = self._enroll(course)
-        print ('Enrollment in %s result: %s'
-               % (course.location.url(), str(data)))
-        return data['success']
+        print ('Enrollment in %s result status code: %s'
+               % (course.location.url(), str(resp.status_code)))
+        return resp.status_code == 200
 
     def enroll(self, course):
         """Enroll the currently logged-in user, and check that it worked."""
-        data = self._enroll(course)
-        self.assertTrue(data['success'])
+        result = self.try_enroll(course)
+        self.assertTrue(result)
 
     def unenroll(self, course):
         """Unenroll the currently logged-in user, and check that it worked."""
@@ -246,8 +241,7 @@ class LoginEnrollmentTestCase(TestCase):
             'enrollment_action': 'unenroll',
             'course_id': course.id,
         })
-        data = parse_json(resp)
-        self.assertTrue(data['success'])
+        self.assertTrue(resp.status_code == 200)
 
     def check_for_get_code(self, code, url):
         """

lms/static/sass/multicourse/_course_about.scss

@@ -154,6 +154,15 @@
             @include transition();
             width: flex-grid(5, 8);
           }
+
+          #register_error {
+            background: $error-red;
+            border: 1px solid rgb(202, 17, 17);
+            color: rgb(143, 14, 14);
+            display: none;
+            padding: 12px;
+            margin-top: 5px;
+          }
         }
       }
 

lms/templates/courseware/course_about.html

@@ -13,42 +13,26 @@
 
 <%block name="js_extra">
 
-  % if not registered:
-    %if user.is_authenticated():
-      ## If the user is authenticated, clicking the enroll button just submits a form
-      <script type="text/javascript">
-      (function() {
-        $(".register").click(function() {
-          $("#class_enroll_form").submit();
-        });
-
-        $(document).delegate('#class_enroll_form', 'ajax:success', function(data, json, xhr) {
-          if(json.success) {
-            location.href="${reverse('dashboard')}";
-          }else{
-          $('#register_message').html('<p class="inline-error">' + json.error + "</p>");
-          }
-        });
-      })(this)
-      </script>
-    %else:
-      ## If the user is not authenticated, clicking the enroll button pops up the register
-      ## field. We also slip in the registration fields into the login/register fields so
-      ## the user is automatically registered after logging in / registering
-      <script type="text/javascript">
-      (function() {
-        $(".register").click(function() {
-          if ($(".login_form .enroll_fieldset").length === 0) {
-            $(".login_form").append( $(".enroll_fieldset").first().clone() );
-          }
-          if ($(".register_form .enroll_fieldset").length === 0) {
-            $(".register_form").append( $(".enroll_fieldset").first().clone() );
-          }
-        });
-      })(this)
-      </script>
-    %endif
-  %endif
+  <script type="text/javascript">
+  (function() {
+    $(".register").click(function(event) {
+      $("#class_enroll_form").submit();
+      event.preventDefault();
+    });
+
+    $('#class_enroll_form').on('ajax:complete', function(event, xhr) {
+      if(xhr.status == 200) {
+        location.href = "${reverse('dashboard')}";
+      } else if (xhr.status == 403) {
+        location.href = "${reverse('register_user')}?course_id=${course.id}&enrollment_action=enroll";
+      } else {
+        $('#register_error').html(
+            (xhr.responseText ? xhr.responseText : 'An error occurred. Please try again later.')
+        ).css("display", "block");
+      }
+    });
+  })(this)
+  </script>
 
   <script src="${static.url('js/course_info.js')}"></script>
 </%block>
@@ -66,8 +50,7 @@
         </hgroup>
 
         <div class="main-cta">
-        %if user.is_authenticated():
-            %if registered:
+        %if user.is_authenticated() and registered:
 	              %if show_courseware_link:
                     <a href="${course_target}">
                 %endif
@@ -76,16 +59,9 @@
                     <strong>View Courseware</strong>
                     </a>
                 %endif
-            %else:
-               <a href="#" class="register">Register for ${course.number}</a>
-	             <div id="register_message"></div>
-            %endif
         %else:
-            <a href="#signup-modal" class="register" rel="leanModal" data-notice='You must Sign Up
-% if not settings.MITX_FEATURES['DISABLE_LOGIN_BUTTON']:
-		     or <a href="#login-modal" rel="leanModal">Log In</a>
-% endif
-		     to enroll.'>Register for ${course.number}</a>
+            <a href="#" class="register">Register for ${course.number}</a>
+            <div id="register_error"></div>
         %endif
         </div>
 

lms/templates/dashboard.html

@@ -59,14 +59,16 @@
       $("#unenroll_course_number").text( $(event.target).data("course-number") );
     });
 
-    $(document).delegate('#unenroll_form', 'ajax:success', function(data, json, xhr) {
-      if(json.success) {
-        location.href="${reverse('dashboard')}";
+    $('#unenroll_form').on('ajax:complete', function(event, xhr) {
+      if(xhr.status == 200) {
+        location.href = "${reverse('dashboard')}";
+      } else if (xhr.status == 403) {
+        location.href = "${reverse('signin_user')}?course_id=" +
+          $("#unenroll_course_id").val() + "&enrollment_action=unenroll";
       } else {
-        if($('#unenroll_error').length == 0) {
-          $('#unenroll_form').prepend('<div id="unenroll_error" class="modal-form-error"></div>');
-        }
-        $('#unenroll_error').text(json.error).stop().css("display", "block");
+        $('#unenroll_error').html(
+          xhr.responseText ? xhr.responseText : "An error occurred. Please try again later."
+        ).stop().css("display", "block");
       }
     });
 
@@ -355,6 +357,8 @@
       <hr/>
     </header>
 
+    <div id="unenroll_error" class="modal-form-error"></div>
+
     <form id="unenroll_form" method="post" data-remote="true" action="${reverse('change_enrollment')}">
       <input name="course_id" id="unenroll_course_id" type="hidden" />
       <input name="enrollment_action" type="hidden" value="unenroll" />

lms/templates/login.html

@@ -126,6 +126,11 @@
         </ol>
       </fieldset>
 
+% if course_id and enrollment_action:
+      <input type="hidden" name="enrollment_action" value="${enrollment_action | h}" />
+      <input type="hidden" name="course_id" value="${course_id | h}" />
+% endif
+
       <div class="form-actions">
         <button name="submit" type="submit" id="submit" class="action action-primary action-update">Log into My edX Account <span class="orn-plus">+</span> Access My Courses</button>
       </div>

lms/templates/register.html

@@ -213,6 +213,11 @@
         </ol>
       </fieldset>
 
+% if course_id and enrollment_action:
+      <input type="hidden" name="enrollment_action" value="${enrollment_action | h}" />
+      <input type="hidden" name="course_id" value="${course_id | h}" />
+% endif
+
       <div class="form-actions">
         <button name="submit" type="submit" id="submit" class="action action-primary action-update">Register <span class="orn-plus">+</span> Create My Account</button>
       </div>

lms/urls.py

@@ -180,7 +180,7 @@ if settings.COURSEWARE_ENABLED:
 
         url(r'^courses/?$', 'branding.views.courses', name="courses"),
         url(r'^change_enrollment$',
-            'student.views.change_enrollment_view', name="change_enrollment"),
+            'student.views.change_enrollment', name="change_enrollment"),
 
         #About the course
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/about$',