Skip to content

E
edx-platform
Commit 6c49e0db by Christina Roberts
Options

Merge pull request #2105 from edx/christina/reorder-privates 

Fix bug about draft verticals reordering.
parents 9abe8ae3 38c516e9
Showing with 117 additions and 9 deletions
cms/djangoapps/contentstore/tests/test_import_draft_order.py 0 → 100644
from django.test.utils import override_settings
from xmodule.modulestore.xml_importer import import_from_xml
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.django import modulestore
from xmodule.modulestore import Location
from contentstore.tests.modulestore_config import TEST_MODULESTORE
# This test is in the CMS module because the test configuration to use a draft
# modulestore is dependent on django.
@override_settings(MODULESTORE=TEST_MODULESTORE)
class DraftReorderTestCase(ModuleStoreTestCase):
def test_order(self):
store = modulestore('direct')
draft_store = modulestore('default')
import_from_xml(store, 'common/test/data/', ['import_draft_order'], draft_store=draft_store)
sequential = draft_store.get_item(
Location('i4x', 'test_org', 'import_draft_order', 'sequential', '0f4f7649b10141b0bdc9922dcf94515a', None)
)
verticals = sequential.children
# The order that files are read in from the file system is not guaranteed (cannot rely on
# alphabetical ordering, for example). Therefore, I have added a lot of variation in filename and desired
# ordering so that the test reliably failed with the bug, at least on Linux.
#
# 'a', 'b', 'c', 'd', and 'z' are all drafts, with 'index_in_children_list' of
# 2 , 4 , 6 , 5 , and 0 respectively.
#
# '5a05be9d59fc4bb79282c94c9e6b88c7' and 'second' are public verticals.
self.assertEqual(7, len(verticals))
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/z', verticals[0])
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/5a05be9d59fc4bb79282c94c9e6b88c7', verticals[1])
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/a', verticals[2])
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/second', verticals[3])
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/b', verticals[4])
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/d', verticals[5])
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/c', verticals[6])
# Now also test that the verticals in a second sequential are correct.
sequential = draft_store.get_item(
Location('i4x', 'test_org', 'import_draft_order', 'sequential', 'secondseq', None)
)
verticals = sequential.children
# 'asecond' and 'zsecond' are drafts with 'index_in_children_list' 0 and 2, respectively.
# 'secondsubsection' is a public vertical.
self.assertEqual(3, len(verticals))
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/asecond', verticals[0])
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/secondsubsection', verticals[1])
self.assertEqual(u'i4x://test_org/import_draft_order/vertical/zsecond', verticals[2])
common/lib/xmodule/xmodule/modulestore/xml_importer.py
......@@ -381,10 +381,18 @@ def import_course_draft(
# create a new 'System' object which will manage the importing
errorlog = make_error_tracker()
# The course_dir as passed to ImportSystem is expected to just be relative, not
# the complete path including data_dir. ImportSystem will concatenate the two together.
data_dir = xml_module_store.data_dir
# Whether or not data_dir ends with a "/" differs in production vs. test.
if not data_dir.endswith("/"):
data_dir += "/"
draft_course_dir = draft_dir.replace(data_dir, '', 1)
system = ImportSystem(
xmlstore=xml_module_store,
course_id=target_location_namespace.course_id,
course_dir=draft_dir,
course_dir=draft_course_dir,
policy={},
error_tracker=errorlog.tracker,
parent_tracker=ParentTracker(),
......@@ -393,6 +401,10 @@ def import_course_draft(
# now walk the /vertical directory where each file in there
# will be a draft copy of the Vertical
# First it is necessary to order the draft items by their desired index in the child list
# (order os.walk returns them in is not guaranteed).
drafts = dict()
for dirname, dirnames, filenames in os.walk(draft_dir + "/vertical"):
for filename in filenames:
module_path = os.path.join(dirname, filename)
......@@ -434,6 +446,29 @@ def import_course_draft(
descriptor = system.process_xml(xml)
# HACK: since we are doing partial imports of drafts
# the vertical doesn't have the 'url-name' set in the
# attributes (they are normally in the parent object,
# 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)
index = int(descriptor.xml_attributes['index_in_children_list'])
if index in drafts:
drafts[index].append(descriptor)
else:
drafts[index] = [descriptor]
except Exception, e:
logging.exception('There was an error. {err}'.format(
err=unicode(e)
))
# For each index_in_children_list key, there is a list of vertical descriptors.
for key in sorted(drafts.iterkeys()):
for descriptor in drafts[key]:
try:
def _import_module(module):
module.location = module.location._replace(revision='draft')
# make sure our parent has us in its list of children
......@@ -467,14 +502,6 @@ def import_course_draft(
for child in module.get_children():
_import_module(child)
# HACK: since we are doing partial imports of drafts
# the vertical doesn't have the 'url-name' set in the
# attributes (they are normally in the parent object,
# 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)
_import_module(descriptor)
except Exception, e:
......
common/test/data/import_draft_order/chapter/3247df3732ea492380e45a4ea1918ffa.xml 0 → 100644
<chapter display_name="New Section Name">
<sequential url_name="0f4f7649b10141b0bdc9922dcf94515a"/>
<sequential url_name="secondseq"/>
</chapter>
common/test/data/import_draft_order/course.xml 0 → 100644
<course url_name="import_draft_order" org="test_org" course="import_draft_order"/>
common/test/data/import_draft_order/course/import_draft_order.xml 0 → 100644
<course display_name="reorder privates">
<chapter url_name="3247df3732ea492380e45a4ea1918ffa"/>
</course>
common/test/data/import_draft_order/drafts/vertical/a.xml 0 → 100644
<vertical display_name="Private Unit 2" parent_sequential_url="i4x://test_org/test_course/sequential/0f4f7649b10141b0bdc9922dcf94515a" index_in_children_list="2"/>
common/test/data/import_draft_order/drafts/vertical/asecond.xml 0 → 100644
<vertical display_name="Private Unit 1" parent_sequential_url="i4x://test_org/test_course/sequential/secondseq" index_in_children_list="0"/>
common/test/data/import_draft_order/drafts/vertical/b.xml 0 → 100644
<vertical display_name="Private Unit 3" parent_sequential_url="i4x://test_org/test_course/sequential/0f4f7649b10141b0bdc9922dcf94515a" index_in_children_list="4"/>
common/test/data/import_draft_order/drafts/vertical/c.xml 0 → 100644
<vertical display_name="Private Unit 5" parent_sequential_url="i4x://test_org/test_course/sequential/0f4f7649b10141b0bdc9922dcf94515a" index_in_children_list="6"/>
common/test/data/import_draft_order/drafts/vertical/d.xml 0 → 100644
<vertical display_name="Private Unit 4" parent_sequential_url="i4x://test_org/test_course/sequential/0f4f7649b10141b0bdc9922dcf94515a" index_in_children_list="5"/>
common/test/data/import_draft_order/drafts/vertical/z.xml 0 → 100644
<vertical display_name="Private Unit 1" parent_sequential_url="i4x://test_org/test_course/sequential/0f4f7649b10141b0bdc9922dcf94515a" index_in_children_list="0"/>
common/test/data/import_draft_order/drafts/vertical/zsecond.xml 0 → 100644
<vertical display_name="Private Unit 2" parent_sequential_url="i4x://test_org/test_course/sequential/secondseq" index_in_children_list="2"/>
common/test/data/import_draft_order/policies/import_draft_order/grading_policy.json 0 → 100644
{"GRADER": [{"short_label": "HW", "min_count": 12, "type": "Homework", "drop_count": 2, "weight": 0.15}, {"min_count": 12, "type": "Lab", "drop_count": 2, "weight": 0.15}, {"short_label": "Midterm", "min_count": 1, "type": "Midterm Exam", "drop_count": 0, "weight": 0.3}, {"short_label": "Final", "min_count": 1, "type": "Final Exam", "drop_count": 0, "weight": 0.4}], "GRADE_CUTOFFS": {"Pass": 0.5}}
common/test/data/import_draft_order/policies/import_draft_order/policy.json 0 → 100644
{"course/import_draft_order": {"tabs": [{"type": "courseware", "name": "Courseware"}, {"type": "course_info", "name": "Course Info"}, {"type": "discussion", "name": "Discussion"}, {"type": "wiki", "name": "Wiki"}, {"type": "progress", "name": "Progress"}], "display_name": "reorder privates", "discussion_topics": {"General": {"id": "i4x-test_org-test_course-course-import_draft_order"}}}}
common/test/data/import_draft_order/sequential/0f4f7649b10141b0bdc9922dcf94515a.xml 0 → 100644
<sequential display_name="Subsection" start="1970-01-01T00:00:00Z">
<vertical url_name="5a05be9d59fc4bb79282c94c9e6b88c7"/>
<vertical url_name="second"/>
</sequential>
common/test/data/import_draft_order/sequential/secondseq.xml 0 → 100644
<sequential display_name="Second Subsection" start="1970-01-01T00:00:00Z">
<vertical url_name="secondsubsection"/>
</sequential>
common/test/data/import_draft_order/vertical/second.xml 0 → 100644
<vertical display_name="Public Unit 2"/>
common/test/data/import_draft_order/vertical/secondsubsection.xml 0 → 100644
<vertical display_name="Public Unit in Second Subsection"/>
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