diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py
index 94bfa55b58..b35f00f0e2 100644
--- a/cms/djangoapps/contentstore/views/assets.py
+++ b/cms/djangoapps/contentstore/views/assets.py
@@ -348,6 +348,8 @@ def generate_export_course(request, org, course, name):
try:
export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name, modulestore())
except SerializationError, e:
+ logging.exception('There was an error exporting course {0}. {1}'.format(course_module.location, unicode(e)))
+
unit = None
failed_item = None
parent = None
@@ -380,6 +382,7 @@ def generate_export_course(request, org, course, name):
})
})
except Exception, e:
+ logging.exception('There was an error exporting course {0}. {1}'.format(course_module.location, unicode(e)))
return render_to_response('export.html', {
'context_course': course_module,
'successful_import_redirect_url': '',
diff --git a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js
index e8a30f6e9c..a34f33ba4c 100644
--- a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js
+++ b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js
@@ -312,15 +312,34 @@ function () {
var newIndex;
if (this.videoCaption.loaded) {
- time = Math.round(Time.convert(time, this.speed, '1.0') * 1000 + 250);
+ // Current mode === 'flash' can only be for YouTube videos. So, we
+ // don't have to also check for videoType === 'youtube'.
+ if (this.currentPlayerMode === 'flash') {
+ // Total play time changes with speed change. Also there is
+ // a 250 ms delay we have to take into account.
+ time = Math.round(
+ Time.convert(time, this.speed, '1.0') * 1000 + 250
+ );
+ } else {
+ // Total play time remains constant when speed changes.
+ time = Math.round(parseInt(time, 10) * 1000);
+ }
+
newIndex = this.videoCaption.search(time);
- if (newIndex !== void 0 && this.videoCaption.currentIndex !== newIndex) {
+ if (
+ newIndex !== void 0 &&
+ this.videoCaption.currentIndex !== newIndex
+ ) {
if (this.videoCaption.currentIndex) {
- this.videoCaption.subtitlesEl.find('li.current').removeClass('current');
+ this.videoCaption.subtitlesEl
+ .find('li.current')
+ .removeClass('current');
}
- this.videoCaption.subtitlesEl.find("li[data-index='" + newIndex + "']").addClass('current');
+ this.videoCaption.subtitlesEl
+ .find("li[data-index='" + newIndex + "']")
+ .addClass('current');
this.videoCaption.currentIndex = newIndex;
@@ -333,9 +352,29 @@ function () {
var time;
event.preventDefault();
- time = Math.round(Time.convert($(event.target).data('start'), '1.0', this.speed) / 1000);
- this.trigger('videoPlayer.onCaptionSeek', {'type': 'onCaptionSeek', 'time': time});
+ // Current mode === 'flash' can only be for YouTube videos. So, we
+ // don't have to also check for videoType === 'youtube'.
+ if (this.currentPlayerMode === 'flash') {
+ // Total play time changes with speed change. Also there is
+ // a 250 ms delay we have to take into account.
+ time = Math.round(
+ Time.convert(
+ $(event.target).data('start'), '1.0', this.speed
+ ) / 1000
+ );
+ } else {
+ // Total play time remains constant when speed changes.
+ time = parseInt($(event.target).data('start'), 10)/1000;
+ }
+
+ this.trigger(
+ 'videoPlayer.onCaptionSeek',
+ {
+ 'type': 'onCaptionSeek',
+ 'time': time
+ }
+ );
}
function calculateOffset(element) {
diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py
index 4a13d565cc..a6a7d86510 100644
--- a/common/lib/xmodule/xmodule/tests/test_video.py
+++ b/common/lib/xmodule/xmodule/tests/test_video.py
@@ -15,6 +15,7 @@ the course, section, subsection, unit, etc.
import unittest
from . import LogicTest
+from lxml import etree
from .import get_test_system
from xmodule.modulestore import Location
from xmodule.video_module import VideoDescriptor, _create_youtube_string
@@ -64,6 +65,32 @@ class VideoModuleTest(LogicTest):
'1.25': '',
'1.50': ''})
+ def test_parse_youtube_invalid(self):
+ """Ensure that ids that are invalid return an empty dict"""
+
+ # invalid id
+ youtube_str = 'thisisaninvalidid'
+ output = VideoDescriptor._parse_youtube(youtube_str)
+ self.assertEqual(output, {'0.75': '',
+ '1.00': '',
+ '1.25': '',
+ '1.50': ''})
+ # another invalid id
+ youtube_str = ',::,:,,'
+ output = VideoDescriptor._parse_youtube(youtube_str)
+ self.assertEqual(output, {'0.75': '',
+ '1.00': '',
+ '1.25': '',
+ '1.50': ''})
+
+ # and another one, partially invalid
+ youtube_str = '0.75_BAD!!!,1.0:AXdE34_U,1.25:KLHF9K_Y,1.5:VO3SxfeD,'
+ output = VideoDescriptor._parse_youtube(youtube_str)
+ self.assertEqual(output, {'0.75': '',
+ '1.00': 'AXdE34_U',
+ '1.25': 'KLHF9K_Y',
+ '1.50': 'VO3SxfeD'})
+
def test_parse_youtube_key_format(self):
"""
Make sure that inconsistent speed keys are parsed correctly.
@@ -263,6 +290,62 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'data': ''
})
+ def test_from_xml_double_quotes(self):
+ """
+ Make sure we can handle the double-quoted string format (which was used for exporting for
+ a few weeks).
+ """
+ module_system = DummySystem(load_error_modules=True)
+ xml_data ='''
+
+ '''
+ output = VideoDescriptor.from_xml(xml_data, module_system)
+ self.assert_attributes_equal(output, {
+ 'youtube_id_0_75': 'OEoXaMPEzf65',
+ 'youtube_id_1_0': 'OEoXaMPEzf10',
+ 'youtube_id_1_25': 'OEoXaMPEzf125',
+ 'youtube_id_1_5': 'OEoXaMPEzf15',
+ 'show_captions': False,
+ 'start_time': 0.0,
+ 'end_time': 0.0,
+ 'track': 'http://download_track',
+ 'source': 'http://download_video',
+ 'html5_sources': ["source_1", "source_2"],
+ 'data': ''
+ })
+
+ def test_from_xml_double_quote_concatenated_youtube(self):
+ module_system = DummySystem(load_error_modules=True)
+ xml_data = '''
+
+ '''
+ output = VideoDescriptor.from_xml(xml_data, module_system)
+ self.assert_attributes_equal(output, {
+ 'youtube_id_0_75': '',
+ 'youtube_id_1_0': 'p2Q6BrNhdh8',
+ 'youtube_id_1_25': '1EeWXzPdhSA',
+ 'youtube_id_1_5': '',
+ 'show_captions': True,
+ 'start_time': 0.0,
+ 'end_time': 0.0,
+ 'track': '',
+ 'source': '',
+ 'html5_sources': [],
+ 'data': ''
+ })
+
def test_old_video_format(self):
"""
Test backwards compatibility with VideoModule's XML format.
@@ -344,7 +427,7 @@ class VideoExportTestCase(unittest.TestCase):
desc.track = 'http://www.example.com/track'
desc.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg']
- xml = desc.export_to_xml(None) # We don't use the `resource_fs` parameter
+ xml = desc.definition_to_xml(None) # We don't use the `resource_fs` parameter
expected = dedent('''\
''')
- self.assertEquals(expected, xml)
+ self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
def test_export_to_xml_empty_parameters(self):
"""Test XML export with defaults."""
@@ -361,7 +444,7 @@ class VideoExportTestCase(unittest.TestCase):
location = Location(["i4x", "edX", "video", "default", "SampleProblem1"])
desc = VideoDescriptor(module_system, {'location': location})
- xml = desc.export_to_xml(None)
+ xml = desc.definition_to_xml(None)
expected = '\n'
- self.assertEquals(expected, xml)
+ self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
index 407547d9bf..7830ff77b4 100644
--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -240,7 +240,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
video = cls(system, model_data)
return video
- def export_to_xml(self, resource_fs):
+ def definition_to_xml(self, resource_fs):
"""
Returns an xml string representing this module.
"""
@@ -266,7 +266,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
if key in fields and fields[key].default == getattr(self, key):
continue
if value:
- xml.set(key, str(value))
+ xml.set(key, unicode(value))
for source in self.html5_sources:
ele = etree.Element('source')
@@ -277,7 +277,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
ele = etree.Element('track')
ele.set('src', self.track)
xml.append(ele)
- return etree.tostring(xml, pretty_print=True)
+ return xml
@staticmethod
def _parse_youtube(data):
@@ -287,19 +287,19 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
XML-based courses.
"""
ret = {'0.75': '', '1.00': '', '1.25': '', '1.50': ''}
- if data == '':
- return ret
+
videos = data.split(',')
for video in videos:
pieces = video.split(':')
- # HACK
- # To elaborate somewhat: in many LMS tests, the keys for
- # Youtube IDs are inconsistent. Sometimes a particular
- # speed isn't present, and formatting is also inconsistent
- # ('1.0' versus '1.00'). So it's necessary to either do
- # something like this or update all the tests to work
- # properly.
- ret['%.2f' % float(pieces[0])] = pieces[1]
+ try:
+ speed = '%.2f' % float(pieces[0]) # normalize speed
+ # Handle the fact that youtube IDs got double-quoted for a period of time.
+ # Note: we pass in "VideoFields.youtube_id_1_0" so we deserialize as a String--
+ # it doesn't matter what the actual speed is for the purposes of deserializing.
+ youtube_id = VideoDescriptor._deserialize(VideoFields.youtube_id_1_0.name, pieces[1])
+ ret[speed] = youtube_id
+ except (ValueError, IndexError):
+ log.warning('Invalid YouTube ID: %s' % video)
return ret
@staticmethod
@@ -312,7 +312,6 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
model_data = {}
conversions = {
- 'show_captions': json.loads,
'start_time': VideoDescriptor._parse_time,
'end_time': VideoDescriptor._parse_time
}
@@ -351,10 +350,21 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
# Convert XML attrs into Python values.
if attr in conversions:
value = conversions[attr](value)
+ else:
+ # We export values with json.dumps (well, except for Strings, but
+ # for about a month we did it for Strings also).
+ value = VideoDescriptor._deserialize(attr, value)
model_data[attr] = value
return model_data
+ @classmethod
+ def _deserialize(cls, attr, value):
+ """
+ Handles deserializing values that may have been encoded with json.dumps.
+ """
+ return cls.get_map_for_field(attr).from_xml(value)
+
@staticmethod
def _parse_time(str_time):
"""Converts s in '12:34:45' format to seconds. If s is
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 3556f3f0f3..87d9413334 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -173,11 +173,11 @@ class XModule(XModuleFields, HTMLSnippet, XBlock):
# don't need to set category as it will automatically get from descriptor
elif isinstance(self.location, Location):
self.url_name = self.location.name
- if not hasattr(self, 'category'):
+ if getattr(self, 'category', None) is None:
self.category = self.location.category
elif isinstance(self.location, BlockUsageLocator):
self.url_name = self.location.usage_id
- if not hasattr(self, 'category'):
+ if getattr(self, 'category', None) is None:
raise InsufficientSpecificationError()
else:
raise InsufficientSpecificationError()
@@ -467,11 +467,11 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock):
self.system = self.runtime
if isinstance(self.location, Location):
self.url_name = self.location.name
- if not hasattr(self, 'category'):
+ if getattr(self, 'category', None) is None:
self.category = self.location.category
elif isinstance(self.location, BlockUsageLocator):
self.url_name = self.location.usage_id
- if not hasattr(self, 'category'):
+ if getattr(self, 'category', None) is None:
raise InsufficientSpecificationError()
else:
raise InsufficientSpecificationError()
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 5b8d2c8aee..b0b7f300ed 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -167,6 +167,11 @@ class XmlDescriptor(XModuleDescriptor):
@classmethod
def get_map_for_field(cls, attr):
+ """
+ Returns a serialize/deserialize AttrMap for the given field of a class.
+
+ Searches through fields defined by cls to find one named attr.
+ """
for field in set(cls.fields + cls.lms.fields):
if field.name == attr:
from_xml = lambda val: deserialize_field(field, val)
diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py
index e3c40079c3..8874a5686c 100644
--- a/lms/djangoapps/courseware/grades.py
+++ b/lms/djangoapps/courseware/grades.py
@@ -358,6 +358,8 @@ def get_score(course_id, user, problem_descriptor, module_creator, model_data_ca
# with the LMS, so they need to always be scored. (E.g. foldit.)
if problem_descriptor.always_recalculate_grades:
problem = module_creator(problem_descriptor)
+ if problem is None:
+ return (None, None)
score = problem.get_score()
if score is not None:
return (score['score'], score['total'])