Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix issues with overrides on duplicated RCBs.

Browse Source
This commit is contained in:
Jonathan Piacenti
2015-03-25 21:00:39 +00:00
parent 037a189828
commit a6c00635b2
4 changed files with 97 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
cms/djangoapps/contentstore/tests/test_libraries.py
View File

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

16
cms/djangoapps/contentstore/views/item.py
View File

@@ -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.

74
common/lib/xmodule/xmodule/library_content_module.py
View File

@@ -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

13
common/lib/xmodule/xmodule/library_tools.py
View File

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