From f30ad29b213719a63a81804b2a3130591d71aea7 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Fri, 21 Nov 2014 17:54:27 -0500 Subject: [PATCH] Imports must replace drafts w/ published in split PLAT-299 --- .../modulestore/split_mongo/split_draft.py | 20 +++----- .../tests/test_mixed_modulestore.py | 50 ++++++++++++++++++- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index dbf3914de8..1ee64f3f05 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -416,20 +416,12 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli new_usage_key = course_key.make_usage_key(block_type, block_id) if self.get_branch_setting() == ModuleStoreEnum.Branch.published_only: - # if importing a direct only, override existing draft - if block_type in DIRECT_ONLY_CATEGORIES: - draft_course = course_key.for_branch(ModuleStoreEnum.BranchName.draft) - with self.branch_setting(ModuleStoreEnum.Branch.draft_preferred, draft_course): - draft = self.import_xblock(user_id, draft_course, block_type, block_id, fields, runtime) - self._auto_publish_no_children(draft.location, block_type, user_id) - return self.get_item(new_usage_key.for_branch(ModuleStoreEnum.BranchName.published)) - # check whether it's new to draft (PLAT_297, need to check even if not new to pub) - elif not self.has_item(new_usage_key.for_branch(ModuleStoreEnum.BranchName.draft)): - # add to draft too - draft_course = course_key.for_branch(ModuleStoreEnum.BranchName.draft) - with self.branch_setting(ModuleStoreEnum.Branch.draft_preferred, draft_course): - draft = self.import_xblock(user_id, draft_course, block_type, block_id, fields, runtime) - return self.publish(draft.location, user_id, blacklist=EXCLUDE_ALL) + # override existing draft (PLAT-297, PLAT-299). NOTE: this has the effect of removing + # any local changes w/ the import. + draft_course = course_key.for_branch(ModuleStoreEnum.BranchName.draft) + with self.branch_setting(ModuleStoreEnum.Branch.draft_preferred, draft_course): + draft_block = self.import_xblock(user_id, draft_course, block_type, block_id, fields, runtime) + return self.publish(draft_block.location.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs) # do the import partitioned_fields = self.partition_fields_by_scope(block_type, fields) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 252c701b3a..87e1dc34d7 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -23,6 +23,7 @@ from xmodule.contentstore.content import StaticContent import mimetypes from opaque_keys.edx.keys import CourseKey from xmodule.modulestore.xml_importer import import_from_xml +from nose import SkipTest if not settings.configured: settings.configure() @@ -1873,7 +1874,7 @@ class TestMixedModuleStore(CourseComparisonTest): self.assertCoursesEqual(source_store, source_course_key, dest_store, dest_course_id) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_import_edit_import(self, default): + def test_import_delete_import(self, default): """ Test that deleting an element after import and then re-importing restores that element in draft as well as published branches (PLAT_297) @@ -1918,6 +1919,53 @@ class TestMixedModuleStore(CourseComparisonTest): with self.store.branch_setting(ModuleStoreEnum.Branch.published_only, course_id): self.assertTrue(self.store.has_item(vertical_loc)) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_import_edit_import(self, default): + """ + Test that editing an element after import and then re-importing resets the draft and pub'd + to the imported pub'd value (PLAT-299) + """ + if default == ModuleStoreEnum.Type.mongo: + raise SkipTest + # set the default modulestore + with MongoContentstoreBuilder().build() as contentstore: + self.store = MixedModuleStore( + contentstore=contentstore, + create_modulestore_instance=create_modulestore_instance, + mappings={}, + **self.OPTIONS + ) + self.addCleanup(self.store.close_all_connections) + with self.store.default_store(default): + dest_course_key = self.store.make_course_key('a', 'course', 'course') + courses = import_from_xml( + self.store, self.user_id, DATA_DIR, ['toy'], load_error_modules=False, + static_content_store=contentstore, + target_course_id=dest_course_key, + create_new_course_if_not_present=True, + ) + course_id = courses[0].id + # no need to verify course content here as test_cross_modulestore_import_export does that + # delete the vertical + vertical_loc = course_id.make_usage_key('vertical', 'vertical_test') + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course_id): + vertical = self.store.get_item(vertical_loc) + vertical.display_name = "4" + self.store.update_item(vertical, self.user_id) + + # now re-import + import_from_xml( + self.store, self.user_id, DATA_DIR, ['toy'], load_error_modules=False, + static_content_store=contentstore, + target_course_id=dest_course_key, + ) + # verify it's the same in both published and draft (toy has no drafts) + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course_id): + draft_vertical = self.store.get_item(vertical_loc) + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only, course_id): + published_vertical = self.store.get_item(vertical_loc) + self.assertEqual(draft_vertical.display_name, published_vertical.display_name) + # ============================================================================================================ # General utils for not using django settings # ============================================================================================================