Merge pull request #4934 from edx/benmcmorran/course-settings-error

Fix error when changing course details on split

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):

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):
             """

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)