diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f865c09..0e161e4 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -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 the top. Include a label indicating the component affected. +Studio: Remove XML from the video component editor. All settings are +moved to be edited as metadata. + XModule: Only write out assets files if the contents have changed. XModule: Don't delete generated xmodule asset files when compiling (for diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index d833c6f..e0f20d3 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -171,6 +171,16 @@ def open_new_unit(step): world.css_click('a.new-unit-item') +@step('when I view the video it (.*) show the captions') +def shows_captions(step, show_captions): + # Prevent cookies from overriding course settings + world.browser.cookies.delete('hide_captions') + if show_captions == 'does not': + assert world.css_find('.video')[0].has_class('closed') + else: + assert world.is_css_not_present('.video.closed') + + def type_in_codemirror(index, text): world.css_click(".CodeMirror", index=index) g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea") diff --git a/cms/djangoapps/contentstore/features/video-editor.feature b/cms/djangoapps/contentstore/features/video-editor.feature index 4c2a460..f28ee56 100644 --- a/cms/djangoapps/contentstore/features/video-editor.feature +++ b/cms/djangoapps/contentstore/features/video-editor.feature @@ -4,10 +4,20 @@ Feature: Video Component Editor Scenario: User can view metadata Given I have created a Video component And I edit and select Settings - Then I see only the Video display name setting + Then I see the correct settings and default values Scenario: User can modify display name Given I have created a Video component And I edit and select Settings Then I can modify the display name And my display name change is persisted on save + + Scenario: Captions are hidden when "show captions" is false + Given I have created a Video component + And I have set "show captions" to False + Then when I view the video it does not show the captions + + Scenario: Captions are shown when "show captions" is true + Given I have created a Video component + And I have set "show captions" to True + Then when I view the video it does show the captions diff --git a/cms/djangoapps/contentstore/features/video-editor.py b/cms/djangoapps/contentstore/features/video-editor.py index 2742357..987b495 100644 --- a/cms/djangoapps/contentstore/features/video-editor.py +++ b/cms/djangoapps/contentstore/features/video-editor.py @@ -4,6 +4,20 @@ from lettuce import world, step -@step('I see only the video display name setting$') -def i_see_only_the_video_display_name(step): - world.verify_all_setting_entries([['Display Name', "default", True]]) +@step('I see the correct settings and default values$') +def i_see_the_correct_settings_and_values(step): + world.verify_all_setting_entries([['Default Speed', 'OEoXaMPEzfM', False], + ['Display Name', 'default', True], + ['Download Track', '', False], + ['Download Video', '', False], + ['Show Captions', 'True', False], + ['Speed: .75x', '', False], + ['Speed: 1.25x', '', False], + ['Speed: 1.5x', '', False]]) + + +@step('I have set "show captions" to (.*)') +def set_show_captions(step, setting): + world.css_click('a.edit-button') + world.browser.select('Show Captions', setting) + world.css_click('a.save-button') diff --git a/cms/djangoapps/contentstore/features/video.feature b/cms/djangoapps/contentstore/features/video.feature index 0129732..e4caa70 100644 --- a/cms/djangoapps/contentstore/features/video.feature +++ b/cms/djangoapps/contentstore/features/video.feature @@ -9,7 +9,16 @@ Feature: Video Component Given I have clicked the new unit button Then creating a video takes a single click - Scenario: Captions are shown correctly + Scenario: Captions are hidden correctly Given I have created a Video component And I have hidden captions Then when I view the video it does not show the captions + + Scenario: Captions are shown correctly + Given I have created a Video component + Then when I view the video it does show the captions + + Scenario: Captions are toggled correctly + Given I have created a Video component + And I have toggled captions + Then when I view the video it does show the captions diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index c48b36a..190f8e9 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -18,11 +18,16 @@ def video_takes_a_single_click(_step): assert(world.is_css_present('.xmodule_VideoModule')) -@step('I have hidden captions') -def set_show_captions_false(step): - world.css_click('a.hide-subtitles') - - -@step('when I view the video it does not show the captions') -def does_not_show_captions(step): - assert world.css_find('.video')[0].has_class('closed') +@step('I have (hidden|toggled) captions') +def hide_or_show_captions(step, shown): + button_css = 'a.hide-subtitles' + if shown == 'hidden': + world.css_click(button_css) + if shown == 'toggled': + world.css_click(button_css) + # When we click the first time, a tooltip shows up. We want to + # click the button rather than the tooltip, so move the mouse + # away to make it disappear. + button = world.css_find(button_css) + button.mouse_out() + world.css_click(button_css) diff --git a/common/lib/xmodule/xmodule/js/fixtures/video.html b/common/lib/xmodule/xmodule/js/fixtures/video.html index e86a24d..1b27255 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video.html @@ -2,7 +2,8 @@ <div id="video_example"> <div id="example"> <div id="video_id" class="video" - data-streams="0.75:slowerSpeedYoutubeId,1.0:normalSpeedYoutubeId" + data-youtube-id-0-75="slowerSpeedYoutubeId" + data-youtube-id-1-0="normalSpeedYoutubeId" data-show-captions="true" data-start="" data-end="" @@ -18,4 +19,4 @@ </div> </div> </div> -</div> \ No newline at end of file +</div> diff --git a/common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee index a83fa39..0ba0cc8 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee @@ -5,7 +5,6 @@ describe 'Video', -> loadFixtures 'video.html' jasmine.stubRequests() - @videosDefinition = '0.75:slowerSpeedYoutubeId,1.0:normalSpeedYoutubeId' @slowerSpeedYoutubeId = 'slowerSpeedYoutubeId' @normalSpeedYoutubeId = 'normalSpeedYoutubeId' metadata = @@ -30,7 +29,7 @@ describe 'Video', -> beforeEach -> spyOn(window.Video.prototype, 'fetchMetadata').andCallFake -> @metadata = metadata - @video = new Video '#example', @videosDefinition + @video = new Video '#example' it 'reset the current video player', -> expect(window.player).toBeNull() @@ -60,7 +59,7 @@ describe 'Video', -> @originalYT = window.YT window.YT = { Player: true } spyOn(window, 'VideoPlayer').andReturn(@stubVideoPlayer) - @video = new Video '#example', @videosDefinition + @video = new Video '#example' afterEach -> window.YT = @originalYT @@ -73,7 +72,7 @@ describe 'Video', -> beforeEach -> @originalYT = window.YT window.YT = {} - @video = new Video '#example', @videosDefinition + @video = new Video '#example' afterEach -> window.YT = @originalYT @@ -86,7 +85,7 @@ describe 'Video', -> @originalYT = window.YT window.YT = {} spyOn(window, 'VideoPlayer').andReturn(@stubVideoPlayer) - @video = new Video '#example', @videosDefinition + @video = new Video '#example' window.onYouTubePlayerAPIReady() afterEach -> @@ -99,7 +98,7 @@ describe 'Video', -> describe 'youtubeId', -> beforeEach -> $.cookie.andReturn '1.0' - @video = new Video '#example', @videosDefinition + @video = new Video '#example' describe 'with speed', -> it 'return the video id for given speed', -> @@ -112,7 +111,7 @@ describe 'Video', -> describe 'setSpeed', -> beforeEach -> - @video = new Video '#example', @videosDefinition + @video = new Video '#example' describe 'when new speed is available', -> beforeEach -> @@ -133,14 +132,14 @@ describe 'Video', -> describe 'getDuration', -> beforeEach -> - @video = new Video '#example', @videosDefinition + @video = new Video '#example' it 'return duration for current video', -> expect(@video.getDuration()).toEqual 200 describe 'log', -> beforeEach -> - @video = new Video '#example', @videosDefinition + @video = new Video '#example' @video.setSpeed '1.0' spyOn Logger, 'log' @video.player = { currentTime: 25 } diff --git a/common/lib/xmodule/xmodule/js/src/video/display.coffee b/common/lib/xmodule/xmodule/js/src/video/display.coffee index 0393fe8..d9c31b2 100644 --- a/common/lib/xmodule/xmodule/js/src/video/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/video/display.coffee @@ -8,7 +8,7 @@ class @Video @show_captions = @el.data('show-captions') window.player = null @el = $("#video_#{@id}") - @parseVideos @el.data('streams') + @parseVideos() @fetchMetadata() @parseSpeed() $("#video_#{@id}").data('video', this).addClass('video-load-complete') @@ -27,10 +27,14 @@ class @Video parseVideos: (videos) -> @videos = {} - $.each videos.split(/,/), (index, video) => - video = video.split(/:/) - speed = parseFloat(video[0]).toFixed(2).replace /\.00$/, '.0' - @videos[speed] = video[1] + if @el.data('youtube-id-0-75') + @videos['0.75'] = @el.data('youtube-id-0-75') + if @el.data('youtube-id-1-0') + @videos['1.0'] = @el.data('youtube-id-1-0') + if @el.data('youtube-id-1-25') + @videos['1.25'] = @el.data('youtube-id-1-25') + if @el.data('youtube-id-1-5') + @videos['1.50'] = @el.data('youtube-id-1-5') parseSpeed: -> @setSpeed($.cookie('video_speed')) diff --git a/common/lib/xmodule/xmodule/templates/video/default.yaml b/common/lib/xmodule/xmodule/templates/video/default.yaml index 0ce2046..048e739 100644 --- a/common/lib/xmodule/xmodule/templates/video/default.yaml +++ b/common/lib/xmodule/xmodule/templates/video/default.yaml @@ -1,7 +1,5 @@ --- metadata: display_name: default - data_dir: a_made_up_name -data: | - <video youtube="0.75:JMD_ifUUfsU,1.0:OEoXaMPEzfM,1.25:AKqURZnYqpk,1.50:DYpADpL7jAY"/> +data: "" children: [] diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 677dd4d..4e9a9f9 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -336,8 +336,8 @@ class ImportTestCase(BaseCourseTestCase): location = Location(["i4x", "edX", "toy", "video", "Welcome"]) toy_video = modulestore.get_instance(toy_id, location) two_toy_video = modulestore.get_instance(two_toy_id, location) - self.assertEqual(etree.fromstring(toy_video.data).get('youtube'), "1.0:p2Q6BrNhdh8") - self.assertEqual(etree.fromstring(two_toy_video.data).get('youtube'), "1.0:p2Q6BrNhdh9") + self.assertEqual(toy_video.youtube_id_1_0, "p2Q6BrNhdh8") + self.assertEqual(two_toy_video.youtube_id_1_0, "p2Q6BrNhdh9") def test_colon_in_url_name(self): """Ensure that colons in url_names convert to file paths properly""" diff --git a/common/lib/xmodule/xmodule/tests/test_video_module.py b/common/lib/xmodule/xmodule/tests/test_video_module.py new file mode 100644 index 0000000..f516e1a --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_video_module.py @@ -0,0 +1,76 @@ +# -*- coding: utf-8 -*- +import unittest + +from xmodule.video_module import VideoDescriptor +from .test_import import DummySystem + + +class VideoDescriptorImportTestCase(unittest.TestCase): + """ + Make sure that VideoDescriptor can import an old XML-based video correctly. + """ + + def test_from_xml(self): + module_system = DummySystem(load_error_modules=True) + xml_data = ''' + <video display_name="Test Video" + youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8" + show_captions="false" + from="00:00:01" + to="00:01:00"> + <source src="http://www.example.com/source.mp4"/> + <track src="http://www.example.com/track"/> + </video> + ''' + output = VideoDescriptor.from_xml(xml_data, module_system) + self.assertEquals(output.youtube_id_0_75, 'izygArpw-Qo') + self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8') + self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA') + self.assertEquals(output.youtube_id_1_5, 'rABDYkeK0x8') + self.assertEquals(output.show_captions, False) + self.assertEquals(output.start_time, 1.0) + self.assertEquals(output.end_time, 60) + self.assertEquals(output.track, 'http://www.example.com/track') + self.assertEquals(output.source, 'http://www.example.com/source.mp4') + + def test_from_xml_missing_attributes(self): + """ + Ensure that attributes have the right values if they aren't + explicitly set in XML. + """ + module_system = DummySystem(load_error_modules=True) + xml_data = ''' + <video display_name="Test Video" + youtube="1.0:p2Q6BrNhdh8,1.25:1EeWXzPdhSA" + show_captions="true"> + <source src="http://www.example.com/source.mp4"/> + <track src="http://www.example.com/track"/> + </video> + ''' + output = VideoDescriptor.from_xml(xml_data, module_system) + self.assertEquals(output.youtube_id_0_75, '') + self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8') + self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA') + self.assertEquals(output.youtube_id_1_5, '') + self.assertEquals(output.show_captions, True) + self.assertEquals(output.start_time, 0.0) + self.assertEquals(output.end_time, 0.0) + self.assertEquals(output.track, 'http://www.example.com/track') + self.assertEquals(output.source, 'http://www.example.com/source.mp4') + + def test_from_xml_no_attributes(self): + """ + Make sure settings are correct if none are explicitly set in XML. + """ + module_system = DummySystem(load_error_modules=True) + xml_data = '<video></video>' + output = VideoDescriptor.from_xml(xml_data, module_system) + self.assertEquals(output.youtube_id_0_75, '') + self.assertEquals(output.youtube_id_1_0, 'OEoXaMPEzfM') + self.assertEquals(output.youtube_id_1_25, '') + self.assertEquals(output.youtube_id_1_5, '') + self.assertEquals(output.show_captions, True) + self.assertEquals(output.start_time, 0.0) + self.assertEquals(output.end_time, 0.0) + self.assertEquals(output.track, '') + self.assertEquals(output.source, '') diff --git a/common/lib/xmodule/xmodule/tests/test_video_xml.py b/common/lib/xmodule/xmodule/tests/test_video_xml.py index 54606ba..0818707 100644 --- a/common/lib/xmodule/xmodule/tests/test_video_xml.py +++ b/common/lib/xmodule/xmodule/tests/test_video_xml.py @@ -18,7 +18,7 @@ import unittest from mock import Mock from lxml import etree -from xmodule.video_module import VideoDescriptor, VideoModule +from xmodule.video_module import VideoDescriptor, VideoModule, _parse_time, _parse_youtube from xmodule.modulestore import Location from xmodule.tests import get_test_system from xmodule.tests.test_logic import LogicTest @@ -49,7 +49,7 @@ class VideoFactory(object): "SampleProblem1"]) model_data = {'data': VideoFactory.sample_problem_xml_youtube, 'location': location} - descriptor = Mock(weight="1") + descriptor = Mock(weight="1", url_name="SampleProblem1") system = get_test_system() system.render_template = lambda template, context: context @@ -67,69 +67,57 @@ class VideoModuleLogicTest(LogicTest): 'data': '<video />' } - def test_get_timeframe_no_parameters(self): - """Make sure that timeframe() works correctly w/o parameters""" - xmltree = etree.fromstring('<video>test</video>') - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, ('', '')) - - def test_get_timeframe_with_one_parameter(self): - """Make sure that timeframe() works correctly with one parameter""" - xmltree = etree.fromstring( - '<video from="00:04:07">test</video>' - ) - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, (247, '')) - - def test_get_timeframe_with_two_parameters(self): - """Make sure that timeframe() works correctly with two parameters""" - xmltree = etree.fromstring( - '''<video - from="00:04:07" - to="13:04:39" - >test</video>''' - ) - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, (247, 47079)) - - -class VideoModuleUnitTest(unittest.TestCase): - """Unit tests for Video Xmodule.""" - - def test_video_constructor(self): - """Make sure that all parameters extracted correclty from xml""" - module = VideoFactory.create() - - # `get_html` return only context, cause we - # overwrite `system.render_template` - context = module.get_html() - expected_context = { - 'track': None, - 'show_captions': 'true', - 'display_name': 'SampleProblem1', - 'id': module.location.html_id(), - 'end': 3610.0, - 'caption_asset_path': '/static/subs/', - 'source': '.../mit-3091x/M-3091X-FA12-L21-3_100.mp4', - 'streams': '0.75:jNCf2gIqpeE,1.0:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg', - 'normal_speed_video_id': 'ZwkTiUPN0mg', - 'position': 0, - 'start': 3603.0 - } - self.assertDictEqual(context, expected_context) - - self.assertEqual( - module.youtube, - '0.75:jNCf2gIqpeE,1.0:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg') - - self.assertEqual( - module.video_list(), - module.youtube) - - self.assertEqual( - module.position, - 0) - - self.assertDictEqual( - json.loads(module.get_instance_state()), - {'position': 0}) + def test_parse_time(self): + """Ensure that times are parsed correctly into seconds.""" + output = _parse_time('00:04:07') + self.assertEqual(output, 247) + + def test_parse_time_none(self): + """Check parsing of None.""" + output = _parse_time(None) + self.assertEqual(output, '') + + def test_parse_time_empty(self): + """Check parsing of the empty string.""" + output = _parse_time('') + self.assertEqual(output, '') + + def test_parse_youtube(self): + """Test parsing old-style Youtube ID strings into a dict.""" + youtube_str = '0.75:jNCf2gIqpeE,1.00:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg' + output = _parse_youtube(youtube_str) + self.assertEqual(output, {'0.75': 'jNCf2gIqpeE', + '1.00': 'ZwkTiUPN0mg', + '1.25': 'rsq9auxASqI', + '1.50': 'kMyNdzVHHgg'}) + + def test_parse_youtube_one_video(self): + """ + Ensure that all keys are present and missing speeds map to the + empty string. + """ + youtube_str = '0.75:jNCf2gIqpeE' + output = _parse_youtube(youtube_str) + self.assertEqual(output, {'0.75': 'jNCf2gIqpeE', + '1.00': '', + '1.25': '', + '1.50': ''}) + + def test_parse_youtube_key_format(self): + """ + Make sure that inconsistent speed keys are parsed correctly. + """ + youtube_str = '1.00:p2Q6BrNhdh8' + youtube_str_hack = '1.0:p2Q6BrNhdh8' + self.assertEqual(_parse_youtube(youtube_str), _parse_youtube(youtube_str_hack)) + + def test_parse_youtube_empty(self): + """ + Some courses have empty youtube attributes, so we should handle + that well. + """ + self.assertEqual(_parse_youtube(''), + {'0.75': '', + '1.00': '', + '1.25': '', + '1.50': ''}) diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index 77d83ca..6344da7 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -6,23 +6,31 @@ import logging from lxml import etree from pkg_resources import resource_string, resource_listdir +import datetime +import time from django.http import Http404 from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor -from xblock.core import Integer, Scope, String - -import datetime -import time +from xmodule.editing_module import MetadataOnlyEditingDescriptor +from xblock.core import Integer, Scope, String, Float, Boolean log = logging.getLogger(__name__) class VideoFields(object): """Fields for `VideoModule` and `VideoDescriptor`.""" - data = String(help="XML data for the problem", scope=Scope.content) position = Integer(help="Current position in the video", scope=Scope.user_state, default=0) + show_captions = Boolean(help="This controls whether or not captions are shown by default.", display_name="Show Captions", scope=Scope.settings, default=True) + youtube_id_1_0 = String(help="This is the Youtube ID reference for the normal speed video.", display_name="Default Speed", scope=Scope.settings, default="OEoXaMPEzfM") + youtube_id_0_75 = String(help="The Youtube ID for the .75x speed video.", display_name="Speed: .75x", scope=Scope.settings, default="") + youtube_id_1_25 = String(help="The Youtube ID for the 1.25x speed video.", display_name="Speed: 1.25x", scope=Scope.settings, default="") + youtube_id_1_5 = String(help="The Youtube ID for the 1.5x speed video.", display_name="Speed: 1.5x", scope=Scope.settings, default="") + start_time = Float(help="Time the video starts", display_name="Start Time", scope=Scope.settings, default=0.0) + end_time = Float(help="Time the video ends", display_name="End Time", scope=Scope.settings, default=0.0) + source = String(help="The external URL to download the video. This appears as a link beneath the video.", display_name="Download Video", scope=Scope.settings, default="") + track = String(help="The external URL to download the subtitle track. This appears as a link beneath the video.", display_name="Download Track", scope=Scope.settings, default="") class VideoModule(VideoFields, XModule): @@ -46,54 +54,6 @@ class VideoModule(VideoFields, XModule): def __init__(self, *args, **kwargs): XModule.__init__(self, *args, **kwargs) - xmltree = etree.fromstring(self.data) - self.youtube = xmltree.get('youtube') - self.show_captions = xmltree.get('show_captions', 'true') - self.source = self._get_source(xmltree) - self.track = self._get_track(xmltree) - self.start_time, self.end_time = self.get_timeframe(xmltree) - - def _get_source(self, xmltree): - """Find the first valid source.""" - return self._get_first_external(xmltree, 'source') - - def _get_track(self, xmltree): - """Find the first valid track.""" - return self._get_first_external(xmltree, 'track') - - def _get_first_external(self, xmltree, tag): - """ - Will return the first valid element - of the given tag. - 'valid' means has a non-empty 'src' attribute - """ - result = None - for element in xmltree.findall(tag): - src = element.get('src') - if src: - result = src - break - return result - - def get_timeframe(self, xmltree): - """ Converts 'from' and 'to' parameters in video tag to seconds. - If there are no parameters, returns empty string. """ - - def parse_time(str_time): - """Converts s in '12:34:45' format to seconds. If s is - None, returns empty string""" - if str_time is None: - return '' - else: - obj_time = time.strptime(str_time, '%H:%M:%S') - return datetime.timedelta( - hours=obj_time.tm_hour, - minutes=obj_time.tm_min, - seconds=obj_time.tm_sec - ).total_seconds() - - return parse_time(xmltree.get('from')), parse_time(xmltree.get('to')) - def handle_ajax(self, dispatch, get): """This is not being called right now and we raise 404 error.""" log.debug(u"GET {0}".format(get)) @@ -104,38 +64,135 @@ class VideoModule(VideoFields, XModule): """Return information about state (position).""" return json.dumps({'position': self.position}) - def video_list(self): - """Return video list.""" - return self.youtube - def get_html(self): - # We normally let JS parse this, but in the case that we need a hacked - # out <object> player because YouTube has broken their <iframe> API for - # the third time in a year, we need to extract it server side. - normal_speed_video_id = None # The 1.0 speed video - - # video_list() example: - # "0.75:nugHYNiD3fI,1.0:7m8pab1MfYY,1.25:3CxdPGXShq8,1.50:F-D7bOFCnXA" - for video_id_str in self.video_list().split(","): - if video_id_str.startswith("1.0:"): - normal_speed_video_id = video_id_str.split(":")[1] - return self.system.render_template('video.html', { - 'streams': self.video_list(), + 'youtube_id_0_75': self.youtube_id_0_75, + 'youtube_id_1_0': self.youtube_id_1_0, + 'youtube_id_1_25': self.youtube_id_1_25, + 'youtube_id_1_5': self.youtube_id_1_5, 'id': self.location.html_id(), 'position': self.position, 'source': self.source, 'track': self.track, 'display_name': self.display_name_with_default, 'caption_asset_path': "/static/subs/", - 'show_captions': self.show_captions, + 'show_captions': 'true' if self.show_captions else 'false', 'start': self.start_time, - 'end': self.end_time, - 'normal_speed_video_id': normal_speed_video_id + 'end': self.end_time }) -class VideoDescriptor(VideoFields, RawDescriptor): - """Descriptor for `VideoModule`.""" +class VideoDescriptor(VideoFields, + MetadataOnlyEditingDescriptor, + RawDescriptor): module_class = VideoModule template_dir_name = "video" + + @property + def non_editable_metadata_fields(self): + non_editable_fields = super(MetadataOnlyEditingDescriptor, self).non_editable_metadata_fields + non_editable_fields.extend([VideoModule.start_time, + VideoModule.end_time]) + return non_editable_fields + + @classmethod + def from_xml(cls, xml_data, system, org=None, course=None): + """ + Creates an instance of this descriptor from the supplied xml_data. + This may be overridden by subclasses + + xml_data: A string of xml that will be translated into data and children for + this module + system: A DescriptorSystem for interacting with external resources + org and course are optional strings that will be used in the generated modules + url identifiers + """ + video = super(VideoDescriptor, cls).from_xml(xml_data, system, org, course) + xml = etree.fromstring(xml_data) + + display_name = xml.get('display_name') + if display_name: + video.display_name = display_name + + youtube = xml.get('youtube') + if youtube: + speeds = _parse_youtube(youtube) + if speeds['0.75']: + video.youtube_id_0_75 = speeds['0.75'] + if speeds['1.00']: + video.youtube_id_1_0 = speeds['1.00'] + if speeds['1.25']: + video.youtube_id_1_25 = speeds['1.25'] + if speeds['1.50']: + video.youtube_id_1_5 = speeds['1.50'] + + show_captions = xml.get('show_captions') + if show_captions: + video.show_captions = json.loads(show_captions) + + source = _get_first_external(xml, 'source') + if source: + video.source = source + + track = _get_first_external(xml, 'track') + if track: + video.track = track + + start_time = _parse_time(xml.get('from')) + if start_time: + video.start_time = start_time + + end_time = _parse_time(xml.get('to')) + if end_time: + video.end_time = end_time + + return video + + +def _get_first_external(xmltree, tag): + """ + Returns the src attribute of the nested `tag` in `xmltree`, if it + exists. + """ + for element in xmltree.findall(tag): + src = element.get('src') + if src: + return src + return None + + +def _parse_youtube(data): + """ + Parses a string of Youtube IDs such as "1.0:AXdE34_U,1.5:VO3SxfeD" + into a dictionary. Necessary for backwards compatibility with + 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] + return ret + + +def _parse_time(str_time): + """Converts s in '12:34:45' format to seconds. If s is + None, returns empty string""" + if str_time is None or str_time == '': + return '' + else: + obj_time = time.strptime(str_time, '%H:%M:%S') + return datetime.timedelta( + hours=obj_time.tm_hour, + minutes=obj_time.tm_min, + seconds=obj_time.tm_sec + ).total_seconds() diff --git a/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml b/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml index 35e4704..600dc90 100644 --- a/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml +++ b/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml @@ -16,13 +16,16 @@ <vertical slug="vertical_94" graceperiod="1 day 12 hours 59 minutes 59 seconds" showanswer="attempted" rerandomize="never"> <video - youtube="0.75:XNh13VZhThQ,1.0:XbDRmF6J0K0,1.25:JDty12WEQWk,1.50:wELKGj-5iyM" - slug="What_s_next" name="What's next" /> + youtube_id_0_75=""XNh13VZhThQ"" + youtube_id_1_0=""XbDRmF6J0K0"" + youtube_id_1_25=""JDty12WEQWk"" + youtube_id_1_5=""wELKGj-5iyM"" + slug="What_s_next" + name="What's next"/> <html slug="html_95">Minor correction: Six elements (five resistors)… </html> <customtag tag="S1" slug="discuss_96" impl="discuss" /> </vertical> - <randomize url_name="PS1_Q4" display_name="Problem 4: Read a Molecule"> <vertical> <html slug="html_900"> diff --git a/common/test/data/full/vertical/vertical_58.xml b/common/test/data/full/vertical/vertical_58.xml index 5707e6a..c5034bf 100644 --- a/common/test/data/full/vertical/vertical_58.xml +++ b/common/test/data/full/vertical/vertical_58.xml @@ -1,5 +1,5 @@ <sequential> - <video youtube="1.50:8kARlsUt9lM,1.25:4cLA-IME32w,1.0:pFOrD8k9_p4,0.75:CcgAYu0n0bg" slug="S1V9_Demo_Setup_-_Lumped_Elements" name="S1V9: Demo Setup - Lumped Elements"/> + <video youtube_id_1_5=""8kARlsUt9lM"" youtube_id_1_25=""4cLA-IME32w"" youtube_id_1_0=""pFOrD8k9_p4"" youtube_id_0_75=""CcgAYu0n0bg"" slug="S1V9_Demo_Setup_-_Lumped_Elements" name="S1V9: Demo Setup - Lumped Elements"/> <customtag tag="S1" slug="discuss_59" impl="discuss"/> <customtag page="29" slug="book_60" impl="book"/> <customtag lecnum="1" slug="slides_61" impl="slides"/> diff --git a/common/test/data/full/vertical/vertical_89.xml b/common/test/data/full/vertical/vertical_89.xml index cf2dd23..aa58ffa 100644 --- a/common/test/data/full/vertical/vertical_89.xml +++ b/common/test/data/full/vertical/vertical_89.xml @@ -3,7 +3,7 @@ <!-- UTF-8 characters are acceptable… HTML entities are not --> <h1>Inline content…</h1> </html> - <video youtube="1.50:vl9xrfxcr38,1.25:qxNX4REGqx4,1.0:BGU1poJDgOY,0.75:8rK9vnpystQ" slug="S1V14_Summary" name="S1V14: Summary"/> + <video youtube_id_1_5=""vl9xrfxcr38"" youtube_id_1_25=""qxNX4REGqx4"" youtube_id_1_0=""BGU1poJDgOY"" youtube_id_0_75=""8rK9vnpystQ"" slug="S1V14_Summary" name="S1V14: Summary"/> <customtag tag="S1" slug="discuss_91" impl="discuss"/> <customtag page="70" slug="book_92" impl="book"/> <customtag lecnum="1" slug="slides_93" impl="slides"/> diff --git a/common/test/data/full/vertical/vertical_94.xml b/common/test/data/full/vertical/vertical_94.xml index c979604..46b6085 100644 --- a/common/test/data/full/vertical/vertical_94.xml +++ b/common/test/data/full/vertical/vertical_94.xml @@ -1,5 +1,5 @@ <sequential> - <video youtube="0.75:3NIegrCmA5k,1.0:eLAyO33baQ8,1.25:m1zWi_sh4Aw,1.50:EG-fRTJln_E" slug="S2V1_Review_KVL_KCL" name="S2V1: Review KVL, KCL"/> + <video youtube_id_0_75=""3NIegrCmA5k"" youtube_id_1_0=""eLAyO33baQ8"" youtube_id_1_25=""m1zWi_sh4Aw"" youtube_id_1_5=""EG-fRTJln_E"" slug="S2V1_Review_KVL_KCL" name="S2V1: Review KVL, KCL"/> <customtag tag="S2" slug="discuss_95" impl="discuss"/> <customtag page="54" slug="book_96" impl="book"/> <customtag lecnum="2" slug="slides_97" impl="slides"/> diff --git a/common/test/data/full/vertical/vertical_98.xml b/common/test/data/full/vertical/vertical_98.xml index 5afa197..f48b068 100644 --- a/common/test/data/full/vertical/vertical_98.xml +++ b/common/test/data/full/vertical/vertical_98.xml @@ -1,5 +1,5 @@ <sequential> - <video youtube="0.75:S_1NaY5te8Q,1.0:G_2F9wivspM,1.25:b-r7dISY-Uc,1.50:jjxHom0oXWk" slug="S2V2_Demo-_KVL_KCL" name="S2V2: Demo- KVL, KCL"/> + <video youtube_id_0_75=""S_1NaY5te8Q"" youtube_id_1_0=""G_2F9wivspM"" youtube_id_1_25=""b-r7dISY-Uc"" youtube_id_1_5=""jjxHom0oXWk"" slug="S2V2_Demo-_KVL_KCL" name="S2V2: Demo- KVL, KCL"/> <customtag tag="S2" slug="discuss_99" impl="discuss"/> <customtag page="56" slug="book_100" impl="book"/> <customtag lecnum="2" slug="slides_101" impl="slides"/> diff --git a/common/test/data/full/video/welcome.xml b/common/test/data/full/video/welcome.xml index 53eef55..7f56ae4 100644 --- a/common/test/data/full/video/welcome.xml +++ b/common/test/data/full/video/welcome.xml @@ -1 +1 @@ -<video youtube="0.75:izygArpw-Qo,1.0:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" format="Video" display_name="Welcome…"/> +<video youtube_id_0_75=""izygArpw-Qo"" youtube_id_1_0=""p2Q6BrNhdh8"" youtube_id_1_25=""1EeWXzPdhSA"" youtube_id_1_5=""rABDYkeK0x8"" format="Video" display_name="Welcome…"/> diff --git a/common/test/data/simple/course.xml b/common/test/data/simple/course.xml index b86a189..c9bb5ec 100644 --- a/common/test/data/simple/course.xml +++ b/common/test/data/simple/course.xml @@ -1,13 +1,13 @@ <course name="A Simple Course" org="edX" course="simple" graceperiod="1 day 5 hours 59 minutes 59 seconds" slug="2012_Fall"> <chapter name="Overview"> - <video name="Welcome" youtube="0.75:izygArpw-Qo,1.0:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8"/> + <video name="Welcome" youtube_id_0_75=""izygArpw-Qo"" youtube_id_1_0=""p2Q6BrNhdh8"" youtube_id_1_25=""1EeWXzPdhSA"" youtube_id_1_5=""rABDYkeK0x8""/> <videosequence format="Lecture Sequence" name="A simple sequence"> <html name="toylab" filename="toylab"/> - <video name="S0V1: Video Resources" youtube="0.75:EuzkdzfR0i8,1.0:1bK-WdDi6Qw,1.25:0v1VzoDVUTM,1.50:Bxk_-ZJb240"/> + <video name="S0V1: Video Resources" youtube_id_0_75=""EuzkdzfR0i8"" youtube_id_1_0=""1bK-WdDi6Qw"" youtube_id_1_25=""0v1VzoDVUTM"" youtube_id_1_5=""Bxk_-ZJb240""/> </videosequence> <section name="Lecture 2"> <sequential> - <video youtube="1.0:TBvX7HzxexQ"/> + <video youtube_id_1_0=""TBvX7HzxexQ""/> <problem name="L1 Problem 1" points="1" type="lecture" showanswer="attempted" filename="L1_Problem_1" rerandomize="never"/> </sequential> </section> @@ -18,7 +18,7 @@ <problem type="lecture" showanswer="attempted" rerandomize="true" display_name="A simple coding problem" name="Simple coding problem" filename="ps01-simple" url_name="ps01-simple"/> </sequential> </section> - <video name="Lost Video" youtube="1.0:TBvX7HzxexQ"/> + <video name="Lost Video" youtube_id_1_0=""TBvX7HzxexQ""/> <sequential format="Lecture Sequence" url_name='test_sequence'> <vertical url_name='test_vertical'> <html url_name='test_html'> diff --git a/common/test/data/test_exam_registration/course/2012_Fall.xml b/common/test/data/test_exam_registration/course/2012_Fall.xml index 77eca9f..1e3dbf0 100644 --- a/common/test/data/test_exam_registration/course/2012_Fall.xml +++ b/common/test/data/test_exam_registration/course/2012_Fall.xml @@ -2,9 +2,9 @@ <chapter url_name="Overview"> <videosequence url_name="Toy_Videos"> <html url_name="toylab"/> - <video url_name="Video_Resources" youtube="1.0:1bK-WdDi6Qw"/> + <video url_name="Video_Resources" youtube_id_1_0=""1bK-WdDi6Qw""/> </videosequence> - <video url_name="Welcome" youtube="1.0:p2Q6BrNhdh8"/> + <video url_name="Welcome" youtube_id_1_0=""p2Q6BrNhdh8""/> </chapter> <chapter url_name="Ch2"> <html url_name="test_html"> diff --git a/common/test/data/test_start_date/course/2012_Fall.xml b/common/test/data/test_start_date/course/2012_Fall.xml index 77eca9f..1e3dbf0 100644 --- a/common/test/data/test_start_date/course/2012_Fall.xml +++ b/common/test/data/test_start_date/course/2012_Fall.xml @@ -2,9 +2,9 @@ <chapter url_name="Overview"> <videosequence url_name="Toy_Videos"> <html url_name="toylab"/> - <video url_name="Video_Resources" youtube="1.0:1bK-WdDi6Qw"/> + <video url_name="Video_Resources" youtube_id_1_0=""1bK-WdDi6Qw""/> </videosequence> - <video url_name="Welcome" youtube="1.0:p2Q6BrNhdh8"/> + <video url_name="Welcome" youtube_id_1_0=""p2Q6BrNhdh8""/> </chapter> <chapter url_name="Ch2"> <html url_name="test_html"> diff --git a/common/test/data/toy/chapter/secret/magic.xml b/common/test/data/toy/chapter/secret/magic.xml index dd16380..0b2c7f6 100644 --- a/common/test/data/toy/chapter/secret/magic.xml +++ b/common/test/data/toy/chapter/secret/magic.xml @@ -1,3 +1,3 @@ <chapter> - <video url_name="toyvideo" youtube="blahblah"/> + <video url_name="toyvideo" youtube_id_1_0=""OEoXaMPEzfM""/> </chapter> diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml index 46dba8b..44d05da 100644 --- a/common/test/data/toy/course/2012_Fall.xml +++ b/common/test/data/toy/course/2012_Fall.xml @@ -2,11 +2,11 @@ <chapter url_name="Overview"> <videosequence url_name="Toy_Videos"> <html url_name="secret:toylab"/> - <video url_name="Video_Resources" youtube="1.0:1bK-WdDi6Qw"/> + <video url_name="Video_Resources" youtube_id_1_0=""1bK-WdDi6Qw""/> </videosequence> - <video url_name="Welcome" youtube="1.0:p2Q6BrNhdh8"/> - <video url_name="video_123456789012" youtube="1.0:p2Q6BrNhdh8"/> - <video url_name="video_123456789012" youtube="1.0:p2Q6BrNhdh8"/> + <video url_name="Welcome" youtube_id_1_0=""p2Q6BrNhdh8""/> + <video url_name="video_123456789012" youtube_id_1_0=""p2Q6BrNhdh8""/> + <video url_name="video_4f66f493ac8f" youtube_id_1_0=""p2Q6BrNhdh8""/> </chapter> <chapter url_name="secret:magic"/> </course> diff --git a/common/test/data/two_toys/video/Video_Resources.xml b/common/test/data/two_toys/video/Video_Resources.xml index 88657ed..a78f6df 100644 --- a/common/test/data/two_toys/video/Video_Resources.xml +++ b/common/test/data/two_toys/video/Video_Resources.xml @@ -1 +1 @@ -<video youtube="1.0:1bK-WdDi6Qw" display_name="Video Resources"/> +<video youtube_id_1_0=""1bK-WdDi6Qw"" display_name="Video Resources"/> diff --git a/common/test/data/two_toys/video/Welcome.xml b/common/test/data/two_toys/video/Welcome.xml index 7f7e238..9c61bcd 100644 --- a/common/test/data/two_toys/video/Welcome.xml +++ b/common/test/data/two_toys/video/Welcome.xml @@ -1 +1 @@ -<video youtube="1.0:p2Q6BrNhdh9" display_name="Welcome"/> +<video youtube_id_1_0="p2Q6BrNhdh9" display_name="Welcome"/> diff --git a/lms/templates/video.html b/lms/templates/video.html index 91b5f63..77c8a5e 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -19,16 +19,17 @@ width="640" height="390"></embed> </object> %else: -<div id="video_${id}" class="video" - - % if not settings.MITX_FEATURES['STUB_VIDEO_FOR_TESTING']: - data-streams="${streams}" - % endif - - data-show-captions="${show_captions}" - data-start="${start}" data-end="${end}" - data-caption-asset-path="${caption_asset_path}" - data-autoplay="${settings.MITX_FEATURES['AUTOPLAY_VIDEOS']}"> + <div id="video_${id}" + class="video" + data-youtube-id-0-75="${youtube_id_0_75}" + data-youtube-id-1-0="${youtube_id_1_0}" + data-youtube-id-1-25="${youtube_id_1_25}" + data-youtube-id-1-5="${youtube_id_1_5}" + data-show-captions="${show_captions}" + data-start="${start}" + data-end="${end}" + data-caption-asset-path="${caption_asset_path}" + data-autoplay="${settings.MITX_FEATURES['AUTOPLAY_VIDEOS']}"> <div class="tc-wrapper"> <article class="video-wrapper"> <section class="video-player">