Error on import into split

if import restores a deleted draft element which was never published
PLAT_297

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
 

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)

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