MA-2164 add youtube when mobile video encoding missing

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

@@ -589,6 +589,20 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
 
         return xml
 
+    def create_youtube_url(self, youtube_id):
+        """
+
+        Args:
+            youtube_id: The ID of the video to create a link for
+
+        Returns:
+            A full youtube url to the video whose ID is passed in
+        """
+        if youtube_id:
+            return 'https://www.youtube.com/watch?v={0}'.format(youtube_id)
+        else:
+            return ''
+
     def get_context(self):
         """
         Extend context by data for transcript basic tab.
@@ -612,10 +626,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
                 if val_youtube_id:
                     video_id = val_youtube_id
 
-            if video_id:
-                return 'http://youtu.be/{0}'.format(video_id)
-            else:
-                return ''
+            return self.create_youtube_url(video_id)
 
         _ = self.runtime.service(self, "i18n").ugettext
         video_url.update({
@@ -848,7 +859,8 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
                     val_video_data = edxval_api.get_video_info(self.edx_video_id)
                     # Unfortunately, the VAL API is inconsistent in how it returns the encodings, so remap here.
                     for enc_vid in val_video_data.pop('encoded_videos'):
-                        encoded_videos[enc_vid['profile']] = {key: enc_vid[key] for key in ["url", "file_size"]}
+                        if enc_vid['profile'] in video_profile_names:
+                            encoded_videos[enc_vid['profile']] = {key: enc_vid[key] for key in ["url", "file_size"]}
                 except edxval_api.ValVideoNotFoundError:
                     pass
 
@@ -861,6 +873,14 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
                     "file_size": 0,  # File size is unknown for fallback URLs
                 }
 
+            # Include youtube link if there is no encoding for mobile- ie only a fallback URL or no encodings at all
+            # We are including a fallback URL for older versions of the mobile app that don't handle Youtube urls
+            if self.youtube_id_1_0:
+                encoded_videos["youtube"] = {
+                    "url": self.create_youtube_url(self.youtube_id_1_0),
+                    "file_size": 0,  # File size is not relevant for external link
+                }
+
         transcripts_info = self.get_transcripts_info()
         transcripts = {
             lang: self.runtime.handler_url(self, 'transcript', 'download', query="lang=" + lang, thirdparty=True)

common/test/acceptance/pages/studio/video/video.py

@@ -53,7 +53,7 @@ DISPLAY_NAME = "Component Display Name"
 DEFAULT_SETTINGS = [
     # basic
     [DISPLAY_NAME, 'Video', False],
-    ['Default Video URL', 'http://youtu.be/3_yD_cEKoCk, , ', False],
+    ['Default Video URL', 'https://www.youtube.com/watch?v=3_yD_cEKoCk, , ', False],
 
     # advanced
     [DISPLAY_NAME, 'Video', False],

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -1013,14 +1013,17 @@ class TestVideoDescriptorStudentViewJson(TestCase):
         'file_size': 222,
     }
     TEST_EDX_VIDEO_ID = 'test_edx_video_id'
+    TEST_YOUTUBE_ID = 'test_youtube_id'
+    TEST_YOUTUBE_EXPECTED_URL = 'https://www.youtube.com/watch?v=test_youtube_id'
 
     def setUp(self):
         super(TestVideoDescriptorStudentViewJson, self).setUp()
-        sample_xml = (
-            "<video display_name='Test Video'> " +
-            "<source src='" + self.TEST_SOURCE_URL + "'/> " +
-            "<transcript language='" + self.TEST_LANGUAGE + "' src='german_translation.srt' /> " +
-            "</video>"
+        video_declaration = "<video display_name='Test Video' youtube_id_1_0=\'" + self.TEST_YOUTUBE_ID + "\'>"
+        sample_xml = ''.join([
+            video_declaration,
+            "<source src='", self.TEST_SOURCE_URL, "'/> ",
+            "<transcript language='", self.TEST_LANGUAGE, "' src='german_translation.srt' /> ",
+            "</video>"]
         )
         self.transcript_url = "transcript_url"
         self.video = instantiate_descriptor(data=sample_xml)
@@ -1055,7 +1058,24 @@ class TestVideoDescriptorStudentViewJson(TestCase):
         }
         return self.video.student_view_data(context)
 
-    def verify_result_with_fallback_url(self, result):
+    def verify_result_with_fallback_and_youtube(self, result):
+        """
+        Verifies the result is as expected when returning "fallback" video data (not from VAL).
+        """
+        self.assertDictEqual(
+            result,
+            {
+                "only_on_web": False,
+                "duration": None,
+                "transcripts": {self.TEST_LANGUAGE: self.transcript_url},
+                "encoded_videos": {
+                    "fallback": {"url": self.TEST_SOURCE_URL, "file_size": 0},
+                    "youtube": {"url": self.TEST_YOUTUBE_EXPECTED_URL, "file_size": 0}
+                },
+            }
+        )
+
+    def verify_result_with_youtube_url(self, result):
         """
         Verifies the result is as expected when returning "fallback" video data (not from VAL).
         """
@@ -1065,7 +1085,7 @@ class TestVideoDescriptorStudentViewJson(TestCase):
                 "only_on_web": False,
                 "duration": None,
                 "transcripts": {self.TEST_LANGUAGE: self.transcript_url},
-                "encoded_videos": {"fallback": {"url": self.TEST_SOURCE_URL, "file_size": 0}},
+                "encoded_videos": {"youtube": {"url": self.TEST_YOUTUBE_EXPECTED_URL, "file_size": 0}},
             }
         )
 
@@ -1093,21 +1113,55 @@ class TestVideoDescriptorStudentViewJson(TestCase):
 
     def test_no_edx_video_id(self):
         result = self.get_result()
-        self.verify_result_with_fallback_url(result)
+        self.verify_result_with_fallback_and_youtube(result)
+
+    def test_no_edx_video_id_and_no_fallback(self):
+        video_declaration = "<video display_name='Test Video' youtube_id_1_0=\'{}\'>".format(self.TEST_YOUTUBE_ID)
+        # the video has no source listed, only a youtube link, so no fallback url will be provided
+        sample_xml = ''.join([
+            video_declaration,
+            "<transcript language='", self.TEST_LANGUAGE, "' src='german_translation.srt' /> ",
+            "</video>"
+        ])
+        self.transcript_url = "transcript_url"
+        self.video = instantiate_descriptor(data=sample_xml)
+        self.video.runtime.handler_url = Mock(return_value=self.transcript_url)
+        result = self.get_result()
+        self.verify_result_with_youtube_url(result)
 
-    @ddt.data(
-        *itertools.product([True, False], [True, False], [True, False])
-    )
-    @ddt.unpack
-    def test_with_edx_video_id(self, allow_cache_miss, video_exists_in_val, associate_course_in_val):
+    @ddt.data(True, False)
+    def test_with_edx_video_id_video_associated_in_val(self, allow_cache_miss):
+        """
+        Tests retrieving a video that is stored in VAL and associated with a course in VAL.
+        """
         self.video.edx_video_id = self.TEST_EDX_VIDEO_ID
-        if video_exists_in_val:
-            self.setup_val_video(associate_course_in_val)
+        self.setup_val_video(associate_course_in_val=True)
+        # the video is associated in VAL so no cache miss should ever happen but test retrieval in both contexts
         result = self.get_result(allow_cache_miss)
-        if video_exists_in_val and (associate_course_in_val or allow_cache_miss):
+        self.verify_result_with_val_profile(result)
+
+    @ddt.data(True, False)
+    def test_with_edx_video_id_video_unassociated_in_val(self, allow_cache_miss):
+        """
+        Tests retrieving a video that is stored in VAL but not associated with a course in VAL.
+        """
+        self.video.edx_video_id = self.TEST_EDX_VIDEO_ID
+        self.setup_val_video(associate_course_in_val=False)
+        result = self.get_result(allow_cache_miss)
+        if allow_cache_miss:
             self.verify_result_with_val_profile(result)
         else:
-            self.verify_result_with_fallback_url(result)
+            self.verify_result_with_fallback_and_youtube(result)
+
+    @ddt.data(True, False)
+    def test_with_edx_video_id_video_not_in_val(self, allow_cache_miss):
+        """
+        Tests retrieving a video that is not stored in VAL.
+        """
+        self.video.edx_video_id = self.TEST_EDX_VIDEO_ID
+        # The video is not is VAL so in contexts that do and don't allow cache misses we should always get a fallback
+        result = self.get_result(allow_cache_miss)
+        self.verify_result_with_fallback_and_youtube(result)
 
 
 @attr('shard_1')