From f8f9b91c16b1cf5351f0af6be5242f023d79c541 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 1 Oct 2015 10:21:35 -0400 Subject: [PATCH] Make unpublish raise the same errors in split and old-mongo when asked to unpublish a DIRECT_ONLY_CATEGORY --- .../modulestore/draft_and_published.py | 5 +++ .../xmodule/modulestore/mongo/draft.py | 4 ++ .../modulestore/split_mongo/split_draft.py | 3 ++ .../xmodule/modulestore/tests/test_publish.py | 37 +------------------ .../ccx/tests/test_ccx_modulestore.py | 2 +- 5 files changed, 15 insertions(+), 36 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py index 77daa462ab..79f567e66b 100644 --- a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py +++ b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py @@ -87,6 +87,11 @@ class ModuleStoreDraftAndPublished(BranchSettingMixin, BulkOperationsMixin): @abstractmethod def unpublish(self, location, user_id): + """ + Turn the published version into a draft, removing the published version. + + Raises: InvalidVersionError if called on a DIRECT_ONLY_CATEGORY + """ raise NotImplementedError @abstractmethod diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 6e6335d3bd..b75a337e63 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -754,6 +754,10 @@ class DraftModuleStore(MongoModuleStore): NOTE: unlike publish, this gives an error if called above the draftable level as it's intended to remove things from the published version """ + # ensure we are not creating a DRAFT of an item that is direct-only + if location.category in DIRECT_ONLY_CATEGORIES: + raise InvalidVersionError(location) + self._verify_branch_setting(ModuleStoreEnum.Branch.draft_preferred) self._convert_to_draft(location, user_id, delete_published=True) 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 b9f8dc85d4..bdd4696106 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -377,6 +377,9 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli Deletes the published version of the item. Returns the newly unpublished item. """ + if location.block_type in DIRECT_ONLY_CATEGORIES: + raise InvalidVersionError(location) + with self.bulk_operations(location.course_key): self.delete_item(location, user_id, revision=ModuleStoreEnum.RevisionOption.published_only) return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.draft), **kwargs) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py index d35eafabc6..7b824e2c75 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_publish.py @@ -925,49 +925,16 @@ class ElementalUnpublishingTests(DraftPublishedOpBaseTestSetup): self.assertOLXIsDraftOnly(block_list_unpublished_children) self.assertOLXIsDraftOnly(block_list_untouched) - @ddt.data(DRAFT_MODULESTORE_SETUP, MongoModulestoreBuilder()) - def test_unpublish_old_mongo_draft_sequential(self, modulestore_builder): + @ddt.data(SPLIT_MODULESTORE_SETUP, DRAFT_MODULESTORE_SETUP, MongoModulestoreBuilder()) + def test_unpublish_draft_sequential(self, modulestore_builder): with self._setup_test(modulestore_builder): - # MODULESTORE_DIFFERENCE: - # In old Mongo, you cannot successfully unpublish an autopublished sequential. - # An exception is thrown. block_list_to_unpublish = ( ('sequential', 'sequential03'), ) with self.assertRaises(InvalidVersionError): self.unpublish(block_list_to_unpublish) - @ddt.data(SPLIT_MODULESTORE_SETUP) - def test_unpublish_split_draft_sequential(self, modulestore_builder): - with self._setup_test(modulestore_builder): - - # MODULESTORE_DIFFERENCE: - # In Split, the sequential is deleted. - # The sequential's children are orphaned - but they stay in - # the same draft state they were before. - block_list_to_unpublish = ( - ('sequential', 'sequential03'), - ) - block_list_unpublished_children = ( - ('vertical', 'vertical06'), - ('vertical', 'vertical07'), - ('html', 'html12'), - ('html', 'html13'), - ('html', 'html14'), - ('html', 'html15'), - ) - # The autopublished sequential is published - its children are draft. - self.assertOLXIsPublishedOnly(block_list_to_unpublish) - self.assertOLXIsDraftOnly(block_list_unpublished_children) - # Unpublish the sequential. - self.unpublish(block_list_to_unpublish) - # Since the sequential was autopublished, a draft version of the sequential never existed. - # So unpublishing the sequential doesn't make it a draft - it deletes it! - self.assertOLXIsDeleted(block_list_to_unpublish) - # Its children are orphaned and remain as drafts. - self.assertOLXIsDraftOnly(block_list_unpublished_children) - @ddt.ddt class ElementalDeleteItemTests(DraftPublishedOpBaseTestSetup): diff --git a/lms/djangoapps/ccx/tests/test_ccx_modulestore.py b/lms/djangoapps/ccx/tests/test_ccx_modulestore.py index b46e0fabbc..161037a50f 100644 --- a/lms/djangoapps/ccx/tests/test_ccx_modulestore.py +++ b/lms/djangoapps/ccx/tests/test_ccx_modulestore.py @@ -45,7 +45,7 @@ class TestCCXModulestoreWrapper(SharedModuleStoreTestCase): ) for _ in xrange(2) for s in sequentials ] cls.blocks = [ - ItemFactory.create(parent=v) for _ in xrange(2) for v in verticals + ItemFactory.create(parent=v, category='html') for _ in xrange(2) for v in verticals ] def setUp(self):