diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index 0ff0bfabce..b484c74a09 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -3,6 +3,7 @@ Content library unit tests that require the CMS runtime. """ from contentstore.tests.utils import AjaxEnabledTestClient, parse_json from contentstore.utils import reverse_url, reverse_usage_url, reverse_library_url +from contentstore.views.item import _duplicate_item from contentstore.views.preview import _load_preview_module from contentstore.views.tests.test_library import LIBRARY_REST_URL import ddt @@ -726,6 +727,7 @@ class TestLibraryAccess(SignalDisconnectTestMixin, LibraryTestCase): self.assertEqual(len(lc_block.children), 1 if expected_result else 0) +@ddt.ddt class TestOverrides(LibraryTestCase): """ Test that overriding block Scope.settings fields from a library in a specific course works @@ -822,7 +824,8 @@ class TestOverrides(LibraryTestCase): self.assertEqual(self.problem.definition_locator.definition_id, definition_id) self.assertEqual(self.problem_in_course.definition_locator.definition_id, definition_id) - def test_persistent_overrides(self): + @ddt.data(False, True) + def test_persistent_overrides(self, duplicate): """ Test that when we override Scope.settings values in a course, the override values persist even when the block is refreshed @@ -834,7 +837,14 @@ class TestOverrides(LibraryTestCase): self.problem_in_course.weight = new_weight modulestore().update_item(self.problem_in_course, self.user.id) - self.problem_in_course = modulestore().get_item(self.problem_in_course.location) + if duplicate: + # Check that this also works when the RCB is duplicated. + self.lc_block = modulestore().get_item( + _duplicate_item(self.course.location, self.lc_block.location, self.user) + ) + self.problem_in_course = modulestore().get_item(self.lc_block.children[0]) + else: + self.problem_in_course = modulestore().get_item(self.problem_in_course.location) self.assertEqual(self.problem_in_course.display_name, new_display_name) self.assertEqual(self.problem_in_course.weight, new_weight) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 1de8db3413..fc876a37db 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -593,17 +593,27 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ runtime=source_item.runtime, ) + handle_children = True + handle_parenting = True + + if hasattr(dest_module, 'studio_post_duplicate'): + # Allow an XBlock to do anything fancy it may need to when duplicated from another block. + # These blocks may handle their own children or parenting if needed. Let them return booleans to + # let us know if we need to handle these or not. + handle_children, handle_parenting = dest_module.studio_post_duplicate(store, parent_usage_key, source_item) + # Children are not automatically copied over (and not all xblocks have a 'children' attribute). # Because DAGs are not fully supported, we need to actually duplicate each child as well. - if source_item.has_children: - dest_module.children = [] + if source_item.has_children and handle_children: + dest_module.children = dest_module.children or [] for child in source_item.children: dupe = _duplicate_item(dest_module.location, child, user=user) if dupe not in dest_module.children: # _duplicate_item may add the child for us. dest_module.children.append(dupe) store.update_item(dest_module, user.id) - if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: + # pylint: disable=protected-access + if ('detached' not in source_item.runtime.load_block_type(category)._class_tags) and handle_parenting: parent = store.get_item(parent_usage_key) # If source was already a child of the parent, add duplicate immediately afterward. # Otherwise, add child to end. diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index d80ae4f82b..a8e329effc 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -306,6 +306,29 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe non_editable_fields.extend([LibraryContentFields.mode, LibraryContentFields.source_library_version]) return non_editable_fields + def get_tools(self): + """ + Grab the library tools service or raise an error. + """ + lib_tools = self.runtime.service(self, 'library_tools') + if not lib_tools: + # This error is diagnostic. The user won't see it, but it may be helpful + # during debugging. + return Response(_(u"Course does not support Library tools."), status=400) + return lib_tools + + def get_user_id(self): + """ + Get the ID of the current user. + """ + user_service = self.runtime.service(self, 'user') + if user_service: + # May be None when creating bok choy test fixtures + user_id = user_service.get_current_user().opt_attrs.get('edx-platform.user_id', None) + else: + user_id = None + return user_id + @XBlock.handler def refresh_children(self, request=None, suffix=None): # pylint: disable=unused-argument """ @@ -320,21 +343,50 @@ class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDe the version number of the libraries used, so we easily determine if this block is up to date or not. """ - lib_tools = self.runtime.service(self, 'library_tools') - if not lib_tools: - # This error is diagnostic. The user won't see it, but it may be helpful - # during debugging. - return Response(_(u"Course does not support Library tools."), status=400) - user_service = self.runtime.service(self, 'user') + lib_tools = self.get_tools() user_perms = self.runtime.service(self, 'studio_user_permissions') - if user_service: - # May be None when creating bok choy test fixtures - user_id = user_service.get_current_user().opt_attrs.get('edx-platform.user_id', None) - else: - user_id = None + user_id = self.get_user_id() lib_tools.update_children(self, user_id, user_perms) return Response() + # pylint: disable=unused-argument + def studio_post_duplicate(self, store, parent_usage_key, source_block): + """ + Used by the studio after basic duplication of a source block. We handle the children + ourselves, because we have to properly reference the library upstream and set the overrides. + + Otherwise we'll end up losing data on the next refresh. + """ + # The first task will be to refresh our copy of the library to generate the children. + # We must do this at the currently set version of the library block. Otherwise we may not have + # exactly the same children-- someone may be duplicating an out of date block, after all. + lib_tools = self.get_tools() + user_id = self.get_user_id() + user_perms = self.runtime.service(self, 'studio_user_permissions') + # pylint: disable=no-member + lib_tools.update_children(self, user_id, user_perms, version=self.source_library_version) + + # Copy over any overridden settings the course author may have applied to the blocks. + def copy_overrides(source, dest): + """ + Copy any overrides the user has made on blocks in this library. + """ + for field_name in source.fields.keys(): + field = dest.fields[field_name] + if field.scope == Scope.settings and field.is_set_on(source): + setattr(dest, field_name, getattr(source, field_name)) + if source.has_children: + source_children = [store.get_item(source_key) for source_key in source.children] + dest_children = [store.get_item(dest_key) for dest_key in dest.children] + for source_child, dest_child in zip(source_children, dest_children): + copy_overrides(source_child, dest_child) + store.update_item(dest, user_id) + + copy_overrides(source_block, self) + + # Don't handle children. Handle parenting. + return False, True + def _validate_library_version(self, validation, lib_tools, version, library_key): """ Validates library version diff --git a/common/lib/xmodule/xmodule/library_tools.py b/common/lib/xmodule/xmodule/library_tools.py index 40afbbb549..da248457da 100644 --- a/common/lib/xmodule/xmodule/library_tools.py +++ b/common/lib/xmodule/xmodule/library_tools.py @@ -16,16 +16,21 @@ class LibraryToolsService(object): def __init__(self, modulestore): self.store = modulestore - def _get_library(self, library_key): + def _get_library(self, library_key, version=None): """ Given a library key like "library-v1:ProblemX+PR0B", return the 'library' XBlock with meta-information about the library. + A specific version may be specified. + Returns None on error. """ if not isinstance(library_key, LibraryLocator): library_key = LibraryLocator.from_string(library_key) - assert library_key.version_guid is None + + library_key = LibraryLocator( + org=library_key.org, library=library_key.library, branch=library_key.branch, version_guid=version + ) try: return self.store.get_library(library_key, remove_version=False, remove_branch=False) @@ -102,7 +107,7 @@ class LibraryToolsService(object): """ return self.store.check_supports(block.location.course_key, 'copy_from_template') - def update_children(self, dest_block, user_id, user_perms=None): + def update_children(self, dest_block, user_id, user_perms=None, version=None): """ This method is to be used when the library that a LibraryContentModule references has been updated. It will re-fetch all matching blocks from @@ -123,7 +128,7 @@ class LibraryToolsService(object): source_blocks = [] library_key = dest_block.source_library_key - library = self._get_library(library_key) + library = self._get_library(library_key, version=version) if library is None: raise ValueError("Requested library not found.") if user_perms and not user_perms.can_read(library_key):