diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 130ef01bb0..b75a0be37f 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -595,6 +595,18 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ else: duplicate_metadata['display_name'] = _("Duplicate of '{0}'").format(source_item.display_name) + asides_to_create = [] + for aside in source_item.runtime.get_asides(source_item): + for field in aside.fields.values(): + if field.scope in (Scope.settings, Scope.content,) and field.is_set_on(aside): + asides_to_create.append(aside) + break + + for aside in asides_to_create: + for field in aside.fields.values(): + if field.scope not in (Scope.settings, Scope.content,): + field.delete_from(aside) + dest_module = store.create_item( user.id, dest_usage_key.course_key, @@ -603,6 +615,7 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), metadata=duplicate_metadata, runtime=source_item.runtime, + asides=asides_to_create ) children_handled = False diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 03024fb587..0e790b227a 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -32,6 +32,11 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, TEST_DAT from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory, check_mongo_calls, CourseFactory from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW from xmodule.course_module import DEFAULT_START_DATE +from xblock.core import XBlockAside +from xblock.fields import Scope, String, ScopeIds +from xblock.fragment import Fragment +from xblock.runtime import DictKeyValueStore, KvsFieldData +from xblock.test.tools import TestRuntime from xblock.exceptions import NoSuchHandlerError from xblock_django.user_service import DjangoXBlockUserService from opaque_keys.edx.keys import UsageKey, CourseKey @@ -39,6 +44,22 @@ from opaque_keys.edx.locations import Location from xmodule.partitions.partitions import Group, UserPartition +class AsideTest(XBlockAside): + """ + Test xblock aside class + """ + FRAG_CONTENT = u"
Aside Foo rendered
" + + field11 = String(default="aside1_default_value1", scope=Scope.content) + field12 = String(default="aside1_default_value2", scope=Scope.settings) + field13 = String(default="aside1_default_value3", scope=Scope.parent) + + @XBlockAside.aside_for('student_view') + def student_view_aside(self, block, context): # pylint: disable=unused-argument + """Add to the student view""" + return Fragment(self.FRAG_CONTENT) + + class ItemTest(CourseTestCase): """ Base test class for create, save, and delete """ def setUp(self): @@ -176,7 +197,8 @@ class GetItemTest(ItemTest): # Add a problem beneath a child vertical child_vertical_usage_key = self._create_vertical(parent_usage_key=root_usage_key) - resp = self.create_xblock(parent_usage_key=child_vertical_usage_key, category='problem', boilerplate='multiplechoice.yaml') + resp = self.create_xblock(parent_usage_key=child_vertical_usage_key, category='problem', + boilerplate='multiplechoice.yaml') self.assertEqual(resp.status_code, 200) # Get the preview HTML @@ -201,7 +223,8 @@ class GetItemTest(ItemTest): self.assertEqual(resp.status_code, 200) wrapper_usage_key = self.response_usage_key(resp) - resp = self.create_xblock(parent_usage_key=wrapper_usage_key, category='problem', boilerplate='multiplechoice.yaml') + resp = self.create_xblock(parent_usage_key=wrapper_usage_key, category='problem', + boilerplate='multiplechoice.yaml') self.assertEqual(resp.status_code, 200) # Get the preview HTML and verify the View -> link is present. @@ -223,9 +246,11 @@ class GetItemTest(ItemTest): root_usage_key = self._create_vertical() resp = self.create_xblock(category='split_test', parent_usage_key=root_usage_key) split_test_usage_key = self.response_usage_key(resp) - resp = self.create_xblock(parent_usage_key=split_test_usage_key, category='html', boilerplate='announcement.yaml') + resp = self.create_xblock(parent_usage_key=split_test_usage_key, category='html', + boilerplate='announcement.yaml') self.assertEqual(resp.status_code, 200) - resp = self.create_xblock(parent_usage_key=split_test_usage_key, category='html', boilerplate='zooming_image.yaml') + resp = self.create_xblock(parent_usage_key=split_test_usage_key, category='html', + boilerplate='zooming_image.yaml') self.assertEqual(resp.status_code, 200) html, __ = self._get_container_preview(split_test_usage_key) self.assertIn('Announcement', html) @@ -265,7 +290,8 @@ class GetItemTest(ItemTest): } response = self.client.put( - reverse_course_url('group_configurations_detail_handler', self.course.id, kwargs={'group_configuration_id': 0}), + reverse_course_url('group_configurations_detail_handler', self.course.id, + kwargs={'group_configuration_id': 0}), data=json.dumps(GROUP_CONFIGURATION_JSON), content_type="application/json", HTTP_ACCEPT="application/json", @@ -445,7 +471,96 @@ class TestCreateItem(ItemTest): self.assertEquals(new_tab.display_name, 'Empty') -class TestDuplicateItem(ItemTest): +class DuplicateHelper(object): + """ + Helper mixin class for TestDuplicateItem and TestDuplicateItemWithAsides + """ + def _duplicate_and_verify(self, source_usage_key, parent_usage_key, check_asides=False): + """ Duplicates the source, parenting to supplied parent. Then does equality check. """ + usage_key = self._duplicate_item(parent_usage_key, source_usage_key) + # pylint: disable=no-member + self.assertTrue( + self._check_equality(source_usage_key, usage_key, parent_usage_key, check_asides=check_asides), + "Duplicated item differs from original" + ) + + def _check_equality(self, source_usage_key, duplicate_usage_key, parent_usage_key=None, check_asides=False): + """ + Gets source and duplicated items from the modulestore using supplied usage keys. + Then verifies that they represent equivalent items (modulo parents and other + known things that may differ). + """ + # pylint: disable=no-member + original_item = self.get_item_from_modulestore(source_usage_key) + duplicated_item = self.get_item_from_modulestore(duplicate_usage_key) + + if check_asides: + original_asides = original_item.runtime.get_asides(original_item) + duplicated_asides = duplicated_item.runtime.get_asides(duplicated_item) + self.assertEqual(len(original_asides), 1) + self.assertEqual(len(duplicated_asides), 1) + self.assertEqual(original_asides[0].field11, duplicated_asides[0].field11) + self.assertEqual(original_asides[0].field12, duplicated_asides[0].field12) + self.assertNotEqual(original_asides[0].field13, duplicated_asides[0].field13) + self.assertEqual(duplicated_asides[0].field13, 'aside1_default_value3') + + self.assertNotEqual( + unicode(original_item.location), + unicode(duplicated_item.location), + "Location of duplicate should be different from original" + ) + + # Parent will only be equal for root of duplicated structure, in the case + # where an item is duplicated in-place. + if parent_usage_key and unicode(original_item.parent) == unicode(parent_usage_key): + self.assertEqual( + unicode(parent_usage_key), unicode(duplicated_item.parent), + "Parent of duplicate should equal parent of source for root xblock when duplicated in-place" + ) + else: + self.assertNotEqual( + unicode(original_item.parent), unicode(duplicated_item.parent), + "Parent duplicate should be different from source" + ) + + # Set the location, display name, and parent to be the same so we can make sure the rest of the + # duplicate is equal. + duplicated_item.location = original_item.location + duplicated_item.display_name = original_item.display_name + duplicated_item.parent = original_item.parent + + # Children will also be duplicated, so for the purposes of testing equality, we will set + # the children to the original after recursively checking the children. + if original_item.has_children: + self.assertEqual( + len(original_item.children), + len(duplicated_item.children), + "Duplicated item differs in number of children" + ) + for i in xrange(len(original_item.children)): + if not self._check_equality(original_item.children[i], duplicated_item.children[i]): + return False + duplicated_item.children = original_item.children + + return original_item == duplicated_item + + def _duplicate_item(self, parent_usage_key, source_usage_key, display_name=None): + """ + Duplicates the source. + """ + # pylint: disable=no-member + data = { + 'parent_locator': unicode(parent_usage_key), + 'duplicate_source_locator': unicode(source_usage_key) + } + if display_name is not None: + data['display_name'] = display_name + + resp = self.client.ajax_post(reverse('contentstore.views.xblock_handler'), json.dumps(data)) + return self.response_usage_key(resp) + + +class TestDuplicateItem(ItemTest, DuplicateHelper): """ Test the duplicate method. """ @@ -461,7 +576,8 @@ class TestDuplicateItem(ItemTest): self.seq_usage_key = self.response_usage_key(resp) # create problem and an html component - resp = self.create_xblock(parent_usage_key=self.seq_usage_key, category='problem', boilerplate='multiplechoice.yaml') + resp = self.create_xblock(parent_usage_key=self.seq_usage_key, category='problem', + boilerplate='multiplechoice.yaml') self.problem_usage_key = self.response_usage_key(resp) resp = self.create_xblock(parent_usage_key=self.seq_usage_key, category='html') @@ -475,67 +591,10 @@ class TestDuplicateItem(ItemTest): Tests that a duplicated xblock is identical to the original, except for location and display name. """ - def duplicate_and_verify(source_usage_key, parent_usage_key): - """ Duplicates the source, parenting to supplied parent. Then does equality check. """ - usage_key = self._duplicate_item(parent_usage_key, source_usage_key) - self.assertTrue( - check_equality(source_usage_key, usage_key, parent_usage_key), - "Duplicated item differs from original" - ) - - def check_equality(source_usage_key, duplicate_usage_key, parent_usage_key=None): - """ - Gets source and duplicated items from the modulestore using supplied usage keys. - Then verifies that they represent equivalent items (modulo parents and other - known things that may differ). - """ - original_item = self.get_item_from_modulestore(source_usage_key) - duplicated_item = self.get_item_from_modulestore(duplicate_usage_key) - - self.assertNotEqual( - unicode(original_item.location), - unicode(duplicated_item.location), - "Location of duplicate should be different from original" - ) - - # Parent will only be equal for root of duplicated structure, in the case - # where an item is duplicated in-place. - if parent_usage_key and unicode(original_item.parent) == unicode(parent_usage_key): - self.assertEqual( - unicode(parent_usage_key), unicode(duplicated_item.parent), - "Parent of duplicate should equal parent of source for root xblock when duplicated in-place" - ) - else: - self.assertNotEqual( - unicode(original_item.parent), unicode(duplicated_item.parent), - "Parent duplicate should be different from source" - ) - - # Set the location, display name, and parent to be the same so we can make sure the rest of the - # duplicate is equal. - duplicated_item.location = original_item.location - duplicated_item.display_name = original_item.display_name - duplicated_item.parent = original_item.parent - - # Children will also be duplicated, so for the purposes of testing equality, we will set - # the children to the original after recursively checking the children. - if original_item.has_children: - self.assertEqual( - len(original_item.children), - len(duplicated_item.children), - "Duplicated item differs in number of children" - ) - for i in xrange(len(original_item.children)): - if not check_equality(original_item.children[i], duplicated_item.children[i]): - return False - duplicated_item.children = original_item.children - - return original_item == duplicated_item - - duplicate_and_verify(self.problem_usage_key, self.seq_usage_key) - duplicate_and_verify(self.html_usage_key, self.seq_usage_key) - duplicate_and_verify(self.seq_usage_key, self.chapter_usage_key) - duplicate_and_verify(self.chapter_usage_key, self.usage_key) + self._duplicate_and_verify(self.problem_usage_key, self.seq_usage_key) + self._duplicate_and_verify(self.html_usage_key, self.seq_usage_key) + self._duplicate_and_verify(self.seq_usage_key, self.chapter_usage_key) + self._duplicate_and_verify(self.chapter_usage_key, self.usage_key) def test_ordering(self): """ @@ -599,16 +658,67 @@ class TestDuplicateItem(ItemTest): # Now send a custom display name for the duplicate. verify_name(self.seq_usage_key, self.chapter_usage_key, "customized name", display_name="customized name") - def _duplicate_item(self, parent_usage_key, source_usage_key, display_name=None): - data = { - 'parent_locator': unicode(parent_usage_key), - 'duplicate_source_locator': unicode(source_usage_key) - } - if display_name is not None: - data['display_name'] = display_name - resp = self.client.ajax_post(reverse('contentstore.views.xblock_handler'), json.dumps(data)) - return self.response_usage_key(resp) +class TestDuplicateItemWithAsides(ItemTest, DuplicateHelper): + """ + Test the duplicate method for blocks with asides. + """ + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + def setUp(self): + """ Creates the test course structure and a few components to 'duplicate'. """ + super(TestDuplicateItemWithAsides, self).setUp() + # Create a parent chapter + resp = self.create_xblock(parent_usage_key=self.usage_key, category='chapter') + self.chapter_usage_key = self.response_usage_key(resp) + + # create a sequential containing a problem and an html component + resp = self.create_xblock(parent_usage_key=self.chapter_usage_key, category='sequential') + self.seq_usage_key = self.response_usage_key(resp) + + # create problem and an html component + resp = self.create_xblock(parent_usage_key=self.seq_usage_key, category='problem', + boilerplate='multiplechoice.yaml') + self.problem_usage_key = self.response_usage_key(resp) + + resp = self.create_xblock(parent_usage_key=self.seq_usage_key, category='html') + self.html_usage_key = self.response_usage_key(resp) + + @XBlockAside.register_temp_plugin(AsideTest, 'test_aside') + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside']) + def test_duplicate_equality_with_asides(self): + """ + Tests that a duplicated xblock aside is identical to the original + """ + def create_aside(usage_key, block_type): + """ + Helper function to create aside + """ + item = self.get_item_from_modulestore(usage_key) + + key_store = DictKeyValueStore() + field_data = KvsFieldData(key_store) + runtime = TestRuntime(services={'field-data': field_data}) # pylint: disable=abstract-class-instantiated + + def_id = runtime.id_generator.create_definition(block_type) + usage_id = runtime.id_generator.create_usage(def_id) + + aside = AsideTest(scope_ids=ScopeIds('user', block_type, def_id, usage_id), runtime=runtime) + aside.field11 = '%s_new_value11' % block_type + aside.field12 = '%s_new_value12' % block_type + aside.field13 = '%s_new_value13' % block_type + + self.store.update_item(item, self.user.id, asides=[aside]) + + create_aside(self.html_usage_key, 'html') + create_aside(self.problem_usage_key, 'problem') + create_aside(self.seq_usage_key, 'seq') + create_aside(self.chapter_usage_key, 'chapter') + + self._duplicate_and_verify(self.problem_usage_key, self.seq_usage_key, check_asides=True) + self._duplicate_and_verify(self.html_usage_key, self.seq_usage_key, check_asides=True) + self._duplicate_and_verify(self.seq_usage_key, self.chapter_usage_key, check_asides=True) class TestEditItemSetup(ItemTest): @@ -862,7 +972,8 @@ class TestEditItem(TestEditItemSetup): data={'publish': 'discard_changes'} ) self._verify_published_with_no_draft(self.problem_usage_key) - published = modulestore().get_item(self.problem_usage_key, revision=ModuleStoreEnum.RevisionOption.published_only) + published = modulestore().get_item(self.problem_usage_key, + revision=ModuleStoreEnum.RevisionOption.published_only) self.assertIsNone(published.due) def test_republish(self): @@ -969,7 +1080,8 @@ class TestEditItem(TestEditItemSetup): data={'publish': 'make_public'} ) self._verify_published_with_no_draft(self.problem_usage_key) - published = modulestore().get_item(self.problem_usage_key, revision=ModuleStoreEnum.RevisionOption.published_only) + published = modulestore().get_item(self.problem_usage_key, + revision=ModuleStoreEnum.RevisionOption.published_only) # Now make a draft self.client.ajax_post( @@ -1318,7 +1430,8 @@ class TestComponentHandler(TestCase): self.descriptor.handle = create_response - self.assertEquals(component_handler(self.request, self.usage_key_string, 'dummy_handler').status_code, status_code) + self.assertEquals(component_handler(self.request, self.usage_key_string, 'dummy_handler').status_code, + status_code) class TestComponentTemplates(CourseTestCase): @@ -1932,7 +2045,8 @@ class TestXBlockPublishingInfo(ItemTest): if path: direct_child_xblock_info = self._get_child_xblock_info(xblock_info, path[0]) remaining_path = path[1:] if len(path) > 1 else None - self._verify_xblock_info_state(direct_child_xblock_info, xblock_info_field, expected_state, remaining_path, should_equal) + self._verify_xblock_info_state(direct_child_xblock_info, xblock_info_field, + expected_state, remaining_path, should_equal) else: if should_equal: self.assertEqual(xblock_info[xblock_info_field], expected_state) @@ -2102,7 +2216,8 @@ class TestXBlockPublishingInfo(ItemTest): self._create_child(sequential, 'vertical', "Unit") self._create_child(sequential, 'vertical', "Locked Unit", staff_only=True) xblock_info = self._get_xblock_info(chapter.location) - self._verify_visibility_state(xblock_info, VisibilityState.staff_only, self.FIRST_SUBSECTION_PATH, should_equal=False) + self._verify_visibility_state(xblock_info, VisibilityState.staff_only, self.FIRST_SUBSECTION_PATH, + should_equal=False) self._verify_visibility_state(xblock_info, VisibilityState.staff_only, self.FIRST_UNIT_PATH, should_equal=False) self._verify_visibility_state(xblock_info, VisibilityState.staff_only, self.SECOND_UNIT_PATH) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py index da3417236e..c40f44f78f 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_mongo_kvs.py @@ -47,7 +47,7 @@ class SplitMongoKVS(InheritanceKeyValueStore): def get(self, key): if key.block_family == XBlockAside.entry_point: - if key.scope not in [Scope.settings, Scope.content]: + if key.scope not in self.VALID_SCOPES: raise InvalidScopeError(key, self.VALID_SCOPES) if key.block_scope_id.block_type not in self.aside_fields: @@ -139,6 +139,9 @@ class SplitMongoKVS(InheritanceKeyValueStore): Is the given field explicitly set in this kvs (not inherited nor default) """ # handle any special cases + if key.scope not in self.VALID_SCOPES: + return False + if key.scope == Scope.content: self._load_definition() elif key.scope == Scope.parent: