Merge pull request #11841 from CredoReference/copy_question_should_also_copy_tags

Duplicate item in Studio should also duplicate related xblock aside

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

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"<p>Aside Foo rendered</p>"
+
+    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)
 

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: