diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index bb410105802f..724886621d88 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -53,5 +53,5 @@ def handle(self, *args, **options): 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) diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_import.py b/cms/djangoapps/contentstore/management/commands/tests/test_import.py index 3a3cac70d11e..055d132f1201 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_import.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_import.py @@ -12,6 +12,7 @@ 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 @@ def setUp(self): with open(os.path.join(self.good_dir, "course", "{0[2]}.xml".format(self.COURSE_ID)), "w+") as f: f.write('') + # 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(''.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('') + def test_forum_seed(self): """ Tests that forum roles were created with import. @@ -48,3 +60,18 @@ def test_forum_seed(self): 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))) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index b9dd91d09e79..cd4a694d2ed9 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/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