Merge pull request #16590 from open-craft/jill/video-block-all-sources

Adds all HTML5 sources to the returned video API data

common/lib/xmodule/xmodule/tests/test_video.py

@@ -799,6 +799,92 @@ class VideoExportTestCase(VideoDescriptorTestBase):
 
 
 @ddt.ddt
+@patch.object(settings, 'FEATURES', create=True, new={
+    'FALLBACK_TO_ENGLISH_TRANSCRIPTS': False,
+})
+class VideoDescriptorStudentViewDataTestCase(unittest.TestCase):
+    """
+    Make sure that VideoDescriptor returns the expected student_view_data.
+    """
+
+    VIDEO_URL_1 = 'http://www.example.com/source_low.mp4'
+    VIDEO_URL_2 = 'http://www.example.com/source_med.mp4'
+    VIDEO_URL_3 = 'http://www.example.com/source_high.mp4'
+
+    @ddt.data(
+        # Ensure no extra data is returned if video module configured only for web display.
+        (
+            {'only_on_web': True},
+            {'only_on_web': True},
+        ),
+        # Ensure that the deprecated `source` attribute is included in the `all_sources` list.
+        (
+            {
+                'only_on_web': False,
+                'youtube_id_1_0': None,
+                'source': VIDEO_URL_1,
+            },
+            {
+                'only_on_web': False,
+                'duration': None,
+                'transcripts': {},
+                'encoded_videos': {
+                    'fallback': {'url': VIDEO_URL_1, 'file_size': 0},
+                },
+                'all_sources': [VIDEO_URL_1],
+            },
+        ),
+        # Ensure that `html5_sources` take precendence over deprecated `source` url
+        (
+            {
+                'only_on_web': False,
+                'youtube_id_1_0': None,
+                'source': VIDEO_URL_1,
+                'html5_sources': [VIDEO_URL_2, VIDEO_URL_3],
+            },
+            {
+                'only_on_web': False,
+                'duration': None,
+                'transcripts': {},
+                'encoded_videos': {
+                    'fallback': {'url': VIDEO_URL_2, 'file_size': 0},
+                },
+                'all_sources': [VIDEO_URL_2, VIDEO_URL_3, VIDEO_URL_1],
+            },
+        ),
+        # Ensure that YouTube URLs are included in `encoded_videos`, but not `all_sources`.
+        (
+            {
+                'only_on_web': False,
+                'youtube_id_1_0': 'abc',
+                'html5_sources': [VIDEO_URL_2, VIDEO_URL_3],
+            },
+            {
+                'only_on_web': False,
+                'duration': None,
+                'transcripts': {},
+                'encoded_videos': {
+                    'fallback': {'url': VIDEO_URL_2, 'file_size': 0},
+                    'youtube': {'url': 'https://www.youtube.com/watch?v=abc', 'file_size': 0},
+                },
+                'all_sources': [VIDEO_URL_2, VIDEO_URL_3],
+            },
+        ),
+    )
+    @ddt.unpack
+    @patch('xmodule.video_module.video_module.is_val_transcript_feature_enabled_for_course')
+    def test_student_view_data(self, field_data, expected_student_view_data, mock_transcript_feature):
+        """
+        Ensure that student_view_data returns the expected results for video modules.
+        """
+        mock_transcript_feature.return_value = False
+        descriptor = instantiate_descriptor(**field_data)
+        descriptor.runtime.course_id = MagicMock()
+        student_view_data = descriptor.student_view_data()
+        self.assertEquals(student_view_data, expected_student_view_data)
+
+
+@ddt.ddt
 @patch.object(settings, 'YOUTUBE', create=True, new={
     # YouTube JavaScript API
     'API': 'www.youtube.com/iframe_api',

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

@@ -979,6 +979,11 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
 
         encoded_videos = {}
         val_video_data = {}
+        all_sources = self.html5_sources or []
+
+        # `source` is a deprecated field, but we include it for backwards compatibility.
+        if self.source:
+            all_sources.append(self.source)
 
         # Check in VAL data first if edx_video_id exists
         if self.edx_video_id:
@@ -1010,10 +1015,9 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
 
         # Fall back to other video URLs in the video module if not found in VAL
         if not encoded_videos:
-            video_url = self.html5_sources[0] if self.html5_sources else self.source
-            if video_url:
+            if all_sources:
                 encoded_videos["fallback"] = {
-                    "url": video_url,
+                    "url": all_sources[0],
                     "file_size": 0,  # File size is unknown for fallback URLs
                 }
 
@@ -1038,4 +1042,5 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
             "duration": val_video_data.get('duration', None),
             "transcripts": transcripts,
             "encoded_videos": encoded_videos,
+            "all_sources": all_sources,
         }

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -1366,6 +1366,7 @@ class TestVideoDescriptorStudentViewJson(TestCase):
                     "fallback": {"url": self.TEST_SOURCE_URL, "file_size": 0},
                     "youtube": {"url": self.TEST_YOUTUBE_EXPECTED_URL, "file_size": 0}
                 },
