From 5bf2c23f595e28f3b8d4c483f421a16ada927868 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Fri, 3 Apr 2015 13:51:03 +0000 Subject: [PATCH] Fixed issue with block IDs being different across library versions. --- common/lib/xmodule/xmodule/library_tools.py | 5 ++++- .../xmodule/modulestore/split_mongo/split.py | 20 +++++++++++++------ .../modulestore/split_mongo/split_draft.py | 5 ++++- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index d6afb9226b..28a490607c 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -144,7 +144,10 @@ class LibraryToolsService(object): with self.store.bulk_operations(dest_block.location.course_key): dest_block.source_library_version = unicode(library.location.library_key.version_guid) self.store.update_item(dest_block, user_id) - dest_block.children = self.store.copy_from_template(source_blocks, dest_block.location, user_id) + head_validation = not version + dest_block.children = self.store.copy_from_template( + source_blocks, dest_block.location, user_id, head_validation=head_validation + ) # ^-- copy_from_template updates the children in the DB # but we must also set .children here to avoid overwriting the DB again diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index c78085bc2a..e76eca743a 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -804,6 +804,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): :param course_key: any subclass of CourseLocator """ + if not course_key.version_guid: + head_validation = True if head_validation and course_key.org and course_key.course and course_key.run: if course_key.branch is None: raise InsufficientSpecificationError(course_key) @@ -2171,7 +2173,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): self._update_head(destination_course, index_entry, destination_course.branch, destination_structure['_id']) @contract(source_keys="list(BlockUsageLocator)", dest_usage=BlockUsageLocator) - def copy_from_template(self, source_keys, dest_usage, user_id): + def copy_from_template(self, source_keys, dest_usage, user_id, head_validation=True): """ Flexible mechanism for inheriting content from an external course/library/etc. @@ -2210,7 +2212,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): raise ItemNotFoundError("branch is required for all source keys when using copy_from_template") if course_key not in source_structures: with self.bulk_operations(course_key): - source_structures[course_key] = self._lookup_course(course_key).structure + source_structures[course_key] = self._lookup_course( + course_key, head_validation=head_validation + ).structure destination_course = dest_usage.course_key with self.bulk_operations(destination_course): @@ -2227,7 +2231,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # The descendants() method used above adds the block itself, which we don't consider a descendant. orig_descendants.remove(block_key) new_descendants = self._copy_from_template( - source_structures, source_keys, dest_structure, block_key, user_id + source_structures, source_keys, dest_structure, block_key, user_id, head_validation ) # Update the edit info: @@ -2251,7 +2255,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): for k in dest_structure['blocks'][block_key].fields['children'] ] - def _copy_from_template(self, source_structures, source_keys, dest_structure, new_parent_block_key, user_id): + def _copy_from_template( + self, source_structures, source_keys, dest_structure, new_parent_block_key, user_id, head_validation + ): """ Internal recursive implementation of copy_from_template() @@ -2265,8 +2271,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): for usage_key in source_keys: src_course_key = usage_key.course_key + hashable_source_id = src_course_key.for_version(None) block_key = BlockKey(usage_key.block_type, usage_key.block_id) source_structure = source_structures[src_course_key] + if block_key not in source_structure['blocks']: raise ItemNotFoundError(usage_key) source_block_info = source_structure['blocks'][block_key] @@ -2274,7 +2282,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # Compute a new block ID. This new block ID must be consistent when this # method is called with the same (source_key, dest_structure) pair unique_data = "{}:{}:{}".format( - unicode(src_course_key).encode("utf-8"), + unicode(hashable_source_id).encode("utf-8"), block_key.id, new_parent_block_key.id, ) @@ -2320,7 +2328,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if children: children = [src_course_key.make_usage_key(child.type, child.id) for child in children] new_blocks |= self._copy_from_template( - source_structures, children, dest_structure, new_block_key, user_id + source_structures, children, dest_structure, new_block_key, user_id, head_validation ) new_blocks.add(new_block_key) 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 64c74bde9f..22ca4342e3 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -104,7 +104,10 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli """ source_keys = [self._map_revision_to_branch(key) for key in source_keys] dest_key = self._map_revision_to_branch(dest_key) - new_keys = super(DraftVersioningModuleStore, self).copy_from_template(source_keys, dest_key, user_id) + head_validation = kwargs.get('head_validation') + new_keys = super(DraftVersioningModuleStore, self).copy_from_template( + source_keys, dest_key, user_id, head_validation + ) if dest_key.branch == ModuleStoreEnum.BranchName.draft: # Check if any of new_keys or their descendants need to be auto-published. # We don't use _auto_publish_no_children since children may need to be published.