Merge pull request #9432 from edx/shr/bug/PLAT-799-prevent-published-orphan-creation

Prevent publish orphans

common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py

@@ -198,6 +198,16 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
                 branches_to_delete = [ModuleStoreEnum.BranchName.published, ModuleStoreEnum.BranchName.draft]
             elif revision is None:
                 branches_to_delete = [ModuleStoreEnum.BranchName.draft]
+                draft_location = location.for_branch(ModuleStoreEnum.BranchName.draft)
+                try:
+                    item = self.get_item(draft_location)
+                    if getattr(item, 'has_children', False):
+                        # If item have has_published_version then delete published children also.
+                        if self.has_published_version(item):
+                            branches_to_delete.insert(0, ModuleStoreEnum.BranchName.published)
+                except ItemNotFoundError:
+                    # Raises ValueError as in function description
+                    raise ValueError("Cannot delete a block that does not exist")
             else:
                 raise UnsupportedRevisionError(
                     [

common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py

@@ -2465,6 +2465,102 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
                     # Verify that the signal was emitted
                     self.assertEqual(receiver.call_count, 1)
 
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_delete_published_item_creates_no_orphans(self, default_store):
+        """
+        Tests delete published item dont create any oprhans in course
+        """
+        self.initdb(default_store)
+        course_locator = self.course.id
+
+        chapter = self.store.create_child(
+            self.user_id, self.course.location, 'chapter', block_id='section_one'
+        )
+
+        sequential = self.store.create_child(
+            self.user_id, chapter.location, 'sequential', block_id='subsection_one'
+        )
+
+        vertical = self.store.create_child(
+            self.user_id, sequential.location, 'vertical', block_id='moon_unit'
+        )
+
+        problem = self.store.create_child(
+            self.user_id, vertical.location, 'problem', block_id='problem'
+        )
+
+        self.store.publish(chapter.location, self.user_id)
+        # Verify that there are no changes
+        self.assertFalse(self._has_changes(chapter.location))
+        self.assertFalse(self._has_changes(sequential.location))
+        self.assertFalse(self._has_changes(vertical.location))
+        self.assertFalse(self._has_changes(problem.location))
+
+        # No oprhans in course
+        course_orphans = self.store.get_orphans(course_locator)
+        self.assertEqual(len(course_orphans), 0)
+        self.store.delete_item(vertical.location, self.user_id)
+        # No oprhans in course after delete
+        course_orphans = self.store.get_orphans(course_locator)
+        self.assertEqual(len(course_orphans), 0)
+
+        if default_store == ModuleStoreEnum.Type.split:
+            course_locator_publish = course_locator.for_branch(ModuleStoreEnum.BranchName.published)
+            # No published oprhans after delete
+            course_publish_orphans = self.store.get_orphans(course_locator_publish)
+            self.assertEqual(len(course_publish_orphans), 0)
+
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_delete_draft_item_creates_no_orphans(self, default_store):
+        """
+        Tests delete draft item create no oprhans in course
+        """
+        self.initdb(default_store)
+        course_locator = self.course.id
+
+        chapter = self.store.create_child(
+            self.user_id, self.course.location, 'chapter', block_id='section_one'
+        )
+
+        sequential = self.store.create_child(
+            self.user_id, chapter.location, 'sequential', block_id='subsection_one'
+        )
+
+        vertical = self.store.create_child(
+            self.user_id, sequential.location, 'vertical', block_id='moon_unit'
+        )
+
+        problem = self.store.create_child(
+            self.user_id, vertical.location, 'problem', block_id='problem'
+        )
+
+        self.store.publish(chapter.location, self.user_id)
+        # Verify that there are no changes
+        self.assertFalse(self._has_changes(chapter.location))
+        self.assertFalse(self._has_changes(sequential.location))
+        self.assertFalse(self._has_changes(vertical.location))
+        self.assertFalse(self._has_changes(problem.location))
+
+        # No oprhans in course
+        course_orphans = self.store.get_orphans(course_locator)
+        self.assertEqual(len(course_orphans), 0)
+
+        problem.display_name = 'changed'
+        problem = self.store.update_item(problem, self.user_id)
+        self.assertTrue(self._has_changes(vertical.location))
+        self.assertTrue(self._has_changes(problem.location))
+
+        self.store.delete_item(vertical.location, self.user_id)
+        # No oprhans in course after delete
+        course_orphans = self.store.get_orphans(course_locator)
+        self.assertEqual(len(course_orphans), 0)
+
+        if default_store == ModuleStoreEnum.Type.split:
+            course_locator_publish = course_locator.for_branch(ModuleStoreEnum.BranchName.published)
+            # No published oprhans after delete
+            course_publish_orphans = self.store.get_orphans(course_locator_publish)
+            self.assertEqual(len(course_publish_orphans), 0)
+
 
 @ddt.ddt
 @attr('mongo')