Rationalize all cases around direct-only categories (both as blocks and parents) and delete

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

@@ -3036,9 +3036,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         Delete the orphan and any of its descendants which no longer have parents.
         """
         if len(self._get_parents_from_structure(orphan, structure)) == 0:
-            for child in structure['blocks'][orphan].fields.get('children', []):
+            orphan_data = structure['blocks'].pop(orphan)
+            for child in orphan_data.fields.get('children', []):
                 self._delete_if_true_orphan(BlockKey(*child), structure)
-            del structure['blocks'][orphan]
 
     @contract(returns=BlockData)
     def _new_block(self, user_id, category, block_fields, definition_id, new_id, raw=False, block_defaults=None):

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

@@ -189,42 +189,41 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli
                     currently only provided by contentstore.views.item.orphan_handler
                 Otherwise, raises a ValueError.
         """
+        allowed_revisions = [
+            None,
+            ModuleStoreEnum.RevisionOption.published_only,
+            ModuleStoreEnum.RevisionOption.all
+        ]
+        if revision not in allowed_revisions:
+            raise UnsupportedRevisionError(allowed_revisions)
+
+        autopublish_parent = False
         with self.bulk_operations(location.course_key):
             if isinstance(location, LibraryUsageLocator):
                 branches_to_delete = [ModuleStoreEnum.BranchName.library]  # Libraries don't yet have draft/publish support
-            elif revision == ModuleStoreEnum.RevisionOption.published_only:
-                branches_to_delete = [ModuleStoreEnum.BranchName.published]
+            elif location.category in DIRECT_ONLY_CATEGORIES:
+                branches_to_delete = [ModuleStoreEnum.BranchName.published, ModuleStoreEnum.BranchName.draft]
             elif revision == ModuleStoreEnum.RevisionOption.all:
                 branches_to_delete = [ModuleStoreEnum.BranchName.published, ModuleStoreEnum.BranchName.draft]
-            elif revision is None:
-                branches_to_delete = [ModuleStoreEnum.BranchName.draft]
-
-                if location.category in DIRECT_ONLY_CATEGORIES:
-                    branches_to_delete.append(ModuleStoreEnum.BranchName.published)
             else:
-                raise UnsupportedRevisionError(
-                    [
-                        None,
-                        ModuleStoreEnum.RevisionOption.published_only,
-                        ModuleStoreEnum.RevisionOption.all
-                    ]
-                )
+                if revision == ModuleStoreEnum.RevisionOption.published_only:
+                    branches_to_delete = [ModuleStoreEnum.BranchName.published]
+                elif revision is None:
+                    branches_to_delete = [ModuleStoreEnum.BranchName.draft]
+                    parent_loc = self.get_parent_location(location.for_branch(ModuleStoreEnum.BranchName.draft))
+                    autopublish_parent = (
+                        not skip_auto_publish and
+                        parent_loc is not None and
+                        parent_loc.block_type in DIRECT_ONLY_CATEGORIES
+                    )
 
             self._flag_publish_event(location.course_key)
             for branch in branches_to_delete:
                 branched_location = location.for_branch(branch)
-                parent_loc = self.get_parent_location(branched_location)
                 super(DraftVersioningModuleStore, self).delete_item(branched_location, user_id)
-                # publish parent w/o child if deleted element is direct only (not based on type of parent)
-                # publish vertical to behave more like the old mongo/draft modulestore - TNL-2593
-                if (
-                        branch == ModuleStoreEnum.BranchName.draft and
-                        branched_location.block_type in (DIRECT_ONLY_CATEGORIES + ['vertical']) and
-                        parent_loc and
-                        not skip_auto_publish
-                ):
-                    # will publish if its not an orphan
-                    self.publish(parent_loc.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs)
+
+            if autopublish_parent:
+                self.publish(parent_loc.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs)
 
     def _map_revision_to_branch(self, key, revision=None):
         """

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

@@ -1122,20 +1122,7 @@ class ElementalDeleteItemTests(DraftPublishedOpBaseTestSetup):
             self.assertOLXIsPublishedOnly(block_list_to_delete)
             self.delete_item(block_list_to_delete, revision=revision)
             self._check_for_item_deletion(block_list_to_delete, result)
-            # MODULESTORE_DIFFERENCE
-            if self.is_split_modulestore:
-                # Split:
-                if revision == ModuleStoreEnum.RevisionOption.published_only:
-                    # If deleting published_only items, the children that are drafts remain.
-                    self.assertOLXIsDraftOnly(block_list_children)
-                else:
-                    self.assertOLXIsDeleted(block_list_children)
-            elif self.is_old_mongo_modulestore:
-                # Old Mongo:
-                # If deleting draft_only or both items, the drafts will be deleted.
-                self.assertOLXIsDeleted(block_list_children)
-            else:
-                raise Exception("Must test either Old Mongo or Split modulestore!")
+            self.assertOLXIsDeleted(block_list_children)
 
     @ddt.data(*itertools.product(
         MODULESTORE_SETUPS,
@@ -1176,20 +1163,7 @@ class ElementalDeleteItemTests(DraftPublishedOpBaseTestSetup):
             self.delete_item(block_list_to_delete, revision=revision)
             self._check_for_item_deletion(block_list_to_delete, result)
             self.assertOLXIsDeleted(autopublished_children)
-            # MODULESTORE_DIFFERENCE
-            if self.is_split_modulestore:
-                # Split:
-                if revision == ModuleStoreEnum.RevisionOption.published_only:
-                    # If deleting published_only items, the children that are drafts remain.
-                    self.assertOLXIsDraftOnly(block_list_draft_children)
-                else:
-                    self.assertOLXIsDeleted(block_list_draft_children)
-            elif self.is_old_mongo_modulestore:
-                # Old Mongo:
-                # If deleting draft_only or both items, the drafts will be deleted.
-                self.assertOLXIsDeleted(block_list_draft_children)
-            else:
-                raise Exception("Must test either Old Mongo or Split modulestore!")
+            self.assertOLXIsDeleted(block_list_draft_children)
 
 
 @ddt.ddt