Merge pull request #18032 from edx/ammar/EDUCATOR-2761-basic-tab-upload-edx-video-id

basic tab attach uploads with edx_video_id
This commit is contained in:
Muhammad Ammar
2018-05-03 11:20:32 +05:00
committed by GitHub
10 changed files with 126 additions and 34 deletions

View File

@@ -195,7 +195,7 @@ class TestUploadTranscripts(BaseTranscripts):
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 @@ class TestUploadTranscripts(BaseTranscripts):
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 @@ class TestUploadTranscripts(BaseTranscripts):
# 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 @@ class TestUploadTranscripts(BaseTranscripts):
"""
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 @@ class TestUploadTranscripts(BaseTranscripts):
"""
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 @@ class TestUploadTranscripts(BaseTranscripts):
"""
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 @@ class TestUploadTranscripts(BaseTranscripts):
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 @@ class TestUploadTranscripts(BaseTranscripts):
# 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 @@ class TestUploadTranscripts(BaseTranscripts):
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):

View File

@@ -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):

View File

@@ -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}
}));
});
});

View File

@@ -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() {

View File

@@ -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) {

View File

@@ -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();

View File

@@ -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');
}
});

View File

@@ -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
});
},

View File

@@ -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)
);

View File

@@ -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({