Merge remote-tracking branch 'origin/master' into fix/vik/sa-and-coe

.pylintrc

@@ -35,6 +35,7 @@ load-plugins=
 # it should appear only once).
 disable=
 # C0301: Line too long
+# C0302: Too many lines in module
 # W0141: Used builtin function 'map'
 # W0142: Used * or ** magic
 # R0201: Method could be a function
@@ -42,8 +43,11 @@ disable=
 # R0902: Too many instance attributes
 # R0903: Too few public methods (1/2)
 # R0904: Too many public methods
+# R0911: Too many return statements
+# R0912: Too many branches
 # R0913: Too many arguments
-    C0301,W0141,W0142,R0201,R0901,R0902,R0903,R0904,R0913
+# R0914: Too many local variables
+    C0301,C0302,W0141,W0142,R0201,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914
 
 
 [REPORTS]
@@ -92,7 +96,7 @@ zope=no
 # List of members which are set dynamically and missed by pylint inference
 # system, and so shouldn't trigger E0201 when accessed. Python regular
 # expressions are accepted.
-generated-members=REQUEST,acl_users,aq_parent,objects,DoesNotExist,can_read,can_write,get_url,size
+generated-members=REQUEST,acl_users,aq_parent,objects,DoesNotExist,can_read,can_write,get_url,size,content
 
 
 [BASIC]

apt-packages.txt

@@ -22,5 +22,4 @@ libreadline6
 libreadline6-dev
 mongodb
 nodejs
-npm
 coffeescript

cms/djangoapps/auth/authz.py

-import logging
-import sys
-
 from django.contrib.auth.models import User, Group
 from django.core.exceptions import PermissionDenied
 
@@ -131,7 +128,7 @@ def remove_user_from_course_group(caller, user, location, role):
         raise PermissionDenied
 
     # see if the user is actually in that role, if not then we don't have to do anything
-    if is_user_in_course_group_role(user, location, role) == True:
+    if is_user_in_course_group_role(user, location, role):
         groupname = get_course_groupname_for_role(location, role)
 
         group = Group.objects.get(name=groupname)

cms/djangoapps/contentstore/course_info_model.py

@@ -97,8 +97,7 @@ def update_course_updates(location, update, passed_id=None):
         if (len(new_html_parsed) == 1):
             content = new_html_parsed[0].tail
         else:
