From 7b2d67f2096de37c5cad893dc4a5ed7cac69b8c7 Mon Sep 17 00:00:00 2001 From: polesye Date: Thu, 29 May 2014 14:58:38 +0300 Subject: [PATCH] BLD-1049: Allow the video player to work with redirected links. --- CHANGELOG.rst | 2 + .../contentstore/features/transcripts.feature | 48 ++- .../contentstore/features/transcripts.py | 18 +- .../js/spec/video/transcripts/utils_spec.js | 66 ++-- .../spec/video/transcripts/videolist_spec.js | 210 +++++++++---- .../video/transcripts/metadata_videolist.js | 286 +++++++++--------- .../js/views/video/transcripts/utils.js | 281 +++++++++-------- .../xmodule/js/fixtures/video_all.html | 4 +- .../xmodule/js/fixtures/video_html5.html | 4 +- .../xmodule/js/spec/video/general_spec.js | 47 --- .../xmodule/js/src/video/01_initialize.js | 81 +---- .../xmodule/js/src/video/02_html5_video.js | 85 +++--- .../xmodule/js/src/video/03_video_player.js | 2 +- .../lib/xmodule/xmodule/tests/test_video.py | 14 +- .../xmodule/video_module/video_module.py | 19 +- .../courseware/tests/test_video_mongo.py | 51 ++-- lms/templates/video.html | 9 +- 17 files changed, 627 insertions(+), 600 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f27c08a853..930981610e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Blades: Fix bug with incorrect link format and redirection. BLD-1049 + Blades: Fix bug with incorrect RelativeTime value after XML serialization. BLD-1060 LMS: Update bulk email implementation to lessen load on the database diff --git a/cms/djangoapps/contentstore/features/transcripts.feature b/cms/djangoapps/contentstore/features/transcripts.feature index d69d12a49f..e2687f2b05 100644 --- a/cms/djangoapps/contentstore/features/transcripts.feature +++ b/cms/djangoapps/contentstore/features/transcripts.feature @@ -34,14 +34,22 @@ Feature: CMS Transcripts And I expect inputs are enabled #User input URL with incorrect format - And I enter a "htt://link.c" source to field number 1 + And I enter a "http://link.c" source to field number 1 Then I see error message "url_format" # Currently we are working with 1st field. It means, that if 1st field # contain incorrect value, 2nd and 3rd fields should be disabled until # 1st field will be filled by correct correct value And I expect 2, 3 inputs are disabled - # We are not clearing fields here, - # Because we changing same field. + + #User input URL with incorrect format + And I enter a "http://goo.gl/pxxZrg" source to field number 1 + And I enter a "http://goo.gl/pxxZrg" source to field number 2 + Then I see error message "links_duplication" + And I expect 1, 3 inputs are disabled + + And I clear fields + And I expect inputs are enabled + And I enter a "http://youtu.be/t_not_exist" source to field number 1 Then I do not see error message And I expect inputs are enabled @@ -699,3 +707,37 @@ Feature: CMS Transcripts And I edit the component Then I see status message "found" + + #37 Uploading subtitles with different file name than file + Scenario: Shortened link: File name and name of subs are different + Given I have created a Video component + And I edit the component + + And I enter a "http://goo.gl/pxxZrg" source to field number 1 + And I see status message "not found" + And I upload the transcripts file "uk_transcripts.srt" + Then I see status message "uploaded_successfully" + And I see value "pxxZrg" in the field "Default Timed Transcript" + + And I save changes + Then when I view the video it does show the captions + + And I edit the component + Then I see status message "found" + + #38 Uploading subtitles with different file name than file + Scenario: Relative link: File name and name of subs are different + Given I have created a Video component + And I edit the component + + And I enter a "/gizmo.webm" source to field number 1 + And I see status message "not found" + And I upload the transcripts file "uk_transcripts.srt" + Then I see status message "uploaded_successfully" + And I see value "gizmo" in the field "Default Timed Transcript" + + And I save changes + Then when I view the video it does show the captions + + And I edit the component + Then I see status message "found" diff --git a/cms/djangoapps/contentstore/features/transcripts.py b/cms/djangoapps/contentstore/features/transcripts.py index 4e70912c3d..371ceea0db 100644 --- a/cms/djangoapps/contentstore/features/transcripts.py +++ b/cms/djangoapps/contentstore/features/transcripts.py @@ -19,6 +19,7 @@ DELAY = 0.5 ERROR_MESSAGES = { 'url_format': u'Incorrect url format.', 'file_type': u'Link types should be unique.', + 'links_duplication': u'Links should be unique.', } STATUSES = { @@ -43,7 +44,7 @@ TRANSCRIPTS_BUTTONS = { 'import': ('.setting-import', 'Import YouTube Transcript'), 'download_to_edit': ('.setting-download', 'Download Transcript for Editing'), 'disabled_download_to_edit': ('.setting-download.is-disabled', 'Download Transcript for Editing'), - 'upload_new_timed_transcripts': ('.setting-upload', 'Upload New Transcript'), + 'upload_new_timed_transcripts': ('.setting-upload', 'Upload New Transcript'), 'replace': ('.setting-replace', 'Yes, replace the edX transcript with the YouTube transcript'), 'choose': ('.setting-choose', 'Timed Transcript from {}'), 'use_existing': ('.setting-use-existing', 'Use Current Transcript'), @@ -118,8 +119,7 @@ def i_see_status_message(_step, status): assert world.css_has_text(SELECTORS['status_bar'], STATUSES[status]) DOWNLOAD_BUTTON = TRANSCRIPTS_BUTTONS["download_to_edit"][0] - if world.is_css_present(DOWNLOAD_BUTTON, wait_time=1) \ - and not world.css_find(DOWNLOAD_BUTTON)[0].has_class('is-disabled'): + if world.is_css_present(DOWNLOAD_BUTTON, wait_time=1) and not world.css_find(DOWNLOAD_BUTTON)[0].has_class('is-disabled'): assert _transcripts_are_downloaded() @@ -210,7 +210,7 @@ def check_text_in_the_captions(_step, text): @step('I see value "([^"]*)" in the field "([^"]*)"$') def check_transcripts_field(_step, values, field_name): world.select_editor_tab('Advanced') - tab = world.css_find('#settings-tab').first; + tab = world.css_find('#settings-tab').first field_id = '#' + tab.find_by_xpath('.//label[text()="%s"]' % field_name.strip())[0]['for'] values_list = [i.strip() == world.css_value(field_id) for i in values.split('|')] assert any(values_list) @@ -229,19 +229,19 @@ def open_tab(_step, tab_name): @step('I set value "([^"]*)" to the field "([^"]*)"$') def set_value_transcripts_field(_step, value, field_name): - tab = world.css_find('#settings-tab').first; + tab = world.css_find('#settings-tab').first XPATH = './/label[text()="{name}"]'.format(name=field_name) SELECTOR = '#' + tab.find_by_xpath(XPATH)[0]['for'] element = world.css_find(SELECTOR).first if element['type'] == 'text': SCRIPT = '$("{selector}").val("{value}").change()'.format( - selector=SELECTOR, - value=value - ) + selector=SELECTOR, + value=value + ) world.browser.execute_script(SCRIPT) assert world.css_has_value(SELECTOR, value) else: - assert False, 'Incorrect element type.'; + assert False, 'Incorrect element type.' world.wait_for_ajax_complete() diff --git a/cms/static/js/spec/video/transcripts/utils_spec.js b/cms/static/js/spec/video/transcripts/utils_spec.js index 68e09cdb4c..6658c9594a 100644 --- a/cms/static/js/spec/video/transcripts/utils_spec.js +++ b/cms/static/js/spec/video/transcripts/utils_spec.js @@ -26,7 +26,7 @@ describe('Transcripts.Utils', function () { } (videoId)), html5FileName = 'file_name', html5LinksList = (function (videoName) { - var videoTypes = ['mp4', 'webm'], + var videoTypes = ['mp4', 'webm', 'm4v', 'ogv'], links = [ 'http://somelink.com/%s.%s?param=1¶m=2#hash', 'http://somelink.com/%s.%s#hash', @@ -34,6 +34,7 @@ describe('Transcripts.Utils', function () { 'http://somelink.com/%s.%s', 'ftp://somelink.com/%s.%s', 'https://somelink.com/%s.%s', + 'https://somelink.com/sub/sub/%s.%s', 'http://cdn.somecdn.net/v/%s.%s', 'somelink.com/%s.%s', '%s.%s' @@ -48,7 +49,25 @@ describe('Transcripts.Utils', function () { return data; - } (html5FileName)); + } (html5FileName)), + otherLinkId = 'other_link_id', + otherLinksList = (function (linkId) { + var links = [ + 'http://goo.gl/%s?param=1¶m=2#hash', + 'http://goo.gl/%s?param=1¶m=2', + 'http://goo.gl/%s#hash', + 'http://goo.gl/%s', + 'http://goo.gl/%s', + 'ftp://goo.gl/%s', + 'https://goo.gl/%s', + '%s' + ]; + + return $.map(links, function (link) { + return _str.sprintf(link, linkId); + }); + + } (otherLinkId)); describe('Method: getField', function (){ var collection, @@ -107,7 +126,6 @@ describe('Transcripts.Utils', function () { }); describe('Wrong arguments ', function () { - beforeEach(function(){ spyOn(console, 'log'); }); @@ -124,18 +142,9 @@ describe('Transcripts.Utils', function () { expect(result).toBeUndefined(); }); - it('videoId is wrong', function () { - var videoId = 'wrong_id', - link = 'http://youtu.be/' + videoId, - result = Utils.parseYoutubeLink(link); - - expect(result).toBeUndefined(); - }); - var wrongUrls = [ - 'http://youtu.bee/' + videoId, 'http://youtu.be/', - 'example.com', + '/static/example', 'http://google.com/somevideo.mp4' ]; @@ -163,10 +172,20 @@ describe('Transcripts.Utils', function () { }); }); }); + + $.each(otherLinksList, function (index, link) { + it(link, function () { + var result = Utils.parseHTML5Link(link); + + expect(result).toEqual({ + video: otherLinkId, + type: 'other' + }); + }); + }); }); describe('Wrong arguments ', function () { - beforeEach(function(){ spyOn(console, 'log'); }); @@ -184,15 +203,11 @@ describe('Transcripts.Utils', function () { }); var html5WrongUrls = [ - 'http://youtu.bee/' + videoId, 'http://youtu.be/', - 'example.com', - 'http://google.com/somevideo.mp1', - 'http://google.com/somevideomp4', - 'http://google.com/somevideo_mp4', - 'http://google.com/somevideo:mp4', - 'http://google.com/somevideo', - 'http://google.com/somevideo.webm_' + 'http://example.com/.mp4', + 'http://example.com/video_name.', + 'http://example.com/', + 'http://example.com' ]; $.each(html5WrongUrls, function (index, link) { @@ -248,6 +263,13 @@ describe('Transcripts.Utils', function () { }); describe('Wrong arguments ', function () { + it('youtube videoId is wrong', function () { + var videoId = 'wrong_id', + link = 'http://youtu.be/' + videoId, + result = Utils.parseLink(link); + + expect(result).toEqual({ mode : 'incorrect' }); + }); it('no arguments', function () { var result = Utils.parseLink(); diff --git a/cms/static/js/spec/video/transcripts/videolist_spec.js b/cms/static/js/spec/video/transcripts/videolist_spec.js index 47b54051e0..b13e81040e 100644 --- a/cms/static/js/spec/video/transcripts/videolist_spec.js +++ b/cms/static/js/spec/video/transcripts/videolist_spec.js @@ -1,11 +1,13 @@ define( [ - "jquery", "underscore", - "js/views/video/transcripts/utils", "js/views/video/transcripts/metadata_videolist", - "js/views/metadata", "js/models/metadata", "js/views/abstract_editor", - "sinon", "xmodule", "jasmine-jquery" + 'jquery', 'underscore', + 'js/views/video/transcripts/utils', + 'js/views/video/transcripts/metadata_videolist', 'js/models/metadata', + 'js/views/abstract_editor', + 'sinon', 'xmodule', 'jasmine-jquery' ], -function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, sinon) { +function ($, _, Utils, VideoList, MetadataModel, AbstractEditor, sinon) { + 'use strict'; describe('CMS.Views.Metadata.VideoList', function () { var videoListEntryTemplate = readFixtures( 'video/transcripts/metadata-videolist-entry.underscore' @@ -14,19 +16,19 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s component_locator = 'component_locator', videoList = [ { - mode: "youtube", - type: "youtube", - video: "12345678901" + mode: 'youtube', + type: 'youtube', + video: '12345678901' }, { - mode: "html5", - type: "mp4", - video: "video" + mode: 'html5', + type: 'mp4', + video: 'video' }, { - mode: "html5", - type: "webm", - video: "video" + mode: 'html5', + type: 'webm', + video: 'video' } ], modelStub = { @@ -54,7 +56,7 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s sinonXhr = sinon.fakeServer.create(); sinonXhr.respondWith([ 200, - { "Content-Type": "application/json"}, + { 'Content-Type': 'application/json'}, response ]); @@ -65,15 +67,15 @@ function ($, _, Utils, VideoList, MetadataView, MetadataModel, AbstractEditor, s 'data-locator': component_locator }), model = new MetadataModel(modelStub), - videoList, $el; + $el; setFixtures(tpl); appendSetFixtures( - $("