From f317244ac4e5f9e7f8cc58e462484a4455fb2f2f Mon Sep 17 00:00:00 2001 From: Peter Fogg Date: Fri, 19 Jul 2013 11:47:28 -0400 Subject: [PATCH] Convert Video Alpha to metadata-only. --- CHANGELOG.rst | 3 + .../contentstore/features/common.py | 20 + .../features/videoalpha-editor.feature | 25 ++ .../features/videoalpha-editor.py | 18 + .../contentstore/features/videoalpha.feature | 6 + .../contentstore/features/videoalpha.py | 10 + cms/templates/widgets/tabs-aggregator.html | 2 +- .../widgets/tabs/metadata-edit-tab.html | 4 + .../xmodule/xmodule/tests/test_videoalpha.py | 332 +++++++++++++++-- .../lib/xmodule/xmodule/videoalpha_module.py | 345 +++++++++++++----- lms/djangoapps/courseware/tests/__init__.py | 6 +- .../courseware/tests/test_videoalpha_mongo.py | 69 ++-- .../courseware/tests/test_videoalpha_xml.py | 87 ++--- 13 files changed, 727 insertions(+), 200 deletions(-) create mode 100644 cms/djangoapps/contentstore/features/videoalpha-editor.feature create mode 100644 cms/djangoapps/contentstore/features/videoalpha-editor.py create mode 100644 cms/djangoapps/contentstore/features/videoalpha.feature create mode 100644 cms/djangoapps/contentstore/features/videoalpha.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cba3d2dbf8..9de2890c37 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -42,6 +42,9 @@ Common: Add a manage.py that knows about edx-platform specific settings and proj Common: Added *experimental* support for jsinput type. +Studio: Remove XML from HTML5 video component editor. All settings are +moved to be edited as metadata. + Common: Added setting to specify Celery Broker vhost Common: Utilize new XBlock bulk save API in LMS and CMS. diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index 8b23bc3635..30ee0518c1 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -228,6 +228,26 @@ def i_created_a_video_component(step): ) +@step('I have created a Video Alpha component$') +def i_created_video_alpha(step): + step.given('I have enabled the videoalpha advanced module') + world.css_click('a.course-link') + step.given('I have added a new subsection') + step.given('I expand the first section') + world.css_click('a.new-unit-item') + world.css_click('.large-advanced-icon') + world.click_component_from_menu('videoalpha', None, '.xmodule_VideoAlphaModule') + + +@step('I have enabled the (.*) advanced module$') +def i_enabled_the_advanced_module(step, module): + step.given('I have opened a new course section in Studio') + world.css_click('.nav-course-settings') + world.css_click('.nav-course-settings-advanced') + type_in_codemirror(0, '["%s"]' % module) + press_the_notification_button(step, 'Save') + + @step('I have clicked the new unit button') def open_new_unit(step): step.given('I have opened a new course section in Studio') diff --git a/cms/djangoapps/contentstore/features/videoalpha-editor.feature b/cms/djangoapps/contentstore/features/videoalpha-editor.feature new file mode 100644 index 0000000000..6021772c1c --- /dev/null +++ b/cms/djangoapps/contentstore/features/videoalpha-editor.feature @@ -0,0 +1,25 @@ +Feature: Video Alpha Component Editor + As a course author, I want to be able to create videoalpha components. + + Scenario: User can view metadata + Given I have created a Video Alpha component + And I edit and select Settings + Then I see the correct videoalpha settings and default values + + Scenario: User can modify display name + Given I have created a Video Alpha component + And I edit and select Settings + Then I can modify the display name + And my display name change is persisted on save + + @skip + 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 + + @skip + 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/videoalpha-editor.py b/cms/djangoapps/contentstore/features/videoalpha-editor.py new file mode 100644 index 0000000000..2d882d2fda --- /dev/null +++ b/cms/djangoapps/contentstore/features/videoalpha-editor.py @@ -0,0 +1,18 @@ +# disable missing docstring +# pylint: disable=C0111 + +from lettuce import world, step + + +@step('I see the correct videoalpha settings and default values$') +def correct_videoalpha_settings(_step): + world.verify_all_setting_entries([['Default Speed', '', False], + ['Display Name', 'Video Alpha', False], + ['Download Track', '', False], + ['Download Video', '', False], + ['HTML5 Subtitles', '', False], + ['Show Captions', 'True', False], + ['Speed: .75x', '', False], + ['Speed: 1.25x', '', False], + ['Speed: 1.5x', '', False], + ['Video Sources', '', False]]) diff --git a/cms/djangoapps/contentstore/features/videoalpha.feature b/cms/djangoapps/contentstore/features/videoalpha.feature new file mode 100644 index 0000000000..58f32da68b --- /dev/null +++ b/cms/djangoapps/contentstore/features/videoalpha.feature @@ -0,0 +1,6 @@ +Feature: Video Alpha Component + As a course author, I want to be able to view my created videos in Studio. + + Scenario: Autoplay is disabled in Studio + Given I have created a Video Alpha component + Then when I view the video alpha it does not have autoplay enabled diff --git a/cms/djangoapps/contentstore/features/videoalpha.py b/cms/djangoapps/contentstore/features/videoalpha.py new file mode 100644 index 0000000000..f95284a00a --- /dev/null +++ b/cms/djangoapps/contentstore/features/videoalpha.py @@ -0,0 +1,10 @@ +# disable missing docstring +# pylint: disable=C0111 + +from lettuce import world, step + + +@step('when I view the video alpha it does not have autoplay enabled') +def does_not_autoplay(_step): + assert world.css_find('.videoalpha')[0]['data-autoplay'] == 'False' + assert world.css_has_class('.video_control', 'play') diff --git a/cms/templates/widgets/tabs-aggregator.html b/cms/templates/widgets/tabs-aggregator.html index d5953005b5..02c34d4dc1 100644 --- a/cms/templates/widgets/tabs-aggregator.html +++ b/cms/templates/widgets/tabs-aggregator.html @@ -9,7 +9,7 @@ % endfor -
+
% for tab in tabs:
<%include file="${tab['template']}" args="tabName=tab['name']"/> diff --git a/cms/templates/widgets/tabs/metadata-edit-tab.html b/cms/templates/widgets/tabs/metadata-edit-tab.html index 4fc8314afd..0caf9d2078 100644 --- a/cms/templates/widgets/tabs/metadata-edit-tab.html +++ b/cms/templates/widgets/tabs/metadata-edit-tab.html @@ -20,5 +20,9 @@ <%static:include path="js/metadata-option-entry.underscore" /> + +
diff --git a/common/lib/xmodule/xmodule/tests/test_videoalpha.py b/common/lib/xmodule/xmodule/tests/test_videoalpha.py index 659bfb2fa0..cb122b2e9a 100644 --- a/common/lib/xmodule/xmodule/tests/test_videoalpha.py +++ b/common/lib/xmodule/xmodule/tests/test_videoalpha.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +#pylint: disable=W0212 """Test for Video Alpha Xmodule functional logic. These tests data readed from xml or from mongo. @@ -13,11 +14,15 @@ the course, section, subsection, unit, etc. """ import unittest -from xmodule.videoalpha_module import VideoAlphaDescriptor from . import LogicTest -from lxml import etree -from pkg_resources import resource_string from .import get_test_system +from xmodule.modulestore import Location +from xmodule.videoalpha_module import VideoAlphaDescriptor, _create_youtube_string +from xmodule.video_module import VideoDescriptor +from .test_import import DummySystem + +from textwrap import dedent + class VideoAlphaModuleTest(LogicTest): """Logic tests for VideoAlpha Xmodule.""" @@ -27,30 +32,62 @@ class VideoAlphaModuleTest(LogicTest): 'data': '' } - def test_get_timeframe_no_parameters(self): - "Make sure that timeframe() works correctly w/o parameters" - xmltree = etree.fromstring('test') - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, ('', '')) + def test_parse_time_empty(self): + """Ensure parse_time returns correctly with None or empty string.""" + expected = '' + self.assertEqual(VideoAlphaDescriptor._parse_time(None), expected) + self.assertEqual(VideoAlphaDescriptor._parse_time(''), expected) - def test_get_timeframe_with_one_parameter(self): - "Make sure that timeframe() works correctly with one parameter" - xmltree = etree.fromstring( - 'test' - ) - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, (247, '')) + def test_parse_time(self): + """Ensure that times are parsed correctly into seconds.""" + expected = 247 + output = VideoAlphaDescriptor._parse_time('00:04:07') + self.assertEqual(output, expected) - def test_get_timeframe_with_two_parameters(self): - "Make sure that timeframe() works correctly with two parameters" - xmltree = etree.fromstring( - '''test''' + 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 = VideoAlphaDescriptor._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 = VideoAlphaDescriptor._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( + VideoAlphaDescriptor._parse_youtube(youtube_str), + VideoAlphaDescriptor._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( + VideoAlphaDescriptor._parse_youtube(''), + {'0.75': '', + '1.00': '', + '1.25': '', + '1.50': ''} ) - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, (247, 47079)) class VideoAlphaDescriptorTest(unittest.TestCase): @@ -65,17 +102,248 @@ class VideoAlphaDescriptorTest(unittest.TestCase): def test_get_context(self): """"test get_context""" correct_tabs = [ - { - 'name': "XML", - 'template': "videoalpha/codemirror-edit.html", - 'css': {'scss': [resource_string(__name__, - '../css/tabs/codemirror.scss')]}, - 'current': True, - }, { 'name': "Settings", - 'template': "tabs/metadata-edit-tab.html" + 'template': "tabs/metadata-edit-tab.html", + 'current': True } ] rendered_context = self.descriptor.get_context() self.assertListEqual(rendered_context['tabs'], correct_tabs) + + def test_create_youtube_string(self): + """ + Test that Youtube ID strings are correctly created when writing + back out to XML. + """ + system = DummySystem(load_error_modules=True) + location = Location(["i4x", "edX", "videoalpha", "default", "SampleProblem1"]) + model_data = {'location': location} + descriptor = VideoAlphaDescriptor(system, model_data) + descriptor.youtube_id_0_75 = 'izygArpw-Qo' + descriptor.youtube_id_1_0 = 'p2Q6BrNhdh8' + descriptor.youtube_id_1_25 = '1EeWXzPdhSA' + descriptor.youtube_id_1_5 = 'rABDYkeK0x8' + expected = "0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" + self.assertEqual(_create_youtube_string(descriptor), expected) + + def test_create_youtube_string_missing(self): + """ + Test that Youtube IDs which aren't explicitly set aren't included + in the output string. + """ + system = DummySystem(load_error_modules=True) + location = Location(["i4x", "edX", "videoalpha", "default", "SampleProblem1"]) + model_data = {'location': location} + descriptor = VideoAlphaDescriptor(system, model_data) + descriptor.youtube_id_0_75 = 'izygArpw-Qo' + descriptor.youtube_id_1_0 = 'p2Q6BrNhdh8' + descriptor.youtube_id_1_25 = '1EeWXzPdhSA' + expected = "0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA" + self.assertEqual(_create_youtube_string(descriptor), expected) + + +class VideoAlphaDescriptorImportTestCase(unittest.TestCase): + """ + Make sure that VideoAlphaDescriptor can import an old XML-based video correctly. + """ + + def test_constructor(self): + sample_xml = ''' + + + + + + ''' + location = Location(["i4x", "edX", "videoalpha", "default", + "SampleProblem1"]) + model_data = {'data': sample_xml, + 'location': location} + system = DummySystem(load_error_modules=True) + descriptor = VideoAlphaDescriptor(system, model_data) + self.assertEquals(descriptor.youtube_id_0_75, 'izygArpw-Qo') + self.assertEquals(descriptor.youtube_id_1_0, 'p2Q6BrNhdh8') + self.assertEquals(descriptor.youtube_id_1_25, '1EeWXzPdhSA') + self.assertEquals(descriptor.youtube_id_1_5, 'rABDYkeK0x8') + self.assertEquals(descriptor.show_captions, False) + self.assertEquals(descriptor.start_time, 1.0) + self.assertEquals(descriptor.end_time, 60) + self.assertEquals(descriptor.track, 'http://www.example.com/track') + self.assertEquals(descriptor.source, 'http://www.example.com/source.mp4') + self.assertEquals(descriptor.html5_sources, ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg']) + self.assertEquals(descriptor.data, '') + + def test_from_xml(self): + module_system = DummySystem(load_error_modules=True) + xml_data = ''' + + + + + ''' + output = VideoAlphaDescriptor.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') + self.assertEquals(output.html5_sources, ['http://www.example.com/source.mp4']) + self.assertEquals(output.data, '') + + 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 = ''' + + + + + ''' + output = VideoAlphaDescriptor.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') + self.assertEquals(output.html5_sources, ['http://www.example.com/source.mp4']) + self.assertEquals(output.data, '') + + 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 = '' + output = VideoAlphaDescriptor.from_xml(xml_data, module_system) + self.assertEquals(output.youtube_id_0_75, '') + self.assertEquals(output.youtube_id_1_0, '') + 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, '') + self.assertEquals(output.html5_sources, []) + self.assertEquals(output.data, '') + + def test_old_video_format(self): + """ + Test backwards compatibility with VideoModule's XML format. + """ + module_system = DummySystem(load_error_modules=True) + xml_data = """ + + + + + """ + output = VideoAlphaDescriptor.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') + self.assertEquals(output.html5_sources, ['http://www.example.com/source.mp4']) + self.assertEquals(output.data, '') + + def test_old_video_data(self): + module_system = DummySystem(load_error_modules=True) + xml_data = """ + + """ + video = VideoDescriptor.from_xml(xml_data, module_system) + video_alpha = VideoAlphaDescriptor(module_system, video._model_data) + self.assertEquals(video_alpha.youtube_id_0_75, 'izygArpw-Qo') + self.assertEquals(video_alpha.youtube_id_1_0, 'p2Q6BrNhdh8') + self.assertEquals(video_alpha.youtube_id_1_25, '1EeWXzPdhSA') + self.assertEquals(video_alpha.youtube_id_1_5, 'rABDYkeK0x8') + self.assertEquals(video_alpha.show_captions, False) + self.assertEquals(video_alpha.start_time, 1.0) + self.assertEquals(video_alpha.end_time, 60) + self.assertEquals(video_alpha.track, 'http://www.example.com/track') + self.assertEquals(video_alpha.source, 'http://www.example.com/source.mp4') + self.assertEquals(video_alpha.html5_sources, ['http://www.example.com/source.mp4']) + self.assertEquals(video_alpha.data, '') + + +class VideoAlphaExportTestCase(unittest.TestCase): + """ + Make sure that VideoAlphaDescriptor can export itself to XML + correctly. + """ + + def test_export_to_xml(self): + """Test that we write the correct XML on export.""" + module_system = DummySystem(load_error_modules=True) + location = Location(["i4x", "edX", "videoalpha", "default", "SampleProblem1"]) + desc = VideoAlphaDescriptor(module_system, {'location': location}) + + desc.youtube_id_0_75 = 'izygArpw-Qo' + desc.youtube_id_1_0 = 'p2Q6BrNhdh8' + desc.youtube_id_1_25 = '1EeWXzPdhSA' + desc.youtube_id_1_5 = 'rABDYkeK0x8' + desc.show_captions = False + desc.start_time = 1.0 + desc.end_time = 60 + 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 + expected = dedent('''\ + + + + + + ''') + + self.assertEquals(expected, xml) + + def test_export_to_xml_empty_parameters(self): + """Test XML export with defaults.""" + module_system = DummySystem(load_error_modules=True) + location = Location(["i4x", "edX", "videoalpha", "default", "SampleProblem1"]) + desc = VideoAlphaDescriptor(module_system, {'location': location}) + + xml = desc.export_to_xml(None) + expected = '\n' + + self.assertEquals(expected, xml) diff --git a/common/lib/xmodule/xmodule/videoalpha_module.py b/common/lib/xmodule/xmodule/videoalpha_module.py index 3ac3d5157c..a238f7d768 100644 --- a/common/lib/xmodule/xmodule/videoalpha_module.py +++ b/common/lib/xmodule/xmodule/videoalpha_module.py @@ -14,7 +14,7 @@ import json import logging from lxml import etree -from pkg_resources import resource_string, resource_listdir +from pkg_resources import resource_string from django.http import Http404 from django.conf import settings @@ -25,31 +25,93 @@ from xmodule.raw_module import RawDescriptor from xmodule.modulestore.mongo import MongoModuleStore from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent -from xblock.core import Integer, Scope, String +from xblock.core import Scope, String, Boolean, Float, List, Integer import datetime import time -import textwrap log = logging.getLogger(__name__) class VideoAlphaFields(object): """Fields for `VideoAlphaModule` and `VideoAlphaDescriptor`.""" - data = String(help="XML data for the problem", - default=textwrap.dedent('''\ - - - - - '''), - scope=Scope.content) - position = Integer(help="Current position in the video", scope=Scope.user_state, default=0) display_name = String( display_name="Display Name", help="Display name for this module", default="Video Alpha", scope=Scope.settings ) + 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 + ) + # TODO (pfogg): Do we want to show these to the user if HTML5 sources are preferred? + 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="" + ) + 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="" + ) + html5_sources = List( + help="A comma-separated list of filenames to be used with HTML5 video.", + display_name="Video Sources", + 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="" + ) + sub = String( + help="The name of the subtitle track (for non-Youtube videos).", + display_name="HTML5 Subtitles", + scope=Scope.settings, + default="" + ) class VideoAlphaModule(VideoAlphaFields, XModule): @@ -85,72 +147,6 @@ class VideoAlphaModule(VideoAlphaFields, XModule): css = {'scss': [resource_string(__name__, 'css/videoalpha/display.scss')]} js_module_name = "VideoAlpha" - def __init__(self, *args, **kwargs): - XModule.__init__(self, *args, **kwargs) - xmltree = etree.fromstring(self.data) - - # Front-end expects an empty string, or a properly formatted string with YouTube IDs. - self.youtube_streams = xmltree.get('youtube', '') - - self.sub = xmltree.get('sub') - - self.autoplay = xmltree.get('autoplay') or '' - if self.autoplay.lower() not in ['true', 'false']: - self.autoplay = 'true' - - self.position = 0 - self.show_captions = xmltree.get('show_captions', 'true') - self.sources = { - 'main': self._get_source(xmltree), - 'mp4': self._get_source(xmltree, ['mp4']), - 'webm': self._get_source(xmltree, ['webm']), - 'ogv': self._get_source(xmltree, ['ogv']), - } - self.track = self._get_track(xmltree) - self.start_time, self.end_time = self.get_timeframe(xmltree) - - def _get_source(self, xmltree, exts=None): - """Find the first valid source, which ends with one of `exts`.""" - exts = ['mp4', 'ogv', 'avi', 'webm'] if exts is None else exts - condition = lambda src: any([src.endswith(ext) for ext in exts]) - return self._get_first_external(xmltree, 'source', condition) - - def _get_track(self, xmltree): - """Find the first valid track.""" - return self._get_first_external(xmltree, 'track') - - def _get_first_external(self, xmltree, tag, condition=bool): - """Will return the first 'valid' element of the given tag. - 'valid' means that `condition('src' attribute) == True` - """ - result = None - - for element in xmltree.findall(tag): - src = element.get('src') - if condition(src): - result = src - break - return result - - def get_timeframe(self, xmltree): - """ Converts 'start_time' and 'end_time' 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('start_time')), parse_time(xmltree.get('end_time')) - def handle_ajax(self, dispatch, data): """This is not being called right now and we raise 404 error.""" log.debug(u"GET {0}".format(data)) @@ -169,12 +165,15 @@ class VideoAlphaModule(VideoAlphaFields, XModule): # cdodge: filesystem static content support. caption_asset_path = "/static/subs/" + get_ext = lambda filename: filename.rpartition('.')[-1] + sources = {get_ext(src): src for src in self.html5_sources} + sources['main'] = self.source + return self.system.render_template('videoalpha.html', { - 'youtube_streams': self.youtube_streams, + 'youtube_streams': _create_youtube_string(self), 'id': self.location.html_id(), 'sub': self.sub, - 'autoplay': self.autoplay, - 'sources': self.sources, + 'sources': sources, 'track': self.track, 'display_name': self.display_name_with_default, # This won't work when we move to data that @@ -193,18 +192,188 @@ class VideoAlphaDescriptor(VideoAlphaFields, TabsEditingDescriptor, RawDescripto module_class = VideoAlphaModule tabs = [ - { - 'name': "XML", - 'template': "videoalpha/codemirror-edit.html", - 'css': {'scss': [resource_string(__name__, 'css/tabs/codemirror.scss')]}, - 'current': True, - }, # { # 'name': "Subtitles", # 'template': "videoalpha/subtitles.html", # }, { 'name': "Settings", - 'template': "tabs/metadata-edit-tab.html" + 'template': "tabs/metadata-edit-tab.html", + 'current': True } ] + + def __init__(self, *args, **kwargs): + super(VideoAlphaDescriptor, self).__init__(*args, **kwargs) + if self.data: + model_data = VideoAlphaDescriptor._parse_video_xml(self.data) + self._model_data.update(model_data) + del self.data + + @property + def non_editable_metadata_fields(self): + non_editable_fields = super(TabsEditingDescriptor, self).non_editable_metadata_fields + non_editable_fields.extend([VideoAlphaFields.start_time, + VideoAlphaFields.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 + """ + model_data = VideoAlphaDescriptor._parse_video_xml(xml_data) + video = cls(system, model_data) + return video + + def export_to_xml(self, resource_fs): + """ + Returns an xml string representing this module, and all modules + underneath it. May also write required resources out to resource_fs + + Assumes that modules have single parentage (that no module appears twice + in the same course), and that it is thus safe to nest modules as xml + children as appropriate. + + The returned XML should be able to be parsed back into an identical + XModuleDescriptor using the from_xml method with the same system, org, + and course + """ + xml = etree.Element('videoalpha') + attrs = { + 'display_name': self.display_name, + 'show_captions': json.dumps(self.show_captions), + 'youtube': _create_youtube_string(self), + 'start_time': datetime.timedelta(seconds=self.start_time), + 'end_time': datetime.timedelta(seconds=self.end_time), + 'sub': self.sub + } + for key, value in attrs.items(): + if value: + xml.set(key, str(value)) + + for source in self.html5_sources: + ele = etree.Element('source') + ele.set('src', source) + xml.append(ele) + + if self.track: + ele = etree.Element('track') + ele.set('src', self.track) + xml.append(ele) + + return etree.tostring(xml, pretty_print=True) + + @staticmethod + 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 + + @staticmethod + def _parse_video_xml(xml_data): + """ + Parse video fields out of xml_data. The fields are set if they are + present in the XML. + """ + xml = etree.fromstring(xml_data) + model_data = {} + + conversions = { + 'show_captions': json.loads, + 'start_time': VideoAlphaDescriptor._parse_time, + 'end_time': VideoAlphaDescriptor._parse_time + } + + # VideoModule and VideoAlphaModule use different names for + # these attributes -- need to convert between them + video_compat = { + 'from': 'start_time', + 'to': 'end_time' + } + + for attr, value in xml.items(): + if attr in video_compat: + attr = video_compat[attr] + if attr == 'youtube': + speeds = VideoAlphaDescriptor._parse_youtube(value) + for speed, youtube_id in speeds.items(): + # should have made these youtube_id_1_00 for + # cleanliness, but hindsight doesn't need glasses + normalized_speed = speed[:-1] if speed.endswith('0') else speed + if youtube_id != '': + model_data['youtube_id_{0}'.format(normalized_speed.replace('.', '_'))] = youtube_id + else: + # Convert XML attrs into Python values. + if attr in conversions: + value = conversions[attr](value) + model_data[attr] = value + + sources = xml.findall('source') + if sources: + model_data['html5_sources'] = [ele.get('src') for ele in sources] + model_data['source'] = model_data['html5_sources'][0] + + track = xml.find('track') + if track is not None: + model_data['track'] = track.get('src') + + return model_data + + @staticmethod + 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() + + +def _create_youtube_string(module): + """ + Create a string of Youtube IDs from `module`'s metadata + attributes. Only writes a speed if an ID is present in the + module. Necessary for backwards compatibility with XML-based + courses. + """ + youtube_ids = [ + module.youtube_id_0_75, + module.youtube_id_1_0, + module.youtube_id_1_25, + module.youtube_id_1_5 + ] + youtube_speeds = ['0.75', '1.00', '1.25', '1.50'] + return ','.join([':'.join(pair) + for pair + in zip(youtube_speeds, youtube_ids) + if pair[1]]) diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index cdb8bb8ea8..40f1df0fbc 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/lms/djangoapps/courseware/tests/__init__.py @@ -81,12 +81,16 @@ class BaseTestXmodule(ModuleStoreTestCase): # Allow us to assert that the template was called in the same way from # different code paths while maintaining the type returned by render_template self.runtime.render_template = lambda template, context: u'{!r}, {!r}'.format(template, sorted(context.items())) + model_data = {'location': self.item_descriptor.location} model_data.update(self.MODEL_DATA) self.item_module = self.item_descriptor.module_class( - self.runtime, self.item_descriptor, model_data + self.runtime, + self.item_descriptor, + model_data ) + self.item_url = Location(self.item_module.location).url() # login all users for acces to Xmodule diff --git a/lms/djangoapps/courseware/tests/test_videoalpha_mongo.py b/lms/djangoapps/courseware/tests/test_videoalpha_mongo.py index 30cf556b9f..b4341f8508 100644 --- a/lms/djangoapps/courseware/tests/test_videoalpha_mongo.py +++ b/lms/djangoapps/courseware/tests/test_videoalpha_mongo.py @@ -4,6 +4,7 @@ from . import BaseTestXmodule from .test_videoalpha_xml import SOURCE_XML from django.conf import settings +from xmodule.videoalpha_module import _create_youtube_string class TestVideo(BaseTestXmodule): @@ -15,6 +16,14 @@ class TestVideo(BaseTestXmodule): 'data': DATA } + def setUp(self): + # Since the VideoAlphaDescriptor changes `self._model_data`, + # we need to instantiate `self.item_module` through + # `self.item_descriptor` rather than directly constructing it + super(TestVideo, self).setUp() + self.item_module = self.item_descriptor.xmodule(self.runtime) + self.item_module.runtime.render_template = lambda template, context: context + def test_handle_ajax_dispatch(self): responses = { user.username: self.clients[user.username].post( @@ -34,22 +43,31 @@ class TestVideo(BaseTestXmodule): def test_videoalpha_constructor(self): """Make sure that all parameters extracted correclty from xml""" - fragment = self.runtime.render(self.item_module, None, 'student_view') + context = self.item_module.get_html() + + sources = { + 'main': '.../mit-3091x/M-3091X-FA12-L21-3_100.mp4', + 'mp4': '.../mit-3091x/M-3091X-FA12-L21-3_100.mp4', + 'webm': '.../mit-3091x/M-3091X-FA12-L21-3_100.webm', + 'ogv': '.../mit-3091x/M-3091X-FA12-L21-3_100.ogv' + } + expected_context = { 'data_dir': getattr(self, 'data_dir', None), 'caption_asset_path': '/c4x/MITx/999/asset/subs_', - 'show_captions': self.item_module.show_captions, - 'display_name': self.item_module.display_name_with_default, - 'end': self.item_module.end_time, + 'show_captions': True, + 'display_name': 'A Name', + 'end': 3610.0, 'id': self.item_module.location.html_id(), - 'sources': self.item_module.sources, - 'start': self.item_module.start_time, - 'sub': self.item_module.sub, - 'track': self.item_module.track, - 'youtube_streams': self.item_module.youtube_streams, + 'sources': sources, + 'start': 3603.0, + 'sub': 'a_sub_file.srt.sjson', + 'track': '', + 'youtube_streams': _create_youtube_string(self.item_module), 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) } - self.assertEqual(fragment.content, self.runtime.render_template('videoalpha.html', expected_context)) + + self.assertEqual(context, expected_context) class TestVideoNonYouTube(TestVideo): @@ -57,9 +75,8 @@ class TestVideoNonYouTube(TestVideo): DATA = """ @@ -75,20 +92,28 @@ class TestVideoNonYouTube(TestVideo): """Make sure that if the 'youtube' attribute is omitted in XML, then the template generates an empty string for the YouTube streams. """ + sources = { + u'main': u'.../mit-3091x/M-3091X-FA12-L21-3_100.mp4', + u'mp4': u'.../mit-3091x/M-3091X-FA12-L21-3_100.mp4', + u'webm': u'.../mit-3091x/M-3091X-FA12-L21-3_100.webm', + u'ogv': u'.../mit-3091x/M-3091X-FA12-L21-3_100.ogv' + } + + context = self.item_module.get_html() - fragment = self.runtime.render(self.item_module, None, 'student_view') expected_context = { 'data_dir': getattr(self, 'data_dir', None), 'caption_asset_path': '/c4x/MITx/999/asset/subs_', - 'show_captions': self.item_module.show_captions, - 'display_name': self.item_module.display_name_with_default, - 'end': self.item_module.end_time, + 'show_captions': True, + 'display_name': 'A Name', + 'end': 3610.0, 'id': self.item_module.location.html_id(), - 'sources': self.item_module.sources, - 'start': self.item_module.start_time, - 'sub': self.item_module.sub, - 'track': self.item_module.track, + 'sources': sources, + 'start': 3603.0, + 'sub': 'a_sub_file.srt.sjson', + 'track': '', 'youtube_streams': '', 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) } - self.assertEqual(fragment.content, self.runtime.render_template('videoalpha.html', expected_context)) + + self.assertEqual(context, expected_context) diff --git a/lms/djangoapps/courseware/tests/test_videoalpha_xml.py b/lms/djangoapps/courseware/tests/test_videoalpha_xml.py index eaa5d1f502..b381d406f5 100644 --- a/lms/djangoapps/courseware/tests/test_videoalpha_xml.py +++ b/lms/djangoapps/courseware/tests/test_videoalpha_xml.py @@ -15,23 +15,19 @@ course, section, subsection, unit, etc. import json import unittest -from mock import Mock -from lxml import etree from django.conf import settings -from xmodule.videoalpha_module import VideoAlphaDescriptor, VideoAlphaModule +from xmodule.videoalpha_module import VideoAlphaDescriptor, _create_youtube_string from xmodule.modulestore import Location from xmodule.tests import get_test_system -from xmodule.tests import LogicTest SOURCE_XML = """ @@ -54,74 +50,53 @@ class VideoAlphaFactory(object): """Method return VideoAlpha Xmodule instance.""" location = Location(["i4x", "edX", "videoalpha", "default", "SampleProblem1"]) - model_data = {'data': VideoAlphaFactory.sample_problem_xml_youtube} - - descriptor = Mock(weight="1") + model_data = {'data': VideoAlphaFactory.sample_problem_xml_youtube, + 'location': location} system = get_test_system() system.render_template = lambda template, context: context - VideoAlphaModule.location = location - module = VideoAlphaModule(system, descriptor, model_data) + + descriptor = VideoAlphaDescriptor(system, model_data) + + module = descriptor.xmodule(system) return module -class VideoAlphaModuleTest(LogicTest): - """Tests for logic of VideoAlpha Xmodule.""" - - descriptor_class = VideoAlphaDescriptor - - raw_model_data = { - 'data': '' - } - - def test_get_timeframe_no_parameters(self): - xmltree = etree.fromstring('test') - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, ('', '')) - - def test_get_timeframe_with_one_parameter(self): - xmltree = etree.fromstring( - 'test' - ) - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, (247, '')) - - def test_get_timeframe_with_two_parameters(self): - xmltree = etree.fromstring( - '''test''' - ) - output = self.xmodule.get_timeframe(xmltree) - self.assertEqual(output, (247, 47079)) - - class VideoAlphaModuleUnitTest(unittest.TestCase): """Unit tests for VideoAlpha Xmodule.""" - def test_videoalpha_constructor(self): + def test_videoalpha_get_html(self): """Make sure that all parameters extracted correclty from xml""" module = VideoAlphaFactory.create() - module.runtime.render_template = lambda template, context: u'{!r}, {!r}'.format(template, sorted(context.items())) + module.runtime.render_template = lambda template, context: context + + sources = { + 'main': '.../mit-3091x/M-3091X-FA12-L21-3_100.mp4', + 'mp4': '.../mit-3091x/M-3091X-FA12-L21-3_100.mp4', + 'ogv': '.../mit-3091x/M-3091X-FA12-L21-3_100.ogv', + 'webm': '.../mit-3091x/M-3091X-FA12-L21-3_100.webm', + } - fragment = module.runtime.render(module, None, 'student_view') expected_context = { 'caption_asset_path': '/static/subs/', - 'sub': module.sub, + 'sub': 'a_sub_file.srt.sjson', 'data_dir': getattr(self, 'data_dir', None), - 'display_name': module.display_name_with_default, - 'end': module.end_time, - 'start': module.start_time, + 'display_name': 'A Name', + 'end': 3610.0, + 'start': 3603.0, 'id': module.location.html_id(), - 'show_captions': module.show_captions, - 'sources': module.sources, - 'youtube_streams': module.youtube_streams, - 'track': module.track, + 'show_captions': True, + 'sources': sources, + 'youtube_streams': _create_youtube_string(module), + 'track': '', 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) } - self.assertEqual(fragment.content, module.runtime.render_template('videoalpha.html', expected_context)) + + self.assertEqual(module.get_html(), expected_context) + + def test_videoalpha_instance_state(self): + module = VideoAlphaFactory.create() self.assertDictEqual( json.loads(module.get_instance_state()),