diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py index 3adde6d8d2..ab4db39ff5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py @@ -2,14 +2,15 @@ Tests for XML importer. """ import mock +from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from xblock.core import XBlock -from xblock.fields import String, Scope, ScopeIds +from xblock.fields import String, Scope, ScopeIds, List from xblock.runtime import Runtime, KvsFieldData, DictKeyValueStore from xmodule.x_module import XModuleMixin from opaque_keys.edx.locations import Location from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.inheritance import InheritanceMixin -from xmodule.modulestore.xml_importer import _import_module_and_update_references +from xmodule.modulestore.xml_importer import _import_module_and_update_references, _update_module_location from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.tests import DATA_DIR @@ -229,3 +230,90 @@ class RemapNamespaceTest(ModuleStoreNoSettings): self.assertNotIn( 'graded', new_version.get_explicitly_set_fields_by_scope(scope=Scope.settings) ) + + +class StubXBlockWithMutableFields(StubXBlock): + """ + Stub XBlock used for testing mutable fields and children + """ + has_children = True + + test_mutable_content_field = List( + help="A mutable content field that will be explicitly set", + scope=Scope.content, + ) + + test_mutable_settings_field = List( + help="A mutable settings field that will be explicitly set", + scope=Scope.settings, + ) + + +class UpdateLocationTest(ModuleStoreNoSettings): + """ + Test that updating location preserves "is_set_on" status on fields + """ + CONTENT_FIELDS = ['test_content_field', 'test_mutable_content_field'] + SETTINGS_FIELDS = ['test_settings_field', 'test_mutable_settings_field'] + CHILDREN_FIELDS = ['children'] + + 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', 'mutablestubxblock', '123', 'import') + self.xblock = StubXBlockWithMutableFields(self.runtime, self.field_data, self.scope_ids) + + self.fake_children_locations = [ + BlockUsageLocator(CourseLocator('org', 'course', 'run'), 'mutablestubxblock', 'child1'), + BlockUsageLocator(CourseLocator('org', 'course', 'run'), 'mutablestubxblock', 'child2'), + ] + + super(UpdateLocationTest, self).setUp() + + def _check_explicitly_set(self, block, scope, expected_explicitly_set_fields, should_be_set=False): + """ Gets fields that are explicitly set on block and checks if they are marked as explicitly set or not """ + actual_explicitly_set_fields = block.get_explicitly_set_fields_by_scope(scope=scope) + assertion = self.assertIn if should_be_set else self.assertNotIn + for field in expected_explicitly_set_fields: + assertion(field, actual_explicitly_set_fields) + + def test_update_locations_native_xblock(self): + """ Update locations updates location and keeps values and "is_set_on" status """ + # Set the XBlock's location + self.xblock.location = Location("org", "import", "run", "category", "stubxblock") + + # Explicitly set the content, settings and children fields + self.xblock.test_content_field = 'Explicitly set' + self.xblock.test_settings_field = 'Explicitly set' + self.xblock.test_mutable_content_field = [1, 2, 3] + self.xblock.test_mutable_settings_field = ["a", "s", "d"] + self.xblock.children = self.fake_children_locations # pylint:disable=attribute-defined-outside-init + self.xblock.save() + + # Update location + target_location = self.xblock.location.replace(revision='draft') + _update_module_location(self.xblock, target_location) + new_version = self.xblock # _update_module_location updates in-place + + # Check the XBlock's location + self.assertEqual(new_version.location, target_location) + + # Check the values of the fields. + # The content, settings and children fields should be preserved + self.assertEqual(new_version.test_content_field, 'Explicitly set') + self.assertEqual(new_version.test_settings_field, 'Explicitly set') + self.assertEqual(new_version.test_mutable_content_field, [1, 2, 3]) + self.assertEqual(new_version.test_mutable_settings_field, ["a", "s", "d"]) + self.assertEqual(new_version.children, self.fake_children_locations) + + # Expect that these fields are marked explicitly set + self._check_explicitly_set(new_version, Scope.content, self.CONTENT_FIELDS, should_be_set=True) + self._check_explicitly_set(new_version, Scope.settings, self.SETTINGS_FIELDS, should_be_set=True) + self._check_explicitly_set(new_version, Scope.children, self.CHILDREN_FIELDS, should_be_set=True) + + # Expect these fields pass "is_set_on" test + for field in self.CONTENT_FIELDS + self.SETTINGS_FIELDS + self.CHILDREN_FIELDS: + self.assertTrue(new_version.fields[field].is_set_on(new_version)) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index fb6b7838a4..c1b0598a84 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -1154,7 +1154,8 @@ def _update_module_location(module, new_location): else: rekey_fields = ( module.get_explicitly_set_fields_by_scope(Scope.content).keys() + - module.get_explicitly_set_fields_by_scope(Scope.settings).keys() + module.get_explicitly_set_fields_by_scope(Scope.settings).keys() + + module.get_explicitly_set_fields_by_scope(Scope.children).keys() ) module.location = new_location diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index d321183375..b4c7e94883 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -492,10 +492,13 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase): # sure if there's a way to poke the test harness to do so. So, we'll # just inject the override field storage in this brute force manner. OverrideFieldData.provider_classes = None + # pylint: disable=protected-access for block in iter_blocks(course): - block._field_data = OverrideFieldData.wrap( # pylint: disable=protected-access - coach, block._field_data) # pylint: disable=protected-access - block._field_data_cache = {'tabs': [], 'discussion_topics': []} # pylint: disable=protected-access + block._field_data = OverrideFieldData.wrap(coach, block._field_data) + new_cache = {'tabs': [], 'discussion_topics': []} + if 'grading_policy' in block._field_data_cache: + new_cache['grading_policy'] = block._field_data_cache['grading_policy'] + block._field_data_cache = new_cache def cleanup_provider_classes(): """ diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 48de1ded3e..3368a251b7 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -27,7 +27,7 @@ git+https://github.com/mfogel/django-settings-context-processor.git@b758c3930862 git+https://github.com/pmitros/pyfs.git@96e1922348bfe6d99201b9512a9ed946c87b7e0b # Our libraries: --e git+https://github.com/edx/XBlock.git@aed464a0e2f7478e93157150ac04133a745f5f46#egg=XBlock +-e git+https://github.com/edx/XBlock.git@1934a2978cdd3e2414486c74b76e3040ff1fb138#egg=XBlock -e git+https://github.com/edx/codejail.git@6b17c33a89bef0ac510926b1d7fea2748b73aadd#egg=codejail -e git+https://github.com/edx/js-test-tool.git@v0.1.6#egg=js_test_tool -e git+https://github.com/edx/event-tracking.git@0.1.0#egg=event-tracking