Merge pull request #2372 from edx/db/create-account-proper-status-code

Use status code 400 when there is a validation error in creating an account

cms/djangoapps/contentstore/tests/tests.py

@@ -111,7 +111,7 @@ class AuthTestCase(ContentStoreTestCase):
     def test_create_account_errors(self):
         # No post data -- should fail
         resp = self.client.post('/create_account', {})
-        self.assertEqual(resp.status_code, 200)
+        self.assertEqual(resp.status_code, 400)
         data = parse_json(resp)
         self.assertEqual(data['success'], False)
 

cms/templates/signup.html

@@ -107,31 +107,25 @@ require(["jquery", "jquery.cookie"], function($) {
     $("label").removeClass("is-focused");
   });
 
-  // form validation
-  function postJSON(url, data, callback) {
-    $.ajax({type:'POST',
-      url: url,
-      dataType: 'json',
-      data: data,
-      success: callback,
-      headers : {'X-CSRFToken': $.cookie('csrftoken')}
-    });
-  }
-
   $('form#register_form').submit(function(e) {
     e.preventDefault();
     var submit_data = $('#register_form').serialize();
 
-    postJSON('/create_account',
-      submit_data,
-      function(json) {
-        if(json.success) {
-          location.href = "${'/course'}";
-        } else {
-          $('#register_error').html(json.value).stop().addClass('is-shown');
-        }
-      }
-    );
+    $.ajax({
+      url: '/create_account',
+      type: 'POST',
+      dataType: 'json',
+      data: submit_data,
+      headers: {'X-CSRFToken': $.cookie('csrftoken')},
+      success: function(json) {
+        location.href = "/course";
+      },
+      error: function(jqXHR, textStatus, errorThrown) {
+        json = $.parseJSON(jqXHR.responseText);
+        $('#register_error').html(json.value).stop().addClass('is-shown');
+      },
+      notifyOnError: false
+    });
   });
 });
   </script>

common/djangoapps/student/views.py

@@ -897,13 +897,13 @@ def create_account(request, post_override=None):
         if a not in post_vars:
             js['value'] = _("Error (401 {field}). E-mail us.").format(field=a)
             js['field'] = a
-            return HttpResponse(json.dumps(js))
+            return JsonResponse(js, status=400)
 
     if extra_fields.get('honor_code', 'required') == 'required' and \
             post_vars.get('honor_code', 'false') != u'true':
         js['value'] = _("To enroll, you must follow the honor code.").format(field=a)
         js['field'] = 'honor_code'
