Skip to content

E
edx-platform
Commit 5bacfcc3 by Anton Stupak
Options

Merge pull request #2045 from edx/anton/video-merge-fields 

Merge "video sources" and "download video" into the same field
parents 636122c0 645007f9
Showing with 71 additions and 19 deletions
CHANGELOG.rst
...@@ -5,6 +5,9 @@ These are notable changes in edx-platform. This is a rolling list of changes, ...@@ -5,6 +5,9 @@ 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 in roughly chronological order, most recent first. Add your entries at or near
the top. Include a label indicating the component affected. the top. Include a label indicating the component affected.
Blades: Change the download video field to a dropdown that will allow students
to download the first source listed in the alternate sources. BLD-364.
Blades: Change the track field to a dropdown that will allow students Blades: Change the track field to a dropdown that will allow students
to download the transcript of the video without timecodes. BLD-368. to download the transcript of the video without timecodes. BLD-368.
......
cms/djangoapps/contentstore/features/video-editor.py
...@@ -40,12 +40,12 @@ def correct_video_settings(_step): ...@@ -40,12 +40,12 @@ def correct_video_settings(_step):
# advanced # advanced
['Display Name', 'Video', False], ['Display Name', 'Video', False],
['Download Video', '', False],
['End Time', '00:00:00', False], ['End Time', '00:00:00', False],
['HTML5 Transcript', '', False], ['HTML5 Transcript', '', False],
['Show Transcript', 'True', False], ['Show Transcript', 'True', False],
['Start Time', '00:00:00', False], ['Start Time', '00:00:00', False],
['Transcript Download Allowed', 'False', False], ['Transcript Download Allowed', 'False', False],
['Video Download Allowed', 'False', False],
['Video Sources', '', False], ['Video Sources', '', False],
['Youtube ID', 'OEoXaMPEzfM', False], ['Youtube ID', 'OEoXaMPEzfM', False],
['Youtube ID for .75x speed', '', False], ['Youtube ID for .75x speed', '', False],
......
cms/static/js/views/metadata.js
...@@ -106,7 +106,6 @@ function(BaseView, _, MetadataModel, AbstractEditor, VideoList) { ...@@ -106,7 +106,6 @@ function(BaseView, _, MetadataModel, AbstractEditor, VideoList) {
} }
}, },
getValueFromEditor : function () { getValueFromEditor : function () {
return this.$el.find('#' + this.uniqueId).val(); return this.$el.find('#' + this.uniqueId).val();
}, },
......
common/lib/xmodule/xmodule/tests/test_video.py
...@@ -187,6 +187,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -187,6 +187,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8" youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8"
show_captions="false" show_captions="false"
download_track="true" download_track="true"
download_video="true"
start_time="00:00:01" start_time="00:00:01"
end_time="00:01:00"> end_time="00:01:00">
<source src="http://www.example.com/source.mp4"/> <source src="http://www.example.com/source.mp4"/>
...@@ -207,6 +208,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -207,6 +208,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'youtube_id_1_0': 'p2Q6BrNhdh8', 'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA', 'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8', 'youtube_id_1_5': 'rABDYkeK0x8',
'download_video': True,
'show_captions': False, 'show_captions': False,
'start_time': datetime.timedelta(seconds=1), 'start_time': datetime.timedelta(seconds=1),
'end_time': datetime.timedelta(seconds=60), 'end_time': datetime.timedelta(seconds=60),
...@@ -224,6 +226,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -224,6 +226,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
show_captions="false" show_captions="false"
download_track="false" download_track="false"
start_time="00:00:01" start_time="00:00:01"
download_video="false"
end_time="00:01:00"> end_time="00:01:00">
<source src="http://www.example.com/source.mp4"/> <source src="http://www.example.com/source.mp4"/>
<track src="http://www.example.com/track"/> <track src="http://www.example.com/track"/>
...@@ -240,7 +243,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -240,7 +243,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'end_time': datetime.timedelta(seconds=60), 'end_time': datetime.timedelta(seconds=60),
'track': 'http://www.example.com/track', 'track': 'http://www.example.com/track',
'download_track': False, 'download_track': False,
'source': 'http://www.example.com/source.mp4', 'download_video': False,
'html5_sources': ['http://www.example.com/source.mp4'], 'html5_sources': ['http://www.example.com/source.mp4'],
'data': '' 'data': ''
}) })
...@@ -269,7 +272,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -269,7 +272,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'end_time': datetime.timedelta(seconds=0.0), 'end_time': datetime.timedelta(seconds=0.0),
'track': '', 'track': '',
'download_track': False, 'download_track': False,
'source': 'http://www.example.com/source.mp4', 'download_video': False,
'html5_sources': ['http://www.example.com/source.mp4'], 'html5_sources': ['http://www.example.com/source.mp4'],
'data': '' 'data': ''
}) })
...@@ -291,7 +294,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -291,7 +294,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'end_time': datetime.timedelta(seconds=0.0), 'end_time': datetime.timedelta(seconds=0.0),
'track': '', 'track': '',
'download_track': False, 'download_track': False,
'source': '', 'download_video': False,
'html5_sources': [], 'html5_sources': [],
'data': '' 'data': ''
}) })
...@@ -306,7 +309,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -306,7 +309,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
<video display_name="&quot;display_name&quot;" <video display_name="&quot;display_name&quot;"
html5_sources="[&quot;source_1&quot;, &quot;source_2&quot;]" html5_sources="[&quot;source_1&quot;, &quot;source_2&quot;]"
show_captions="false" show_captions="false"
source="&quot;http://download_video&quot;" download_video="true"
sub="&quot;html5_subtitles&quot;" sub="&quot;html5_subtitles&quot;"
track="&quot;http://download_track&quot;" track="&quot;http://download_track&quot;"
download_track="true" download_track="true"
...@@ -327,7 +330,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -327,7 +330,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'end_time': datetime.timedelta(seconds=0.0), 'end_time': datetime.timedelta(seconds=0.0),
'track': 'http://download_track', 'track': 'http://download_track',
'download_track': True, 'download_track': True,
'source': 'http://download_video', 'download_video': True,
'html5_sources': ["source_1", "source_2"], 'html5_sources': ["source_1", "source_2"],
'data': '' 'data': ''
}) })
...@@ -350,7 +353,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -350,7 +353,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'end_time': datetime.timedelta(seconds=0.0), 'end_time': datetime.timedelta(seconds=0.0),
'track': '', 'track': '',
'download_track': False, 'download_track': False,
'source': '', 'download_video': False,
'html5_sources': [], 'html5_sources': [],
'data': '' 'data': ''
}) })
...@@ -364,6 +367,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -364,6 +367,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
<video display_name="Test Video" <video display_name="Test Video"
youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8" youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8"
show_captions="false" show_captions="false"
source="http://www.example.com/source.mp4"
from="00:00:01" from="00:00:01"
to="00:01:00"> to="00:01:00">
<source src="http://www.example.com/source.mp4"/> <source src="http://www.example.com/source.mp4"/>
...@@ -382,7 +386,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): ...@@ -382,7 +386,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
'track': 'http://www.example.com/track', 'track': 'http://www.example.com/track',
'download_track': True, 'download_track': True,
'html5_sources': ['http://www.example.com/source.mp4'], 'html5_sources': ['http://www.example.com/source.mp4'],
'data': '' 'data': '',
}) })
def test_old_video_data(self): def test_old_video_data(self):
...@@ -473,10 +477,11 @@ class VideoExportTestCase(unittest.TestCase): ...@@ -473,10 +477,11 @@ class VideoExportTestCase(unittest.TestCase):
desc.track = 'http://www.example.com/track' desc.track = 'http://www.example.com/track'
desc.download_track = True desc.download_track = True
desc.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg'] desc.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg']
desc.download_video = True
xml = desc.definition_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 = etree.fromstring('''\ expected = etree.fromstring('''\
<video url_name="SampleProblem1" start_time="0:00:01" youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" show_captions="false" end_time="0:01:00" download_track="true"> <video url_name="SampleProblem1" start_time="0:00:01" youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" show_captions="false" end_time="0:01:00" download_video="true" download_track="true">
<source src="http://www.example.com/source.mp4"/> <source src="http://www.example.com/source.mp4"/>
<source src="http://www.example.com/source.ogg"/> <source src="http://www.example.com/source.ogg"/>
<track src="http://www.example.com/track"/> <track src="http://www.example.com/track"/>
...@@ -501,10 +506,11 @@ class VideoExportTestCase(unittest.TestCase): ...@@ -501,10 +506,11 @@ class VideoExportTestCase(unittest.TestCase):
desc.track = 'http://www.example.com/track' desc.track = 'http://www.example.com/track'
desc.download_track = True desc.download_track = True
desc.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg'] desc.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg']
desc.download_video = True
xml = desc.definition_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 = etree.fromstring('''\ expected = etree.fromstring('''\
<video url_name="SampleProblem1" start_time="0:00:05" youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" show_captions="false" download_track="true"> <video url_name="SampleProblem1" start_time="0:00:05" youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" show_captions="false" download_video="true" download_track="true">
<source src="http://www.example.com/source.mp4"/> <source src="http://www.example.com/source.mp4"/>
<source src="http://www.example.com/source.ogg"/> <source src="http://www.example.com/source.ogg"/>
<track src="http://www.example.com/track"/> <track src="http://www.example.com/track"/>
......
common/lib/xmodule/xmodule/video_module.py
...@@ -98,12 +98,20 @@ class VideoFields(object): ...@@ -98,12 +98,20 @@ class VideoFields(object):
) )
#front-end code of video player checks logical validity of (start_time, end_time) pair. #front-end code of video player checks logical validity of (start_time, end_time) pair.
# `source` is deprecated field and should not be used in future.
# `download_video` is used instead.
source = String( source = String(
help="The external URL to download the video. This appears as a link beneath the video.", help="The external URL to download the video.",
display_name="Download Video", display_name="Download Video",
scope=Scope.settings, scope=Scope.settings,
default="" default=""
) )
download_video = Boolean(
help="Show a link beneath the video to allow students to download the video. Note: You must add at least one video source below.",
display_name="Video Download Allowed",
scope=Scope.settings,
default=False
)
html5_sources = List( html5_sources = List(
help="A list of filenames to be used with HTML5 video. The first supported filetype will be displayed.", help="A list of filenames to be used with HTML5 video. The first supported filetype will be displayed.",
display_name="Video Sources", display_name="Video Sources",
...@@ -181,7 +189,12 @@ class VideoModule(VideoFields, XModule): ...@@ -181,7 +189,12 @@ class VideoModule(VideoFields, XModule):
get_ext = lambda filename: filename.rpartition('.')[-1] get_ext = lambda filename: filename.rpartition('.')[-1]
sources = {get_ext(src): src for src in self.html5_sources} sources = {get_ext(src): src for src in self.html5_sources}
sources['main'] = self.source
if self.download_video:
if self.source:
sources['main'] = self.source
elif self.html5_sources:
sources['main'] = self.html5_sources[0]
if self.download_track: if self.download_track:
if self.track: if self.track:
...@@ -281,6 +294,14 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor ...@@ -281,6 +294,14 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
If `track` field exists show `track` field on front-end as not-editable If `track` field exists show `track` field on front-end as not-editable
but clearable. Dropdown `download_track` is a new field and it has value but clearable. Dropdown `download_track` is a new field and it has value
True. True.
`source` is deprecated field.
a) If `source` exists and `source` is not `html5_sources`: show `source`
field on front-end as not-editable but clearable. Dropdown is a new
field `download_video` and it has value True.
b) If `source` is cleared it is not shown anymore.
c) If `source` exists and `source` in `html5_sources`, do not show `source`
field. `download_video` field has value True.
''' '''
super(VideoDescriptor, self).__init__(*args, **kwargs) super(VideoDescriptor, self).__init__(*args, **kwargs)
# For backwards compatibility -- if we've got XML data, parse # For backwards compatibility -- if we've got XML data, parse
...@@ -290,21 +311,43 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor ...@@ -290,21 +311,43 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
self._field_data.set_many(self, field_data) self._field_data.set_many(self, field_data)
del self.data del self.data
editable_fields = self.editable_metadata_fields
self.track_visible = False self.track_visible = False
if self.track: if self.track:
self.track_visible = True self.track_visible = True
download_track = self.editable_metadata_fields['download_track'] download_track = editable_fields['download_track']
if not download_track['explicitly_set']: if not download_track['explicitly_set']:
self.download_track = True self.download_track = True
self.source_visible = False
if self.source:
# If `source` field value exist in the `html5_sources` field values,
# then delete `source` field value and use value from `html5_sources` field.
if self.source in self.html5_sources:
self.source = '' # Delete source field value.
self.download_video = True
else: # Otherwise, `source` field value will be used.
self.source_visible = True
download_video = editable_fields['download_video']
if not download_video['explicitly_set']:
self.download_video = True
@property @property
def editable_metadata_fields(self): def editable_metadata_fields(self):
editable_fields = super(VideoDescriptor, self).editable_metadata_fields editable_fields = super(VideoDescriptor, self).editable_metadata_fields
if self.track_visible: if hasattr(self, 'track_visible'):
editable_fields['track']['non_editable'] = True if self.track_visible:
else: editable_fields['track']['non_editable'] = True
editable_fields.pop('track') else:
editable_fields.pop('track')
if hasattr(self, 'source_visible'):
if self.source_visible:
editable_fields['source']['non_editable'] = True
else:
editable_fields.pop('source')
return editable_fields return editable_fields
...@@ -359,6 +402,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor ...@@ -359,6 +402,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
'end_time': self.end_time, 'end_time': self.end_time,
'sub': self.sub, 'sub': self.sub,
'download_track': json.dumps(self.download_track), 'download_track': json.dumps(self.download_track),
'download_video': json.dumps(self.download_video),
} }
for key, value in attrs.items(): for key, value in attrs.items():
# Mild workaround to ensure that tests pass -- if a field # Mild workaround to ensure that tests pass -- if a field
...@@ -468,7 +512,6 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor ...@@ -468,7 +512,6 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
sources = xml.findall('source') sources = xml.findall('source')
if sources: if sources:
field_data['html5_sources'] = [ele.get('src') for ele in sources] field_data['html5_sources'] = [ele.get('src') for ele in sources]
field_data['source'] = field_data['html5_sources'][0]
track = xml.find('track') track = xml.find('track')
if track is not None: if track is not None:
......
lms/djangoapps/courseware/tests/test_video_xml.py
...@@ -31,6 +31,7 @@ SOURCE_XML = """ ...@@ -31,6 +31,7 @@ SOURCE_XML = """
display_name="A Name" display_name="A Name"
youtube="0.75:jNCf2gIqpeE,1.0:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg" youtube="0.75:jNCf2gIqpeE,1.0:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg"
sub="a_sub_file.srt.sjson" sub="a_sub_file.srt.sjson"
download_video="true"
start_time="01:00:03" end_time="01:00:10" start_time="01:00:03" end_time="01:00:10"
> >
<source src="example.mp4"/> <source src="example.mp4"/>
......
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