Skip to content

E
edx-platform
Commit b0b8257e by Christina Roberts
Options

Merge pull request #2543 from edx/christina/reference_bug 

Rewrite Reference and ReferenceList fields on import
parents 302b5746 894c40b8
Showing with 100 additions and 26 deletions
CHANGELOG.rst
......@@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes,
in roughly chronological order, most recent first. Add your entries at or near
the top. Include a label indicating the component affected.
Studo: Fix import/export bug with conditional modules. STUD-149
Blades: Persist student progress in video. BLD-385.
Blades: Fix for the list metadata editor that gets into a bad state where "Add"
......
cms/djangoapps/contentstore/tests/test_import_nostatic.py → cms/djangoapps/contentstore/tests/test_import.py
#pylint: disable=E1101
'''
Tests for importing with no static
'''
"""
Tests for import_from_xml using the mongo modulestore.
"""
from django.test.client import Client
from django.test.utils import override_settings
......@@ -32,7 +32,7 @@ TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().
@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE, MODULESTORE=TEST_MODULESTORE)
class ContentStoreImportNoStaticTest(ModuleStoreTestCase):
class ContentStoreImportTest(ModuleStoreTestCase):
"""
Tests that rely on the toy and test_import_course courses.
NOTE: refactor using CourseFactory so they do not.
......@@ -130,7 +130,52 @@ class ContentStoreImportNoStaticTest(ModuleStoreTestCase):
self.assertIn('/static/', handouts.data)
def test_tab_name_imports_correctly(self):
module_store, content_store, course, course_location = self.load_test_import_course()
_module_store, _content_store, course, _course_location = self.load_test_import_course()
print "course tabs = {0}".format(course.tabs)
self.assertEqual(course.tabs[2]['name'],'Syllabus')
self.assertEqual(course.tabs[2]['name'], 'Syllabus')
def test_rewrite_reference_list(self):
module_store = modulestore('direct')
target_location = Location(['i4x', 'testX', 'conditional_copy', 'course', 'copy_run'])
import_from_xml(
module_store,
'common/test/data/',
['conditional'],
target_location_namespace=target_location
)
conditional_module = module_store.get_item(
Location(['i4x', 'testX', 'conditional_copy', 'conditional', 'condone'])
)
self.assertIsNotNone(conditional_module)
self.assertListEqual(
[
u'i4x://testX/conditional_copy/problem/choiceprob',
u'i4x://edX/different_course/html/for_testing_import_rewrites'
],
conditional_module.sources_list
)
self.assertListEqual(
[
u'i4x://testX/conditional_copy/html/congrats',
u'i4x://testX/conditional_copy/html/secret_page'
],
conditional_module.show_tag_list
)
def test_rewrite_reference(self):
module_store = modulestore('direct')
target_location = Location(['i4x', 'testX', 'peergrading_copy', 'course', 'copy_run'])
import_from_xml(
module_store,
'common/test/data/',
['open_ended'],
target_location_namespace=target_location
)
peergrading_module = module_store.get_item(
Location(['i4x', 'testX', 'peergrading_copy', 'peergrading', 'PeerGradingLinked'])
)
self.assertIsNotNone(peergrading_module)
self.assertEqual(
u'i4x://testX/peergrading_copy/combinedopenended/SampleQuestion',
peergrading_module.link_to_location
)
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 xblock.fields import Scope, Reference, ReferenceList
from xmodule.contentstore.content import StaticContent
from .inheritance import own_metadata
from xmodule.errortracker import make_error_tracker
......@@ -424,7 +425,7 @@ def import_course_draft(
# aka sequential), so we have to replace the location.name
# with the XML filename that is part of the pack
fn, fileExtension = os.path.splitext(filename)
descriptor.location = descriptor.location._replace(name=fn)
descriptor.location = descriptor.location.replace(name=fn)
index = int(descriptor.xml_attributes['index_in_children_list'])
if index in drafts:
......@@ -442,13 +443,13 @@ def import_course_draft(
for descriptor in drafts[key]:
try:
def _import_module(module):
module.location = module.location._replace(revision='draft')
module.location = module.location.replace(revision='draft')
# make sure our parent has us in its list of children
# this is to make sure private only verticals show up
# in the list of children since they would have been
# filtered out from the non-draft store export
if module.location.category == 'vertical':
non_draft_location = module.location._replace(revision=None)
non_draft_location = module.location.replace(revision=None)
sequential_url = module.xml_attributes['parent_sequential_url']
index = int(module.xml_attributes['index_in_children_list'])
......@@ -456,7 +457,7 @@ def import_course_draft(
# IMPORTANT: Be sure to update the sequential
# in the NEW namespace
seq_location = seq_location._replace(
seq_location = seq_location.replace(
org=target_location_namespace.org,
course=target_location_namespace.course
)
......@@ -486,20 +487,21 @@ def remap_namespace(module, target_location_namespace):
if target_location_namespace is None:
return module
original_location = module.location
# 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':
module.location = module.location._replace(
module.location = module.location.replace(
tag=target_location_namespace.tag,
org=target_location_namespace.org,
course=target_location_namespace.course
)
else:
original_location = module.location
#
# module is a course module
#
module.location = module.location._replace(
module.location = module.location.replace(
tag=target_location_namespace.tag,
org=target_location_namespace.org,
course=target_location_namespace.course,
......@@ -524,22 +526,41 @@ def remap_namespace(module, target_location_namespace):
module.save()
# then remap children pointers since they too will be re-namespaced
all_fields = module.get_explicitly_set_fields_by_scope(Scope.content)
all_fields.update(module.get_explicitly_set_fields_by_scope(Scope.settings))
if hasattr(module, 'children'):
children_locs = module.children
if children_locs is not None and children_locs != []:
new_locs = []
for child in children_locs:
child_loc = Location(child)
new_child_loc = child_loc._replace(
all_fields['children'] = module.children
def convert_ref(reference):
"""
Convert a reference to the new namespace, but only
if the original namespace matched the original course.
Otherwise, returns the input value.
"""
new_ref = reference
ref = Location(reference)
in_original_namespace = (original_location.tag == ref.tag and
original_location.org == ref.org and
original_location.course == ref.course)
if in_original_namespace:
new_ref = ref.replace(
tag=target_location_namespace.tag,
org=target_location_namespace.org,
course=target_location_namespace.course
)
new_locs.append(new_child_loc.url())
).url()
return new_ref
module.children = new_locs
for field in all_fields:
if isinstance(module.fields.get(field), Reference):
new_ref = convert_ref(getattr(module, field))
setattr(module, field, new_ref)
module.save()
elif isinstance(module.fields.get(field), ReferenceList):
references = getattr(module, field)
new_references = [convert_ref(reference) for reference in references]
setattr(module, field, new_references)
module.save()
return module
......
common/test/data/conditional/README.md
course for testing conditional module
Course for testing conditional module.
Note that 'i4x://edX/different_course/html/for_testing_import_rewrites' in sources
is only present for testing that import does NOT rewrite references to
courses that differ from the original course being imported.
It is not expected that i4x://edX/different_course/html/for_testing_import_rewrites will render.
common/test/data/conditional/conditional/condone.xml
<conditional correct="True" sources="i4x://edX/cond_test/problem/choiceprob">
<conditional correct="True" sources="i4x://edX/cond_test/problem/choiceprob; i4x://edX/different_course/html/for_testing_import_rewrites">
<html>Conditionally shown page</html>
<show sources="i4x://edX/cond_test/html/congrats; i4x://edX/cond_test/html/secret_page"/>
</conditional>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment