diff --git a/cms/envs/common.py b/cms/envs/common.py index f036200ac3a37464a2b56eb3837242a2880921bc..94ffa02b15e3adc2a5b2db789a40c73daead5472 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -247,6 +247,11 @@ FEATURES = { # Show the language selector in the header 'SHOW_HEADER_LANGUAGE_SELECTOR': False, + # At edX it's safe to assume that English transcripts are always available + # This is not the case for all installations. + # The default value in {lms,cms}/envs/common.py and xmodule/tests/test_video.py should be consistent. + 'FALLBACK_TO_ENGLISH_TRANSCRIPTS': True, + # Set this to False to facilitate cleaning up invalid xml from your modulestore. 'ENABLE_XBLOCK_XML_VALIDATION': True, diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index 45e93fab74ec780fe2c59eae785bba56feb63581..6df6a9f0a04743f952a422dc7d71c31ec310ba58 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -792,67 +792,44 @@ class VideoExportTestCase(VideoDescriptorTestBase): @ddt.ddt +@patch.object(settings, 'YOUTUBE', create=True, new={ + # YouTube JavaScript API + 'API': 'www.youtube.com/iframe_api', + + # URL to get YouTube metadata + 'METADATA_URL': 'www.googleapis.com/youtube/v3/videos/', + + # Current youtube api for requesting transcripts. + # For example: http://video.google.com/timedtext?lang=en&v=j_jEn79vS3g. + 'TEXT_API': { + 'url': 'video.google.com/timedtext', + 'params': { + 'lang': 'en', + 'v': 'set_youtube_id_of_11_symbols_here', + }, + }, +}) +@patch.object(settings, 'CONTENTSTORE', create=True, new={ + 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', + 'DOC_STORE_CONFIG': { + 'host': 'edx.devstack.mongo' if 'BOK_CHOY_HOSTNAME' in os.environ else 'localhost', + 'db': 'test_xcontent_%s' % uuid4().hex, + }, + # allow for additional options that can be keyed on a name, e.g. 'trashcan' + 'ADDITIONAL_OPTIONS': { + 'trashcan': { + 'bucket': 'trash_fs' + } + } +}) +@patch.object(settings, 'FEATURES', create=True, new={ + # The default value in {lms,cms}/envs/common.py and xmodule/tests/test_video.py should be consistent. + 'FALLBACK_TO_ENGLISH_TRANSCRIPTS': True, +}) class VideoDescriptorIndexingTestCase(unittest.TestCase): """ Make sure that VideoDescriptor can format data for indexing as expected. """ - def setUp(self): - """ - Overrides YOUTUBE and CONTENTSTORE settings - """ - super(VideoDescriptorIndexingTestCase, self).setUp() - self.youtube_setting = getattr(settings, "YOUTUBE", None) - self.contentstore_setting = getattr(settings, "CONTENTSTORE", None) - settings.YOUTUBE = { - # YouTube JavaScript API - 'API': 'www.youtube.com/iframe_api', - - # URL to get YouTube metadata - 'METADATA_URL': 'www.googleapis.com/youtube/v3/videos/', - - # Current youtube api for requesting transcripts. - # For example: http://video.google.com/timedtext?lang=en&v=j_jEn79vS3g. - 'TEXT_API': { - 'url': 'video.google.com/timedtext', - 'params': { - 'lang': 'en', - 'v': 'set_youtube_id_of_11_symbols_here', - }, - }, - } - - settings.CONTENTSTORE = { - 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', - 'DOC_STORE_CONFIG': { - 'host': 'edx.devstack.mongo' if 'BOK_CHOY_HOSTNAME' in os.environ else 'localhost', - 'db': 'test_xcontent_%s' % uuid4().hex, - }, - # allow for additional options that can be keyed on a name, e.g. 'trashcan' - 'ADDITIONAL_OPTIONS': { - 'trashcan': { - 'bucket': 'trash_fs' - } - } - } - - self.addCleanup(self.cleanup) - - def cleanup(self): - """ - Returns YOUTUBE and CONTENTSTORE settings to a default value - """ - if self.youtube_setting: - settings.YOUTUBE = self.youtube_setting - self.youtube_setting = None - else: - del settings.YOUTUBE - - if self.contentstore_setting: - settings.CONTENTSTORE = self.contentstore_setting - self.contentstore_setting = None - else: - del settings.CONTENTSTORE - def test_video_with_no_subs_index_dictionary(self): """ Test index dictionary of a video module without subtitles. @@ -993,7 +970,7 @@ class VideoDescriptorIndexingTestCase(unittest.TestCase): ''' descriptor = instantiate_descriptor(data=xml_data_transcripts) - translations = descriptor.available_translations(descriptor.get_transcripts_info(), verify_assets=False) + translations = descriptor.available_translations(descriptor.get_transcripts_info()) self.assertEqual(translations, ['hr', 'ge']) def test_video_with_no_transcripts_translation_retrieval(self): @@ -1003,8 +980,14 @@ class VideoDescriptorIndexingTestCase(unittest.TestCase): does not throw an exception. """ descriptor = instantiate_descriptor(data=None) - translations = descriptor.available_translations(descriptor.get_transcripts_info(), verify_assets=False) - self.assertEqual(translations, ['en']) + translations_with_fallback = descriptor.available_translations(descriptor.get_transcripts_info()) + self.assertEqual(translations_with_fallback, ['en']) + + with patch.dict(settings.FEATURES, FALLBACK_TO_ENGLISH_TRANSCRIPTS=False): + # Some organizations don't have English transcripts for all videos + # This feature makes it configurable + translations_no_fallback = descriptor.available_translations(descriptor.get_transcripts_info()) + self.assertEqual(translations_no_fallback, []) @override_settings(ALL_LANGUAGES=ALL_LANGUAGES) def test_video_with_language_do_not_have_transcripts_translation(self): diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index af4f1e22fb61be9885d0767f2015141010be5f56..4e6b96781864c3f47bca6a37dc942010cb4b5aee 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -2,6 +2,7 @@ Utility functions for transcripts. ++++++++++++++++++++++++++++++++++ """ +from django.conf import settings import os import copy import json @@ -556,7 +557,7 @@ class VideoTranscriptsMixin(object): This is necessary for both VideoModule and VideoDescriptor. """ - def available_translations(self, transcripts, verify_assets=True): + def available_translations(self, transcripts, verify_assets=None): """Return a list of language codes for which we have transcripts. Args: @@ -566,13 +567,16 @@ class VideoTranscriptsMixin(object): we might do this is to avoid slamming contentstore() with queries when trying to make a listing of videos and their languages. - Defaults to True. + Defaults to `not FALLBACK_TO_ENGLISH_TRANSCRIPTS`. transcripts (dict): A dict with all transcripts and a sub. Defaults to False """ translations = [] + if verify_assets is None: + verify_assets = not settings.FEATURES.get('FALLBACK_TO_ENGLISH_TRANSCRIPTS') + sub, other_langs = transcripts["sub"], transcripts["transcripts"] # If we're not verifying the assets, we just trust our field values diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index c5256a20655a417a2222a437a123c0c32bc1d388..76e83dc6ffca0d01009cf10c628712d7d85c9a86 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -276,7 +276,7 @@ class VideoStudentViewHandlers(object): elif dispatch.startswith('available_translations'): - available_translations = self.available_translations(transcripts) + available_translations = self.available_translations(transcripts, verify_assets=True) if available_translations: response = Response(json.dumps(available_translations)) response.content_type = 'application/json' diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index f8271473e284a7293dacb7fcb3cacbc7fdce2626..e8645685eaa25ead43ee88bf619b3e374bd87483 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -991,7 +991,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler transcripts_info = self.get_transcripts_info() transcripts = { lang: self.runtime.handler_url(self, 'transcript', 'download', query="lang=" + lang, thirdparty=True) - for lang in self.available_translations(transcripts_info, verify_assets=False) + for lang in self.available_translations(transcripts_info) } return { diff --git a/lms/djangoapps/mobile_api/video_outlines/serializers.py b/lms/djangoapps/mobile_api/video_outlines/serializers.py index 5c0e08315a88a9b0b7f17f33790ddf89c51f5831..b548ffe3948f4df0c0106788cd2a3d0a61d5e31c 100644 --- a/lms/djangoapps/mobile_api/video_outlines/serializers.py +++ b/lms/djangoapps/mobile_api/video_outlines/serializers.py @@ -209,7 +209,7 @@ def video_summary(video_profiles, course_id, video_descriptor, request, local_ca # Transcripts... transcripts_info = video_descriptor.get_transcripts_info() - transcript_langs = video_descriptor.available_translations(transcripts_info, verify_assets=False) + transcript_langs = video_descriptor.available_translations(transcripts_info) transcripts = { lang: reverse( diff --git a/lms/djangoapps/mobile_api/video_outlines/tests.py b/lms/djangoapps/mobile_api/video_outlines/tests.py index 144649729f9df3534c6ebaf949faab5c40d6b022..e2d3a3eca65f683cb68cc834e6c55fcc720aeb71 100644 --- a/lms/djangoapps/mobile_api/video_outlines/tests.py +++ b/lms/djangoapps/mobile_api/video_outlines/tests.py @@ -8,8 +8,10 @@ from collections import namedtuple from uuid import uuid4 import ddt +from django.conf import settings from edxval import api from milestones.tests.utils import MilestonesTestCaseMixin +from mock import patch from nose.plugins.attrib import attr from mobile_api.models import MobileApiConfig @@ -515,6 +517,16 @@ class TestVideoSummaryList(TestVideoAPITestCase, MobileAuthTestMixin, MobileCour 'size': 111 } + # The transcript was not entered, so it should not be found! + # This is the default behaviour at courses.edX.org, based on `FALLBACK_TO_ENGLISH_TRANSCRIPTS` + transcripts_response = self.client.get(expected_output['transcripts']['en']) + self.assertEqual(404, transcripts_response.status_code) + + with patch.dict(settings.FEATURES, FALLBACK_TO_ENGLISH_TRANSCRIPTS=False): + # Other platform installations may override this setting + # This ensures that the server don't return empty English transcripts when there's none! + self.assertFalse(self.api_response().data[0]['summary'].get('transcripts')) + # Testing when video_profiles='mobile_low,mobile_high,youtube' course_outline = self.api_response().data course_outline[0]['summary'].pop("id") diff --git a/lms/envs/common.py b/lms/envs/common.py index 4c39c9e810f8227a7c449ce831e07aac8ad4fecc..8a47fddae80df00c7177fb407a39e78af8881bca 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -346,6 +346,11 @@ FEATURES = { # Show the language selector in the header 'SHOW_HEADER_LANGUAGE_SELECTOR': False, + # At edX it's safe to assume that English transcripts are always available + # This is not the case for all installations. + # The default value in {lms,cms}/envs/common.py and xmodule/tests/test_video.py should be consistent. + 'FALLBACK_TO_ENGLISH_TRANSCRIPTS': True, + # Show the language selector in the footer 'SHOW_FOOTER_LANGUAGE_SELECTOR': False,