-        return HttpResponse(json.dumps(js))
+        return JsonResponse(js, status=400)
 
     # Can't have terms of service for certain SHIB users, like at Stanford
     tos_required = (
@@ -919,7 +919,7 @@ def create_account(request, post_override=None):
         if post_vars.get('terms_of_service', 'false') != u'true':
             js['value'] = _("You must accept the terms of service.").format(field=a)
             js['field'] = 'terms_of_service'
-            return HttpResponse(json.dumps(js))
+            return JsonResponse(js, status=400)
 
     # Confirm appropriate fields are there.
     # TODO: Check e-mail format is correct.
@@ -941,13 +941,13 @@ def create_account(request, post_override=None):
 
         if len(post_vars[field_name]) < min_length:
             error_str = {
-                'username': _('Username must be minimum of two characters long.'),
-                'email': _('A properly formatted e-mail is required.'),
-                'name': _('Your legal name must be a minimum of two characters long.'),
-                'password': _('A valid password is required.'),
-                'terms_of_service': _('Accepting Terms of Service is required.'),
-                'honor_code': _('Agreeing to the Honor Code is required.'),
-                'level_of_education': _('A level of education is required.'),
+                'username': _('Username must be minimum of two characters long'),
+                'email': _('A properly formatted e-mail is required'),
+                'name': _('Your legal name must be a minimum of two characters long'),
+                'password': _('A valid password is required'),
+                'terms_of_service': _('Accepting Terms of Service is required'),
+                'honor_code': _('Agreeing to the Honor Code is required'),
+                'level_of_education': _('A level of education is required'),
                 'gender': _('Your gender is required'),
                 'year_of_birth': _('Your year of birth is required'),
                 'mailing_address': _('Your mailing address is required'),
@@ -957,21 +957,21 @@ def create_account(request, post_override=None):
             }
             js['value'] = error_str[field_name]
             js['field'] = field_name
-            return HttpResponse(json.dumps(js))
+            return JsonResponse(js, status=400)
 
     try:
         validate_email(post_vars['email'])
     except ValidationError:
         js['value'] = _("Valid e-mail is required.").format(field=a)
         js['field'] = 'email'
-        return HttpResponse(json.dumps(js))
+        return JsonResponse(js, status=400)
 
     try:
         validate_slug(post_vars['username'])
     except ValidationError:
         js['value'] = _("Username should only consist of A-Z and 0-9, with no spaces.").format(field=a)
         js['field'] = 'username'
-        return HttpResponse(json.dumps(js))
+        return JsonResponse(js, status=400)
 
     # Ok, looks like everything is legit.  Create the account.
     ret = _do_create_account(post_vars)
@@ -1007,7 +1007,10 @@ def create_account(request, post_override=None):
         except:
             log.warning('Unable to send activation email to user', exc_info=True)
             js['value'] = _('Could not send activation e-mail.')
-            return HttpResponse(json.dumps(js))
+            # What is the correct status code to use here? I think it's 500, because
+            # the problem is on the server's end -- but also, the account was created.
+            # Seems like the core part of the request was successful.
+            return JsonResponse(js, status=500)
 
     # Immediately after a user creates an account, we log them in. They are only
     # logged in until they close the browser. They can't log in again until they click
@@ -1034,14 +1037,12 @@ def create_account(request, post_override=None):
             login_user.save()
             AUDIT_LOG.info(u"Login activated on extauth account - {0} ({1})".format(login_user.username, login_user.email))
 
-    redirect_url = try_change_enrollment(request)
-
     dog_stats_api.increment("common.student.account_created")
 
-    response_params = {'success': True,
-                       'redirect_url': redirect_url}
-
-    response = HttpResponse(json.dumps(response_params))
+    response = JsonResponse({
+        'success': True,
+        'redirect_url': try_change_enrollment(request),
+    })
 
     # set the login cookie for the edx marketing site
     # we want this cookie to be accessed via javascript

lms/templates/register-shib.html

@@ -51,15 +51,17 @@
       });
 
       $('#register-form').on('ajax:success', function(event, json, xhr) {
-        if(json.success) {
-          location.href="${reverse('dashboard')}";
-       } else {
-         toggleSubmitButton(true);
-         $('.status.message.submission-error').addClass('is-shown').focus();
-         $('.status.message.submission-error .message-copy').html(json.value).stop().css("display", "block");
-         $(".field-error").removeClass('field-error');
-         $("[data-field='"+json.field+"']").addClass('field-error')
-       }
+        var url = json.redirect_url || "${reverse('dashboard')}";
+        location.href = url;
+      });
+
+      $('#register-form').on('ajax:error', function(event, jqXHR, textStatus) {
+        toggleSubmitButton(true);
+        json = $.parseJSON(jqXHR.responseText);
+        $('.status.message.submission-error').addClass('is-shown').focus();
+        $('.status.message.submission-error .message-copy').html(json.value).stop().css("display", "block");
+        $(".field-error").removeClass('field-error');
+        $("[data-field='"+json.field+"']").addClass('field-error')
       });
     })(this);
 

lms/templates/register.html

@@ -53,20 +53,17 @@
       });
 
       $('#register-form').on('ajax:success', function(event, json, xhr) {
-        if(json.success) {
-          if(json.redirect_url){
-            location.href=json.redirect_url;
-          }
-          else {
-            location.href="${reverse('dashboard')}";
-          }
-       } else {
-         toggleSubmitButton(true);
-         $('.status.message.submission-error').addClass('is-shown').focus();
-         $('.status.message.submission-error .message-copy').html(json.value).stop().css("display", "block");
-         $(".field-error").removeClass('field-error');
-         $("[data-field='"+json.field+"']").addClass('field-error')
-       }
+        var url = json.redirect_url || "${reverse('dashboard')}";
+        location.href = url;
+      });
+
+      $('#register-form').on('ajax:error', function(event, jqXHR, textStatus) {
+        toggleSubmitButton(true);
+        json = $.parseJSON(jqXHR.responseText);
+        $('.status.message.submission-error').addClass('is-shown').focus();
+        $('.status.message.submission-error .message-copy').html(json.value).stop().css("display", "block");
+        $(".field-error").removeClass('field-error');
+        $("[data-field='"+json.field+"']").addClass('field-error')
       });
     })(this);
 

lms/templates/signup_modal.html

@@ -152,13 +152,13 @@
 <script type="text/javascript">
   (function() {
    $(document).delegate('#register_form', 'ajax:success', function(data, json, xhr) {
-     if(json.success) {
-       location.href="${reverse('dashboard')}";
-     } else {
-       $(".field-error").removeClass('field-error');
-       $('#register_error').html(json.value).stop().css("display", "block");
-       $("[data-field='"+json.field+"']").addClass('field-error')
-     }
+     location.href="${reverse('dashboard')}";
+    });
+   $(document).delegate('#register_form', 'ajax:error', function(event, jqXHR, textStatus) {
+     json = $.parseJSON(jqXHR.responseText);
+     $(".field-error").removeClass('field-error');
+     $('#register_error').html(json.value).stop().css("display", "block");
+     $("[data-field='"+json.field+"']").addClass('field-error')
     });
 
    // removing close link's default behavior