diff --git a/cms/djangoapps/contentstore/views/tests/test_transcripts.py b/cms/djangoapps/contentstore/views/tests/test_transcripts.py index 4c75355203cb..0fae175fd917 100644 --- a/cms/djangoapps/contentstore/views/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/views/tests/test_transcripts.py @@ -195,7 +195,7 @@ def clean_temporary_transcripts(self): self.bad_name_srt_file.close() self.bom_srt_file.close() - def upload_transcript(self, locator, transcript_file): + def upload_transcript(self, locator, transcript_file, edx_video_id=None): """ Uploads a transcript for a video """ @@ -203,6 +203,9 @@ def upload_transcript(self, locator, transcript_file): if locator: payload.update({'locator': locator}) + if edx_video_id is not None: + payload.update({'edx_video_id': edx_video_id}) + if transcript_file: payload.update({'transcript-file': transcript_file}) @@ -233,7 +236,7 @@ def test_transcript_upload_success(self, edx_video_id, include_bom): # Upload a transcript transcript_file = self.bom_srt_file if include_bom else self.good_srt_file - response = self.upload_transcript(self.video_usage_key, transcript_file) + response = self.upload_transcript(self.video_usage_key, transcript_file, '') # Verify the response self.assert_response(response, expected_status_code=200, expected_message='Success') @@ -258,7 +261,7 @@ def test_transcript_upload_without_locator(self): """ Test that transcript upload validation fails if the video locator is missing """ - response = self.upload_transcript(locator=None, transcript_file=self.good_srt_file) + response = self.upload_transcript(locator=None, transcript_file=self.good_srt_file, edx_video_id='') self.assert_response( response, expected_status_code=400, @@ -269,7 +272,7 @@ def test_transcript_upload_without_file(self): """ Test that transcript upload validation fails if transcript file is missing """ - response = self.upload_transcript(locator=self.video_usage_key, transcript_file=None) + response = self.upload_transcript(locator=self.video_usage_key, transcript_file=None, edx_video_id='') self.assert_response( response, expected_status_code=400, @@ -280,7 +283,11 @@ def test_transcript_upload_bad_format(self): """ Test that transcript upload validation fails if transcript format is not SRT """ - response = self.upload_transcript(locator=self.video_usage_key, transcript_file=self.bad_name_srt_file) + response = self.upload_transcript( + locator=self.video_usage_key, + transcript_file=self.bad_name_srt_file, + edx_video_id='' + ) self.assert_response( response, expected_status_code=400, @@ -292,7 +299,11 @@ def test_transcript_upload_bad_content(self): Test that transcript upload validation fails in case of bad transcript content. """ # Request to upload transcript for the video - response = self.upload_transcript(locator=self.video_usage_key, transcript_file=self.bad_data_srt_file) + response = self.upload_transcript( + locator=self.video_usage_key, + transcript_file=self.bad_data_srt_file, + edx_video_id='' + ) self.assert_response( response, expected_status_code=400, @@ -306,7 +317,7 @@ def test_transcript_upload_unknown_category(self): # non_video module setup - i.e. an item whose category is not 'video'. usage_key = self.create_non_video_module() # Request to upload transcript for the item - response = self.upload_transcript(locator=usage_key, transcript_file=self.good_srt_file) + response = self.upload_transcript(locator=usage_key, transcript_file=self.good_srt_file, edx_video_id='') self.assert_response( response, expected_status_code=400, @@ -318,13 +329,47 @@ def test_transcript_upload_non_existent_item(self): Test that transcript upload validation fails in case of invalid item's locator. """ # Request to upload transcript for the item - response = self.upload_transcript(locator='non_existent_locator', transcript_file=self.good_srt_file) + response = self.upload_transcript( + locator='non_existent_locator', + transcript_file=self.good_srt_file, + edx_video_id='' + ) self.assert_response( response, expected_status_code=400, expected_message=u'Cannot find item by locator.' ) + def test_transcript_upload_without_edx_video_id(self): + """ + Test that transcript upload validation fails if the `edx_video_id` is missing + """ + response = self.upload_transcript(locator=self.video_usage_key, transcript_file=self.good_srt_file) + self.assert_response( + response, + expected_status_code=400, + expected_message=u'Video ID is required.' + ) + + def test_transcript_upload_with_non_existant_edx_video_id(self): + """ + Test that transcript upload works as expected if `edx_video_id` set on + video descriptor is different from `edx_video_id` received in POST request. + """ + non_existant_edx_video_id = '1111-2222-3333-4444' + + # Upload with non-existant `edx_video_id` + response = self.upload_transcript( + locator=self.video_usage_key, + transcript_file=self.good_srt_file, + edx_video_id=non_existant_edx_video_id + ) + # Verify the response + self.assert_response(response, expected_status_code=400, expected_message='Invalid Video ID') + + # Verify transcript does not exist for non-existant `edx_video_id` + self.assertIsNone(get_video_transcript_content(non_existant_edx_video_id, language_code=u'en')) + @ddt.ddt class TestChooseTranscripts(BaseTranscripts): diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index ae1c856231bf..99c8b80e3808 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -169,19 +169,22 @@ def validate_transcript_upload_data(request): error, validated_data = None, {} data, files = request.POST, request.FILES video_locator = data.get('locator') + edx_video_id = data.get('edx_video_id') if not video_locator: error = _(u'Video locator is required.') elif 'transcript-file' not in files: error = _(u'A transcript file is required.') elif os.path.splitext(files['transcript-file'].name)[1][1:] != Transcript.SRT: error = _(u'This transcript file type is not supported.') + elif 'edx_video_id' not in data: + error = _(u'Video ID is required.') if not error: error, video = validate_video_module(request, video_locator) if not error: validated_data.update({ 'video': video, - 'edx_video_id': clean_video_id(video.edx_video_id), + 'edx_video_id': clean_video_id(edx_video_id) or clean_video_id(video.edx_video_id), 'transcript_file': files['transcript-file'] }) @@ -212,6 +215,8 @@ def upload_transcripts(request): video.edx_video_id = edx_video_id video.save_with_metadata(request.user) + response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) + try: # Convert 'srt' transcript into the 'sjson' and upload it to # configured transcript storage. For example, S3. @@ -220,7 +225,7 @@ def upload_transcripts(request): input_format=Transcript.SRT, output_format=Transcript.SJSON ) - create_or_update_video_transcript( + transcript_created = create_or_update_video_transcript( video_id=edx_video_id, language_code=u'en', metadata={ @@ -230,7 +235,9 @@ def upload_transcripts(request): }, file_data=ContentFile(sjson_subs), ) - response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) + + if transcript_created is None: + response = JsonResponse({'status': 'Invalid Video ID'}, status=400) except (TranscriptsGenerationException, UnicodeDecodeError): diff --git a/cms/static/js/spec/video/transcripts/file_uploader_spec.js b/cms/static/js/spec/video/transcripts/file_uploader_spec.js index 6516504004fc..fcce1d7e1726 100644 --- a/cms/static/js/spec/video/transcripts/file_uploader_spec.js +++ b/cms/static/js/spec/video/transcripts/file_uploader_spec.js @@ -4,7 +4,7 @@ define( 'js/views/video/transcripts/utils', 'js/views/video/transcripts/file_uploader', 'xmodule', 'jquery.form' ], -function($, _, Backbone, Utils, FileUploader) { +function($, _, Backbone, TranscriptUtils, FileUploader) { 'use strict'; describe('Transcripts.FileUploader', function() { @@ -95,6 +95,12 @@ function($, _, Backbone, Utils, FileUploader) { }); describe('Upload', function() { + var videoId = '123-456-789-0'; + + beforeEach(function() { + TranscriptUtils.Storage.set('edx_video_id', videoId); + }); + it('File is not chosen', function() { spyOn($.fn, 'ajaxSubmit'); view.upload(); @@ -109,6 +115,9 @@ function($, _, Backbone, Utils, FileUploader) { view.upload(); expect(view.$form.ajaxSubmit).toHaveBeenCalled(); + expect(view.$form.ajaxSubmit).toHaveBeenCalledWith(jasmine.objectContaining({ + data: {'edx_video_id': videoId} + })); }); }); diff --git a/cms/static/js/spec/video/transcripts/videolist_spec.js b/cms/static/js/spec/video/transcripts/videolist_spec.js index 2158c8ddbee5..f8dfefebe0a1 100644 --- a/cms/static/js/spec/video/transcripts/videolist_spec.js +++ b/cms/static/js/spec/video/transcripts/videolist_spec.js @@ -99,6 +99,7 @@ function($, _, Backbone, AjaxHelpers, Utils, Editor, VideoList, MetadataModel, A // create mock server this.mockServer = createMockAjaxServer(); + spyOn($.fn, 'on').and.callThrough(); spyOn(Backbone, 'trigger').and.callThrough(); spyOn(Utils, 'command').and.callThrough(); spyOn(abstractEditor, 'initialize').and.callThrough(); @@ -215,14 +216,17 @@ function($, _, Backbone, AjaxHelpers, Utils, Editor, VideoList, MetadataModel, A it('Initialize', function(done) { - var view = createVideoListView(this.mockServer); + var view = createVideoListView(this.mockServer), callArgs; waitsForResponse(this.mockServer) - .then(function() { - expect(abstractEditor.initialize).toHaveBeenCalled(); - expect(MessageManager.prototype.initialize).toHaveBeenCalled(); - expect(view.component_locator).toBe(component_locator); - expect(view.$el).toHandle('input'); - }).always(done); + .then(function() { + expect(abstractEditor.initialize).toHaveBeenCalled(); + expect(MessageManager.prototype.initialize).toHaveBeenCalled(); + expect(view.component_locator).toBe(component_locator); + expect(view.$el).toHandle('input'); + callArgs = view.$el.on.calls.mostRecent().args; + expect(callArgs[0]).toEqual('input'); + expect(callArgs[1]).toEqual('.videolist-settings-item input'); + }).always(done); }); describe('Render', function() { diff --git a/cms/static/js/spec/views/metadata_edit_spec.js b/cms/static/js/spec/views/metadata_edit_spec.js index 9df165f9e5b1..cea1857b3b25 100644 --- a/cms/static/js/spec/views/metadata_edit_spec.js +++ b/cms/static/js/spec/views/metadata_edit_spec.js @@ -5,7 +5,7 @@ */ define(["underscore", "js/models/metadata", "js/collections/metadata", "js/views/metadata", "cms/js/main", "js/views/video/transcripts/utils", 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers'], -function(_, MetadataModel, MetadataCollection, MetadataView, main, Utils, AjaxHelpers) { +function(_, MetadataModel, MetadataCollection, MetadataView, main, TranscriptUtils, AjaxHelpers) { const verifyInputType = function(input, expectedType) { // Some browsers (e.g. FireFox) do not support the "number" // input type. We can accept a "text" input instead @@ -285,8 +285,10 @@ function(_, MetadataModel, MetadataCollection, MetadataView, main, Utils, AjaxHe beforeEach(function() { const model = new MetadataModel(videoIDEntry); + spyOn(TranscriptUtils.Storage, 'set'); this.view = new MetadataView.VideoID({model}); spyOn(Backbone, 'trigger'); + expect(TranscriptUtils.Storage.set).toHaveBeenCalledWith('edx_video_id', this.view.getValueFromEditor()); }); it("triggers correct event on input change", function(done) { diff --git a/cms/static/js/spec/views/upload_spec.js b/cms/static/js/spec/views/upload_spec.js index e1bff80c7827..cf8adec8de3a 100644 --- a/cms/static/js/spec/views/upload_spec.js +++ b/cms/static/js/spec/views/upload_spec.js @@ -1,9 +1,14 @@ -define(["sinon", "js/models/uploads", "js/views/uploads", "js/models/chapter", +define(["underscore", "sinon", "js/models/uploads", "js/views/uploads", "js/models/chapter", "edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers", "js/spec_helpers/modal_helpers"], - (sinon, FileUpload, UploadDialog, Chapter, AjaxHelpers, modal_helpers) => + (_, sinon, FileUpload, UploadDialog, Chapter, AjaxHelpers, modal_helpers) => describe("UploadDialog", function() { - const tpl = readFixtures("upload-dialog.underscore"); + const tpl = readFixtures("upload-dialog.underscore"), + uploadData = { + edx_video_id: '123-456-789-0', + language_code: 'en', + new_language_code: 'ur' + }; beforeEach(function() { let dialogResponse; @@ -27,7 +32,8 @@ define(["sinon", "js/models/uploads", "js/views/uploads", "js/models/chapter", url: CMS.URL.UPLOAD_ASSET, onSuccess: response => { return test.dialogResponse.push(response.response); - } + }, + uploadData: uploadData }); spyOn(view, 'remove').and.callThrough(); @@ -37,6 +43,7 @@ define(["sinon", "js/models/uploads", "js/views/uploads", "js/models/chapter", const jqMockFileInput = jasmine.createSpyObj('jqMockFileInput', ['get', 'replaceWith']); jqMockFileInput.get.and.returnValue(mockFileInput); const originalView$ = view.$; + spyOn($.fn, 'ajaxSubmit').and.callThrough(); spyOn(view, "$").and.callFake(function(selector) { if (selector === "input[type=file]") { return jqMockFileInput; @@ -126,6 +133,9 @@ define(["sinon", "js/models/uploads", "js/views/uploads", "js/models/chapter", view.upload(); expect(this.model.get("uploading")).toBeTruthy(); AjaxHelpers.expectRequest(requests, "POST", "/upload"); + expect($.fn.ajaxSubmit.calls.mostRecent().args[0].data).toEqual( + _.extend({}, uploadData, {notifyOnError: false}) + ); AjaxHelpers.respondWithJson(requests, { response: "dummy_response"}); expect(this.model.get("uploading")).toBeFalsy(); expect(this.model.get("finished")).toBeTruthy(); diff --git a/cms/static/js/views/metadata.js b/cms/static/js/views/metadata.js index f348d9f5a942..dd28c066a7e9 100644 --- a/cms/static/js/views/metadata.js +++ b/cms/static/js/views/metadata.js @@ -4,11 +4,12 @@ define( 'js/views/baseview', 'underscore', 'js/models/metadata', 'js/views/abstract_editor', 'js/models/uploads', 'js/views/uploads', 'js/models/license', 'js/views/license', + 'js/views/video/transcripts/utils', 'js/views/video/transcripts/metadata_videolist', 'js/views/video/translations_editor' ], function(Backbone, BaseView, _, MetadataModel, AbstractEditor, FileUpload, UploadDialog, - LicenseModel, LicenseView, VideoList, VideoTranslations) { + LicenseModel, LicenseView, TranscriptUtils, VideoList, VideoTranslations) { 'use strict'; var Metadata = {}; @@ -138,6 +139,11 @@ function(Backbone, BaseView, _, MetadataModel, AbstractEditor, FileUpload, Uploa ); }, + render: function() { + Metadata.String.prototype.render.apply(this, arguments); + TranscriptUtils.Storage.set('edx_video_id', this.getValueFromEditor()); + }, + clear: function() { this.model.setValue(''); this.inputChange(); @@ -148,6 +154,7 @@ function(Backbone, BaseView, _, MetadataModel, AbstractEditor, FileUpload, Uploa }, inputChange: function() { + TranscriptUtils.Storage.set('edx_video_id', this.getValueFromEditor()); Backbone.trigger('transcripts:basicTabFieldChanged'); } }); diff --git a/cms/static/js/views/video/transcripts/file_uploader.js b/cms/static/js/views/video/transcripts/file_uploader.js index 872578c96492..aba2a2407312 100644 --- a/cms/static/js/views/video/transcripts/file_uploader.js +++ b/cms/static/js/views/video/transcripts/file_uploader.js @@ -3,7 +3,7 @@ define( 'jquery', 'backbone', 'underscore', 'js/views/video/transcripts/utils' ], -function($, Backbone, _, Utils) { +function($, Backbone, _, TranscriptUtils) { var FileUploader = Backbone.View.extend({ invisibleClass: 'is-invisible', @@ -57,6 +57,10 @@ function($, Backbone, _, Utils) { * */ upload: function() { + var data = { + 'edx_video_id': TranscriptUtils.Storage.get('edx_video_id') || '' + }; + if (!this.file) { return; } @@ -64,7 +68,8 @@ function($, Backbone, _, Utils) { this.$form.ajaxSubmit({ beforeSend: this.xhrResetProgressBar, uploadProgress: this.xhrProgressHandler, - complete: this.xhrCompleteHandler + complete: this.xhrCompleteHandler, + data: data }); }, diff --git a/cms/static/js/views/video/transcripts/metadata_videolist.js b/cms/static/js/views/video/transcripts/metadata_videolist.js index 6d27e1343ccc..8cb9b8990451 100644 --- a/cms/static/js/views/video/transcripts/metadata_videolist.js +++ b/cms/static/js/views/video/transcripts/metadata_videolist.js @@ -43,7 +43,7 @@ function($, Backbone, _, AbstractEditor, Utils, MessageManager) { .apply(this, arguments); this.$el.on( - 'input', 'input', + 'input', '.videolist-settings-item input', _.debounce(_.bind(this.inputHandler, this), this.inputDelay) ); diff --git a/cms/static/js/views/video/translations_editor.js b/cms/static/js/views/video/translations_editor.js index 23c67bebe9df..9c82afa18025 100644 --- a/cms/static/js/views/video/translations_editor.js +++ b/cms/static/js/views/video/translations_editor.js @@ -27,6 +27,8 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, ViewUtils, FileUpload templateName: 'metadata-translations-entry', templateItemName: 'metadata-translations-item', + validFileFormats: ['srt'], + initialize: function() { var templateName = _.result(this, 'templateItemName'), tpl = document.getElementById(templateName).text, @@ -38,11 +40,12 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, ViewUtils, FileUpload this.templateItem = _.template(tpl); - // Initialize language map. Keys in this map represent language codes present on - // server. Values will change if user changes the language from a dropdown. - // For example: Initially map will look like {'ar': 'ar', 'zh': 'zh'} and corresponding - // dropdowns will show language names `Arabic` and `Chinese`. If user changes `Chinese` - // to `Russian` then map will become {'ar': 'ar', 'zh': 'ru'} + // Initialize language map. This maps original language to the newly selected language. + // Keys in this map represent language codes present on server, they don't change when + // user selects a language while values represent currently selected language. + // Initially, the map will look like {'ar': 'ar', 'zh': 'zh'} i.e {'original_lang': 'original_lang'} + // and corresponding dropdowns will show language names Arabic and Chinese. If user changes + // Chinese to Russian then map will become {'ar': 'ar', 'zh': 'ru'} i.e {'original_lang': 'new_lang'} _.each(this.model.getDisplayValue(), function(value, lang) { languageMap[lang] = lang; }); @@ -216,7 +219,7 @@ function($, _, HtmlUtils, TranscriptUtils, AbstractEditor, ViewUtils, FileUpload fileUploadModel = new FileUpload({ title: gettext('Upload translation'), - fileFormats: ['srt'] + fileFormats: this.validFileFormats }); videoUploadDialog = new VideoUploadDialog({