diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index 63b98eda64..13574d6449 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -798,6 +798,92 @@ class VideoExportTestCase(VideoDescriptorTestBase): self.assertEqual(xml.get('display_name'), u'\u8fd9\u662f\u6587') +@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 diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index b520b5bee6..4102e1e1a7 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -977,6 +977,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: @@ -1008,10 +1013,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 } @@ -1036,4 +1040,5 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler "duration": val_video_data.get('duration', None), "transcripts": transcripts, "encoded_videos": encoded_videos, + "all_sources": all_sources, } diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index b940ddc9c8..8d1f6055a8 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/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], } ) diff --git a/lms/djangoapps/mobile_api/video_outlines/serializers.py b/lms/djangoapps/mobile_api/video_outlines/serializers.py index 3dcd56567f..0b64e3a4c3 100644 --- a/lms/djangoapps/mobile_api/video_outlines/serializers.py +++ b/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 diff --git a/lms/djangoapps/mobile_api/video_outlines/tests.py b/lms/djangoapps/mobile_api/video_outlines/tests.py index f5513ad39c..a41d2ea1ac 100644 --- a/lms/djangoapps/mobile_api/video_outlines/tests.py +++ b/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()