Many changes to finish up PR. To wit:

-- Changed docstrings to use double quote. -- More consistent naming for youtube IDs (normal_speed_id -> youtube_id_1_0). -- Formatting changes for readability. -- Updated test XML to expect JSON-encoded strings; prevents tests from spuriously failing.

Many changes to finish up PR. To wit:
8159e861 · Peter Fogg · d8f307a5 · 8159e861 · 8159e861 · 8159e861
Commit 8159e861 authored 11 years ago by Peter Fogg
--- a/common/lib/xmodule/xmodule/js/fixtures/video.html
+++ b/common/lib/xmodule/xmodule/js/fixtures/video.html
@@ -3,7 +3,7 @@
    <div id="example">
      <div id="video_id" class="video"
        data-youtube-id-0-75="slowerSpeedYoutubeId"
-        data-normal-speed-video-id="normalSpeedYoutubeId"
+        data-youtube-id-1-0="normalSpeedYoutubeId"
        data-show-captions="true"
        data-start=""
        data-end=""

--- a/common/lib/xmodule/xmodule/js/src/video/display.coffee
+++ b/common/lib/xmodule/xmodule/js/src/video/display.coffee
@@ -29,8 +29,8 @@ class @Video
    @videos = {}
    if @el.data('youtube-id-0-75')
      @videos['0.75'] = @el.data('youtube-id-0-75')
-    if @el.data('normal-speed-video-id')
-      @videos['1.0'] = @el.data('normal-speed-video-id')
+    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')

