Merge pull request #1120 from MITx/feature/christina/misc

Bug fix: Show error if organization name has invalid characters

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

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

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]]: