From e984ca15a13a743ea138d8065a5485e96a2c5640 Mon Sep 17 00:00:00 2001 From: cahrens <christina@edx.org> Date: Tue, 8 Jul 2014 12:59:42 -0400 Subject: [PATCH] Add revert_to_published method. STUD-1860 --- .../lib/xmodule/xmodule/modulestore/mixed.py | 13 +++ .../xmodule/modulestore/mongo/draft.py | 45 ++++++++++ .../xmodule/modulestore/split_mongo/split.py | 12 +++ .../tests/test_mixed_modulestore.py | 82 +++++++++++++++++++ 4 files changed, 152 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 7dfe0f345c7..3ab10b11f8b 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -354,6 +354,19 @@ class MixedModuleStore(ModuleStoreWriteBase): store = self._verify_modulestore_support(location, 'delete_item') store.delete_item(location, user_id=user_id, **kwargs) + def revert_to_published(self, location, user_id=None): + """ + Reverts an item to its last published version (recursively traversing all of its descendants). + If no published version exists, a VersionConflictError is thrown. + + If a published version exists but there is no draft version of this item or any of its descendants, this + method is a no-op. + + :raises InvalidVersionError: if no published version exists for the location specified + """ + store = self._verify_modulestore_support(location, 'revert_to_published') + return store.revert_to_published(location, user_id=user_id) + def close_all_connections(self): """ Close all db connections diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index f883a2d366c..a68df7b72a7 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -597,6 +597,51 @@ class DraftModuleStore(MongoModuleStore): self._verify_branch_setting(ModuleStoreEnum.Branch.draft_preferred) return self._convert_to_draft(location, user_id, delete_published=True) + def revert_to_published(self, location, user_id=None): + """ + Reverts an item to its last published version (recursively traversing all of its descendants). + If no published version exists, a VersionConflictError is thrown. + + If a published version exists but there is no draft version of this item or any of its descendants, this + method is a no-op. It is also a no-op if the root item is in DIRECT_ONLY_CATEGORIES. + + :raises InvalidVersionError: if no published version exists for the location specified + """ + self._verify_branch_setting(ModuleStoreEnum.Branch.draft_preferred) + _verify_revision_is_published(location) + + if location.category in DIRECT_ONLY_CATEGORIES: + return + + if not self.has_item(location, revision=ModuleStoreEnum.RevisionOption.published_only): + raise InvalidVersionError(location) + + def delete_draft_only(root_location): + """ + Helper function that calls delete on the specified location if a draft version of the item exists. + If no draft exists, this function recursively calls itself on the children of the item. + """ + query = root_location.to_deprecated_son(prefix='_id.') + del query['_id.revision'] + versions_found = self.collection.find( + query, {'_id': True, 'definition.children': True}, sort=[SORT_REVISION_FAVOR_DRAFT] + ) + # If 2 versions versions exist, we can assume one is a published version. Go ahead and do the delete + # of the draft version. + if versions_found.count() > 1: + self._delete_subtree(root_location, [as_draft]) + elif versions_found.count() == 1: + # Since this method cannot be called on something in DIRECT_ONLY_CATEGORIES and we call + # delete_subtree as soon as we find an item with a draft version, if there is only 1 version + # it must be published (since adding a child to a published item creates a draft of the parent). + item = versions_found[0] + assert item.get('_id').get('revision') != MongoRevisionKey.draft + for child in item.get('definition', {}).get('children', []): + child_loc = Location.from_deprecated_string(child) + delete_draft_only(child_loc) + + delete_draft_only(location) + def _query_children_for_cache_children(self, course_key, items): # first get non-draft in a round-trip to_process_non_drafts = super(DraftModuleStore, self)._query_children_for_cache_children(course_key, items) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index ccc1123f6f6..114607ce8a0 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1380,6 +1380,18 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): log.info(u"deleting course from split-mongo: %s", course_key) self.db_connection.delete_course_index(index) + def revert_to_published(self, location, user_id=None): + """ + Reverts an item to its last published version (recursively traversing all of its descendants). + If no published version exists, a VersionConflictError is thrown. + + If a published version exists but there is no draft version of this item or any of its descendants, this + method is a no-op. + + :raises InvalidVersionError: if no published version exists for the location specified + """ + raise NotImplementedError() + def get_errored_courses(self): """ This function doesn't make sense for the mongo modulestore, as structures 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 62667936515..b5ed4e1cc90 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -9,6 +9,7 @@ from xmodule.tests import DATA_DIR from opaque_keys.edx.locations import Location from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.exceptions import InvalidVersionError from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from xmodule.modulestore.tests.test_location_mapper import LocMapperSetupSansDjango, loc_mapper @@ -460,6 +461,87 @@ class TestMixedModuleStore(LocMapperSetupSansDjango): (child_to_delete, None, ModuleStoreEnum.RevisionOption.published_only), ]) + + @ddt.data('draft') + def test_revert_to_published_root_draft(self, default_ms): + """ + Test calling revert_to_published on draft vertical. + """ + self.initdb(default_ms) + self._create_block_hierarchy() + self.store.publish(self.course.location, self.user_id) + + # delete leaf problem (will make parent vertical a draft) + self.store.delete_item(self.problem_x1a_1.location, self.user_id) + + draft_parent = self.store.get_item(self.vertical_x1a.location) + self.assertEqual(2, len(draft_parent.children)) + published_parent = self.store.get_item( + self.vertical_x1a.location, + revision=ModuleStoreEnum.RevisionOption.published_only + ) + self.assertEqual(3, len(published_parent.children)) + + self.store.revert_to_published(self.vertical_x1a.location, self.user_id) + reverted_parent = self.store.get_item(self.vertical_x1a.location) + self.assertEqual(3, len(published_parent.children)) + self.assertEqual(reverted_parent, published_parent) + + @ddt.data('draft') + def test_revert_to_published_root_published(self, default_ms): + """ + Test calling revert_to_published on a published vertical with a draft child. + """ + self.initdb(default_ms) + self._create_block_hierarchy() + self.store.publish(self.course.location, self.user_id) + + orig_display_name = self.problem_x1a_1.display_name + + # Change display name of problem and update just it (so parent remains published) + self.problem_x1a_1.display_name = "updated before calling revert" + self.store.update_item(self.problem_x1a_1, self.user_id) + self.store.revert_to_published(self.vertical_x1a.location, self.user_id) + + reverted_problem = self.store.get_item(self.problem_x1a_1.location) + self.assertEqual(orig_display_name, reverted_problem.display_name) + + @ddt.data('draft') + def test_revert_to_published_no_draft(self, default_ms): + """ + Test calling revert_to_published on vertical with no draft content does nothing. + """ + self.initdb(default_ms) + self._create_block_hierarchy() + self.store.publish(self.course.location, self.user_id) + + orig_vertical = self.vertical_x1a + self.store.revert_to_published(self.vertical_x1a.location, self.user_id) + reverted_vertical = self.store.get_item(self.vertical_x1a.location) + self.assertEqual(orig_vertical, reverted_vertical) + + @ddt.data('draft') + def test_revert_to_published_no_published(self, default_ms): + """ + Test calling revert_to_published on vertical with no published version errors. + """ + self.initdb(default_ms) + self._create_block_hierarchy() + with self.assertRaises(InvalidVersionError): + self.store.revert_to_published(self.vertical_x1a.location) + + @ddt.data('draft') + def test_revert_to_published_direct_only(self, default_ms): + """ + Test calling revert_to_published on a direct-only item is a no-op. + """ + self.initdb(default_ms) + self._create_block_hierarchy() + self.store.revert_to_published(self.sequential_x1.location) + reverted_parent = self.store.get_item(self.sequential_x1.location) + # It does not discard the child vertical, even though that child is a draft (with no published version) + self.assertEqual(1, len(reverted_parent.children)) + @ddt.data('draft', 'split') def test_get_orphans(self, default_ms): self.initdb(default_ms) -- GitLab