--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -67,7 +67,7 @@ class VideoModule(VideoFields, XModule):
    def get_html(self):
        return self.system.render_template('video.html', {
            'youtube_id_0_75': self.youtube_id_0_75,
-            'normal_speed_video_id': self.youtube_id_1_0,
+            '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(),
@@ -109,9 +109,11 @@ class VideoDescriptor(VideoFields,
        """
        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)
@@ -123,29 +125,35 @@ class VideoDescriptor(VideoFields,
                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:
@@ -154,11 +162,11 @@ def _get_first_external(xmltree, tag):


 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': ''}
    videos = data.split(',')
    for video in videos:

--- a/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml
+++ b/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml
@@ -16,10 +16,10 @@
    <vertical slug="vertical_94" graceperiod="1 day 12 hours 59 minutes 59 seconds"
        showanswer="attempted" rerandomize="never">
        <video
-            youtube_id_0_75="XNh13VZhThQ"
-            youtube_id_1_0="XbDRmF6J0K0"
-            youtube_id_1_25="JDty12WEQWk"
-            youtube_id_1_5="wELKGj-5iyM"
+            youtube_id_0_75="&quot;XNh13VZhThQ&quot;"
+            youtube_id_1_0="&quot;XbDRmF6J0K0&quot;"
+            youtube_id_1_25="&quot;JDty12WEQWk&quot;"
+            youtube_id_1_5="&quot;wELKGj-5iyM&quot;"
            slug="What_s_next"
            name="What's next"/>
        <html slug="html_95">Minor correction: Six elements (five resistors)…

--- a/common/test/data/full/vertical/vertical_58.xml
+++ b/common/test/data/full/vertical/vertical_58.xml
 <sequential>
-  <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"/>
+  <video youtube_id_1_5="&quot;8kARlsUt9lM&quot;" youtube_id_1_25="&quot;4cLA-IME32w&quot;" youtube_id_1_0="&quot;pFOrD8k9_p4&quot;" youtube_id_0_75="&quot;CcgAYu0n0bg&quot;" 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"/>

--- 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_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"/>
+  <video youtube_id_1_5="&quot;vl9xrfxcr38&quot;" youtube_id_1_25="&quot;qxNX4REGqx4&quot;" youtube_id_1_0="&quot;BGU1poJDgOY&quot;" youtube_id_0_75="&quot;8rK9vnpystQ&quot;" 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"/>

--- a/common/test/data/full/vertical/vertical_94.xml
+++ b/common/test/data/full/vertical/vertical_94.xml
 <sequential>
-  <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"/>
+  <video youtube_id_0_75="&quot;3NIegrCmA5k&quot;" youtube_id_1_0="&quot;eLAyO33baQ8&quot;" youtube_id_1_25="&quot;m1zWi_sh4Aw&quot;" youtube_id_1_5="&quot;EG-fRTJln_E&quot;" 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"/>

--- a/common/test/data/full/vertical/vertical_98.xml
+++ b/common/test/data/full/vertical/vertical_98.xml
 <sequential>
-  <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"/>
+  <video youtube_id_0_75="&quot;S_1NaY5te8Q&quot;" youtube_id_1_0="&quot;G_2F9wivspM&quot;" youtube_id_1_25="&quot;b-r7dISY-Uc&quot;" youtube_id_1_5="&quot;jjxHom0oXWk&quot;" 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"/>

--- a/common/test/data/full/video/welcome.xml
+++ b/common/test/data/full/video/welcome.xml
-<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…"/>
+<video youtube_id_0_75="&quot;izygArpw-Qo&quot;" youtube_id_1_0="&quot;p2Q6BrNhdh8&quot;" youtube_id_1_25="&quot;1EeWXzPdhSA&quot;" youtube_id_1_5="&quot;rABDYkeK0x8&quot;" format="Video" display_name="Welcome…"/>
--- a/common/test/data/simple/course.xml
+++ b/common/test/data/simple/course.xml
 <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_id_0_75="izygArpw-Qo" youtube_id_1_0="p2Q6BrNhdh8" youtube_id_1_25="1EeWXzPdhSA" youtube_id_1_5="rABDYkeK0x8"/>
+    <video name="Welcome" youtube_id_0_75="&quot;izygArpw-Qo&quot;" youtube_id_1_0="&quot;p2Q6BrNhdh8&quot;" youtube_id_1_25="&quot;1EeWXzPdhSA&quot;" youtube_id_1_5="&quot;rABDYkeK0x8&quot;"/>
    <videosequence format="Lecture Sequence" name="A simple sequence">
      <html name="toylab" filename="toylab"/>
-      <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"/>
+      <video name="S0V1: Video Resources" youtube_id_0_75="&quot;EuzkdzfR0i8&quot;" youtube_id_1_0="&quot;1bK-WdDi6Qw&quot;" youtube_id_1_25="&quot;0v1VzoDVUTM&quot;" youtube_id_1_5="&quot;Bxk_-ZJb240&quot;"/>
    </videosequence>
    <section name="Lecture 2">
    <sequential>
-        <video youtube_id_1_0="TBvX7HzxexQ"/>
+        <video youtube_id_1_0="&quot;TBvX7HzxexQ&quot;"/>
        <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_id_1_0="TBvX7HzxexQ"/>
+    <video name="Lost Video" youtube_id_1_0="&quot;TBvX7HzxexQ&quot;"/>
    <sequential format="Lecture Sequence" url_name='test_sequence'>
        <vertical url_name='test_vertical'>
            <html url_name='test_html'>

--- 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_id_1_0="1bK-WdDi6Qw"/>
+      <video url_name="Video_Resources" youtube_id_1_0="&quot;1bK-WdDi6Qw&quot;"/>
    </videosequence>
-    <video url_name="Welcome" youtube_id_1_0="p2Q6BrNhdh8"/>
+    <video url_name="Welcome" youtube_id_1_0="&quot;p2Q6BrNhdh8&quot;"/>
  </chapter>
  <chapter url_name="Ch2">
     <html url_name="test_html">

--- 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_id_1_0="1bK-WdDi6Qw"/>
+      <video url_name="Video_Resources" youtube_id_1_0="&quot;1bK-WdDi6Qw&quot;"/>
    </videosequence>
-    <video url_name="Welcome" youtube_id_1_0="p2Q6BrNhdh8"/>
+    <video url_name="Welcome" youtube_id_1_0="&quot;p2Q6BrNhdh8&quot;"/>
  </chapter>
  <chapter url_name="Ch2">
     <html url_name="test_html">

--- a/common/test/data/toy/chapter/secret/magic.xml
+++ b/common/test/data/toy/chapter/secret/magic.xml
 <chapter>
-    <video url_name="toyvideo" youtube_id_1_0="OEoXaMPEzfM"/>
+    <video url_name="toyvideo" youtube_id_1_0="&quot;OEoXaMPEzfM&quot;"/>
 </chapter>
--- 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_id_1_0="1bK-WdDi6Qw"/>
+      <video url_name="Video_Resources" youtube_id_1_0="&quot;1bK-WdDi6Qw&quot;"/>
    </videosequence>
-    <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"/>
+    <video url_name="Welcome" youtube_id_1_0="&quot;p2Q6BrNhdh8&quot;"/>
+    <video url_name="video_123456789012" youtube_id_1_0="&quot;p2Q6BrNhdh8&quot;"/>
+    <video url_name="video_4f66f493ac8f" youtube_id_1_0="&quot;p2Q6BrNhdh8&quot;"/>
  </chapter>
  <chapter url_name="secret:magic"/>
 </course>
--- a/common/test/data/two_toys/video/Video_Resources.xml
+++ b/common/test/data/two_toys/video/Video_Resources.xml
-<video youtube_id_1_0="1bK-WdDi6Qw" display_name="Video Resources"/>
+<video youtube_id_1_0="&quot;1bK-WdDi6Qw&quot;" display_name="Video Resources"/>
--- a/lms/templates/video.html
+++ b/lms/templates/video.html
@@ -21,12 +21,12 @@
      </section>
    </div>
  </div>
-%elif settings.MITX_FEATURES.get('USE_YOUTUBE_OBJECT_API') and normal_speed_video_id:
+%elif settings.MITX_FEATURES.get('USE_YOUTUBE_OBJECT_API') and youtube_id_1_0:
  <object width="640" height="390">
    <param name="movie"
-           value="https://www.youtube.com/v/${normal_speed_video_id}?version=3&amp;autoplay=1&amp;rel=0"></param>
+           value="https://www.youtube.com/v/${youtube_id_1_0}?version=3&amp;autoplay=1&amp;rel=0"></param>
    <param name="allowScriptAccess" value="always"></param>
-    <embed src="https://www.youtube.com/v/${normal_speed_video_id}?version=3&amp;autoplay=1&amp;rel=0"
+    <embed src="https://www.youtube.com/v/${youtube_id_1_0}?version=3&amp;autoplay=1&amp;rel=0"
           type="application/x-shockwave-flash"
           allowscriptaccess="always"
           width="640" height="390"></embed>
@@ -35,7 +35,7 @@
  <div id="video_${id}"
       class="video"
       data-youtube-id-0-75="${youtube_id_0_75}"
-       data-normal-speed-video-id="${normal_speed_video_id}"
+       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}"