Merge pull request #1471 from edx/dhm/bug_course_case

Require course ids to be case-insensitively unique

cms/djangoapps/auth/authz.py

@@ -195,7 +195,7 @@ def is_user_in_course_group_role(user, location, role, check_staff=True):
         # all "is_staff" flagged accounts belong to all groups
         if check_staff and user.is_staff:
             return True
-        return user.groups.filter(name=get_course_groupname_for_role(location, role)).count() > 0
+        return user.groups.filter(name=get_course_groupname_for_role(location, role)).exists()
 
     return False
 

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -1436,6 +1436,20 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.')
 
+    def test_create_course_case_change(self):
+        """Test new course creation - error path due to case insensitive name equality"""
+        self.course_data['number'] = 'capital'
+        self.client.post(reverse('create_new_course'), self.course_data)
+        cache_current = self.course_data['org']
+        self.course_data['org'] = self.course_data['org'].lower()
+        self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.')
+        self.course_data['org'] = cache_current
+
+        self.client.post(reverse('create_new_course'), self.course_data)
+        cache_current = self.course_data['number']
+        self.course_data['number'] = self.course_data['number'].upper()
+        self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.')
+
     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'

cms/djangoapps/contentstore/views/course.py

@@ -49,6 +49,8 @@ from student.models import CourseEnrollment
 
 from xmodule.html_module import AboutDescriptor
 from xmodule.modulestore.locator import BlockUsageLocator
+import re
+import bson
 __all__ = ['create_new_course', 'course_info', 'course_handler',
            'course_info_updates', 'get_course_settings',
            'course_config_graders_page',
@@ -197,15 +199,17 @@ def create_new_course(request):
                 'course number so that it is unique.'),
         })
 
-    course_search_location = [
-        'i4x',
-        dest_location.org,
-        dest_location.course,
-        'course',
-        None
-    ]
-    courses = modulestore().get_items(course_search_location)
-    if len(courses) > 0:
+    # dhm: this query breaks the abstraction, but I'll fix it when I do my suspended refactoring of this
+    # file for new locators. get_items should accept a query rather than requiring it be a legal location
+    course_search_location = bson.son.SON({
+        '_id.tag': 'i4x',
+        # cannot pass regex to Location constructor; thus this hack
+        '_id.org': re.compile(dest_location.org, re.IGNORECASE),
+        '_id.course': re.compile(dest_location.course, re.IGNORECASE),
+        '_id.category': 'course',
+    })
+    courses = modulestore().collection.find(course_search_location, fields=('_id'))
+    if courses.count() > 0:
         return JsonResponse({
             'ErrMsg': _('There is already a course defined with the same '
                 'organization and course number. Please '