+                "all_sources": [self.TEST_SOURCE_URL],
             }
         )
 
@@ -1380,6 +1381,7 @@ class TestVideoDescriptorStudentViewJson(TestCase):
                 "duration": None,
                 "transcripts": {self.TEST_LANGUAGE: self.transcript_url},
                 "encoded_videos": {"youtube": {"url": self.TEST_YOUTUBE_EXPECTED_URL, "file_size": 0}},
+                "all_sources": [],
             }
         )
 
@@ -1397,6 +1399,7 @@ class TestVideoDescriptorStudentViewJson(TestCase):
                 "only_on_web": False,
                 "duration": self.TEST_DURATION,
                 "transcripts": {self.TEST_LANGUAGE: self.transcript_url},
+                'all_sources': [self.TEST_SOURCE_URL],
             }
         )
 

lms/djangoapps/mobile_api/video_outlines/serializers.py

@@ -172,6 +172,8 @@ def video_summary(video_profiles, course_id, video_descriptor, request, local_ca
         "only_on_web": video_descriptor.only_on_web,
     }
 
+    all_sources = []
+
     if video_descriptor.only_on_web:
         ret = {
             "video_url": None,
@@ -180,6 +182,7 @@ def video_summary(video_profiles, course_id, video_descriptor, request, local_ca
             "size": 0,
             "transcripts": {},
             "language": None,
+            "all_sources": all_sources,
         }
         ret.update(always_available_data)
         return ret
@@ -201,9 +204,13 @@ def video_summary(video_profiles, course_id, video_descriptor, request, local_ca
     # Then fall back to VideoDescriptor fields for video URLs
     elif video_descriptor.html5_sources:
         video_url = video_descriptor.html5_sources[0]
+        all_sources = video_descriptor.html5_sources
     else:
         video_url = video_descriptor.source
 
+    if video_descriptor.source:
+        all_sources.append(video_descriptor.source)
+
     # Get duration/size, else default
     duration = video_data.get('duration', None)
     size = default_encoded_video.get('file_size', 0)
@@ -236,7 +243,8 @@ def video_summary(video_profiles, course_id, video_descriptor, request, local_ca
         "size": size,
         "transcripts": transcripts,
         "language": video_descriptor.get_default_transcript_language(transcripts_info),
-        "encoded_videos": video_data.get('profiles')
+        "encoded_videos": video_data.get('profiles'),
+        "all_sources": all_sources,
     }
     ret.update(always_available_data)
     return ret

lms/djangoapps/mobile_api/video_outlines/tests.py

@@ -66,6 +66,7 @@ class TestVideoAPITestCase(MobileAPITestCase):
         self.edx_video_id = 'testing-123'
         self.video_url = 'http://val.edx.org/val/video.mp4'
         self.video_url_high = 'http://val.edx.org/val/video_high.mp4'
+        self.video_url_low = 'http://val.edx.org/val/video_low.mp4'
         self.youtube_url = 'http://val.edx.org/val/youtube.mp4'
         self.html5_video_url = 'http://video.edx.org/html5/video.mp4'
 
@@ -461,7 +462,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour
         self.assertEqual(course_outline[0]["summary"]["category"], "video")
         self.assertTrue(course_outline[0]["summary"]["only_on_web"])
 
-    def test_mobile_api_config(self):
+    def test_mobile_api_video_profiles(self):
         """
         Tests VideoSummaryList with different MobileApiConfig video_profiles
         """
@@ -496,6 +497,7 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour
         )
 
         expected_output = {
+            'all_sources': [],
             'category': u'video',
             'video_thumbnail_url': None,
             'language': u'en',
@@ -559,6 +561,39 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour
         course_outline[0]['summary'].pop("id")
         self.assertEqual(course_outline[0]['summary'], expected_output)
 
+    def test_mobile_api_html5_sources(self):
+        """
+        Tests VideoSummaryList without the video pipeline, using fallback HTML5 video URLs
+        """
+        self.login_and_enroll()
+        descriptor = ItemFactory.create(
+            parent=self.other_unit,
+            category="video",
+            display_name=u"testing html5 sources",
+            edx_video_id=None,
+            source=self.video_url_high,
+            html5_sources=[self.video_url_low],
+        )
+        expected_output = {
+            'all_sources': [self.video_url_low, self.video_url_high],
+            'category': u'video',
+            'video_thumbnail_url': None,
+            'language': u'en',
+            'id': unicode(descriptor.scope_ids.usage_id),
+            'name': u'testing html5 sources',
+            'video_url': self.video_url_low,
+            'duration': None,
+            'transcripts': {
+                'en': 'http://testserver/api/mobile/v0.5/video_outlines/transcripts/{}/testing_html5_sources/en'.format(self.course.id)  # pylint: disable=line-too-long
+            },
+            'only_on_web': False,
+            'encoded_videos': None,
+            'size': 0,
+        }
+
+        course_outline = self.api_response().data
+        self.assertEqual(course_outline[0]['summary'], expected_output)
+
     def test_video_not_in_val(self):
         self.login_and_enroll()
         self._create_video_with_subs()