diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 3599a28148..78ef997f61 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -138,8 +138,7 @@ class MixedModuleStore(ModuleStoreWriteBase): def get_item(self, usage_key, depth=0, **kwargs): """ - This method is explicitly not implemented as we need a course_id to disambiguate - We should be able to fix this when the data-model rearchitecting is done + see parent doc """ store = self._get_modulestore_for_courseid(usage_key.course_key) return store.get_item(usage_key, depth, **kwargs) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 067af3bd8f..35e9926885 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -506,15 +506,13 @@ class DraftModuleStore(MongoModuleStore): # 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 for parent_location in parent_locations: - # don't remove from direct_only parent if other versions of this still exists + # don't remove from direct_only parent if other versions of this still exists (this code + # assumes that there's only one parent_location in this case) if not is_item_direct_only and parent_location.category in DIRECT_ONLY_CATEGORIES: - # see if other version of root exists - alt_location = location.replace( - revision=MongoRevisionKey.published - if location.revision == MongoRevisionKey.draft - else MongoRevisionKey.draft - ) - if super(DraftModuleStore, self).has_item(alt_location): + # see if other version of to-be-deleted root exists + query = location.to_deprecated_son(prefix='_id.') + del query['_id.revision'] + if self.collection.find(query).count() > 1: continue parent_block = super(DraftModuleStore, self).get_item(parent_location) 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 b3a3fd6010..b404d7457b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -17,6 +17,7 @@ from xmodule.modulestore.tests.test_location_mapper import LocMapperSetupSansDja # before importing the module from django.conf import settings from opaque_keys.edx.locations import SlashSeparatedCourseKey +from xmodule.modulestore.mongo.base import MongoRevisionKey if not settings.configured: settings.configure() from xmodule.modulestore.mixed import MixedModuleStore @@ -326,6 +327,42 @@ class TestMixedModuleStore(LocMapperSetupSansDjango): with self.assertRaises(ItemNotFoundError): self.store.get_item(self.writable_chapter_location) + # 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='private' + ) + private_leaf = self.store.create_item( + # don't use course_location as it may not be the repr + private_vert.location, 'html', user_id=self.user_id, block_id='private_leaf' + ) + + # verify pre delete state (just to verify that the test is valid) + self.assertTrue(hasattr(private_vert, 'is_draft') or private_vert.location.branch == ModuleStoreEnum.BranchName.draft) + if hasattr(private_vert.location, 'version_guid'): + # change to the HEAD version + vert_loc = private_vert.location.for_version(private_leaf.location.version_guid) + else: + vert_loc = private_vert.location + self.assertTrue(self.store.has_item(vert_loc)) + self.assertTrue(self.store.has_item(private_leaf.location)) + course = self.store.get_course(self.course_locations[self.MONGO_COURSEID].course_key, 0) + self.assertIn(vert_loc, course.children) + + # 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) + if hasattr(private_vert.location, 'version_guid'): + # change to the HEAD version + vert_loc = private_vert.location.for_version(course.location.version_guid) + leaf_loc = private_leaf.location.for_version(course.location.version_guid) + else: + vert_loc = private_vert.location + leaf_loc = private_leaf.location + self.assertFalse(self.store.has_item(vert_loc)) + self.assertFalse(self.store.has_item(leaf_loc)) + self.assertNotIn(vert_loc, course.children) + @ddt.data('draft', 'split') def test_get_courses(self, default_ms): self.initdb(default_ms)