diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index 73b0bf01b7..a6b1da42a3 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -592,3 +592,17 @@ class VideoTranscriptsMixin(object): raise ValueError return content, filename, Transcript.mime_types[transcript_format] + + def get_default_transcript_language(self): + """ + Returns the default transcript language for this video module. + """ + if self.transcript_language in self.transcripts: + transcript_language = self.transcript_language + elif self.sub: + transcript_language = u'en' + elif len(self.transcripts) > 0: + transcript_language = sorted(self.transcripts)[0] + else: + transcript_language = u'en' + return transcript_language diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 6a323a9139..d10bd65790 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -152,12 +152,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, transcript_language = u'en' languages = {'en': 'English'} else: - if self.transcript_language in self.transcripts: - transcript_language = self.transcript_language - elif self.sub: - transcript_language = u'en' - else: - transcript_language = sorted(self.transcripts.keys())[0] + transcript_language = self.get_default_transcript_language() native_languages = {lang: label for lang, label in settings.LANGUAGES if len(lang) == 2} languages = { @@ -177,7 +172,6 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, sorted_languages = OrderedDict(sorted_languages) return track_url, transcript_language, sorted_languages - def get_html(self): transcript_download_format = self.transcript_download_format if not (self.download_track and self.track) else None sources = filter(None, self.html5_sources) @@ -201,16 +195,22 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, # internally for download links (source, html5_sources) and the youtube # stream. if self.edx_video_id and edxval_api: - val_video_urls = edxval_api.get_urls_for_profiles( - self.edx_video_id, ["desktop_mp4", "youtube"] - ) - # VAL will always give us the keys for the profiles we asked for, but - # if it doesn't have an encoded video entry for that Video + Profile, the - # value will map to `None` - if val_video_urls["desktop_mp4"] and self.download_video: - download_video_link = val_video_urls["desktop_mp4"] - if val_video_urls["youtube"]: - youtube_streams = "1.00:{}".format(val_video_urls["youtube"]) + try: + val_video_urls = edxval_api.get_urls_for_profiles( + self.edx_video_id, ["desktop_mp4", "youtube"] + ) + # VAL will always give us the keys for the profiles we asked for, but + # if it doesn't have an encoded video entry for that Video + Profile, the + # value will map to `None` + if val_video_urls["desktop_mp4"] and self.download_video: + download_video_link = val_video_urls["desktop_mp4"] + if val_video_urls["youtube"]: + youtube_streams = "1.00:{}".format(val_video_urls["youtube"]) + except edxval_api.ValInternalError: + # VAL raises this exception if it can't find data for the edx video ID. This can happen if the + # course data is ported to a machine that does not have the VAL data. So for now, pass on this + # exception and fallback to whatever we find in the VideoDescriptor. + log.warning("Could not retrieve information from VAL for edx Video ID: %s.", self.edx_video_id) # If there was no edx_video_id, or if there was no download specified # for it, we fall back on whatever we find in the VideoDescriptor diff --git a/lms/djangoapps/mobile_api/users/serializers.py b/lms/djangoapps/mobile_api/users/serializers.py index 19014212d9..b0ae321a3c 100644 --- a/lms/djangoapps/mobile_api/users/serializers.py +++ b/lms/djangoapps/mobile_api/users/serializers.py @@ -44,7 +44,7 @@ class CourseField(serializers.RelatedField): return { "id": course_id, "name": course.display_name, - "number": course.number, + "number": course.display_number_with_default, "org": course.display_org_with_default, "start": course.start, "end": course.end, diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index d923aa3a90..06046d72f5 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -4,6 +4,7 @@ Tests for users API from rest_framework.test import APITestCase from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.django import modulestore from courseware.tests.factories import UserFactory from django.core.urlresolvers import reverse from mobile_api.users.serializers import CourseEnrollmentSerializer @@ -93,3 +94,16 @@ class TestUserApi(ModuleStoreTestCase, APITestCase): serialized = CourseEnrollmentSerializer(CourseEnrollment.enrollments_for_user(self.user)[0]).data # pylint: disable=E1101 self.assertEqual(serialized['course']['video_outline'], None) self.assertEqual(serialized['course']['name'], self.course.display_name) + self.assertEqual(serialized['course']['number'], self.course.id.course) + self.assertEqual(serialized['course']['org'], self.course.id.org) + + def test_course_serializer_with_display_overrides(self): + self.course.display_coursenumber = "overridden_number" + self.course.display_organization = "overridden_org" + modulestore().update_item(self.course, self.user.id) + + self.client.login(username=self.username, password=self.password) + self._enroll() + serialized = CourseEnrollmentSerializer(CourseEnrollment.enrollments_for_user(self.user)[0]).data # pylint: disable=E1101 + self.assertEqual(serialized['course']['number'], self.course.display_coursenumber) + self.assertEqual(serialized['course']['org'], self.course.display_organization) diff --git a/lms/djangoapps/mobile_api/video_outlines/serializers.py b/lms/djangoapps/mobile_api/video_outlines/serializers.py index 1d312c599d..029e72a7e7 100644 --- a/lms/djangoapps/mobile_api/video_outlines/serializers.py +++ b/lms/djangoapps/mobile_api/video_outlines/serializers.py @@ -40,7 +40,8 @@ class BlockOutline(object): block = child_to_parent[block] if block is not self.start_block: block_path.append({ - 'name': block.display_name, + # to be consistent with other edx-platform clients, return the defaulted display name + 'name': block.display_name_with_default, 'category': block.category, }) return reversed(block_path) @@ -76,20 +77,30 @@ class BlockOutline(object): kwargs=kwargs, request=self.request, ) - return unit_url, section_url + return unit_url, section_url, block_path user = self.request.user while stack: curr_block = stack.pop() + if curr_block.hide_from_toc: + # For now, if the 'hide_from_toc' setting is set on the block, do not traverse down + # the hierarchy. The reason being is that these blocks may not have human-readable names + # to display on the mobile clients. + # Eventually, we'll need to figure out how we want these blocks to be displayed on the + # mobile clients. As, they are still accessible in the browser, just not navigatable + # from the table-of-contents. + continue + if curr_block.category in self.categories_to_outliner: if not has_access(user, 'load', curr_block, course_key=self.course_id): continue summary_fn = self.categories_to_outliner[curr_block.category] block_path = list(path(curr_block)) - unit_url, section_url = find_urls(curr_block) + unit_url, section_url, _ = find_urls(curr_block) + yield { "path": block_path, "named_path": [b["name"] for b in block_path[:-1]], @@ -145,7 +156,7 @@ def video_summary(course, course_id, video_descriptor, request, local_cache): "size": size, "name": video_descriptor.display_name, "transcripts": transcripts, - "language": video_descriptor.transcript_language, + "language": video_descriptor.get_default_transcript_language(), "category": video_descriptor.category, "id": unicode(video_descriptor.scope_ids.usage_id), } diff --git a/lms/djangoapps/mobile_api/video_outlines/tests.py b/lms/djangoapps/mobile_api/video_outlines/tests.py index 167359ca6b..134826552c 100644 --- a/lms/djangoapps/mobile_api/video_outlines/tests.py +++ b/lms/djangoapps/mobile_api/video_outlines/tests.py @@ -4,6 +4,7 @@ Tests for video outline API from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.video_module import transcripts_utils from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.django import modulestore from courseware.tests.factories import UserFactory from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE from django.core.urlresolvers import reverse @@ -27,13 +28,13 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase): super(TestVideoOutline, self).setUp() self.user = UserFactory.create() self.course = CourseFactory.create(mobile_available=True) - section = ItemFactory.create( + self.section = ItemFactory.create( parent_location=self.course.location, category="chapter", display_name=u"test factory section omega \u03a9", ) self.sub_section = ItemFactory.create( - parent_location=section.location, + parent_location=self.section.location, category="sequential", display_name=u"test subsection omega \u03a9", ) @@ -50,6 +51,12 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase): metadata={'graded': True, 'format': 'Homework'}, display_name=u"test unit omega 2 \u03a9", ) + self.nameless_unit = ItemFactory.create( + parent_location=self.sub_section.location, + category="vertical", + metadata={'graded': True, 'format': 'Homework'}, + display_name=None, + ) self.edx_video_id = 'testing-123' @@ -90,15 +97,28 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase): } ]}) - subid = uuid4().hex - self.video = ItemFactory.create( - parent_location=self.unit.location, - category="video", - edx_video_id=self.edx_video_id, - display_name=u"test video omega \u03a9", - sub=subid - ) + self.client.login(username=self.user.username, password='test') + def test_course_not_available(self): + nonmobile = CourseFactory.create() + url = reverse('video-summary-list', kwargs={'course_id': unicode(nonmobile.id)}) + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + + def _get_video_summary_list(self): + """ + Calls the video-summary-list endpoint, expecting a success response + """ + url = reverse('video-summary-list', kwargs={'course_id': unicode(self.course.id)}) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + return response.data # pylint: disable=E1103 + + def _create_video_with_subs(self): + """ + Creates and returns a video with stored subtitles. + """ + subid = uuid4().hex transcripts_utils.save_subs_to_store({ 'start': [100, 200, 240, 390, 1000], 'end': [200, 240, 380, 1000, 1500], @@ -111,16 +131,16 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase): ]}, subid, self.course) - - self.client.login(username=self.user.username, password='test') - - def test_course_not_available(self): - nonmobile = CourseFactory.create() - url = reverse('video-summary-list', kwargs={'course_id': unicode(nonmobile.id)}) - response = self.client.get(url) - self.assertEqual(response.status_code, 403) + return ItemFactory.create( + parent_location=self.unit.location, + category="video", + edx_video_id=self.edx_video_id, + display_name=u"test video omega \u03a9", + sub=subid + ) def test_course_list(self): + self._create_video_with_subs() ItemFactory.create( parent_location=self.other_unit.location, category="video", @@ -133,7 +153,6 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase): display_name=u"test video omega 3 \u03a9", source=self.html5_video_url ) - ItemFactory.create( parent_location=self.unit.location, category="video", @@ -142,11 +161,7 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase): visible_to_staff_only=True, ) - url = reverse('video-summary-list', kwargs={'course_id': unicode(self.course.id)}) - response = self.client.get(url) - - self.assertEqual(response.status_code, 200) - course_outline = response.data # pylint: disable=E1103 + course_outline = self._get_video_summary_list() self.assertEqual(len(course_outline), 3) vid = course_outline[0] self.assertTrue('test_subsection_omega_%CE%A9' in vid['section_url']) @@ -157,14 +172,77 @@ class TestVideoOutline(ModuleStoreTestCase, APITestCase): self.assertTrue('en' in vid['summary']['transcripts']) self.assertEqual(course_outline[1]['summary']['video_url'], self.html5_video_url) self.assertEqual(course_outline[1]['summary']['size'], 0) + self.assertEqual(course_outline[1]['path'][2]['name'], self.other_unit.display_name) self.assertEqual(course_outline[2]['summary']['video_url'], self.html5_video_url) self.assertEqual(course_outline[2]['summary']['size'], 0) - def test_transcripts(self): + def test_course_list_with_nameless_unit(self): + ItemFactory.create( + parent_location=self.nameless_unit.location, + category="video", + edx_video_id=self.edx_video_id, + display_name=u"test draft video omega 2 \u03a9" + ) + course_outline = self._get_video_summary_list() + self.assertEqual(len(course_outline), 1) + self.assertEqual(course_outline[0]['path'][2]['name'], self.nameless_unit.location.block_id) + + def test_course_list_with_hidden_blocks(self): + hidden_subsection = ItemFactory.create( + parent_location=self.section.location, + category="sequential", + hide_from_toc=True, + ) + unit_within_hidden_subsection = ItemFactory.create( + parent_location=hidden_subsection.location, + category="vertical", + ) + hidden_unit = ItemFactory.create( + parent_location=self.sub_section.location, + category="vertical", + hide_from_toc=True, + ) + ItemFactory.create( + parent_location=unit_within_hidden_subsection.location, + category="video", + edx_video_id=self.edx_video_id, + ) + ItemFactory.create( + parent_location=hidden_unit.location, + category="video", + edx_video_id=self.edx_video_id, + ) + course_outline = self._get_video_summary_list() + self.assertEqual(len(course_outline), 0) + + def test_course_list_transcripts(self): + video = ItemFactory.create( + parent_location=self.nameless_unit.location, + category="video", + edx_video_id=self.edx_video_id, + display_name=u"test draft video omega 2 \u03a9" + ) + transcript_cases = [ + ({}, "en"), + ({"en": 1}, "en"), + ({"lang1": 1}, "lang1"), + ({"lang1": 1, "en": 2}, "en"), + ({"lang1": 1, "lang2": 2}, "lang1"), + ] + + for transcript_case in transcript_cases: + video.transcripts = transcript_case[0] + modulestore().update_item(video, self.user.id) + course_outline = self._get_video_summary_list() + self.assertEqual(len(course_outline), 1) + self.assertEqual(course_outline[0]['summary']['language'], transcript_case[1]) + + def test_transcripts_detail(self): + video = self._create_video_with_subs() kwargs = { 'course_id': unicode(self.course.id), - 'block_id': unicode(self.video.scope_ids.usage_id.block_id), + 'block_id': unicode(video.scope_ids.usage_id.block_id), 'lang': 'pl' } url = reverse('video-transcripts-detail', kwargs=kwargs) diff --git a/lms/envs/common.py b/lms/envs/common.py index 1ad593393d..1ba359bfac 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -281,10 +281,6 @@ FEATURES = { # ENABLE_OAUTH2_PROVIDER to True 'ENABLE_MOBILE_REST_API': False, - # Video Abstraction Layer used to allow video teams to manage video assets - # independently of courseware. https://github.com/edx/edx-val - 'ENABLE_VIDEO_ABSTRACTION_LAYER_API': False, - # Enable the new dashboard, account, and profile pages 'ENABLE_NEW_DASHBOARD': False, diff --git a/lms/urls.py b/lms/urls.py index d4f0bf7dc6..41af12c90f 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -78,6 +78,8 @@ urlpatterns = ('', # nopep8 if settings.FEATURES["ENABLE_MOBILE_REST_API"]: urlpatterns += ( url(r'^api/mobile/v0.5/', include('mobile_api.urls')), + # Video Abstraction Layer used to allow video teams to manage video assets + # independently of courseware. https://github.com/edx/edx-val url(r'^api/val/v0/', include('edxval.urls')), )