diff --git a/cms/static/js/views/uploads.js b/cms/static/js/views/uploads.js index 1b7142b6b0..9f70363cb7 100644 --- a/cms/static/js/views/uploads.js +++ b/cms/static/js/views/uploads.js @@ -80,7 +80,7 @@ define(['jquery', 'underscore', 'gettext', 'js/views/modals/base_modal', 'jquery var uploadAjaxData = _.extend({}, this.uploadData); // don't show the generic error notification; we're in a modal, // and we're better off modifying it instead. - uploadAjaxData['notifyOnError'] = false; + uploadAjaxData.notifyOnError = false; if (e && e.preventDefault) { e.preventDefault(); } this.model.set('uploading', true); diff --git a/cms/static/js/views/video/translations_editor.js b/cms/static/js/views/video/translations_editor.js index a68dcee05a..23c67bebe9 100644 --- a/cms/static/js/views/video/translations_editor.js +++ b/cms/static/js/views/video/translations_editor.js @@ -1,9 +1,9 @@ define( [ 'jquery', 'underscore', 'edx-ui-toolkit/js/utils/html-utils', 'js/views/video/transcripts/utils', - 'js/views/abstract_editor', 'js/models/uploads', 'js/views/uploads' + 'js/views/abstract_editor', 'common/js/components/utils/view_utils', 'js/models/uploads', 'js/views/uploads' ], -function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDialog) { +function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, ViewUtils, FileUpload, UploadDialog) { 'use strict'; var VideoUploadDialog = UploadDialog.extend({ @@ -18,7 +18,6 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDia var Translations = AbstractEditor.extend({ events: { - 'click .setting-clear': 'clear', 'click .create-setting': 'addEntry', 'click .remove-setting': 'removeEntry', 'click .upload-setting': 'upload', @@ -48,7 +47,6 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDia languageMap[lang] = lang; }); TranscriptUtils.Storage.set('languageMap', languageMap); - AbstractEditor.prototype.initialize.apply(this, arguments); }, @@ -128,7 +126,7 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDia _.each(values, function(value, newLang) { var html = $(self.templateItem({ newLang: newLang, - originalLang: _.findKey(languageMap, function(lang){ return lang === newLang}) || '', + originalLang: _.findKey(languageMap, function(lang) { return lang === newLang; }) || '', value: value, url: self.model.get('urlRoot') })).prepend(dropdown.clone().val(newLang))[0]; @@ -148,26 +146,49 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDia }, removeEntry: function(event) { - event.preventDefault(); - var $currentListItemEl = $(event.currentTarget).parent(), + var self = this, + $currentListItemEl = $(event.currentTarget).parent(), originalLang = $currentListItemEl.data('original-lang'), selectedLang = $currentListItemEl.find('select option:selected').val(), - languageMap = TranscriptUtils.Storage.get('languageMap'); + languageMap = TranscriptUtils.Storage.get('languageMap'), + edxVideoIdField = TranscriptUtils.getField(self.model.collection, 'edx_video_id'); - // re-render dropdowns - this.setValueInEditor(this.getAllLanguageDropdownElementsData(false, selectedLang)); + event.preventDefault(); - // remvove the `originalLang` from `languageMap` + /* + There is a scenario when a user adds an empty video translation item and + removes it. In such cases, omitting will have no harm on the model + values or languages map. + */ if (originalLang) { - TranscriptUtils.Storage.set('languageMap', _.omit(languageMap, originalLang)); + ViewUtils.confirmThenRunOperation( + gettext('Are you sure you want to remove this transcript?'), + gettext('If you remove this transcript, the transcript will not be available for this component.'), + gettext('Remove Transcript'), + function() { + ViewUtils.runOperationShowingMessage( + gettext('Removing'), + function() { + return $.ajax({ + url: self.model.get('urlRoot'), + type: 'DELETE', + data: JSON.stringify({lang: originalLang, edx_video_id: edxVideoIdField.getValue()}) + }).done(function() { + self.setValueInEditor(self.getAllLanguageDropdownElementsData(false, selectedLang)); + TranscriptUtils.Storage.set('languageMap', _.omit(languageMap, originalLang)); + }); + } + ); + } + ); + } else { + this.setValueInEditor(this.getAllLanguageDropdownElementsData(false, selectedLang)); } this.$el.find('.create-setting').removeClass('is-disabled').attr('aria-disabled', false); }, upload: function(event) { - event.preventDefault(); - var self = this, $target = $(event.currentTarget), $listItem = $target.parents('li.list-settings-item'), @@ -178,10 +199,12 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDia uploadData, videoUploadDialog; + event.preventDefault(); + // That's the case when an author is // uploading a new transcript. if (!originalLang) { - originalLang = newLang + originalLang = newLang; } // Transcript data payload @@ -208,7 +231,7 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDia // new language entry to be added to languageMap newLangObject[newLang] = newLang; - //Update edx-video-id + // Update edx-video-id edxVideoIdField.setValue(response.edx_video_id); // Update language map by omitting original lang and adding new lang @@ -228,13 +251,6 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDia this.$el.find('.create-setting').removeClass('is-disabled').attr('aria-disabled', false); }, - clear: function() { - AbstractEditor.prototype.clear.apply(this, arguments); - if (_.isNull(this.model.getValue())) { - this.$el.find('.create-setting').removeClass('is-disabled').attr('aria-disabled', false); - } - }, - onChangeHandler: function(event) { var $target = $(event.currentTarget), $listItem = $target.parents('li.list-settings-item'), @@ -266,27 +282,26 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, FileUpload, UploadDia // data object will mirror the languageMap. `data` will contain lang to lang map as explained below // {originalLang: originalLang}; original lang not changed // {newLang: originalLang}; original lang changed to a new lang - // {selectedLang: ""}; new lang to be added, no entry in languageMap - _.each(languageDropdownElements, function(languageDropdown, index){ + // {selectedLang: ''}; new lang to be added, no entry in languageMap + _.each(languageDropdownElements, function(languageDropdown) { var language = $(languageDropdown).find(':selected').val(); - data[language] = _.findKey(languageMap, function(lang){ return lang === language}) || ""; + data[language] = _.findKey(languageMap, function(lang) { return lang === language; }) || ''; }); // This is needed to render an empty item that // will be further used to upload a transcript. if (isNew) { - data[""] = ""; + data[''] = ''; } // This Omits a language from the dropdown's data. It is // needed when an item is going to be removed. if (typeof(omittedLanguage) !== 'undefined') { - data = _.omit(data, omittedLanguage) + data = _.omit(data, omittedLanguage); } return data; } - }); return Translations; diff --git a/cms/templates/js/video/metadata-translations-entry.underscore b/cms/templates/js/video/metadata-translations-entry.underscore index 421513dd8f..1afef78cd9 100644 --- a/cms/templates/js/video/metadata-translations-entry.underscore +++ b/cms/templates/js/video/metadata-translations-entry.underscore @@ -7,9 +7,5 @@ <%= gettext("Add") %> <%= model.get('display_name')%> - <%= model.get('help') %> diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index 1e7e193353..19b17f2dd9 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -20,19 +20,21 @@ from xmodule.exceptions import NotFoundError from xmodule.fields import RelativeTime from opaque_keys.edx.locator import CourseLocator -from edxval.api import create_or_update_video_transcript, create_external_video +from edxval.api import create_or_update_video_transcript, create_external_video, delete_video_transcript from .transcripts_utils import ( clean_video_id, get_or_create_sjson, generate_sjson_for_all_speeds, - get_video_transcript_content, + save_to_store, subs_filename, Transcript, TranscriptException, TranscriptsGenerationException, youtube_speed_dict, get_transcript, - get_transcript_from_contentstore + get_transcript_from_contentstore, + remove_subs_from_store, + get_html5_ids ) log = logging.getLogger(__name__) @@ -488,6 +490,36 @@ class VideoStudioViewHandlers(object): }, status=400 ) + elif request.method == 'DELETE': + request_data = request.json + + if 'lang' not in request_data or 'edx_video_id' not in request_data: + return Response(status=400) + + language = request_data['lang'] + edx_video_id = clean_video_id(request_data['edx_video_id']) + + if edx_video_id: + delete_video_transcript(video_id=edx_video_id, language_code=language) + + if language == u'en': + # remove any transcript file from content store for the video ids + possible_sub_ids = [ + self.sub, # pylint: disable=access-member-before-definition + self.youtube_id_1_0 + ] + get_html5_ids(self.html5_sources) + for sub_id in possible_sub_ids: + remove_subs_from_store(sub_id, self, language) + + # update metadata as `en` can also be present in `transcripts` field + remove_subs_from_store(self.transcripts.pop(language, None), self, language) + + # also empty `sub` field + self.sub = '' # pylint: disable=attribute-defined-outside-init + else: + remove_subs_from_store(self.transcripts.pop(language, None), self, language) + + return Response(status=200) elif request.method == 'GET': language = request.GET.get('language_code') diff --git a/common/test/acceptance/pages/studio/video/video.py b/common/test/acceptance/pages/studio/video/video.py index f5bb6468ce..58abc54c85 100644 --- a/common/test/acceptance/pages/studio/video/video.py +++ b/common/test/acceptance/pages/studio/video/video.py @@ -81,6 +81,11 @@ DEFAULT_SETTINGS = [ ['YouTube ID for 1.5x speed', '', False] ] +# field names without clear button +FIELDS_WO_CLEAR = [ + 'Transcript Languages' +] + # We should wait 300 ms for event handler invocation + 200ms for safety. DELAY = 0.5 @@ -346,15 +351,22 @@ class VideoComponentPage(VideoPage): """ Verify that video component has correct default settings. """ - query = '.wrapper-comp-setting' - settings = self.q(css=query).results - if len(DEFAULT_SETTINGS) != len(settings): - return False + def _check_settings_length(): + """Check video settings""" + query = '.wrapper-comp-setting' + settings = self.q(css=query).results + if len(DEFAULT_SETTINGS) == len(settings): + return True, settings + return (False, None) + + settings = Promise(_check_settings_length, 'All video fields are present').fulfill() for counter, setting in enumerate(settings): - is_verified = self._verify_setting_entry(setting, - DEFAULT_SETTINGS[counter][0], - DEFAULT_SETTINGS[counter][1]) + is_verified = self._verify_setting_entry( + setting, + DEFAULT_SETTINGS[counter][0], + DEFAULT_SETTINGS[counter][1] + ) if not is_verified: return is_verified @@ -395,9 +407,8 @@ class VideoComponentPage(VideoPage): if field_value != current_value: return False - # Clear button should be visible(active class is present) for - # every setting that don't have 'metadata-videolist-enum' class - if 'metadata-videolist-enum' not in setting.get_attribute('class'): + # Verify if clear button is active for expected video fields + if field_name not in FIELDS_WO_CLEAR and 'metadata-videolist-enum' not in setting.get_attribute('class'): setting_clear_button = setting.find_elements_by_class_name('setting-clear')[0] if 'active' not in setting_clear_button.get_attribute('class'): return False @@ -543,7 +554,9 @@ class VideoComponentPage(VideoPage): language_code (str): language code """ - self.q(css='.remove-action').filter(lambda el: language_code == el.get_attribute('data-lang')).click() + selector = '.metadata-video-translations .list-settings-item' + translation = self.q(css=selector).filter(lambda el: language_code == el.get_attribute('data-original-lang')) + translation[0].find_element_by_class_name('remove-action').click() @property def upload_status_message(self): 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 5a09b64c3f..cd1a409a02 100644 --- a/common/test/acceptance/tests/video/test_studio_video_editor.py +++ b/common/test/acceptance/tests/video/test_studio_video_editor.py @@ -3,12 +3,14 @@ """ Acceptance tests for CMS Video Editor. """ +import ddt from nose.plugins.attrib import attr - +from common.test.acceptance.pages.common.utils import confirm_prompt from common.test.acceptance.tests.video.test_studio_video_module import CMSVideoBaseTest @attr(shard=6) +@ddt.ddt class VideoEditorTest(CMSVideoBaseTest): """ CMS Video Editor Test Class @@ -263,6 +265,7 @@ class VideoEditorTest(CMSVideoBaseTest): self.open_advanced_tab() self.assertEqual(self.video.translations(), ['zh', 'uk']) self.video.remove_translation('uk') + confirm_prompt(self.video) self.save_unit_settings() self.assertTrue(self.video.is_captions_visible()) unicode_text = "好 各位同学".decode('utf-8') @@ -271,6 +274,7 @@ class VideoEditorTest(CMSVideoBaseTest): self.open_advanced_tab() self.assertEqual(self.video.translations(), ['zh']) self.video.remove_translation('zh') + confirm_prompt(self.video) self.save_unit_settings() self.assertFalse(self.video.is_captions_visible()) @@ -292,9 +296,28 @@ class VideoEditorTest(CMSVideoBaseTest): self.video.upload_translation('uk_transcripts.srt', 'uk') self.assertEqual(self.video.translations(), ['uk']) self.video.remove_translation('uk') + confirm_prompt(self.video) self.save_unit_settings() self.assertFalse(self.video.is_captions_visible()) + def test_translations_entry_remove_works(self): + """ + Scenario: Translations entry removal works correctly when transcript is not uploaded + Given I have created a Video component + And I edit the component + And I open tab "Advanced" + And I click on "+ Add" button for "Transcript Languages" field + Then I click on "Remove" button + And I see newly created entry is removed + """ + self._create_video_component() + self.edit_component() + self.open_advanced_tab() + self.video.click_button("translation_add") + self.assertEqual(self.video.translations_count(), 1) + self.video.remove_translation("") + self.assertEqual(self.video.translations_count(), 0) + def test_cannot_upload_sjson_translation(self): """ Scenario: User cannot upload translations in sjson format @@ -394,6 +417,7 @@ class VideoEditorTest(CMSVideoBaseTest): self.video.upload_translation('chinese_transcripts.srt', 'zh') self.assertEqual(self.video.translations(), ['zh']) self.video.remove_translation('zh') + confirm_prompt(self.video) self.video.upload_translation('uk_transcripts.srt', 'zh') self.save_unit_settings() self.assertTrue(self.video.is_captions_visible()) diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index c941e4fc08..adbb4d2d66 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/lms/djangoapps/courseware/tests/test_video_handlers.py @@ -9,6 +9,7 @@ from datetime import timedelta import ddt import freezegun +from django.core.files.base import ContentFile from django.utils.timezone import now from mock import MagicMock, Mock, patch from nose.plugins.attrib import attr @@ -22,13 +23,14 @@ from xmodule.exceptions import NotFoundError from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.video_module.transcripts_utils import ( - TranscriptException, - TranscriptsGenerationException, Transcript, - edxval_api + edxval_api, + subs_filename, ) from xmodule.x_module import STUDENT_VIEW +from edxval import api + from .helpers import BaseTestXmodule from .test_video_xml import SOURCE_XML @@ -1011,6 +1013,138 @@ class TestStudioTranscriptTranslationPostDispatch(TestVideo): ) +@attr(shard=1) +@ddt.ddt +class TestStudioTranscriptTranslationDeleteDispatch(TestVideo): + """ + Test studio video handler that provide translation transcripts. + + Tests for `translation` dispatch DELETE HTTP method. + """ + EDX_VIDEO_ID, LANGUAGE_CODE_UK, LANGUAGE_CODE_EN = u'an_edx_video_id', u'uk', u'en' + REQUEST_META = {'wsgi.url_scheme': 'http', 'REQUEST_METHOD': 'DELETE'} + SRT_FILE = _create_srt_file() + + @ddt.data( + { + 'params': {'lang': 'uk'} + }, + { + 'params': {'edx_video_id': '12345'} + }, + { + 'params': {} + }, + ) + @ddt.unpack + def test_translation_missing_required_params(self, params): + """ + Verify that DELETE dispatch works as expected when required args are missing from request + """ + request = Request(self.REQUEST_META, body=json.dumps(params)) + response = self.item_descriptor.studio_transcript(request=request, dispatch='translation') + self.assertEqual(response.status_code, 400) + + def test_translation_delete_w_edx_video_id(self): + """ + Verify that DELETE dispatch works as expected when video has edx_video_id + """ + request_body = json.dumps({'lang': self.LANGUAGE_CODE_UK, 'edx_video_id': self.EDX_VIDEO_ID}) + api.create_video({ + 'edx_video_id': self.EDX_VIDEO_ID, + 'status': 'upload', + 'client_video_id': 'awesome.mp4', + 'duration': 0, + 'encoded_videos': [], + 'courses': [unicode(self.course.id)] + }) + api.create_video_transcript( + video_id=self.EDX_VIDEO_ID, + language_code=self.LANGUAGE_CODE_UK, + file_format='srt', + content=ContentFile(SRT_content) + ) + + # verify that a video transcript exists for expected data + self.assertTrue(api.get_video_transcript_data(video_id=self.EDX_VIDEO_ID, language_code=self.LANGUAGE_CODE_UK)) + + request = Request(self.REQUEST_META, body=request_body) + self.item_descriptor.edx_video_id = self.EDX_VIDEO_ID + response = self.item_descriptor.studio_transcript(request=request, dispatch='translation') + self.assertEqual(response.status_code, 200) + + # verify that a video transcript dose not exist for expected data + self.assertFalse(api.get_video_transcript_data(video_id=self.EDX_VIDEO_ID, language_code=self.LANGUAGE_CODE_UK)) + + def test_translation_delete_wo_edx_video_id(self): + """ + Verify that DELETE dispatch works as expected when video has no edx_video_id + """ + request_body = json.dumps({'lang': self.LANGUAGE_CODE_UK, 'edx_video_id': ''}) + srt_file_name_uk = subs_filename('ukrainian_translation.srt', lang=self.LANGUAGE_CODE_UK) + request = Request(self.REQUEST_META, body=request_body) + + # upload and verify that srt file exists in assets + _upload_file(self.SRT_FILE, self.item_descriptor.location, srt_file_name_uk) + self.assertTrue(_check_asset(self.item_descriptor.location, srt_file_name_uk)) + + # verify transcripts field + self.assertNotEqual(self.item_descriptor.transcripts, {}) + self.assertTrue(self.LANGUAGE_CODE_UK in self.item_descriptor.transcripts) + + # make request and verify response + response = self.item_descriptor.studio_transcript(request=request, dispatch='translation') + self.assertEqual(response.status_code, 200) + + # verify that srt file is deleted + self.assertEqual(self.item_descriptor.transcripts, {}) + self.assertFalse(_check_asset(self.item_descriptor.location, srt_file_name_uk)) + + def test_translation_delete_w_english_lang(self): + """ + Verify that DELETE dispatch works as expected for english language translation + """ + request_body = json.dumps({'lang': self.LANGUAGE_CODE_EN, 'edx_video_id': ''}) + srt_file_name_en = subs_filename('english_translation.srt', lang=self.LANGUAGE_CODE_EN) + self.item_descriptor.transcripts['en'] = 'english_translation.srt' + request = Request(self.REQUEST_META, body=request_body) + + # upload and verify that srt file exists in assets + _upload_file(self.SRT_FILE, self.item_descriptor.location, srt_file_name_en) + self.assertTrue(_check_asset(self.item_descriptor.location, srt_file_name_en)) + + # make request and verify response + response = self.item_descriptor.studio_transcript(request=request, dispatch='translation') + self.assertEqual(response.status_code, 200) + + # verify that srt file is deleted + self.assertTrue(self.LANGUAGE_CODE_EN not in self.item_descriptor.transcripts) + self.assertFalse(_check_asset(self.item_descriptor.location, srt_file_name_en)) + + def test_translation_delete_w_sub(self): + """ + Verify that DELETE dispatch works as expected when translation is present against `sub` field + """ + request_body = json.dumps({'lang': self.LANGUAGE_CODE_EN, 'edx_video_id': ''}) + sub_file_name = subs_filename(self.item_descriptor.sub, lang=self.LANGUAGE_CODE_EN) + request = Request(self.REQUEST_META, body=request_body) + + # sub should not be empy + self.assertFalse(self.item_descriptor.sub == u'') + + # upload and verify that srt file exists in assets + _upload_file(self.SRT_FILE, self.item_descriptor.location, sub_file_name) + self.assertTrue(_check_asset(self.item_descriptor.location, sub_file_name)) + + # make request and verify response + response = self.item_descriptor.studio_transcript(request=request, dispatch='translation') + self.assertEqual(response.status_code, 200) + + # verify that sub is empty and transcript is deleted also + self.assertTrue(self.item_descriptor.sub == u'') + self.assertFalse(_check_asset(self.item_descriptor.location, sub_file_name)) + + @attr(shard=1) class TestGetTranscript(TestVideo): """