Merge pull request #3145 from carsongee/cg/course_import_validation

Add test in import code to make sure two overlapping courses aren't imported

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

@@ -53,5 +53,5 @@ class Command(BaseCommand):
         for module in course_items:
             course_id = module.location.course_id
             if not are_permissions_roles_seeded(course_id):
-                self.stdout.write('Seeding forum roles for course {0}'.format(course_id))
+                self.stdout.write('Seeding forum roles for course {0}\n'.format(course_id))
                 seed_permissions_roles(course_id)

cms/djangoapps/contentstore/management/commands/tests/test_import.py

@@ -12,6 +12,7 @@ from django.test.utils import override_settings
 
 from contentstore.tests.modulestore_config import TEST_MODULESTORE
 from django_comment_common.utils import are_permissions_roles_seeded
+from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 
@@ -22,6 +23,7 @@ class TestImport(ModuleStoreTestCase):
     """
 
     COURSE_ID = ['EDx', '0.00x', '2013_Spring', ]
+    DIFF_RUN = ['EDx', '0.00x', '2014_Spring', ]
 
     def setUp(self):
         """
@@ -41,6 +43,16 @@ class TestImport(ModuleStoreTestCase):
         with open(os.path.join(self.good_dir, "course", "{0[2]}.xml".format(self.COURSE_ID)), "w+") as f:
             f.write('<course></course>')
 
+        # Create run changed course xml
+        self.dupe_dir = tempfile.mkdtemp(dir=self.content_dir)
+        os.makedirs(os.path.join(self.dupe_dir, "course"))
+        with open(os.path.join(self.dupe_dir, "course.xml"), "w+") as f:
+            f.write('<course url_name="{0[2]}" org="{0[0]}" '
+                    'course="{0[1]}"/>'.format(self.DIFF_RUN))
+
+        with open(os.path.join(self.dupe_dir, "course", "{0[2]}.xml".format(self.DIFF_RUN)), "w+") as f:
+            f.write('<course></course>')
+
     def test_forum_seed(self):
         """
         Tests that forum roles were created with import.
@@ -48,3 +60,18 @@ class TestImport(ModuleStoreTestCase):
         self.assertFalse(are_permissions_roles_seeded('/'.join(self.COURSE_ID)))
         call_command('import', self.content_dir, self.good_dir)
         self.assertTrue(are_permissions_roles_seeded('/'.join(self.COURSE_ID)))
+
+    def test_duplicate_with_url(self):
+        """
+        Check to make sure an import doesn't import courses that have the
+        same org and course, but they have different runs in order to
+        prevent modulestore "findone" exceptions on deletion
+        """
+        # Load up base course and verify it is available
+        call_command('import', self.content_dir, self.good_dir)
+        store = modulestore()
+        self.assertIsNotNone(store.get_course('/'.join(self.COURSE_ID)))
+
+        # Now load up duped course and verify it doesn't load
+        call_command('import', self.content_dir, self.dupe_dir)
+        self.assertIsNone(store.get_course('/'.join(self.DIFF_RUN)))

common/lib/xmodule/xmodule/modulestore/xml_importer.py

@@ -176,6 +176,32 @@ def import_from_xml(
                 if module.scope_ids.block_type == 'course':
                     course_data_path = path(data_dir) / module.data_dir
                     course_location = module.location
+                    course_prefix = u'{0.org}/{0.course}'.format(course_location)
+
+                    # Check to see if a course with the same
+                    # pseudo_course_id, but different run exists in
+                    # the passed store to avoid broken courses
+                    courses = store.get_courses()
+                    bad_run = False
+                    for course in courses:
+                        if course.location.course_id.startswith(course_prefix):
+                            log.debug('Import is overwriting existing course')
+                            # Importing over existing course, check
+                            # that runs match or fail
+                            if course.location.name != module.location.name:
+                                log.error(
+                                    'A course with ID %s exists, and this '
+                                    'course has the same organization and '
+                                    'course number, but a different term that '
+                                    'is fully identified as %s.',
+                                    course.location.course_id,
+                                    module.location.course_id
+                                )
+                                bad_run = True
+                                break
+                    if bad_run:
+                        # Skip this course, but keep trying to import courses
+                        continue
 
                     log.debug('======> IMPORTING course to location {loc}'.format(
                         loc=course_location