-            content = "\n".join([html.tostring(ele)
-                for ele in new_html_parsed[1:]])
+            content = "\n".join([html.tostring(ele) for ele in new_html_parsed[1:]])
 
         return {"id": passed_id,
                 "date": update['date'],

cms/djangoapps/contentstore/features/checklists.py

@@ -6,6 +6,7 @@ from nose.tools import assert_true, assert_equal
 from terrain.steps import reload_the_page
 from selenium.common.exceptions import StaleElementReferenceException
 
+
 ############### ACTIONS ####################
 @step('I select Checklists from the Tools menu$')
 def i_select_checklists(step):

cms/djangoapps/contentstore/management/commands/check_course.py

@@ -59,7 +59,7 @@ class Command(BaseCommand):
         discussion_items = _get_discussion_items(course)
 
         # now query all discussion items via get_items() and compare with the tree-traversal
-        queried_discussion_items = store.get_items(['i4x', course.location.org, course.location.course, 
+        queried_discussion_items = store.get_items(['i4x', course.location.org, course.location.course,
                                                     'discussion', None, None])
 
         for item in queried_discussion_items:

cms/djangoapps/contentstore/management/commands/clone.py

@@ -5,7 +5,6 @@ from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.store_utilities import clone_course
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore import Location
 from xmodule.course_module import CourseDescriptor
 
 from auth.authz import _copy_course_group
@@ -16,8 +15,7 @@ from auth.authz import _copy_course_group
 
 
 class Command(BaseCommand):
-    help = \
-'''Clone a MongoDB backed course to another location'''
+    help = 'Clone a MongoDB backed course to another location'
 
     def handle(self, *args, **options):
         if len(args) != 2:

cms/djangoapps/contentstore/management/commands/delete_course.py

@@ -5,7 +5,6 @@ from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore import Location
 from xmodule.course_module import CourseDescriptor
 from .prompt import query_yes_no
 
@@ -38,7 +37,7 @@ class Command(BaseCommand):
         if query_yes_no("Deleting course {0}. Confirm?".format(loc_str), default="no"):
             if query_yes_no("Are you sure. This action cannot be undone!", default="no"):
                 loc = CourseDescriptor.id_to_location(loc_str)
-                if delete_course(ms, cs, loc, commit) == True:
+                if delete_course(ms, cs, loc, commit):
                     print 'removing User permissions from course....'
                     # in the django layer, we need to remove all the user permissions groups associated with this course
                     if commit:

cms/djangoapps/contentstore/management/commands/export.py

@@ -7,7 +7,6 @@ from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
-from xmodule.modulestore import Location
 from xmodule.course_module import CourseDescriptor
 
 
@@ -15,8 +14,7 @@ unnamed_modules = 0
 
 
 class Command(BaseCommand):
-    help = \
-'''Import the specified data directory into the default ModuleStore'''
+    help = 'Import the specified data directory into the default ModuleStore'
 
     def handle(self, *args, **options):
         if len(args) != 2:

cms/djangoapps/contentstore/management/commands/import.py

@@ -12,8 +12,7 @@ unnamed_modules = 0
 
 
 class Command(BaseCommand):
-    help = \
-'''Import the specified data directory into the default ModuleStore'''
+    help = 'Import the specified data directory into the default ModuleStore'
 
     def handle(self, *args, **options):
         if len(args) == 0:
@@ -28,4 +27,4 @@ class Command(BaseCommand):
             data=data_dir,
             courses=course_dirs)
         import_from_xml(modulestore('direct'), data_dir, course_dirs, load_error_modules=False,
-            static_content_store=contentstore(), verbose=True)
+                        static_content_store=contentstore(), verbose=True)

cms/djangoapps/contentstore/management/commands/prompt.py

@@ -11,8 +11,8 @@ def query_yes_no(question, default="yes"):
 
     The "answer" return value is one of "yes" or "no".
     """
-    valid = {"yes":True,   "y":True,  "ye":True,
-             "no":False,     "n":False}
+    valid = {"yes": True,   "y": True,  "ye": True,
+             "no": False,     "n": False}
     if default is None:
         prompt = " [y/n] "
     elif default == "yes":
@@ -30,5 +30,4 @@ def query_yes_no(question, default="yes"):
         elif choice in valid:
             return valid[choice]
         else:
-            sys.stdout.write("Please respond with 'yes' or 'no' "\
-                             "(or 'y' or 'n').\n")
+            sys.stdout.write("Please respond with 'yes' or 'no' (or 'y' or 'n').\n")

cms/djangoapps/contentstore/management/commands/update_templates.py

 from xmodule.templates import update_templates
 from django.core.management.base import BaseCommand
 
+
 class Command(BaseCommand):
-    help = \
-'''Imports and updates the Studio component templates from the code pack and put in the DB'''
+    help = 'Imports and updates the Studio component templates from the code pack and put in the DB'
 
     def handle(self, *args, **options):
-        update_templates()
\ No newline at end of file
+        update_templates()

cms/djangoapps/contentstore/management/commands/xlint.py

 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_importer import perform_xlint
-from xmodule.modulestore.django import modulestore
-from xmodule.contentstore.django import contentstore
 
 
 unnamed_modules = 0
@@ -9,10 +7,11 @@ unnamed_modules = 0
 
 class Command(BaseCommand):
     help = \
-    '''
-    Verify the structure of courseware as to it's suitability for import
-    To run test: rake cms:xlint DATA_DIR=../data [COURSE_DIR=content-edx-101 (optional parameter)]
-    '''
+        '''
+        Verify the structure of courseware as to it's suitability for import
+        To run test: rake cms:xlint DATA_DIR=../data [COURSE_DIR=content-edx-101 (optional parameter)]
+        '''
+
     def handle(self, *args, **options):
         if len(args) == 0:
             raise CommandError("import requires at least one argument: <data directory> [<course dir>...]")

cms/djangoapps/contentstore/module_info_model.py

 from static_replace import replace_static_urls
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore import Location
-from django.http import Http404
 
 
 def get_module_info(store, location, parent_location=None, rewrite_static_links=False):

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -6,13 +6,14 @@ from django.conf import settings
 from django.core.urlresolvers import reverse
 from path import path
 from tempdir import mkdtemp_clean
-from datetime import timedelta
-import json
 from fs.osfs import OSFS
 import copy
 from json import loads
+import traceback
+from datetime import timedelta
 
 from django.contrib.auth.models import User
+from django.dispatch import Signal
 from contentstore.utils import get_modulestore
 
 from .utils import ModuleStoreTestCase, parse_json
@@ -37,6 +38,7 @@ TEST_DATA_MODULESTORE = copy.deepcopy(settings.MODULESTORE)
 TEST_DATA_MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data')
 TEST_DATA_MODULESTORE['direct']['OPTIONS']['fs_root'] = path('common/test/data')
 
+
 class MongoCollectionFindWrapper(object):
     def __init__(self, original):
         self.original = original
@@ -92,6 +94,33 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         return cnt
 
+    def test_get_items(self):
+        '''
+        This verifies a bug we had where the None setting in get_items() meant 'wildcard'
+        Unfortunately, None = published for the revision field, so get_items() would return
+        both draft and non-draft copies.
+        '''
+        store = modulestore()
+        draft_store = modulestore('draft')
+        import_from_xml(store, 'common/test/data/', ['simple'])
+
+        html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None])
+
+        draft_store.clone_item(html_module.location, html_module.location)
+
+        # now query get_items() to get this location with revision=None, this should just
+        # return back a single item (not 2)
+
+        items = store.get_items(['i4x', 'edX', 'simple', 'html', 'test_html', None])
+        self.assertEqual(len(items), 1)
+        self.assertFalse(getattr(items[0], 'is_draft', False))
+
+        # now refetch from the draft store. Note that even though we pass
+        # None in the revision field, the draft store will replace that with 'draft'
+        items = draft_store.get_items(['i4x', 'edX', 'simple', 'html', 'test_html', None])
+        self.assertEqual(len(items), 1)
+        self.assertTrue(getattr(items[0], 'is_draft', False))
+
     def test_draft_metadata(self):
         '''
         This verifies a bug we had where inherited metadata was getting written to the
@@ -158,32 +187,31 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
     def test_get_depth_with_drafts(self):
         import_from_xml(modulestore(), 'common/test/data/', ['simple'])
 
-        course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', 
-            'course', '2012_Fall', None]), depth=None)
+        course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
+                                                         'course', '2012_Fall', None]), depth=None)
 
         # make sure no draft items have been returned
         num_drafts = self._get_draft_counts(course)
         self.assertEqual(num_drafts, 0)
 
-        problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', 
-            'problem', 'ps01-simple', None]))
+        problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
+                                                          'problem', 'ps01-simple', None]))
 
         # put into draft
         modulestore('draft').clone_item(problem.location, problem.location)
 
         # make sure we can query that item and verify that it is a draft
-        draft_problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', 
-            'problem', 'ps01-simple', None]))
-        self.assertTrue(getattr(draft_problem,'is_draft', False))
+        draft_problem = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
+                                                                'problem', 'ps01-simple', None]))
+        self.assertTrue(getattr(draft_problem, 'is_draft', False))
 
         #now requery with depth
-        course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple', 
-            'course', '2012_Fall', None]), depth=None)
+        course = modulestore('draft').get_item(Location(['i4x', 'edX', 'simple',
+                                                         'course', '2012_Fall', None]), depth=None)
 
         # make sure just one draft item have been returned
         num_drafts = self._get_draft_counts(course)
-        self.assertEqual(num_drafts, 1)       
-
+        self.assertEqual(num_drafts, 1)
 
     def test_static_tab_reordering(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
@@ -215,13 +243,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         module_store = modulestore('direct')
         found = False
 
-        item = None
         items = module_store.get_items(['i4x', 'edX', 'full', 'poll_question', None, None])
         found = len(items) > 0
 
         self.assertTrue(found)
         # check that there's actually content in the 'question' field
-        self.assertGreater(len(items[0].question),0)
+        self.assertGreater(len(items[0].question), 0)
 
     def test_xlint_fails(self):
         err_cnt = perform_xlint('common/test/data', ['full'])
@@ -234,14 +261,15 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None]))
 
-        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
+        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter', 'Week_1', None]))
 
         # make sure the parent no longer points to the child object which was deleted
         self.assertTrue(sequential.location.url() in chapter.children)
 
         self.client.post(reverse('delete_item'),
-            json.dumps({'id': sequential.location.url(), 'delete_children': 'true', 'delete_all_versions': 'true'}),
-            "application/json")
+                         json.dumps({'id': sequential.location.url(), 'delete_children': 'true',
+                                     'delete_all_versions': 'true'}),
+                         "application/json")
 
         found = False
         try:
@@ -252,7 +280,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         self.assertFalse(found)
 
-        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
+        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter', 'Week_1', None]))
 
         # make sure the parent no longer points to the child object which was deleted
         self.assertFalse(sequential.location.url() in chapter.children)
@@ -275,7 +303,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
         module_store = modulestore('direct')
-        content_store = contentstore()
 
         source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
         course = module_store.get_item(source_location)
@@ -288,7 +315,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
-            }
+        }
 
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
@@ -347,17 +374,44 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
     def test_export_course(self):
         module_store = modulestore('direct')
+        draft_store = modulestore('draft')
         content_store = contentstore()
 
         import_from_xml(module_store, 'common/test/data/', ['full'])
         location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
 
+        # get a vertical (and components in it) to put into 'draft'
+        vertical = module_store.get_item(Location(['i4x', 'edX', 'full',
+                                         'vertical', 'vertical_66', None]), depth=1)
+
+        draft_store.clone_item(vertical.location, vertical.location)
+
+        for child in vertical.get_children():
+            draft_store.clone_item(child.location, child.location)
+
         root_dir = path(mkdtemp_clean())
 
+        # now create a private vertical
+        private_vertical = draft_store.clone_item(vertical.location,
+                                                  Location(['i4x', 'edX', 'full', 'vertical', 'a_private_vertical', None]))
+
+        # add private to list of children
+        sequential = module_store.get_item(Location(['i4x', 'edX', 'full',
+                                           'sequential', 'Administrivia_and_Circuit_Elements', None]))
+        private_location_no_draft = private_vertical.location._replace(revision=None)
+        module_store.update_children(sequential.location, sequential.children +
+                                     [private_location_no_draft.url()])
+
+        # read back the sequential, to make sure we have a pointer to
+        sequential = module_store.get_item(Location(['i4x', 'edX', 'full',
+                                                     'sequential', 'Administrivia_and_Circuit_Elements', None]))
+
+        self.assertIn(private_location_no_draft.url(), sequential.children)
+
         print 'Exporting to tempdir = {0}'.format(root_dir)
 
         # export out to a tempdir
-        export_to_xml(module_store, content_store, location, root_dir, 'test_export')
+        export_to_xml(module_store, content_store, location, root_dir, 'test_export', draft_modulestore=draft_store)
 
         # check for static tabs
         self.verify_content_existence(module_store, root_dir, location, 'tabs', 'static_tab', '.html')
@@ -391,20 +445,36 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         delete_course(module_store, content_store, location)
 
         # reimport
-        import_from_xml(module_store, root_dir, ['test_export'])
+        import_from_xml(module_store, root_dir, ['test_export'], draft_store=draft_store)
 
         items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
         self.assertGreater(len(items), 0)
         for descriptor in items:
-            print "Checking {0}....".format(descriptor.location.url())
-            resp = self.client.get(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
-            self.assertEqual(resp.status_code, 200)
+            # don't try to look at private verticals. Right now we're running
+            # the service in non-draft aware
+            if getattr(descriptor, 'is_draft', False):
+                print "Checking {0}....".format(descriptor.location.url())
+                resp = self.client.get(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
+                self.assertEqual(resp.status_code, 200)
+
+        # verify that we have the content in the draft store as well
+        vertical = draft_store.get_item(Location(['i4x', 'edX', 'full',
+                                                  'vertical', 'vertical_66', None]), depth=1)
+
+        self.assertTrue(getattr(vertical, 'is_draft', False))
+        for child in vertical.get_children():
+            self.assertTrue(getattr(child, 'is_draft', False))
+
+        # verify that we have the private vertical
+        test_private_vertical = draft_store.get_item(Location(['i4x', 'edX', 'full',
+                                                               'vertical', 'vertical_66', None]))
+
+        self.assertTrue(getattr(test_private_vertical, 'is_draft', False))
 
         shutil.rmtree(root_dir)
 
     def test_course_handouts_rewrites(self):
         module_store = modulestore('direct')
-        content_store = contentstore()
 
         # import a test course
         import_from_xml(module_store, 'common/test/data/', ['full'])
@@ -437,11 +507,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         # make sure we pre-fetched a known sequential which should be at depth=2
         self.assertTrue(Location(['i4x', 'edX', 'full', 'sequential',
-            'Administrivia_and_Circuit_Elements', None]) in course.system.module_data)
+                                  'Administrivia_and_Circuit_Elements', None]) in course.system.module_data)
 
         # make sure we don't have a specific vertical which should be at depth=3
-        self.assertFalse(Location(['i4x', 'edX', 'full', 'vertical', 'vertical_58',
-            None]) in course.system.module_data)
+        self.assertFalse(Location(['i4x', 'edX', 'full', 'vertical', 'vertical_58', None])
+                         in course.system.module_data)
 
     def test_export_course_with_unknown_metadata(self):
         module_store = modulestore('direct')
@@ -468,10 +538,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
             export_to_xml(module_store, content_store, location, root_dir, 'test_export')
             exported = True
         except Exception:
+            print 'Exception thrown: {0}'.format(traceback.format_exc())
             pass
 
         self.assertTrue(exported)
 
+
 class ContentStoreTest(ModuleStoreTestCase):
     """
     Tests for the CMS ContentStore application.
@@ -506,7 +578,7 @@ class ContentStoreTest(ModuleStoreTestCase):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
-            }
+        }
 
     def test_create_course(self):
         """Test new course creation - happy path"""
