From c0644dc9841e45990082002d6645e831d95fdc18 Mon Sep 17 00:00:00 2001
From: Carson Gee <x@carsongee.com>
Date: Wed, 16 Apr 2014 11:30:27 -0400
Subject: [PATCH] Only do static transcript redirect for english language, and
 don't offer static redirect for download Rename video test to real YouTube-ID

---
 .../xmodule/video_module/video_handlers.py    | 31 ++++++++++-------
 .../courseware/tests/test_video_handlers.py   | 34 ++++---------------
 2 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py
index 722cefacf7..a51655395c 100644
--- a/common/lib/xmodule/xmodule/video_module/video_handlers.py
+++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py
@@ -175,15 +175,21 @@ class VideoStudentViewHandlers(object):
 
     def get_static_transcript(self, request):
         """
-        Return URL for static transcript if it isn't available in the content store
+        Courses that are imported with the --nostatic flag do not show
+        transcripts/captions properly even if those captions are stored inside
+        their static folder. This adds a last resort method of redirecting to
+        the static asset path of the course if the transcript can't be found
+        inside the contentstore and the course has the static_asset_path field
+        set.
         """
-        # Try to return static redirect to the transcript as a last
-        # resort, but return 404 if we don't
         response = Response(status=404)
-        vid_id = request.GET.get('videoId', None)
+        # Only do redirect for English
+        if not self.transcript_language == 'en':
+            return response
 
-        if vid_id:
-            transcript_name = vid_id
+        video_id = request.GET.get('videoId', None)
+        if video_id:
+            transcript_name = video_id
         else:
             transcript_name = self.sub
 
@@ -238,16 +244,18 @@ class VideoStudentViewHandlers(object):
 
             try:
                 transcript = self.translation(request.GET.get('videoId', None))
+            except NotFoundError, ex:
+                log.info(ex.message)
+                # Try to return static URL redirection as last resort
+                # if no translation is required
+                return self.get_static_transcript(request)
             except (
                 TranscriptException,
-                NotFoundError,
                 UnicodeDecodeError,
-                TranscriptException,
                 TranscriptsGenerationException
             ) as ex:
                 log.info(ex.message)
-                # Try to return static URL redirection as last resort
-                return self.get_static_transcript(request)
+                response = Response(status=404)
             else:
                 response = Response(transcript, headerlist=[('Content-Language', language)])
                 response.content_type = Transcript.mime_types['sjson']
@@ -257,8 +265,7 @@ class VideoStudentViewHandlers(object):
                 transcript_content, transcript_filename, transcript_mime_type = self.get_transcript(self.transcript_download_format)
             except (NotFoundError, ValueError, KeyError, UnicodeDecodeError):
                 log.debug("Video@download exception")
-                # Return static URL or 404
-                return self.get_static_transcript(request)
+                return Response(status=404)
             else:
                 response = Response(
                     transcript_content,
diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py
index 296a81acb5..c0030e5d27 100644
--- a/lms/djangoapps/courseware/tests/test_video_handlers.py
+++ b/lms/djangoapps/courseware/tests/test_video_handlers.py
@@ -226,7 +226,7 @@ class TestTranscriptDownloadDispatch(TestVideo):
     DATA = """
         <video show_captions="true"
         display_name="A Name"
-        sub='blahblah'
+        sub='OEoXaMPEzfM'
         >
             <source src="example.mp4"/>
             <source src="example.webm"/>
@@ -279,23 +279,6 @@ class TestTranscriptDownloadDispatch(TestVideo):
         self.assertEqual(response.headers['Content-Type'], 'application/x-subrip; charset=utf-8')
         self.assertEqual(response.headers['Content-Disposition'], 'attachment; filename="塞.srt"')
 
-    def test_download_static_transcript(self):
-        """
-        Set course static_asset_path and ensure we get redirected to that path
-        if it isn't found in the contentstore
-        """
-        self.course.static_asset_path = 'dummy/static'
-        self.course.save()
-        store = editable_modulestore()
-        store.update_item(self.course, 'blahblah')
-        request = Request.blank('/download')
-        response = self.item.transcript(request=request, dispatch='download')
-        self.assertEqual(response.status, '307 Temporary Redirect')
-        self.assertIn(
-            ('Location', '/static/dummy/static/subs_blahblah.srt.sjson'),
-            response.headerlist
-        )
-
 
 class TestTranscriptTranslationGetDispatch(TestVideo):
     """
@@ -429,7 +412,7 @@ class TestTranscriptTranslationGetDispatch(TestVideo):
         self.course.static_asset_path = 'dummy/static'
         self.course.save()
         store = editable_modulestore()
-        store.update_item(self.course, 'blahblah')
+        store.update_item(self.course, 'OEoXaMPEzfM')
 
         # Test youtube style en
         request = Request.blank('/translation/en?videoId=12345')
@@ -441,23 +424,20 @@ class TestTranscriptTranslationGetDispatch(TestVideo):
         )
 
         # Test HTML5 video style
-        self.item.sub = 'blahblah'
+        self.item.sub = 'OEoXaMPEzfM'
         request = Request.blank('/translation/en')
         response = self.item.transcript(request=request, dispatch='translation/en')
         self.assertEqual(response.status, '307 Temporary Redirect')
         self.assertIn(
-            ('Location', '/static/dummy/static/subs_blahblah.srt.sjson'),
+            ('Location', '/static/dummy/static/subs_OEoXaMPEzfM.srt.sjson'),
             response.headerlist
         )
 
-        # Test different language
+        # Test different language to ensure we are just ignoring it since we can't
+        # translate with static fallback
         request = Request.blank('/translation/uk')
         response = self.item.transcript(request=request, dispatch='translation/uk')
-        self.assertEqual(response.status, '307 Temporary Redirect')
-        self.assertIn(
-            ('Location', '/static/dummy/static/uk_subs_blahblah.srt.sjson'),
-            response.headerlist
-        )
+        self.assertEqual(response.status, '404 Not Found')
 
 
 class TestStudioTranscriptTranslationGetDispatch(TestVideo):