Merge pull request #4315 from edx/bug/delete_draft

Deleting a child of a unit should convert the unit to draft

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -409,6 +409,14 @@ class DraftModuleStore(MongoModuleStore):
 
         # remove subtree from its parent
         parent_locations = self._get_raw_parent_locations(location, key_revision=parent_revision)
+        # if no parents, then we're trying to delete something which we should convert to draft
+        if not parent_locations:
+            # find the published parent, convert it to draft, then manipulate the draft
+            parent_locations = self._get_raw_parent_locations(location, key_revision=MongoRevisionKey.published)
+            # parent_locations will still be empty if the object was an orphan
+            if parent_locations:
+                draft_parent = self.convert_to_draft(parent_locations[0], user_id)
+                parent_locations = [draft_parent.location]
         # there could be 2 parents if
         #   Case 1: the draft item moved from one parent to another
         #   Case 2: revision==ModuleStoreEnum.RevisionOption.all and the single parent has 2 versions: draft and published
@@ -561,7 +569,7 @@ class DraftModuleStore(MongoModuleStore):
                             if published_parent == item_location:
                                 # Case 1: child was deleted in draft parent item
                                 # So, delete published version of the child now that we're publishing the draft parent
-                                self._delete_subtree(item_location, [as_published])
+                                self._delete_subtree(orig_child, [as_published])
                             else:
                                 # Case 2: child was moved to a new draft parent item
                                 # So, do not delete the child.  It will be published when the new parent is published.

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

@@ -63,8 +63,8 @@ class TestPublish(SplitWMongoCourseBoostrapper):
         To reproduce a bug (STUD-811) publish a vertical, convert to draft, delete a child, move a child, publish.
         See if deleted and moved children still is connected or exists in db (bug was disconnected but existed)
         """
-        location = self.old_course_key.make_usage_key('vertical', name='Vert1')
-        item = self.draft_mongo.get_item(location, 2)
+        vert_location = self.old_course_key.make_usage_key('vertical', name='Vert1')
+        item = self.draft_mongo.get_item(vert_location, 2)
         # Vert1 has 3 children; so, publishes 4 nodes which may mean 4 inserts & 1 bulk remove
         # 25-June-2014 find calls are 19. Probably due to inheritance recomputation?
         # 02-July-2014 send calls are 7. 5 from above, plus 2 for updating subtree edit info for Chapter1 and course
@@ -73,24 +73,16 @@ class TestPublish(SplitWMongoCourseBoostrapper):
             self.draft_mongo.publish(item.location, self.userid)
 
         # verify status
-        item = self.draft_mongo.get_item(location, 0)
+        item = self.draft_mongo.get_item(vert_location, 0)
         self.assertFalse(getattr(item, 'is_draft', False), "Item was published. Draft should not exist")
         # however, children are still draft, but I'm not sure that's by design
 
-        # convert back to draft
-        self.draft_mongo.convert_to_draft(location, self.userid)
-        # both draft and published should exist
-        draft_vert = self.draft_mongo.get_item(location, 0)
-        self.assertTrue(getattr(draft_vert, 'is_draft', False), "Item was converted to draft but doesn't say so")
-        item = self.old_mongo.get_item(location, 0)
-        self.assertFalse(getattr(item, 'is_draft', False), "Published item doesn't say so")
-
         # delete the draft version of the discussion
         location = self.old_course_key.make_usage_key('discussion', name='Discussion1')
         self.draft_mongo.delete_item(location, self.userid)
 
-        draft_vert = self.draft_mongo.get_item(draft_vert.location, 0)
-        # remove pointer from draft vertical (still there b/c not refetching vert)
+        draft_vert = self.draft_mongo.get_item(vert_location, 0)
+        self.assertTrue(getattr(draft_vert, 'is_draft', False), "Deletion didn't convert parent to draft")
         self.assertNotIn(location, draft_vert.children)
         # move the other child
         other_child_loc = self.old_course_key.make_usage_key('html', name='Html2')
@@ -100,8 +92,8 @@ class TestPublish(SplitWMongoCourseBoostrapper):
         self.draft_mongo.update_item(draft_vert, self.userid)
         self.draft_mongo.update_item(other_vert, self.userid)
         # publish
-        self.draft_mongo.publish(draft_vert.location, self.userid)
-        item = self.old_mongo.get_item(draft_vert.location, 0)
+        self.draft_mongo.publish(vert_location, self.userid)
+        item = self.old_mongo.get_item(vert_location, 0)
         self.assertNotIn(location, item.children)
         self.assertIsNone(self.draft_mongo.get_parent_location(location))
         with self.assertRaises(ItemNotFoundError):