From 210f9328da6354f8df9e70ebb1d6c7ca8933a9d4 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Tue, 21 Jun 2016 15:56:06 +0500 Subject: [PATCH 1/2] Refersh library children for the block during import. TNL-4826 --- .../lib/xmodule/xmodule/modulestore/split_mongo/split.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 2718fc793d..aa531499f2 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -68,6 +68,7 @@ from xblock.core import XBlock from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xmodule.course_module import CourseSummary from xmodule.errortracker import null_error_tracker +from xmodule.library_tools import LibraryToolsService from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import ( BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, @@ -2105,7 +2106,12 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # fetch and return the new item--fetching is unnecessary but a good qc step new_locator = course_key.make_usage_key(block_key.type, block_key.id) - return self.get_item(new_locator, **kwargs) + new_item = self.get_item(new_locator, **kwargs) + + if block_key.type == 'library_content': + # Update imported xblocks' 'source_library_version' to keep it up to date. + LibraryToolsService(self).update_children(new_item, user_id) + return new_item else: return None From aca82be33533a17a049b7b5063f4b4110a2bdc90 Mon Sep 17 00:00:00 2001 From: Qubad786 Date: Fri, 22 Jul 2016 17:02:36 +0500 Subject: [PATCH 2/2] Move fix to importer module and fix branch_setting in MixedModuleStore. --- .../contentstore/tests/test_libraries.py | 6 +- .../views/tests/test_import_export.py | 116 +++++++++++++++++- .../lib/xmodule/xmodule/modulestore/mixed.py | 9 +- .../xmodule/modulestore/split_mongo/split.py | 8 +- .../tests/test_mixed_modulestore.py | 8 ++ .../xmodule/modulestore/xml_importer.py | 19 ++- 6 files changed, 150 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py index b33c7b6356..6f95cd26da 100644 --- a/cms/djangoapps/contentstore/tests/test_libraries.py +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -64,7 +64,7 @@ class LibraryTestCase(ModuleStoreTestCase): self.assertIsInstance(lib_key, LibraryLocator) return lib_key - def _add_library_content_block(self, course, library_key, other_settings=None): + def _add_library_content_block(self, course, library_key, publish_item=False, other_settings=None): """ Helper method to add a LibraryContent block to a course. The block will be configured to select content from the library @@ -75,7 +75,7 @@ class LibraryTestCase(ModuleStoreTestCase): category='library_content', parent_location=course.location, user_id=self.user.id, - publish_item=False, + publish_item=publish_item, source_library_id=unicode(library_key), **(other_settings or {}) ) @@ -159,7 +159,7 @@ class TestLibraries(LibraryTestCase): with modulestore().default_store(ModuleStoreEnum.Type.split): course = CourseFactory.create() - lc_block = self._add_library_content_block(course, self.lib_key, {'max_count': num_to_select}) + lc_block = self._add_library_content_block(course, self.lib_key, other_settings={'max_count': num_to_select}) self.assertEqual(len(lc_block.children), 0) lc_block = self._refresh_children(lc_block) diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 94b022480c..9fcba609e1 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -15,15 +15,17 @@ from uuid import uuid4 from django.test.utils import override_settings from django.conf import settings + +from contentstore.tests.test_libraries import LibraryTestCase from xmodule.contentstore.django import contentstore from xmodule.modulestore.django import modulestore -from xmodule.modulestore.xml_exporter import export_library_to_xml -from xmodule.modulestore.xml_importer import import_library_from_xml +from xmodule.modulestore.xml_exporter import export_library_to_xml, export_course_to_xml +from xmodule.modulestore.xml_importer import import_library_from_xml, import_course_from_xml from xmodule.modulestore import LIBRARY_ROOT, ModuleStoreEnum from contentstore.utils import reverse_course_url from contentstore.tests.utils import CourseTestCase -from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory +from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory, CourseFactory from xmodule.modulestore.tests.utils import ( MongoContentstoreBuilder, SPLIT_MODULESTORE_SETUP, TEST_DATA_DIR ) @@ -697,3 +699,111 @@ class TestLibraryImportExport(CourseTestCase): # Compare the two content libraries for equality. self.assertCoursesEqual(source_library1_key, source_library2_key) + + +@ddt.ddt +@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) +class TestCourseExportImport(LibraryTestCase): + """ + Tests for importing after exporting the course containing content libraries from XML. + """ + def setUp(self): + super(TestCourseExportImport, self).setUp() + self.export_dir = tempfile.mkdtemp() + + # Create a problem in library + ItemFactory.create( + category="problem", + parent_location=self.library.location, + user_id=self.user.id, # pylint: disable=no-member + publish_item=False, + display_name='Test Problem', + data="", + ) + + # Create a source course. + self.source_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) + self.addCleanup(shutil.rmtree, self.export_dir, ignore_errors=True) + + def _setup_source_course_with_library_content(self, publish=False): + """ + Sets up course with library content. + """ + chapter = ItemFactory.create( + parent_location=self.source_course.location, + category='chapter', + display_name='Test Section' + ) + sequential = ItemFactory.create( + parent_location=chapter.location, + category='sequential', + display_name='Test Sequential' + ) + vertical = ItemFactory.create( + category='vertical', + parent_location=sequential.location, + display_name='Test Unit' + ) + lc_block = self._add_library_content_block(vertical, self.lib_key, publish_item=publish) + self._refresh_children(lc_block) + + def get_lib_content_block_children(self, block_location): + """ + Search for library content block to return its immediate children + """ + if block_location.block_type == 'library_content': + return self.store.get_item(block_location).children + + return self.get_lib_content_block_children(self.store.get_item(block_location).children[0]) + + def assert_problem_display_names(self, source_course_location, dest_course_location): + """ + Asserts that problems' display names in both source and destination courses are same. + """ + source_course_lib_children = self.get_lib_content_block_children(source_course_location) + dest_course_lib_children = self.get_lib_content_block_children(dest_course_location) + + self.assertEquals(len(source_course_lib_children), len(dest_course_lib_children)) + + for source_child_location, dest_child_location in zip(source_course_lib_children, dest_course_lib_children): + source_child = self.store.get_item(source_child_location) + dest_child = self.store.get_item(dest_child_location) + self.assertEquals(source_child.display_name, dest_child.display_name) + + @ddt.data(True, False) + def test_library_content_on_course_export_import(self, publish_item): + """ + Verify that library contents in destination and source courses are same after importing + the source course into destination course. + """ + self._setup_source_course_with_library_content(publish=publish_item) + + # Create a course to import source course. + dest_course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) + + # Export the source course. + export_course_to_xml( + self.store, + contentstore(), + self.source_course.location.course_key, + self.export_dir, + 'exported_source_course', + ) + + # Now, import it back to dest_course. + import_course_from_xml( + self.store, + self.user.id, # pylint: disable=no-member + self.export_dir, + ['exported_source_course'], + static_content_store=contentstore(), + target_id=dest_course.location.course_key, + load_error_modules=False, + raise_on_failure=True, + create_if_not_present=True, + ) + + self.assert_problem_display_names( + self.source_course.location, + dest_course.location + ) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 0f726bbaf0..14c5484fd0 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -980,8 +980,13 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): to the given branch_setting. If course_id is None, the default store is used. """ store = self._verify_modulestore_support(course_id, 'branch_setting') - with store.branch_setting(branch_setting, course_id): - yield + previous_thread_branch_setting = getattr(self.thread_cache, 'branch_setting', None) + try: + self.thread_cache.branch_setting = branch_setting + with store.branch_setting(branch_setting, course_id): + yield + finally: + self.thread_cache.branch_setting = previous_thread_branch_setting @contextmanager def bulk_operations(self, course_id, emit_signals=True): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index aa531499f2..2718fc793d 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -68,7 +68,6 @@ from xblock.core import XBlock from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xmodule.course_module import CourseSummary from xmodule.errortracker import null_error_tracker -from xmodule.library_tools import LibraryToolsService from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import ( BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, @@ -2106,12 +2105,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # fetch and return the new item--fetching is unnecessary but a good qc step new_locator = course_key.make_usage_key(block_key.type, block_key.id) - new_item = self.get_item(new_locator, **kwargs) - - if block_key.type == 'library_content': - # Update imported xblocks' 'source_library_version' to keep it up to date. - LibraryToolsService(self).update_children(new_item, user_id) - return new_item + return self.get_item(new_locator, **kwargs) else: return None diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 43dbdd18ee..7e37dd80eb 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1991,6 +1991,14 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): # there should be no published problems with the old name assertNumProblems(problem_original_name, 0) + # verify branch setting is published-only in manager + with self.store.branch_setting(ModuleStoreEnum.Branch.published_only): + self.assertEquals(self.store.get_branch_setting(), ModuleStoreEnum.Branch.published_only) + + # verify branch setting is draft-preferred in manager + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred): + self.assertEquals(self.store.get_branch_setting(), ModuleStoreEnum.Branch.draft_preferred) + def verify_default_store(self, store_type): """ Verifies the default_store property diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 4febc7c546..f81fc04b25 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -30,6 +30,7 @@ import json import re from lxml import etree +from xmodule.library_tools import LibraryToolsService from xmodule.modulestore.xml import XMLModuleStore, LibraryXMLModuleStore, ImportSystem from xblock.runtime import KvsFieldData, DictKeyValueStore from xmodule.x_module import XModuleDescriptor, XModuleMixin @@ -739,11 +740,27 @@ def _update_and_import_module( fields = _update_module_references(module, source_course_id, dest_course_id) asides = module.get_asides() if isinstance(module, XModuleMixin) else None - return store.import_xblock( + block = store.import_xblock( user_id, dest_course_id, module.location.category, module.location.block_id, fields, runtime, asides=asides ) + # TODO: Move this code once the following condition is met. + # Get to the point where XML import is happening inside the + # modulestore that is eventually going to store the data. + # Ticket: https://openedx.atlassian.net/browse/PLAT-1046 + if block.location.category == 'library_content': + # if library exists, update source_library_version and children + # according to this existing library and library content block. + if store.get_library(block.source_library_key): + LibraryToolsService(store).update_children( + block, + user_id, + version=block.source_library_version + ) + + return block + def _import_course_draft( xml_module_store,