diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index fc0fd7cf8e6360e1eaed40e044ec3aef341e5823..bc8c2aa8fa3d817795a4a473322d3995bc5859ef 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/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) diff --git a/cms/templates/signup.html b/cms/templates/signup.html index dd0f4a3aaf791a15f11a0b0f616dfa0132567841..84ba1750a1a7e306482dae96f5bd403d2afe4b8e 100644 --- a/cms/templates/signup.html +++ b/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> diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index a090eafa902d86584229f288e18055ca862233ca..b745e2da671cffac9e2846741bf035c9decf5b77 100644 --- a/common/djangoapps/student/views.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/test_registration_extra_vars.py b/lms/djangoapps/courseware/tests/test_registration_extra_vars.py index 9730d7b91ef22e2b194a32d746f0f00f3916267a..af8c8361a5d5f17df77726088fc341adccabb8e9 100644 --- a/lms/djangoapps/courseware/tests/test_registration_extra_vars.py +++ b/lms/djangoapps/courseware/tests/test_registration_extra_vars.py @@ -6,49 +6,51 @@ import json import uuid from django.conf import settings +from django.test import TestCase from django.core.urlresolvers import reverse from mock import patch -from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_post_code - -class TestExtraRegistrationVariables(LoginEnrollmentTestCase): +class TestExtraRegistrationVariables(TestCase): """ Test that extra registration variables are properly checked according to settings """ - - def _do_register_attempt(self, **extra_fields_values): - """ - Helper method to make the call to the do registration - """ + def setUp(self): + super(TestExtraRegistrationVariables, self).setUp() + self.url = reverse('create_account') username = 'foo_bar' + uuid.uuid4().hex - fields_values = { + self.url_params = { 'username': username, + 'name': username, 'email': 'foo' + uuid.uuid4().hex + '@bar.com', 'password': 'password', - 'name': username, 'terms_of_service': 'true', + 'honor_code': 'true', } - fields_values = dict(fields_values.items() + extra_fields_values.items()) - resp = check_for_post_code(self, 200, reverse('create_account'), fields_values) - data = json.loads(resp.content) - return data def test_default_missing_honor(self): """ By default, the honor code must be required """ - data = self._do_register_attempt(honor_code='') - self.assertEqual(data['success'], False) - self.assertEqual(data['value'], u'To enroll, you must follow the honor code.') + self.url_params['honor_code'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + u'To enroll, you must follow the honor code.', + ) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'honor_code': 'optional'}) def test_optional_honor(self): """ With the honor code is made optional, should pass without extra vars """ - data = self._do_register_attempt(honor_code='') - self.assertEqual(data['success'], True) + self.url_params['honor_code'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertEqual(obj['success'], True) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, { 'level_of_education': 'hidden', @@ -63,89 +65,164 @@ class TestExtraRegistrationVariables(LoginEnrollmentTestCase): """ When the fields are all hidden, should pass without extra vars """ - data = self._do_register_attempt() - self.assertEqual(data['success'], True) + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'city': 'required'}) def test_required_city_missing(self): """ Should require the city if configured as 'required' but missing """ - data = self._do_register_attempt(honor_code='true', city='') - self.assertEqual(data['success'], False) - self.assertEqual(data['value'], u'A city is required') + self.url_params['city'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + u'A city is required', + ) - data = self._do_register_attempt(honor_code='true', city='New York') - self.assertEqual(data['success'], True) + @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'city': 'required'}) + def test_required_city(self): + """ + Should require the city if configured as 'required' but missing + """ + self.url_params['city'] = 'New York' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'country': 'required'}) def test_required_country_missing(self): """ Should require the country if configured as 'required' but missing """ - data = self._do_register_attempt(honor_code='true', country='') - self.assertEqual(data['success'], False) - self.assertEqual(data['value'], u'A country is required') + self.url_params['country'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + u'A country is required', + ) - data = self._do_register_attempt(honor_code='true', country='New York') - self.assertEqual(data['success'], True) + @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'country': 'required'}) + def test_required_country(self): + self.url_params['country'] = 'New York' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'level_of_education': 'required'}) def test_required_level_of_education_missing(self): """ Should require the level_of_education if configured as 'required' but missing """ - data = self._do_register_attempt(honor_code='true', level_of_education='') - self.assertEqual(data['success'], False) - self.assertEqual(data['value'], u'A level of education is required.') + self.url_params['level_of_education'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + u'A level of education is required', + ) - data = self._do_register_attempt(honor_code='true', level_of_education='p') - self.assertEqual(data['success'], True) + @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'level_of_education': 'required'}) + def test_required_level_of_education(self): + self.url_params['level_of_education'] = 'p' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'gender': 'required'}) def test_required_gender_missing(self): """ Should require the gender if configured as 'required' but missing """ - data = self._do_register_attempt(honor_code='true', gender='') - self.assertEqual(data['success'], False) - self.assertEqual(data['value'], u'Your gender is required') + self.url_params['gender'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + u'Your gender is required', + ) - data = self._do_register_attempt(honor_code='true', gender='m') - self.assertEqual(data['success'], True) + @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'gender': 'required'}) + def test_required_gender(self): + self.url_params['gender'] = 'm' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'year_of_birth': 'required'}) def test_required_year_of_birth_missing(self): """ Should require the year_of_birth if configured as 'required' but missing """ - data = self._do_register_attempt(honor_code='true', year_of_birth='') - self.assertEqual(data['success'], False) - self.assertEqual(data['value'], u'Your year of birth is required') + self.url_params['year_of_birth'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + u'Your year of birth is required', + ) - data = self._do_register_attempt(honor_code='true', year_of_birth='1982') - self.assertEqual(data['success'], True) + @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'year_of_birth': 'required'}) + def test_required_year_of_birth(self): + self.url_params['year_of_birth'] = '1982' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'mailing_address': 'required'}) def test_required_mailing_address_missing(self): """ Should require the mailing_address if configured as 'required' but missing """ - data = self._do_register_attempt(honor_code='true', mailing_address='') - self.assertEqual(data['success'], False) - self.assertEqual(data['value'], u'Your mailing address is required') + self.url_params['mailing_address'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + u'Your mailing address is required', + ) - data = self._do_register_attempt(honor_code='true', mailing_address='my address') - self.assertEqual(data['success'], True) + @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'mailing_address': 'required'}) + def test_required_mailing_address(self): + self.url_params['mailing_address'] = 'my address' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'goals': 'required'}) def test_required_goals_missing(self): """ Should require the goals if configured as 'required' but missing """ - data = self._do_register_attempt(honor_code='true', goals='') - self.assertEqual(data['success'], False) - self.assertEqual(data['value'], u'A description of your goals is required') + self.url_params['goals'] = '' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 400) + obj = json.loads(response.content) + self.assertEqual( + obj['value'], + u'A description of your goals is required', + ) - data = self._do_register_attempt(honor_code='true', goals='my goals') - self.assertEqual(data['success'], True) + @patch.dict(settings.REGISTRATION_EXTRA_FIELDS, {'goals': 'required'}) + def test_required_goals(self): + self.url_params['goals'] = 'my goals' + response = self.client.post(self.url, self.url_params) + self.assertEqual(response.status_code, 200) + obj = json.loads(response.content) + self.assertTrue(obj['success']) diff --git a/lms/templates/register-shib.html b/lms/templates/register-shib.html index 5b5a64c1afb56939b2c61614eb2c708ec9bd2145..7724918cf5db48f8d0e7c01a6286dda57139eff1 100644 --- a/lms/templates/register-shib.html +++ b/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); diff --git a/lms/templates/register.html b/lms/templates/register.html index a04ed02f9754d92b73f86730c9bc438063e9cd5b..96c0409c56e60d82539fd8a18537b48d14012f04 100644 --- a/lms/templates/register.html +++ b/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); diff --git a/lms/templates/signup_modal.html b/lms/templates/signup_modal.html index 4f74523af7e67fe5bd7e66dc5ed06889092bbc7d..f80c700c7c3db3db246ab4b41a032e3d03031dd5 100644 --- a/lms/templates/signup_modal.html +++ b/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