From 80fc91bf1ad27639b3f86cdd009500edf48ddafc Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 9 Feb 2016 17:02:05 -0500 Subject: [PATCH] - Add test content library XML. - Ensure that only a course import is saved to the draft branch and then published. A content library has only a single 'library' branch. - Add a fields parameter to the create_library call so the call has the correct number of parameters. - Add tests which import content libraries using different test methods and with different branch settings. - Add test which imports a content library and then exports it. - Use XBlock module version that supports XML-serialized String of None. - Add re-import of content library and equality comparison to test. - Allow get_items to be called on LibraryLocators. --- .../views/tests/test_import_export.py | 135 +++++++++++++++++- .../xmodule/modulestore/split_mongo/split.py | 5 +- .../modulestore/split_mongo/split_draft.py | 8 +- .../xmodule/modulestore/xml_importer.py | 1 + .../data/library_empty_problem/library.xml | 4 + .../policies/assets.json | 1 + .../afe9dbb29b724181944f56617c72b3e5.xml | 1 + .../ba28f97e8f33414e9a5de0068508f7fa.xml | 28 ++++ requirements/edx/github.txt | 2 +- 9 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 common/test/data/library_empty_problem/library.xml create mode 100644 common/test/data/library_empty_problem/policies/assets.json create mode 100644 common/test/data/library_empty_problem/problem/afe9dbb29b724181944f56617c72b3e5.xml create mode 100644 common/test/data/library_empty_problem/problem/ba28f97e8f33414e9a5de0068508f7fa.xml diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 7568c9ca5b..f934efba62 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -2,6 +2,7 @@ Unit tests for course import and export """ import copy +import ddt import json import logging import lxml @@ -15,20 +16,24 @@ from uuid import uuid4 from django.test.utils import override_settings from django.conf import settings 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 import LIBRARY_ROOT +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.utils import ( + MongoContentstoreBuilder, SPLIT_MODULESTORE_SETUP, TEST_DATA_DIR +) +from opaque_keys.edx.locator import LibraryLocator -from contentstore.tests.utils import CourseTestCase from openedx.core.lib.extract_tar import safetar_extractall from student import auth from student.roles import CourseInstructorRole, CourseStaffRole from models.settings.course_metadata import CourseMetadata from util import milestones_helpers -from xmodule.modulestore.django import modulestore from milestones.tests.utils import MilestonesTestCaseMixin TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) @@ -123,6 +128,7 @@ class ImportEntranceExamTestCase(CourseTestCase, MilestonesTestCaseMixin): self.assertEquals(course.entrance_exam_minimum_score_pct, 0.7) +@ddt.ddt @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) class ImportTestCase(CourseTestCase): """ @@ -435,6 +441,73 @@ class ImportTestCase(CourseTestCase): self.assertIn(test_block3.url_name, children) self.assertIn(test_block4.url_name, children) + @ddt.data( + ModuleStoreEnum.Branch.draft_preferred, + ModuleStoreEnum.Branch.published_only, + ) + def test_library_import_branch_settings(self, branch_setting): + """ + Try importing a known good library archive under either branch setting. + The branch setting should have no effect on library import. + """ + with self.store.branch_setting(branch_setting): + library = LibraryFactory.create(modulestore=self.store) + lib_key = library.location.library_key + extract_dir = path(tempfile.mkdtemp(dir=settings.DATA_DIR)) + # the extract_dir needs to be passed as a relative dir to + # import_library_from_xml + extract_dir_relative = path.relpath(extract_dir, settings.DATA_DIR) + + try: + with tarfile.open(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz') as tar: + safetar_extractall(tar, extract_dir) + import_library_from_xml( + self.store, + self.user.id, + settings.GITHUB_REPO_ROOT, + [extract_dir_relative / 'library'], + load_error_modules=False, + static_content_store=contentstore(), + target_id=lib_key + ) + finally: + shutil.rmtree(extract_dir) + + @ddt.data( + ModuleStoreEnum.Branch.draft_preferred, + ModuleStoreEnum.Branch.published_only, + ) + def test_library_import_branch_settings_again(self, branch_setting): + # Construct the contentstore for storing the import + with MongoContentstoreBuilder().build() as source_content: + # Construct the modulestore for storing the import (using the previously created contentstore) + with SPLIT_MODULESTORE_SETUP.build(contentstore=source_content) as source_store: + # Use the test branch setting. + with source_store.branch_setting(branch_setting): + source_library_key = LibraryLocator(org='TestOrg', library='TestProbs') + + extract_dir = path(tempfile.mkdtemp(dir=settings.DATA_DIR)) + # the extract_dir needs to be passed as a relative dir to + # import_library_from_xml + extract_dir_relative = path.relpath(extract_dir, settings.DATA_DIR) + + try: + with tarfile.open(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz') as tar: + safetar_extractall(tar, extract_dir) + import_library_from_xml( + source_store, + self.user.id, + settings.GITHUB_REPO_ROOT, + [extract_dir_relative / 'library'], + static_content_store=source_content, + target_id=source_library_key, + load_error_modules=False, + raise_on_failure=True, + create_if_not_present=True, + ) + finally: + shutil.rmtree(extract_dir) + @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) class ExportTestCase(CourseTestCase): @@ -556,3 +629,59 @@ class ExportTestCase(CourseTestCase): ) self.test_export_targz_urlparam() + + +@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) +class TestLibraryImportExport(CourseTestCase): + """ + Tests for importing content libraries from XML and exporting them to XML. + """ + def setUp(self): + super(TestLibraryImportExport, self).setUp() + self.export_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.export_dir, ignore_errors=True) + + def test_content_library_export_import(self): + library1 = LibraryFactory.create(modulestore=self.store) + source_library1_key = library1.location.library_key + library2 = LibraryFactory.create(modulestore=self.store) + source_library2_key = library2.location.library_key + + import_library_from_xml( + self.store, + 'test_user', + TEST_DATA_DIR, + ['library_empty_problem'], + static_content_store=contentstore(), + target_id=source_library1_key, + load_error_modules=False, + raise_on_failure=True, + create_if_not_present=True, + ) + + export_library_to_xml( + self.store, + contentstore(), + source_library1_key, + self.export_dir, + 'exported_source_library', + ) + + source_library = self.store.get_library(source_library1_key) + self.assertEqual(source_library.url_name, 'library') + + # Import the exported library into a different content library. + import_library_from_xml( + self.store, + 'test_user', + self.export_dir, + ['exported_source_library'], + static_content_store=contentstore(), + target_id=source_library2_key, + load_error_modules=False, + raise_on_failure=True, + create_if_not_present=True, + ) + + # Compare the two content libraries for equality. + self.assertCoursesEqual(source_library1_key, source_library2_key) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 71ebc3351b..0ad465991c 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 opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import ( BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, ) @@ -1160,8 +1161,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): False - if we want only those items which are in the course tree. This would ensure no orphans are fetched. """ - if not isinstance(course_locator, CourseLocator) or course_locator.deprecated: - # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. + if not isinstance(course_locator, CourseKey) or course_locator.deprecated: + # The supplied courselike key is of the wrong type, so it can't possibly be stored in this modulestore. return [] course = self._lookup_course(course_locator) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index dc25acf7ac..5d25886035 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -544,9 +544,11 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli block_id = self.DEFAULT_ROOT_LIBRARY_BLOCK_ID new_usage_key = course_key.make_usage_key(block_type, block_id) - # Only the course import process calls import_xblock(). If the branch setting is published_only, - # then the non-draft blocks are being imported. - if self.get_branch_setting() == ModuleStoreEnum.Branch.published_only: + # Both the course and library import process calls import_xblock(). + # If importing a course -and- the branch setting is published_only, + # then the non-draft course blocks are being imported. + is_course = isinstance(course_key, CourseLocator) + if is_course and self.get_branch_setting() == ModuleStoreEnum.Branch.published_only: # Override any existing drafts (PLAT-297, PLAT-299). This import/publish step removes # any local changes during the course import. draft_course = course_key.for_branch(ModuleStoreEnum.BranchName.draft) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 8ef58dbea6..0e9d388ca9 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -607,6 +607,7 @@ class LibraryImportManager(ImportManager): org=self.target_id.org, library=self.target_id.library, user_id=self.user_id, + fields={"display_name": ""}, ) runtime = library.runtime except DuplicateCourseError: diff --git a/common/test/data/library_empty_problem/library.xml b/common/test/data/library_empty_problem/library.xml new file mode 100644 index 0000000000..7fd662c43b --- /dev/null +++ b/common/test/data/library_empty_problem/library.xml @@ -0,0 +1,4 @@ + + + + diff --git a/common/test/data/library_empty_problem/policies/assets.json b/common/test/data/library_empty_problem/policies/assets.json new file mode 100644 index 0000000000..9e26dfeeb6 --- /dev/null +++ b/common/test/data/library_empty_problem/policies/assets.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/common/test/data/library_empty_problem/problem/afe9dbb29b724181944f56617c72b3e5.xml b/common/test/data/library_empty_problem/problem/afe9dbb29b724181944f56617c72b3e5.xml new file mode 100644 index 0000000000..93091b7fe6 --- /dev/null +++ b/common/test/data/library_empty_problem/problem/afe9dbb29b724181944f56617c72b3e5.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/library_empty_problem/problem/ba28f97e8f33414e9a5de0068508f7fa.xml b/common/test/data/library_empty_problem/problem/ba28f97e8f33414e9a5de0068508f7fa.xml new file mode 100644 index 0000000000..fce7a6b217 --- /dev/null +++ b/common/test/data/library_empty_problem/problem/ba28f97e8f33414e9a5de0068508f7fa.xml @@ -0,0 +1,28 @@ + +

Multiple choice problems allow learners to select only one option. + Learners can see all the options along with the problem text.

+

When you add the problem, be sure to select Settings +to specify a Display Name and other values that apply.

+

You can use the following example problem as a model.

+

Which of the following countries has the largest population?

+ + + Brazil + timely feedback -- explain why an almost correct answer is wrong + + Germany + Indonesia + Russia + + + +
+

Explanation

+

According to September 2014 estimates:

+

The population of Indonesia is approximately 250 million.

+

The population of Brazil is approximately 200 million.

+

The population of Russia is approximately 146 million.

+

The population of Germany is approximately 81 million.

+
+ + diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index c45797f539..c9781dc518 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -70,7 +70,7 @@ git+https://github.com/edx/rfc6266.git@v0.0.5-edx#egg=rfc6266==0.0.5-edx git+https://github.com/edx/lettuce.git@0.2.20.002#egg=lettuce==0.2.20.002 # Our libraries: -git+https://github.com/edx/XBlock.git@xblock-0.4.4#egg=XBlock==0.4.4 +git+https://github.com/edx/XBlock.git@xblock-0.4.5#egg=XBlock==0.4.5 -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.2.1#egg=event-tracking==0.2.1