@@ -533,7 +605,7 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         self.assertEqual(resp.status_code, 200)
         self.assertEqual(data['ErrMsg'],
-            'There is already a course defined with the same organization and course number.')
+                         '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"""
@@ -543,16 +615,16 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         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'.")
+                         "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
         resp = self.client.get(reverse('index'))
         self.assertContains(resp,
-            '<h1 class="title-1">My Courses</h1>',
-            status_code=200,
-            html=True)
+                            '<h1 class="title-1">My Courses</h1>',
+                            status_code=200,
+                            html=True)
 
     def test_course_factory(self):
         """Test that the course factory works correctly."""
@@ -570,25 +642,25 @@ class ContentStoreTest(ModuleStoreTestCase):
         CourseFactory.create(display_name='Robot Super Educational Course')
         resp = self.client.get(reverse('index'))
         self.assertContains(resp,
-            '<span class="class-name">Robot Super Educational Course</span>',
-            status_code=200,
-            html=True)
+                            '<span class="class-name">Robot Super Educational Course</span>',
+                            status_code=200,
+                            html=True)
 
     def test_course_overview_view_with_course(self):
         """Test viewing the course overview page with an existing course"""
         CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
 
         data = {
-                'org': 'MITx',
-                'course': '999',
-                'name': Location.clean('Robot Super Course'),
-                }
+            'org': 'MITx',
+            'course': '999',
+            'name': Location.clean('Robot Super Course'),
+        }
 
         resp = self.client.get(reverse('course_index', kwargs=data))
         self.assertContains(resp,
-            '<article class="courseware-overview" data-course-id="i4x://MITx/999/course/Robot_Super_Course">',
-            status_code=200,
-            html=True)
+                            '<article class="courseware-overview" data-course-id="i4x://MITx/999/course/Robot_Super_Course">',
+                            status_code=200,
+                            html=True)
 
     def test_clone_item(self):
         """Test cloning an item. E.g. creating a new section"""
