From 6acae5b440f45b79625928666c5beac29c7fce38 Mon Sep 17 00:00:00 2001 From: Christina Roberts Date: Mon, 10 Dec 2012 15:19:34 -0500 Subject: [PATCH] Show error if organization name has invalid characters --- cms/djangoapps/contentstore/tests/tests.py | 31 ++++++++++++------- cms/djangoapps/contentstore/views.py | 7 +++-- .../xmodule/xmodule/modulestore/__init__.py | 2 +- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 9ddbe049ad..f5ce0b3692 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -1,7 +1,6 @@ import json from django.test import TestCase from django.test.client import Client -from mock import patch, Mock from override_settings import override_settings from django.conf import settings from django.core.urlresolvers import reverse @@ -9,9 +8,7 @@ from path import path from student.models import Registration from django.contrib.auth.models import User -from xmodule.modulestore.django import modulestore import xmodule.modulestore.django -from xmodule.modulestore import Location from xmodule.modulestore.xml_importer import import_from_xml import copy from factories import * @@ -23,33 +20,33 @@ def parse_json(response): def user(email): - '''look up a user by email''' + """look up a user by email""" return User.objects.get(email=email) def registration(email): - '''look up registration object by email''' + """look up registration object by email""" return Registration.objects.get(user__email=email) class ContentStoreTestCase(TestCase): def _login(self, email, pw): - '''Login. View should always return 200. The success/fail is in the - returned json''' + """Login. View should always return 200. The success/fail is in the + returned json""" resp = self.client.post(reverse('login_post'), {'email': email, 'password': pw}) self.assertEqual(resp.status_code, 200) return resp def login(self, email, pw): - '''Login, check that it worked.''' + """Login, check that it worked.""" resp = self._login(email, pw) data = parse_json(resp) self.assertTrue(data['success']) return resp def _create_account(self, username, email, pw): - '''Try to create an account. No error checking''' + """Try to create an account. No error checking""" resp = self.client.post('/create_account', { 'username': username, 'email': email, @@ -63,7 +60,7 @@ class ContentStoreTestCase(TestCase): return resp def create_account(self, username, email, pw): - '''Create the account and check that it worked''' + """Create the account and check that it worked""" resp = self._create_account(username, email, pw) self.assertEqual(resp.status_code, 200) data = parse_json(resp) @@ -75,8 +72,8 @@ class ContentStoreTestCase(TestCase): return resp def _activate_user(self, email): - '''Look up the activation key for the user, then hit the activate view. - No error checking''' + """Look up the activation key for the user, then hit the activate view. + No error checking""" activation_key = registration(email).activation_key # and now we try to activate @@ -257,6 +254,16 @@ class ContentStoreTest(TestCase): self.assertEqual(data['ErrMsg'], 'There is already a course defined with the same organization and course number.') + def test_create_course_with_bad_organization(self): + """Test new course creation - error path for bad organization name""" + self.course_data['org'] = 'University of California, Berkeley' + resp = self.client.post(reverse('create_new_course'), self.course_data) + data = parse_json(resp) + + self.assertEqual(resp.status_code, 200) + self.assertEqual(data['ErrMsg'], + "Unable to create course 'Robot Super Course'.\n\nInvalid characters in 'University of California, Berkeley'.") + def test_course_index_view_with_no_courses(self): """Test viewing the index page with no courses""" # Create a course so there is something to view diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 780a92035c..938dbc8285 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -23,7 +23,7 @@ from django.core.urlresolvers import reverse from django.conf import settings from xmodule.modulestore import Location -from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError from xmodule.x_module import ModuleSystem from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str @@ -1053,7 +1053,10 @@ def create_new_course(request): number = request.POST.get('number') display_name = request.POST.get('display_name') - dest_location = Location('i4x', org, number, 'course', Location.clean(display_name)) + try: + dest_location = Location('i4x', org, number, 'course', Location.clean(display_name)) + except InvalidLocationError as e: + return HttpResponse(json.dumps({'ErrMsg': "Unable to create course '" + display_name + "'.\n\n" + e.message})) # see if the course already exists existing_course = None diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 55236b4f8e..b9b3e399c1 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -153,7 +153,7 @@ class Location(_LocationBase): def check(val, regexp): if val is not None and regexp.search(val) is not None: log.debug('invalid characters val="%s", list_="%s"' % (val, list_)) - raise InvalidLocationError(location) + raise InvalidLocationError("Invalid characters in '%s'." % (val)) list_ = list(list_) for val in list_[:4] + [list_[5]]: