From ab6c82342e5297713208075157fad143b1175eef Mon Sep 17 00:00:00 2001
From: Carson Gee <x@carsongee.com>
Date: Wed, 2 Apr 2014 16:10:07 -0400
Subject: [PATCH 1/2] Add last resort method to download and showing of video
 transcript

---
 .../xmodule/video_module/video_handlers.py    | 36 ++++++++++-
 .../courseware/tests/test_video_handlers.py   | 59 ++++++++++++++++++-
 2 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py
index fc06701d02..722cefacf7 100644
--- a/common/lib/xmodule/xmodule/video_module/video_handlers.py
+++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py
@@ -11,6 +11,7 @@ from webob import Response
 
 from xblock.core import XBlock
 
+from xmodule.course_module import CourseDescriptor
 from xmodule.exceptions import NotFoundError
 from xmodule.fields import RelativeTime
 
@@ -22,6 +23,7 @@ from .transcripts_utils import (
     youtube_speed_dict,
     Transcript,
     save_to_store,
+    subs_filename
 )
 
 
@@ -171,6 +173,33 @@ class VideoStudentViewHandlers(object):
 
         return content, filename, Transcript.mime_types[transcript_format]
 
+    def get_static_transcript(self, request):
+        """
+        Return URL for static transcript if it isn't available in the content store
+        """
+        # 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)
+
+        if vid_id:
+            transcript_name = vid_id
+        else:
+            transcript_name = self.sub
+
+        if transcript_name:
+            course_location = CourseDescriptor.id_to_location(self.course_id)
+            course = self.descriptor.runtime.modulestore.get_item(course_location)
+            if course.static_asset_path:
+                response = Response(
+                    status=307,
+                    location='/static/{0}/{1}'.format(
+                        course.static_asset_path,
+                        subs_filename(transcript_name, self.transcript_language)
+                    )
+                )
+        return response
+
     @XBlock.handler
     def transcript(self, request, dispatch):
         """
@@ -206,6 +235,7 @@ class VideoStudentViewHandlers(object):
 
             if language != self.transcript_language:
                 self.transcript_language = language
+
             try:
                 transcript = self.translation(request.GET.get('videoId', None))
             except (
@@ -216,7 +246,8 @@ class VideoStudentViewHandlers(object):
                 TranscriptsGenerationException
             ) as ex:
                 log.info(ex.message)
-                response = Response(status=404)
+                # Try to return static URL redirection as last resort
+                return self.get_static_transcript(request)
             else:
                 response = Response(transcript, headerlist=[('Content-Language', language)])
                 response.content_type = Transcript.mime_types['sjson']
@@ -226,7 +257,8 @@ 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 Response(status=404)
+                # Return static URL or 404
+                return self.get_static_transcript(request)
             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 5598e5a3b4..296a81acb5 100644
--- a/lms/djangoapps/courseware/tests/test_video_handlers.py
+++ b/lms/djangoapps/courseware/tests/test_video_handlers.py
@@ -10,8 +10,9 @@ from datetime import timedelta
 from webob import Request
 
 from xmodule.contentstore.content import StaticContent
-from xmodule.modulestore import Location
 from xmodule.contentstore.django import contentstore
+from xmodule.modulestore import Location
+from xmodule.modulestore.django import editable_modulestore
 from . import BaseTestXmodule
 from .test_video_xml import SOURCE_XML
 from cache_toolbox.core import del_cached_content
@@ -225,6 +226,7 @@ class TestTranscriptDownloadDispatch(TestVideo):
     DATA = """
         <video show_captions="true"
         display_name="A Name"
+        sub='blahblah'
         >
             <source src="example.mp4"/>
             <source src="example.webm"/>
@@ -277,6 +279,23 @@ 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):
     """
@@ -402,6 +421,44 @@ class TestTranscriptTranslationGetDispatch(TestVideo):
         response = self.item.transcript(request=request, dispatch='translation/uk')
         self.assertDictEqual(json.loads(response.body), subs)
 
+    def test_translation_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')
+
+        # Test youtube style en
+        request = Request.blank('/translation/en?videoId=12345')
+        response = self.item.transcript(request=request, dispatch='translation/en')
+        self.assertEqual(response.status, '307 Temporary Redirect')
+        self.assertIn(
+            ('Location', '/static/dummy/static/subs_12345.srt.sjson'),
+            response.headerlist
+        )
+
+        # Test HTML5 video style
+        self.item.sub = 'blahblah'
+        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'),
+            response.headerlist
+        )
+
+        # Test different language
+        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
+        )
+
 
 class TestStudioTranscriptTranslationGetDispatch(TestVideo):
     """

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 2/2] 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):