@@ -598,14 +670,13 @@ class ContentStoreTest(ModuleStoreTestCase):
             'parent_location': 'i4x://MITx/999/course/Robot_Super_Course',
             'template': 'i4x://edx/templates/chapter/Empty',
             'display_name': 'Section One',
-            }
+        }
 
         resp = self.client.post(reverse('clone_item'), section_data)
 
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
-        self.assertRegexpMatches(data['id'],
-            '^i4x:\/\/MITx\/999\/chapter\/([0-9]|[a-f]){32}$')
+        self.assertRegexpMatches(data['id'], '^i4x:\/\/MITx\/999\/chapter\/([0-9]|[a-f]){32}$')
 
     def test_capa_module(self):
         """Test that a problem treats markdown specially."""
@@ -614,7 +685,7 @@ class ContentStoreTest(ModuleStoreTestCase):
         problem_data = {
             'parent_location': 'i4x://MITx/999/course/Robot_Super_Course',
             'template': 'i4x://edx/templates/problem/Blank_Common_Problem'
-            }
+        }
 
         resp = self.client.post(reverse('clone_item'), problem_data)
 
@@ -748,6 +819,45 @@ class ContentStoreTest(ModuleStoreTestCase):
         # make sure we found the item (e.g. it didn't error while loading)
         self.assertTrue(did_load_item)
 
