From 8159e861415c58977c69e69827ecba64c95d909b Mon Sep 17 00:00:00 2001
From: Peter Fogg <pfogg@edx.org>
Date: Mon, 17 Jun 2013 11:53:27 -0400
Subject: [PATCH] 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.
---
 .../lib/xmodule/xmodule/js/fixtures/video.html |  2 +-
 .../xmodule/js/src/video/display.coffee        |  4 ++--
 common/lib/xmodule/xmodule/video_module.py     | 18 +++++++++++++-----
 .../Administrivia_and_Circuit_Elements.xml     |  8 ++++----
 common/test/data/full/vertical/vertical_58.xml |  2 +-
 common/test/data/full/vertical/vertical_89.xml |  2 +-
 common/test/data/full/vertical/vertical_94.xml |  2 +-
 common/test/data/full/vertical/vertical_98.xml |  2 +-
 common/test/data/full/video/welcome.xml        |  2 +-
 common/test/data/simple/course.xml             |  8 ++++----
 .../course/2012_Fall.xml                       |  4 ++--
 .../data/test_start_date/course/2012_Fall.xml  |  4 ++--
 common/test/data/toy/chapter/secret/magic.xml  |  2 +-
 common/test/data/toy/course/2012_Fall.xml      |  8 ++++----
 .../data/two_toys/video/Video_Resources.xml    |  2 +-
 lms/templates/video.html                       |  8 ++++----
 16 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/common/lib/xmodule/xmodule/js/fixtures/video.html b/common/lib/xmodule/xmodule/js/fixtures/video.html
index adcc31a2c8..1b27255b1e 100644
--- 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=""
diff --git a/common/lib/xmodule/xmodule/js/src/video/display.coffee b/common/lib/xmodule/xmodule/js/src/video/display.coffee
index 582188f752..d9c31b2c8d 100644
--- 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')
diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py
index 6c10fa0899..dbb35816db 100644
--- 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:
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 c0b157bcc5..600dc906b3 100644
--- 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)…
diff --git a/common/test/data/full/vertical/vertical_58.xml b/common/test/data/full/vertical/vertical_58.xml
index cd262512ae..c5034bf56c 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_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"/>
diff --git a/common/test/data/full/vertical/vertical_89.xml b/common/test/data/full/vertical/vertical_89.xml
index 3f5af20eef..aa58ffa119 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_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"/>
diff --git a/common/test/data/full/vertical/vertical_94.xml b/common/test/data/full/vertical/vertical_94.xml
index 716af41e71..46b608516c 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_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"/>
diff --git a/common/test/data/full/vertical/vertical_98.xml b/common/test/data/full/vertical/vertical_98.xml
index c69959c292..f48b068b7f 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_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"/>
diff --git a/common/test/data/full/video/welcome.xml b/common/test/data/full/video/welcome.xml
index 7a503bd608..7f56ae4df0 100644
--- a/common/test/data/full/video/welcome.xml
+++ b/common/test/data/full/video/welcome.xml
@@ -1 +1 @@
-<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…"/>
diff --git a/common/test/data/simple/course.xml b/common/test/data/simple/course.xml
index 529528ca0a..c9bb5ec8b2 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_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'>
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 9cd963ab63..1e3dbf0757 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_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">
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 9cd963ab63..1e3dbf0757 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_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">
diff --git a/common/test/data/toy/chapter/secret/magic.xml b/common/test/data/toy/chapter/secret/magic.xml
index 62756b8896..0b2c7f643d 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_id_1_0="OEoXaMPEzfM"/>
+    <video url_name="toyvideo" youtube_id_1_0="&quot;OEoXaMPEzfM&quot;"/>
 </chapter>
diff --git a/common/test/data/toy/course/2012_Fall.xml b/common/test/data/toy/course/2012_Fall.xml
index d8e49a50d5..44d05da122 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_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>
diff --git a/common/test/data/two_toys/video/Video_Resources.xml b/common/test/data/two_toys/video/Video_Resources.xml
index e9ce8f2dae..a78f6dfe51 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_id_1_0="1bK-WdDi6Qw" display_name="Video Resources"/>
+<video youtube_id_1_0="&quot;1bK-WdDi6Qw&quot;" display_name="Video Resources"/>
diff --git a/lms/templates/video.html b/lms/templates/video.html
index 9dd0168f39..cd25a9c716 100644
--- 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}"