From 3893f3f06928122bdfd4be25c509a3678b90b6c1 Mon Sep 17 00:00:00 2001
From: Sanford Student <sanfordstudent@C02QJ0RCG8WM.tld>
Date: Thu, 17 Mar 2016 19:56:46 -0400
Subject: [PATCH] MA-2164 add youtube when mobile video encoding missing
---
.../xmodule/video_module/video_module.py | 30 +++++--
.../acceptance/pages/studio/video/video.py | 2 +-
.../courseware/tests/test_video_mongo.py | 88 +++++++++++++++----
3 files changed, 97 insertions(+), 23 deletions(-)
diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py
index 727a0c92e62..96cc2ffceac 100644
--- a/common/lib/xmodule/xmodule/video_module/video_module.py
+++ b/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)
diff --git a/common/test/acceptance/pages/studio/video/video.py b/common/test/acceptance/pages/studio/video/video.py
index ca03863d4db..83c2df6eed8 100644
--- a/common/test/acceptance/pages/studio/video/video.py
+++ b/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],
diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py
index 9d8f70a642e..a617a4b94c0 100644
--- a/lms/djangoapps/courseware/tests/test_video_mongo.py
+++ b/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')
--
GitLab