Commit bf0c4f51 by Don Mitchell

Don't let destructive loc change

leak out to callers.
Also, use the return from update_item rather than assume
it was just side effecting.
LMS-11361
parent e18e18a8
...@@ -314,7 +314,7 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None): ...@@ -314,7 +314,7 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None):
xblock.editor_saved(user, old_metadata, old_content) xblock.editor_saved(user, old_metadata, old_content)
# Update after the callback so any changes made in the callback will get persisted. # Update after the callback so any changes made in the callback will get persisted.
modulestore().update_item(xblock, user.id) return modulestore().update_item(xblock, user.id)
def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, nullout=None, def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, nullout=None,
...@@ -356,7 +356,7 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, ...@@ -356,7 +356,7 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None,
if old_parent_location: if old_parent_location:
old_parent = store.get_item(old_parent_location) old_parent = store.get_item(old_parent_location)
old_parent.children.remove(new_child) old_parent.children.remove(new_child)
_update_with_callback(old_parent, user) old_parent = _update_with_callback(old_parent, user)
else: else:
# the Studio UI currently doesn't present orphaned children, so assume this is an error # the Studio UI currently doesn't present orphaned children, so assume this is an error
return JsonResponse({"error": "Invalid data, possibly caused by concurrent authors."}, 400) return JsonResponse({"error": "Invalid data, possibly caused by concurrent authors."}, 400)
...@@ -411,7 +411,7 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, ...@@ -411,7 +411,7 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None,
field.write_to(xblock, value) field.write_to(xblock, value)
# update the xblock and call any xblock callbacks # update the xblock and call any xblock callbacks
_update_with_callback(xblock, user, old_metadata, old_content) xblock = _update_with_callback(xblock, user, old_metadata, old_content)
# for static tabs, their containing course also records their display name # for static tabs, their containing course also records their display name
if xblock.location.category == 'static_tab': if xblock.location.category == 'static_tab':
...@@ -550,7 +550,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ ...@@ -550,7 +550,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_
dest_module.children.append(dupe) dest_module.children.append(dupe)
store.update_item(dest_module, user.id) store.update_item(dest_module, user.id)
if not 'detached' in source_item.runtime.load_block_type(category)._class_tags: if 'detached' not in source_item.runtime.load_block_type(category)._class_tags:
parent = store.get_item(parent_usage_key) parent = store.get_item(parent_usage_key)
# If source was already a child of the parent, add duplicate immediately afterward. # If source was already a child of the parent, add duplicate immediately afterward.
# Otherwise, add child to end. # Otherwise, add child to end.
......
...@@ -117,7 +117,6 @@ class GetItemTest(ItemTest): ...@@ -117,7 +117,6 @@ class GetItemTest(ItemTest):
with check_mongo_calls(problem_queries): with check_mongo_calls(problem_queries):
self.client.get(reverse_usage_url('xblock_handler', self.populated_usage_keys['problem'][-1])) self.client.get(reverse_usage_url('xblock_handler', self.populated_usage_keys['problem'][-1]))
def test_get_vertical(self): def test_get_vertical(self):
# Add a vertical # Add a vertical
resp = self.create_xblock(category='vertical') resp = self.create_xblock(category='vertical')
...@@ -197,7 +196,6 @@ class GetItemTest(ItemTest): ...@@ -197,7 +196,6 @@ class GetItemTest(ItemTest):
self.assertIn('Announcement', html) self.assertIn('Announcement', html)
self.assertIn('Zooming', html) self.assertIn('Zooming', html)
def test_split_test_edited(self): def test_split_test_edited(self):
""" """
Test that rename of a group changes display name of child vertical. Test that rename of a group changes display name of child vertical.
......
...@@ -89,7 +89,8 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -89,7 +89,8 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
self.publish(location.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs) self.publish(location.version_agnostic(), user_id, blacklist=EXCLUDE_ALL, **kwargs)
def update_item(self, descriptor, user_id, allow_not_found=False, force=False, **kwargs): def update_item(self, descriptor, user_id, allow_not_found=False, force=False, **kwargs):
descriptor.location = self._map_revision_to_branch(descriptor.location) old_descriptor_locn = descriptor.location
descriptor.location = self._map_revision_to_branch(old_descriptor_locn)
with self.bulk_operations(descriptor.location.course_key): with self.bulk_operations(descriptor.location.course_key):
item = super(DraftVersioningModuleStore, self).update_item( item = super(DraftVersioningModuleStore, self).update_item(
descriptor, descriptor,
...@@ -99,6 +100,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS ...@@ -99,6 +100,7 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
**kwargs **kwargs
) )
self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs) self._auto_publish_no_children(item.location, item.location.category, user_id, **kwargs)
descriptor.location = old_descriptor_locn
return item return item
def create_item( def create_item(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment