From 11a07ca8c98fa1a73daa694e5f64e34c33630154 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Mon, 26 Sep 2016 12:40:44 +0500 Subject: [PATCH] Show validation message when no transcript file uploaded for a language in a video component in studio. Also, don't show respective transcript language in video language menu when a related transcript is not uploaded for that language. TNL-5200 --- .../lib/xmodule/xmodule/tests/test_video.py | 80 +++++++++++++++++++ .../xmodule/video_module/transcripts_utils.py | 28 ++++--- .../xmodule/video_module/video_module.py | 36 +++++++++ .../tests/video/test_studio_video_editor.py | 21 +++++ 4 files changed, 154 insertions(+), 11 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index 42328114ca..afec656305 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -21,6 +21,7 @@ from mock import ANY, Mock, patch import ddt from django.conf import settings +from django.test.utils import override_settings from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.keys import CourseKey @@ -28,6 +29,7 @@ from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from xmodule.tests import get_test_descriptor_system +from xmodule.validation import StudioValidationMessage from xmodule.video_module import VideoDescriptor, create_youtube_string from xmodule.video_module.transcripts_utils import download_youtube_subs, save_to_store from . import LogicTest @@ -77,6 +79,12 @@ YOUTUBE_SUBTITLES = ( " that now. The tutorial will continue in the next video." ) +ALL_LANGUAGES = ( + [u"en", u"English"], + [u"eo", u"Esperanto"], + [u"ur", u"Urdu"] +) + def instantiate_descriptor(**field_data): """ @@ -780,6 +788,7 @@ class VideoExportTestCase(VideoDescriptorTestBase): self.assertEqual(xml.get('display_name'), u'\u8fd9\u662f\u6587') +@ddt.ddt class VideoDescriptorIndexingTestCase(unittest.TestCase): """ Make sure that VideoDescriptor can format data for indexing as expected. @@ -993,3 +1002,74 @@ class VideoDescriptorIndexingTestCase(unittest.TestCase): descriptor = instantiate_descriptor(data=None) translations = descriptor.available_translations(descriptor.get_transcripts_info(), verify_assets=False) self.assertEqual(translations, ['en']) + + @override_settings(ALL_LANGUAGES=ALL_LANGUAGES) + def test_video_with_language_do_not_have_transcripts_translation(self): + """ + Test translation retrieval of a video module with + a language having no transcripts uploaded by a user. + """ + xml_data_transcripts = ''' + + ''' + descriptor = instantiate_descriptor(data=xml_data_transcripts) + translations = descriptor.available_translations(descriptor.get_transcripts_info(), verify_assets=False) + self.assertNotEqual(translations, ['ur']) + + def assert_validation_message(self, validation, expected_msg): + """ + Asserts that the validation message has all expected content. + + Args: + validation (StudioValidation): A validation object. + expected_msg (string): An expected validation message. + """ + self.assertFalse(validation.empty) # Validation contains some warning/message + self.assertTrue(validation.summary) + self.assertEqual(StudioValidationMessage.WARNING, validation.summary.type) + self.assertIn(expected_msg, validation.summary.text) + + @ddt.data( + ( + '', + 'There is no transcript file associated with the Urdu language.' + ), + ( + '', + 'There are no transcript files associated with the Esperanto, Urdu languages.' + ), + ) + @ddt.unpack + @override_settings(ALL_LANGUAGES=ALL_LANGUAGES) + def test_no_transcript_validation_message(self, xml_transcripts, expected_validation_msg): + """ + Test the validation message when no associated transcript file uploaded. + """ + xml_data_transcripts = ''' + + '''.format(xml_transcripts=xml_transcripts) + descriptor = instantiate_descriptor(data=xml_data_transcripts) + validation = descriptor.validate() + self.assert_validation_message(validation, expected_validation_msg) diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index 2675000a8e..96a6ba7fa5 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -383,9 +383,7 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat lang, ) except TranscriptException as ex: - # remove key from transcripts because proper srt file does not exist in assets. - item.transcripts.pop(lang) - reraised_message += ' ' + ex.message + pass if reraised_message: item.save_with_metadata(user) raise TranscriptException(reraised_message) @@ -531,6 +529,9 @@ class Transcript(object): """ Return asset location. `location` is module location. """ + # If user transcript filename is empty, raise `TranscriptException` to avoid `InvalidKeyError`. + if not filename: + raise TranscriptException("Transcript not uploaded yet") return StaticContent.compute_location(location.course_key, filename) @staticmethod @@ -670,12 +671,17 @@ class VideoTranscriptsMixin(object): """ if is_bumper: transcripts = copy.deepcopy(get_bumper_settings(self).get('transcripts', {})) - return { - "sub": transcripts.pop("en", ""), - "transcripts": transcripts, - } + sub = transcripts.pop("en", "") else: - return { - "sub": self.sub, - "transcripts": self.transcripts, - } + transcripts = self.transcripts + sub = self.sub + + # Only attach transcripts that are not empty. + transcripts = { + language_code: transcript_file + for language_code, transcript_file in transcripts.items() if transcript_file != '' + } + return { + "sub": sub, + "transcripts": transcripts, + } diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index a8d4db6bd4..0900e66ada 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -36,6 +36,7 @@ from xmodule.raw_module import EmptyDataRawDescriptor from xmodule.xml_module import is_pointer_tag, name_to_pathname, deserialize_field from xmodule.exceptions import NotFoundError from xmodule.contentstore.content import StaticContent +from xmodule.validation import StudioValidationMessage, StudioValidation from .transcripts_utils import VideoTranscriptsMixin, Transcript, get_html5_ids from .video_utils import create_youtube_string, get_poster, rewrite_video_url, format_xml_exception_message @@ -152,6 +153,12 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, ]} js_module_name = "Video" + def validate(self): + """ + Validates the state of this Video Module Instance. + """ + return self.descriptor.validate() + def get_transcripts_for_student(self, transcripts): """Return transcript information necessary for rendering the XModule student view. This is more or less a direct extraction from `get_html`. @@ -427,6 +434,35 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler if not self.fields['download_track'].is_set_on(self) and self.track: self.download_track = True + def validate(self): + """ + Validates the state of this video Module Instance. This + is the override of the general XBlock method, and it will also ask + its superclass to validate. + """ + validation = super(VideoDescriptor, self).validate() + if not isinstance(validation, StudioValidation): + validation = StudioValidation.copy(validation) + + no_transcript_lang = [] + for lang_code, transcript in self.transcripts.items(): + if not transcript: + no_transcript_lang.append([label for code, label in settings.ALL_LANGUAGES if code == lang_code][0]) + + if no_transcript_lang: + ungettext = self.runtime.service(self, "i18n").ungettext + validation.set_summary( + StudioValidationMessage( + StudioValidationMessage.WARNING, + ungettext( + 'There is no transcript file associated with the {lang} language.', + 'There are no transcript files associated with the {lang} languages.', + len(no_transcript_lang) + ).format(lang=', '.join(no_transcript_lang)) + ) + ) + return validation + def editor_saved(self, user, old_metadata, old_content): """ Used to update video values during `self`:save method from CMS. diff --git a/common/test/acceptance/tests/video/test_studio_video_editor.py b/common/test/acceptance/tests/video/test_studio_video_editor.py index 90f43dfc55..84c1e75819 100644 --- a/common/test/acceptance/tests/video/test_studio_video_editor.py +++ b/common/test/acceptance/tests/video/test_studio_video_editor.py @@ -126,6 +126,27 @@ class VideoEditorTest(CMSVideoBaseTest): self.assertIn(unicode_text, self.video.captions_text) self.assertEqual(self.video.caption_languages.keys(), ['zh', 'uk']) + def test_save_language_upload_no_transcript(self): + """ + Scenario: Transcript language is not shown in language menu if no transcript file is uploaded + Given I have created a Video component + And I edit the component + And I open tab "Advanced" + And I add a language "uk" but do not upload an .srt file + And I save changes + When I view the video language menu + Then I am not able to see the language "uk" translation language + """ + self._create_video_component() + self.edit_component() + self.open_advanced_tab() + language_code = 'uk' + self.video.click_button('translation_add') + translations_count = self.video.translations_count() + self.video.select_translation_language(language_code, translations_count - 1) + self.save_unit_settings() + self.assertNotIn(language_code, self.video.caption_languages.keys()) + def test_upload_large_transcript(self): """ Scenario: User can upload transcript file with > 1mb size