diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py new file mode 100644 index 0000000000..9e21a1a636 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py @@ -0,0 +1,122 @@ +""" +Tests for XML importer. +""" +from unittest import TestCase +import mock +from xblock.core import XBlock +from xblock.fields import String, Scope, ScopeIds +from xblock.runtime import Runtime, KvsFieldData, DictKeyValueStore +from xmodule.x_module import XModuleMixin +from xmodule.modulestore import Location +from xmodule.modulestore.inheritance import InheritanceMixin +from xmodule.modulestore.xml_importer import remap_namespace + + +class StubXBlock(XBlock, XModuleMixin, InheritanceMixin): + """ + Stub XBlock used for testing. + """ + test_content_field = String( + help="A content field that will be explicitly set", + scope=Scope.content, + default="default value" + ) + + test_settings_field = String( + help="A settings field that will be explicitly set", + scope=Scope.settings, + default="default value" + ) + + +class RemapNamespaceTest(TestCase): + """ + Test that remapping the namespace from import to the actual course location. + """ + + def setUp(self): + """ + Create a stub XBlock backed by in-memory storage. + """ + self.runtime = mock.MagicMock(Runtime) + self.field_data = KvsFieldData(kvs=DictKeyValueStore()) + self.scope_ids = ScopeIds('Bob', 'stubxblock', '123', 'import') + self.xblock = StubXBlock(self.runtime, self.field_data, self.scope_ids) + + def test_remap_namespace_native_xblock(self): + + # Set the XBlock's location + self.xblock.location = Location("i4x://import/org/run/stubxblock") + + # Explicitly set the content and settings fields + self.xblock.test_content_field = "Explicitly set" + self.xblock.test_settings_field = "Explicitly set" + self.xblock.save() + + # Remap the namespace + target_location_namespace = Location("i4x://course/org/run/stubxblock") + remap_namespace(self.xblock, target_location_namespace) + + # Check the XBlock's location + self.assertEqual(self.xblock.location, target_location_namespace) + + # Check the values of the fields. + # The content and settings fields should be preserved + self.assertEqual(self.xblock.test_content_field, 'Explicitly set') + self.assertEqual(self.xblock.test_settings_field, 'Explicitly set') + + # Expect that these fields are marked explicitly set + self.assertIn( + 'test_content_field', + self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.content) + ) + self.assertIn( + 'test_settings_field', + self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.settings) + ) + + def test_remap_namespace_native_xblock_default_values(self): + + # Set the XBlock's location + self.xblock.location = Location("i4x://import/org/run/stubxblock") + + # Do NOT set any values, so the fields should use the defaults + self.xblock.save() + + # Remap the namespace + target_location_namespace = Location("i4x://course/org/run/stubxblock") + remap_namespace(self.xblock, target_location_namespace) + + # Check the values of the fields. + # The content and settings fields should be the default values + self.assertEqual(self.xblock.test_content_field, 'default value') + self.assertEqual(self.xblock.test_settings_field, 'default value') + + # The fields should NOT appear in the explicitly set fields + self.assertNotIn( + 'test_content_field', + self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.content) + ) + self.assertNotIn( + 'test_settings_field', + self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.settings) + ) + + def test_remap_namespace_native_xblock_inherited_values(self): + + # Set the XBlock's location + self.xblock.location = Location("i4x://import/org/run/stubxblock") + self.xblock.save() + + # Remap the namespace + target_location_namespace = Location("i4x://course/org/run/stubxblock") + remap_namespace(self.xblock, target_location_namespace) + + # Inherited fields should NOT be explicitly set + self.assertNotIn( + 'start', self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.settings) + ) + self.assertNotIn( + 'graded', self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.settings) + ) + diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 2dc7a50067..0e49581efb 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -6,6 +6,7 @@ import json from .xml import XMLModuleStore, ImportSystem, ParentTracker from xmodule.modulestore import Location +from xmodule.x_module import XModuleDescriptor from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xmodule.contentstore.content import StaticContent from .inheritance import own_metadata @@ -488,11 +489,34 @@ def remap_namespace(module, target_location_namespace): # This looks a bit wonky as we need to also change the 'name' of the # imported course to be what the caller passed in if module.location.category != 'course': + + # Retrieve the content and settings fields that have been explicitly set + # to ensure that they are properly re-keyed in the XBlock field data. + if isinstance(module, XModuleDescriptor): + rekey_fields = [] + else: + rekey_fields = ( + module.get_explicitly_set_fields_by_scope(Scope.content).keys() + + module.get_explicitly_set_fields_by_scope(Scope.settings).keys() + ) + module.location = module.location.replace( tag=target_location_namespace.tag, org=target_location_namespace.org, course=target_location_namespace.course ) + + # Native XBlocks store the field data in a key-value store + # in which one component of the key is the XBlock's location (equivalent to "scope_ids"). + # Since we've changed the XBlock's location, we need to re-save + # all the XBlock's fields so they will be stored using the new location in the key. + # However, since XBlocks only save "dirty" fields, we need to first + # explicitly set each field to its current value before triggering the save. + if len(rekey_fields) > 0: + for rekey_field_name in rekey_fields: + setattr(module, rekey_field_name, getattr(module, rekey_field_name)) + module.save() + else: # # module is a course module