+    def test_forum_id_generation(self):
+        import_from_xml(modulestore(), 'common/test/data/', ['full'])
+        module_store = modulestore('direct')
+        new_component_location = Location('i4x', 'edX', 'full', 'discussion', 'new_component')
+        source_template_location = Location('i4x', 'edx', 'templates', 'discussion', 'Discussion_Tag')
+
+        # crate a new module and add it as a child to a vertical
+        module_store.clone_item(source_template_location, new_component_location)
+
+        new_discussion_item = module_store.get_item(new_component_location)
+
+        self.assertNotEquals(new_discussion_item.discussion_id, '$$GUID$$')
+
+    def test_update_modulestore_signal_did_fire(self):
+
+        import_from_xml(modulestore(), 'common/test/data/', ['full'])
+        module_store = modulestore('direct')
+
+        try:
+            module_store.modulestore_update_signal = Signal(providing_args=['modulestore', 'course_id', 'location'])
+
+            self.got_signal = False
+
+            def _signal_hander(modulestore=None, course_id=None, location=None, **kwargs):
+                self.got_signal = True
+
+            module_store.modulestore_update_signal.connect(_signal_hander)
+
+            new_component_location = Location('i4x', 'edX', 'full', 'html', 'new_component')
+            source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page')
+
+            # crate a new module
+            module_store.clone_item(source_template_location, new_component_location)
+
+        finally:
+            module_store.modulestore_update_signal = None
+
+        self.assertTrue(self.got_signal)
+
     def test_metadata_inheritance(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 

cms/djangoapps/contentstore/tests/test_core_caching.py

@@ -23,14 +23,14 @@ class CachingTestCase(TestCase):
     def test_put_and_get(self):
         set_cached_content(self.mockAsset)
         self.assertEqual(self.mockAsset.content, get_cached_content(self.unicodeLocation).content,
-            'should be stored in cache with unicodeLocation')
+                         'should be stored in cache with unicodeLocation')
         self.assertEqual(self.mockAsset.content, get_cached_content(self.nonUnicodeLocation).content,
-            'should be stored in cache with nonUnicodeLocation')
+                         'should be stored in cache with nonUnicodeLocation')
 
     def test_delete(self):
         set_cached_content(self.mockAsset)
         del_cached_content(self.nonUnicodeLocation)
         self.assertEqual(None, get_cached_content(self.unicodeLocation),
-            'should not be stored in cache with unicodeLocation')
+                         'should not be stored in cache with unicodeLocation')
         self.assertEqual(None, get_cached_content(self.nonUnicodeLocation),
-            'should not be stored in cache with nonUnicodeLocation')
+                         'should not be stored in cache with nonUnicodeLocation')

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -8,8 +8,7 @@ from django.core.urlresolvers import reverse
 from django.utils.timezone import UTC
 
 from xmodule.modulestore import Location
-from models.settings.course_details import (CourseDetails,
-                                                    CourseSettingsEncoder)
+from models.settings.course_details import (CourseDetails, CourseSettingsEncoder)
 from models.settings.course_grading import CourseGradingModel
 from contentstore.utils import get_modulestore
 
@@ -21,6 +20,7 @@ from xmodule.modulestore.xml_importer import import_from_xml
 from xmodule.modulestore.django import modulestore
 from xmodule.fields import Date
 
+
 class CourseTestCase(ModuleStoreTestCase):
     def setUp(self):
         """
@@ -87,16 +87,16 @@ class CourseDetailsTestCase(CourseTestCase):
         jsondetails.syllabus = "<a href='foo'>bar</a>"
         # encode - decode to convert date fields and other data which changes form
         self.assertEqual(CourseDetails.update_from_json(jsondetails.__dict__).syllabus,
-                             jsondetails.syllabus, "After set syllabus")
+                         jsondetails.syllabus, "After set syllabus")
         jsondetails.overview = "Overview"
         self.assertEqual(CourseDetails.update_from_json(jsondetails.__dict__).overview,
-                             jsondetails.overview, "After set overview")
+                         jsondetails.overview, "After set overview")
         jsondetails.intro_video = "intro_video"
         self.assertEqual(CourseDetails.update_from_json(jsondetails.__dict__).intro_video,
-                             jsondetails.intro_video, "After set intro_video")
+                         jsondetails.intro_video, "After set intro_video")
         jsondetails.effort = "effort"
         self.assertEqual(CourseDetails.update_from_json(jsondetails.__dict__).effort,
-                             jsondetails.effort, "After set effort")
+                         jsondetails.effort, "After set effort")
 
 
 class CourseDetailsViewTest(CourseTestCase):
@@ -151,8 +151,8 @@ class CourseDetailsViewTest(CourseTestCase):
     @staticmethod
     def struct_to_datetime(struct_time):
         return datetime.datetime(struct_time.tm_year, struct_time.tm_mon,
-            struct_time.tm_mday, struct_time.tm_hour,
-            struct_time.tm_min, struct_time.tm_sec, tzinfo=UTC())
+                                 struct_time.tm_mday, struct_time.tm_hour,
+                                 struct_time.tm_min, struct_time.tm_sec, tzinfo=UTC())
 
     def compare_date_fields(self, details, encoded, context, field):
         if details[field] is not None:
@@ -249,6 +249,7 @@ class CourseGradingTest(CourseTestCase):
         altered_grader = CourseGradingModel.update_grader_from_json(test_grader.course_location, test_grader.graders[1])
         self.assertDictEqual(test_grader.graders[1], altered_grader, "drop_count[1] + 2")
 
+
 class CourseMetadataEditingTest(CourseTestCase):
     def setUp(self):
         CourseTestCase.setUp(self)
@@ -256,7 +257,6 @@ class CourseMetadataEditingTest(CourseTestCase):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
         self.fullcourse_location = Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None])
 
-
     def test_fetch_initial_fields(self):
         test_model = CourseMetadata.fetch(self.course_location)
         self.assertIn('display_name', test_model, 'Missing editable metadata field')
@@ -272,17 +272,17 @@ class CourseMetadataEditingTest(CourseTestCase):
 
     def test_update_from_json(self):
         test_model = CourseMetadata.update_from_json(self.course_location,
-            { "advertised_start" : "start A",
-              "testcenter_info" : { "c" : "test" },
-              "days_early_for_beta" : 2})
+                                                     {"advertised_start": "start A",
+                                                      "testcenter_info": {"c": "test"},
+                                                      "days_early_for_beta": 2})
         self.update_check(test_model)
         # try fresh fetch to ensure persistence
         test_model = CourseMetadata.fetch(self.course_location)
         self.update_check(test_model)
         # now change some of the existing metadata
         test_model = CourseMetadata.update_from_json(self.course_location,
-            { "advertised_start" : "start B",
-              "display_name" : "jolly roger"})
+                                                     {"advertised_start": "start B",
+                                                      "display_name": "jolly roger"})
         self.assertIn('display_name', test_model, 'Missing editable metadata field')
         self.assertEqual(test_model['display_name'], 'jolly roger', "not expected value")
         self.assertIn('advertised_start', test_model, 'Missing revised advertised_start metadata field')
@@ -294,13 +294,12 @@ class CourseMetadataEditingTest(CourseTestCase):
         self.assertIn('advertised_start', test_model, 'Missing new advertised_start metadata field')
         self.assertEqual(test_model['advertised_start'], 'start A', "advertised_start not expected value")
         self.assertIn('testcenter_info', test_model, 'Missing testcenter_info metadata field')
-        self.assertDictEqual(test_model['testcenter_info'], { "c" : "test" }, "testcenter_info not expected value")
+        self.assertDictEqual(test_model['testcenter_info'], {"c": "test"}, "testcenter_info not expected value")
         self.assertIn('days_early_for_beta', test_model, 'Missing days_early_for_beta metadata field')
         self.assertEqual(test_model['days_early_for_beta'], 2, "days_early_for_beta not expected value")
 
-
     def test_delete_key(self):
-        test_model = CourseMetadata.delete_key(self.fullcourse_location, { 'deleteKeys' : ['doesnt_exist', 'showanswer', 'xqa_key']})
+        test_model = CourseMetadata.delete_key(self.fullcourse_location, {'deleteKeys': ['doesnt_exist', 'showanswer', 'xqa_key']})
         # ensure no harm
         self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in')
         self.assertIn('display_name', test_model, 'full missing editable metadata field')

cms/djangoapps/contentstore/tests/test_utils.py

@@ -69,4 +69,5 @@ class UrlReverseTestCase(ModuleStoreTestCase):
         self.assertEquals(
             'https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about',
             utils.get_url_reverse('https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about', course)
-        )
\ No newline at end of file
+        )
+        
\ No newline at end of file

cms/djangoapps/contentstore/tests/tests.py

-import json
-import shutil
 from django.test.client import Client
-from django.conf import settings
 from django.core.urlresolvers import reverse
-from path import path
-import json
-from fs.osfs import OSFS
-import copy
-
-from contentstore.utils import get_modulestore
-
-from xmodule.modulestore import Location
-from xmodule.modulestore.store_utilities import clone_course
-from xmodule.modulestore.store_utilities import delete_course
-from xmodule.modulestore.django import modulestore, _MODULESTORES
-from xmodule.contentstore.django import contentstore
-from xmodule.templates import update_templates
-from xmodule.modulestore.xml_exporter import export_to_xml
-from xmodule.modulestore.xml_importer import import_from_xml
-
-from xmodule.capa_module import CapaDescriptor
-from xmodule.course_module import CourseDescriptor
-from xmodule.seq_module import SequenceDescriptor
-
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+
 from .utils import ModuleStoreTestCase, parse_json, user, registration
 
 
@@ -84,6 +61,7 @@ class ContentStoreTestCase(ModuleStoreTestCase):
         # Now make sure that the user is now actually activated
         self.assertTrue(user(email).is_active)
 
+
 class AuthTestCase(ContentStoreTestCase):
     """Check that various permissions-related things work"""
 
@@ -101,9 +79,9 @@ class AuthTestCase(ContentStoreTestCase):
     def test_public_pages_load(self):
         """Make sure pages that don't require login load without error."""
         pages = (
-                 reverse('login'),
-                 reverse('signup'),
-                 )
+            reverse('login'),
+            reverse('signup'),
+        )
         for page in pages:
             print "Checking '{0}'".format(page)
             self.check_page_get(page, 200)
