diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index e466020c10..6c61cd8419 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -86,14 +86,18 @@ class CourseDetails(object): temploc = course_key.make_usage_key('about', about_key) store = modulestore() if data is None: - store.delete_item(temploc, user.id) + try: + store.delete_item(temploc, user.id) + # Ignore an attempt to delete an item that doesn't exist + except ValueError: + pass else: try: about_item = store.get_item(temploc) except ItemNotFoundError: about_item = store.create_xblock(course.runtime, course.id, 'about', about_key) about_item.data = data - store.update_item(about_item, user.id) + store.update_item(about_item, user.id, allow_not_found=True) @classmethod def update_from_json(cls, course_key, jsondict, user): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 05084d85f3..25b8391818 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1500,17 +1500,20 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): original_structure = self._lookup_course(usage_locator.course_key)['structure'] if original_structure['root'] == usage_locator.block_id: raise ValueError("Cannot delete the root of a course") + if encode_key_for_mongo(usage_locator.block_id) not in original_structure['blocks']: + raise ValueError("Cannot delete a block that does not exist") index_entry = self._get_index_if_valid(usage_locator, force) new_structure = self._version_structure(original_structure, user_id) new_blocks = new_structure['blocks'] new_id = new_structure['_id'] encoded_block_id = self._get_parent_from_structure(usage_locator.block_id, original_structure) - parent_block = new_blocks[encoded_block_id] - parent_block['fields']['children'].remove(usage_locator.block_id) - parent_block['edit_info']['edited_on'] = datetime.datetime.now(UTC) - parent_block['edit_info']['edited_by'] = user_id - parent_block['edit_info']['previous_version'] = parent_block['edit_info']['update_version'] - parent_block['edit_info']['update_version'] = new_id + if encoded_block_id: + parent_block = new_blocks[encoded_block_id] + parent_block['fields']['children'].remove(usage_locator.block_id) + parent_block['edit_info']['edited_on'] = datetime.datetime.now(UTC) + parent_block['edit_info']['edited_by'] = user_id + parent_block['edit_info']['previous_version'] = parent_block['edit_info']['update_version'] + parent_block['edit_info']['update_version'] = new_id def remove_subtree(block_id): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index abe634ed88..05f5497d3c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -1332,6 +1332,8 @@ class TestItemCrud(SplitModuleTest): self.assertFalse(modulestore().has_item(deleted)) with self.assertRaises(VersionConflictError): modulestore().has_item(locn_to_del) + with self.assertRaises(ValueError): + modulestore().delete_item(deleted, self.user_id) self.assertTrue(modulestore().has_item(locn_to_del.course_agnostic())) self.assertNotEqual(new_course_loc.version_guid, course.location.version_guid)