Merge pull request #4527 from edx/oleg/xml-course-transcripts-fix

Fix transcripts in XML course when no static_asset_path is set.

common/lib/xmodule/xmodule/video_module/video_handlers.py

@@ -11,7 +11,6 @@ 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
 
@@ -196,12 +195,12 @@ class VideoStudentViewHandlers(object):
         if transcript_name:
             # Get the asset path for course
             asset_path = None
-            if hasattr(self.descriptor.runtime, 'modulestore'):
-                course = self.descriptor.runtime.modulestore.get_course(self.course_id)
+            course = self.descriptor.runtime.modulestore.get_course(self.course_id)
+            if course.static_asset_path:
                 asset_path = course.static_asset_path
             else:
-                # Handle XML Courses that don't have modulestore in the runtime
-                asset_path = getattr(self.descriptor, 'data_dir', None)
+                # It seems static_asset_path is not set in any XMLModuleStore courses.
+                asset_path = getattr(course, 'data_dir', '')
 
             if asset_path:
                 response = Response(
@@ -251,7 +250,7 @@ class VideoStudentViewHandlers(object):
 
             try:
                 transcript = self.translation(request.GET.get('videoId', None))
-            except NotFoundError, ex:
+            except (TypeError, NotFoundError) as ex:
                 log.info(ex.message)
                 # Try to return static URL redirection as last resort
                 # if no translation is required

lms/djangoapps/courseware/tests/test_video_handlers.py

@@ -8,6 +8,7 @@ import textwrap
 import json
 from datetime import timedelta
 from webob import Request
+from mock import MagicMock, Mock
 
 from xmodule.contentstore.content import StaticContent
 from xmodule.contentstore.django import contentstore
@@ -23,7 +24,6 @@ from xmodule.video_module.transcripts_utils import (
     TranscriptException,
     TranscriptsGenerationException,
 )
-from opaque_keys.edx.locations import AssetLocation
 
 SRT_content = textwrap.dedent("""
         0
@@ -401,16 +401,18 @@ 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):
+    def test_translation_static_transcript_xml_with_data_dirc(self):
         """
-        Set course static_asset_path and ensure we get redirected to that path
-        if it isn't found in the contentstore
+        Test id data_dir is set in XML course.
+
+        Set course data_dir 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 = modulestore()
-        with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
-            store.update_item(self.course, self.user.id)
+        # Simulate data_dir set in course.
+        test_modulestore = MagicMock()
+        attrs = {'get_course.return_value': Mock(data_dir='dummy/static', static_asset_path='')}
+        test_modulestore.configure_mock(**attrs)
+        self.item_descriptor.runtime.modulestore = test_modulestore
 
         # Test youtube style en
         request = Request.blank('/translation/en?videoId=12345')
@@ -437,16 +439,16 @@ class TestTranscriptTranslationGetDispatch(TestVideo):
         response = self.item.transcript(request=request, dispatch='translation/uk')
         self.assertEqual(response.status, '404 Not Found')
 
-    def test_xml_transcript(self):
+    def test_translation_static_transcript(self):
         """
-        Set data_dir and remove runtime modulestore to simulate an XMLModuelStore course.
-        Then run the same tests as static_asset_path run.
+        Set course static_asset_path and ensure we get redirected to that path
+        if it isn't found in the contentstore
         """
-        # Simulate XMLModuleStore xmodule
-        self.item_descriptor.data_dir = 'dummy/static'
-        del self.item_descriptor.runtime.modulestore
-
-        self.assertFalse(self.course.static_asset_path)
+        self.course.static_asset_path = 'dummy/static'
+        self.course.save()
+        store = modulestore()
+        with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id):
+            store.update_item(self.course, self.user.id)
 
         # Test youtube style en
         request = Request.blank('/translation/en?videoId=12345')