@@ -136,13 +114,13 @@ class AuthTestCase(ContentStoreTestCase):
         """Make sure pages that do require login work."""
         auth_pages = (
             reverse('index'),
-            )
+        )
 
         # These are pages that should just load when the user is logged in
         # (no data needed)
         simple_auth_pages = (
             reverse('index'),
-            )
+        )
 
         # need an activated user
         self.test_create_account()

cms/djangoapps/contentstore/utils.py

-import logging
 from django.conf import settings
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
@@ -9,7 +8,8 @@ import copy
 DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info']
 
 #In order to instantiate an open ended tab automatically, need to have this data
-OPEN_ENDED_PANEL = {"name" : "Open Ended Panel", "type" : "open_ended"}
+OPEN_ENDED_PANEL = {"name": "Open Ended Panel", "type": "open_ended"}
+
 
 def get_modulestore(location):
     """
@@ -86,11 +86,10 @@ def get_lms_link_for_item(location, preview=False, course_id=None):
 
     if settings.LMS_BASE is not None:
         if preview:
-            lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE',
-                'preview.' + settings.LMS_BASE)
+            lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE', 'preview.' + settings.LMS_BASE)
         else:
             lms_base = settings.LMS_BASE
-     
+
         lms_link = "//{lms_base}/courses/{course_id}/jump_to/{location}".format(
             lms_base=lms_base,
             course_id=course_id,
@@ -192,6 +191,7 @@ class CoursePageNames:
     CourseOutline = "course_index"
     Checklists = "checklists"
 
+
 def add_open_ended_panel_tab(course):
     """
     Used to add the open ended panel tab to a course if it does not exist.
@@ -208,6 +208,7 @@ def add_open_ended_panel_tab(course):
         changed = True
     return changed, course_tabs
 
+
 def remove_open_ended_panel_tab(course):
     """
     Used to remove the open ended panel tab from a course if it exists.
@@ -220,6 +221,6 @@ def remove_open_ended_panel_tab(course):
     #Check to see if open ended panel is defined in the course
     if OPEN_ENDED_PANEL in course_tabs:
         #Add panel to the tabs if it is not defined
-        course_tabs = [ct for ct in course_tabs if ct!=OPEN_ENDED_PANEL]
+        course_tabs = [ct for ct in course_tabs if ct != OPEN_ENDED_PANEL]
         changed = True
     return changed, course_tabs