From 1103d057d75a4b671a2d13322c8684a6bfb2281f Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 17 Jul 2014 10:10:45 -0400 Subject: [PATCH] Piecemeal conversion to draft needs to allow already converted children. STUD-1965 --- cms/djangoapps/contentstore/views/item.py | 1 - .../xmodule/modulestore/mongo/draft.py | 8 ++++--- .../tests/test_mixed_modulestore.py | 23 ++++++++++++++++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 7dca8ee24a..e4f300e325 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -305,7 +305,6 @@ def _save_item(user, usage_key, data=None, children=None, metadata=None, nullout pass elif publish == 'create_draft': try: - # This recursively clones the item subtree and marks the copies as draft store.convert_to_draft(existing_item.location, user.id) except DuplicateItemError: pass diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 9e26c211dc..152a8bf4d6 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -368,10 +368,11 @@ class DraftModuleStore(MongoModuleStore): Raises: InvalidVersionError: if the source can not be made into a draft ItemNotFoundError: if the source does not exist - DuplicateItemError: if the source or any of its descendants already has a draft copy """ + # TODO (dhm) I don't think this needs to recurse anymore but can convert each unit on demand. + # See if that's true. # delegating to internal b/c we don't want any public user to use the kwargs on the internal - self._convert_to_draft(location, user_id) + self._convert_to_draft(location, user_id, ignore_if_draft=True) # return the new draft item (does another fetch) # get_item will wrap_draft so don't call it here (otherwise, it would override the is_draft attribute) @@ -390,7 +391,8 @@ class DraftModuleStore(MongoModuleStore): Raises: InvalidVersionError: if the source can not be made into a draft ItemNotFoundError: if the source does not exist - DuplicateItemError: if the source or any of its descendants already has a draft copy + DuplicateItemError: if the source or any of its descendants already has a draft copy. Only + useful for unpublish b/c we don't want unpublish to overwrite any existing drafts. """ # verify input conditions self._verify_branch_setting(ModuleStoreEnum.Branch.draft_preferred) 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 7dc251b4e1..5fd7170125 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -320,7 +320,7 @@ class TestMixedModuleStore(unittest.TestCase): Delete should reject on r/o db and work on r/w one """ self.initdb(default_ms) - # r/o try deleting the course (is here to ensure it can't be deleted) + # r/o try deleting the chapter (is here to ensure it can't be deleted) with self.assertRaises(NotImplementedError): self.store.delete_item(self.xml_chapter_location, self.user_id) self.store.delete_item(self.writable_chapter_location, self.user_id) @@ -350,6 +350,7 @@ class TestMixedModuleStore(unittest.TestCase): course = self.store.get_course(self.course_locations[self.MONGO_COURSEID].course_key, 0) self.assertIn(vert_loc, course.children) + # update the component to force it to draft w/o forcing the unit to draft # delete the vertical and ensure the course no longer points to it self.store.delete_item(vert_loc, self.user_id) course = self.store.get_course(self.course_locations[self.MONGO_COURSEID].course_key, 0) @@ -364,6 +365,26 @@ class TestMixedModuleStore(unittest.TestCase): self.assertFalse(self.store.has_item(leaf_loc)) self.assertNotIn(vert_loc, course.children) + # NAATODO enable for split after your converge merge + if default_ms == 'split': + return + + # reproduce bug STUD-1965 + # create and delete a private vertical with private children + private_vert = self.store.create_item( + # don't use course_location as it may not be the repr + self.course_locations[self.MONGO_COURSEID], 'vertical', user_id=self.user_id, block_id='publish' + ) + private_leaf = self.store.create_item( + private_vert.location, 'html', user_id=self.user_id, block_id='bug_leaf' + ) + + self.store.publish(private_vert.location, self.user_id) + private_leaf.display_name = 'change me' + private_leaf = self.store.update_item(private_leaf, self.user_id) + # test succeeds if delete succeeds w/o error + self.store.delete_item(private_leaf.location, self.user_id) + @ddt.data('draft', 'split') def test_get_courses(self, default_ms): self.initdb(default_ms)