From e900a47b3f5e9e577ff4d6a30985c9c6c82a1e18 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Wed, 6 Aug 2014 09:16:05 -0400 Subject: [PATCH] Provide workarounds for skip_draft where other code doesn't want its behavior --- cms/djangoapps/contentstore/tests/utils.py | 9 +++++- .../xmodule/modulestore/split_migrator.py | 2 ++ .../xmodule/modulestore/split_mongo/split.py | 9 ++++-- .../modulestore/split_mongo/split_draft.py | 29 ++++++++++++------- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 20b21065bf..cdd0a4b512 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -247,7 +247,14 @@ class CourseTestCase(ModuleStoreTestCase): course1_items = self.store.get_items(course1_id) course2_items = self.store.get_items(course2_id) self.assertGreater(len(course1_items), 0) # ensure it found content instead of [] == [] - self.assertEqual(len(course1_items), len(course2_items)) + if len(course1_items) != len(course2_items): + course1_block_ids = set([item.location.block_id for item in course1_items]) + course2_block_ids = set([item.location.block_id for item in course2_items]) + raise AssertionError( + u"Course1 extra blocks: {}; course2 extra blocks: {}".format( + course1_block_ids - course2_block_ids, course2_block_ids - course1_block_ids + ) + ) for course1_item in course1_items: course2_item_location = course1_item.location.map_into_course(course2_id) diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index 8fff1be717..9124195636 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -62,6 +62,7 @@ class SplitMigrator(object): new_org, new_course, new_run, user_id, fields=new_fields, master_branch=ModuleStoreEnum.BranchName.published, + skip_auto_publish=True, **kwargs ) @@ -101,6 +102,7 @@ class SplitMigrator(object): module, course_version_locator, new_course.location.block_id ), continue_version=True, + skip_auto_publish=True, **kwargs ) # after done w/ published items, add version for DRAFT pointing to the published structure diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 2a5fa3b7be..7e7840b391 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -953,8 +953,13 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if source_index is None: raise ItemNotFoundError("Cannot find a course at {0}. Aborting".format(source_course_id)) return self.create_course( - dest_course_id.org, dest_course_id.course, dest_course_id.run, user_id, fields=fields, - versions_dict=source_index['versions'], search_targets=source_index['search_targets'], **kwargs + dest_course_id.org, dest_course_id.course, dest_course_id.run, + user_id, + fields=fields, + versions_dict=source_index['versions'], + search_targets=source_index['search_targets'], + skip_auto_publish=True, + **kwargs ) def create_course( diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 02f3ca8141..fb7f452516 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -15,7 +15,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS A subclass of Split that supports a dual-branch fall-back versioning framework with a Draft branch that falls back to a Published branch. """ - def create_course(self, org, course, run, user_id, **kwargs): + def create_course(self, org, course, run, user_id, skip_auto_publish=False, **kwargs): """ Creates and returns the course. @@ -32,14 +32,18 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS item = super(DraftVersioningModuleStore, self).create_course( org, course, run, user_id, master_branch=master_branch, **kwargs ) - self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs) + if master_branch == ModuleStoreEnum.BranchName.draft and not skip_auto_publish: + # any other value is hopefully only cloning or doing something which doesn't want this value add + self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs) - # create any other necessary things as a side effect: ensure they populate the draft branch - # and rely on auto publish to populate the published branch - with self.branch_setting(ModuleStoreEnum.Branch.draft_preferred, item.id): - super(SplitMongoModuleStore, self).create_course( - org, course, run, user_id, **kwargs - ) + # create any other necessary things as a side effect: ensure they populate the draft branch + # and rely on auto publish to populate the published branch: split's create course doesn't + # call super b/c it needs the auto publish above to have happened before any of the create_items + # in this + with self.branch_setting(ModuleStoreEnum.Branch.draft_preferred, item.id): + super(SplitMongoModuleStore, self).create_course( + org, course, run, user_id, **kwargs + ) return item @@ -84,15 +88,20 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS def create_item( self, user_id, course_key, block_type, block_id=None, definition_locator=None, fields=None, - force=False, continue_version=False, **kwargs + force=False, continue_version=False, skip_auto_publish=False, **kwargs ): + """ + Adds skip_auto_publish to behavior or parent. Skip_auto_publish basically just calls the super + and skips all of this wrapper's functionality. + """ course_key = self._map_revision_to_branch(course_key) item = super(DraftVersioningModuleStore, self).create_item( user_id, course_key, block_type, block_id=block_id, definition_locator=definition_locator, fields=fields, force=force, continue_version=continue_version, **kwargs ) - self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs) + if not skip_auto_publish: + self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs) return item def create_child(