diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index 3cf2b558e6..91dc1f8d3c 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -31,8 +31,6 @@ from xmodule.modulestore.xml_exporter import export_to_xml from .access import has_course_access from extract_tar import safetar_extractall -from student import auth -from student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff from util.json_request import JsonResponse from util.views import ensure_valid_course_key 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 6c277f5618..dbf3914de8 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -172,7 +172,9 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli branched_location = location.for_branch(branch) parent_loc = self.get_parent_location(branched_location) SplitMongoModuleStore.delete_item(self, branched_location, user_id) - self._auto_publish_no_children(parent_loc, parent_loc.category, user_id, **kwargs) + # publish parent w/o child if deleted element is direct only (not based on type of parent) + if branch == ModuleStoreEnum.BranchName.draft and branched_location.block_type in DIRECT_ONLY_CATEGORIES: + self.publish(parent_loc.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs) def _map_revision_to_branch(self, key, revision=None): """ @@ -421,15 +423,13 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli 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)) - # if new to published - elif not self.has_item(new_usage_key.for_branch(ModuleStoreEnum.BranchName.published)): - # check whether it's new to draft - if 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) + # 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) # 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 16fbfb15a8..252c701b3a 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -22,6 +22,7 @@ from xmodule.modulestore.tests.test_cross_modulestore_import_export import Mongo from xmodule.contentstore.content import StaticContent import mimetypes from opaque_keys.edx.keys import CourseKey +from xmodule.modulestore.xml_importer import import_from_xml if not settings.configured: settings.configure() @@ -718,7 +719,7 @@ class TestMixedModuleStore(CourseComparisonTest): # Split: # queries: active_versions, draft and published structures, definition (unnecessary) # sends: update published (why?), draft, and active_versions - @ddt.data(('draft', 8, 2), ('split', 4, 3)) + @ddt.data(('draft', 8, 2), ('split', 2, 2)) @ddt.unpack def test_delete_private_vertical(self, default_ms, max_find, max_send): """ @@ -1871,6 +1872,51 @@ class TestMixedModuleStore(CourseComparisonTest): dest_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.split) 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): + """ + Test that deleting an element after import and then re-importing restores that element in draft + as well as published branches (PLAT_297) + """ + # 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') + self.assertTrue(self.store.has_item(vertical_loc)) + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course_id): + self.store.delete_item(vertical_loc, self.user_id) + # verify it's in the published still + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only, course_id): + self.assertTrue(self.store.has_item(vertical_loc)) + + # 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 in both published and draft + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course_id): + self.assertTrue(self.store.has_item(vertical_loc)) + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only, course_id): + self.assertTrue(self.store.has_item(vertical_loc)) # ============================================================================================================ # General utils for not using django settings