From 127bf7ed3f0e309677c1a763c6a15996ea0d4efb Mon Sep 17 00:00:00 2001 From: polesye Date: Mon, 14 Apr 2014 18:52:58 +0300 Subject: [PATCH] BLD-1000: Download handout. --- CHANGELOG.rst | 3 + cms/djangoapps/contentstore/features/video.py | 1 + ...eo-editor.feature => video_editor.feature} | 0 .../{video-editor.py => video_editor.py} | 1 + .../features/video_handout.feature | 71 +++++++ .../contentstore/features/video_handout.py | 40 ++++ cms/static/coffee/spec/main_squire.coffee | 3 +- .../coffee/spec/models/upload_spec.coffee | 8 +- cms/static/js/models/uploads.js | 3 +- .../spec/video/file_uploader_editor_spec.js | 176 ++++++++++++++++++ .../js/spec/video/translations_editor_spec.js | 70 ++----- cms/static/js/views/abstract_editor.js | 21 ++- cms/static/js/views/metadata.js | 70 ++++++- cms/static/sass/views/_unit.scss | 35 ++++ .../metadata-file-uploader-entry.underscore | 9 + .../js/metadata-file-uploader-item.underscore | 3 + cms/templates/widgets/metadata-edit.html | 2 +- .../widgets/tabs/metadata-edit-tab.html | 2 +- .../xmodule/xmodule/css/video/display.scss | 3 +- .../lib/xmodule/xmodule/tests/test_video.py | 15 +- .../xmodule/video_module/video_module.py | 12 ++ .../xmodule/video_module/video_xfields.py | 17 +- .../courseware/tests/test_video_mongo.py | 5 + lms/templates/video.html | 9 +- 24 files changed, 498 insertions(+), 81 deletions(-) rename cms/djangoapps/contentstore/features/{video-editor.feature => video_editor.feature} (100%) rename cms/djangoapps/contentstore/features/{video-editor.py => video_editor.py} (99%) create mode 100644 cms/djangoapps/contentstore/features/video_handout.feature create mode 100644 cms/djangoapps/contentstore/features/video_handout.py create mode 100644 cms/static/js/spec/video/file_uploader_editor_spec.js create mode 100644 cms/templates/js/metadata-file-uploader-entry.underscore create mode 100644 cms/templates/js/metadata-file-uploader-item.underscore diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 01894935bd..02b4982463 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,9 @@ 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: Add an upload button for authors to provide students with an option to +download a handout associated with a video (of arbitrary file format). BLD-1000. + Blades: Show the HD button only if there is an HD version available. BLD-937. Studio: Add edit button to leaf xblocks on the container page. STUD-1306. diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index b8287047af..a19d7c2e67 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -11,6 +11,7 @@ VIDEO_BUTTONS = { 'volume': '.volume', 'play': '.video_control.play', 'pause': '.video_control.pause', + 'handout': '.video-handout.video-download-button a', } SELECTORS = { diff --git a/cms/djangoapps/contentstore/features/video-editor.feature b/cms/djangoapps/contentstore/features/video_editor.feature similarity index 100% rename from cms/djangoapps/contentstore/features/video-editor.feature rename to cms/djangoapps/contentstore/features/video_editor.feature diff --git a/cms/djangoapps/contentstore/features/video-editor.py b/cms/djangoapps/contentstore/features/video_editor.py similarity index 99% rename from cms/djangoapps/contentstore/features/video-editor.py rename to cms/djangoapps/contentstore/features/video_editor.py index 483172a0b8..31f41b6ae8 100644 --- a/cms/djangoapps/contentstore/features/video-editor.py +++ b/cms/djangoapps/contentstore/features/video_editor.py @@ -148,6 +148,7 @@ def correct_video_settings(_step): ['Transcript Display', 'True', False], ['Transcript Download Allowed', 'False', False], ['Transcript Translations', '', False], + ['Upload Handout', '', False], ['Video Download Allowed', 'False', False], ['Video Sources', '', False], ['Youtube ID', 'OEoXaMPEzfM', False], diff --git a/cms/djangoapps/contentstore/features/video_handout.feature b/cms/djangoapps/contentstore/features/video_handout.feature new file mode 100644 index 0000000000..980ae0ed6f --- /dev/null +++ b/cms/djangoapps/contentstore/features/video_handout.feature @@ -0,0 +1,71 @@ +@shard_3 +Feature: CMS Video Component Handout + As a course author, I want to be able to create video handout + + # 1 + Scenario: Handout uploading works correctly + Given I have created a Video component with handout file "textbook.pdf" + And I save changes + Then I can see video button "handout" + And I can download handout file with mime type "application/pdf" + + # 2 + Scenario: Handout downloading works correctly w/ preliminary saving + Given I have created a Video component with handout file "textbook.pdf" + And I save changes + And I edit the component + And I open tab "Advanced" + And I can download handout file in editor with mime type "application/pdf" + + # 3 + Scenario: Handout downloading works correctly w/o preliminary saving + Given I have created a Video component with handout file "textbook.pdf" + And I can download handout file in editor with mime type "application/pdf" + + # 4 + Scenario: Handout clearing works correctly w/ preliminary saving + Given I have created a Video component with handout file "textbook.pdf" + And I save changes + And I can download handout file with mime type "application/pdf" + And I edit the component + And I open tab "Advanced" + And I clear handout + And I save changes + Then I do not see video button "handout" + + # 5 + Scenario: Handout clearing works correctly w/o preliminary saving + Given I have created a Video component with handout file "asset.html" + And I clear handout + And I save changes + Then I do not see video button "handout" + + # 6 + Scenario: User can easy replace the handout by another one w/ preliminary saving + Given I have created a Video component with handout file "asset.html" + And I save changes + Then I can see video button "handout" + And I can download handout file with mime type "text/html" + And I edit the component + And I open tab "Advanced" + And I replace handout file by "textbook.pdf" + And I save changes + Then I can see video button "handout" + And I can download handout file with mime type "application/pdf" + + # 7 + Scenario: User can easy replace the handout by another one w/o preliminary saving + Given I have created a Video component with handout file "asset.html" + And I replace handout file by "textbook.pdf" + And I save changes + Then I can see video button "handout" + And I can download handout file with mime type "application/pdf" + + # 8 + Scenario: Upload file "A" -> Remove it -> Upload file "B" + Given I have created a Video component with handout file "asset.html" + And I clear handout + And I upload handout file "textbook.pdf" + And I save changes + Then I can see video button "handout" + And I can download handout file with mime type "application/pdf" diff --git a/cms/djangoapps/contentstore/features/video_handout.py b/cms/djangoapps/contentstore/features/video_handout.py new file mode 100644 index 0000000000..c847a2a5fb --- /dev/null +++ b/cms/djangoapps/contentstore/features/video_handout.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +# disable missing docstring +# pylint: disable=C0111 + +from lettuce import world, step +from nose.tools import assert_true # pylint: disable=E0611 +from video_editor import RequestHandlerWithSessionId, success_upload_file + + +@step('I (?:upload|replace) handout file(?: by)? "([^"]*)"$') +def upload_handout(step, filename): + world.css_click('.wrapper-comp-setting.file-uploader .upload-action') + success_upload_file(filename) + + +@step('I can download handout file( in editor)? with mime type "([^"]*)"$') +def i_can_download_handout_with_mime_type(_step, is_editor, mime_type): + if is_editor: + selector = '.wrapper-comp-setting.file-uploader .download-action' + else: + selector = '.video-handout.video-download-button a' + + button = world.css_find(selector).first + url = button['href'] + request = RequestHandlerWithSessionId() + assert_true(request.get(url).is_success()) + assert_true(request.check_header('content-type', mime_type)) + + +@step('I clear handout$') +def clear_handout(_step): + world.css_click('.wrapper-comp-setting.file-uploader .setting-clear') + + +@step('I have created a Video component with handout file "([^"]*)"') +def create_video_with_handout(_step, filename): + _step.given('I have created a Video component') + _step.given('I edit the component') + _step.given('I open tab "Advanced"') + _step.given('I upload handout file "{0}"'.format(filename)) diff --git a/cms/static/coffee/spec/main_squire.coffee b/cms/static/coffee/spec/main_squire.coffee index e7e6bef00b..7c85125c7c 100644 --- a/cms/static/coffee/spec/main_squire.coffee +++ b/cms/static/coffee/spec/main_squire.coffee @@ -175,5 +175,6 @@ jasmine.getFixtures().fixturesPath += 'coffee/fixtures' define([ "coffee/spec/views/assets_spec", - "js/spec/video/translations_editor_spec" + "js/spec/video/translations_editor_spec", + "js/spec/video/file_uploader_editor_spec" ]) diff --git a/cms/static/coffee/spec/models/upload_spec.coffee b/cms/static/coffee/spec/models/upload_spec.coffee index c91562b6e5..17cd887b36 100644 --- a/cms/static/coffee/spec/models/upload_spec.coffee +++ b/cms/static/coffee/spec/models/upload_spec.coffee @@ -13,15 +13,15 @@ define ["js/models/uploads"], (FileUpload) -> it "is valid by default", -> expect(@model.isValid()).toBeTruthy() - it "is invalid for text files by default", -> + it "is valid for text files by default", -> file = {"type": "text/plain", "name": "filename.txt"} @model.set("selectedFile", file); - expect(@model.isValid()).toBeFalsy() + expect(@model.isValid()).toBeTruthy() - it "is invalid for PNG files by default", -> + it "is valid for PNG files by default", -> file = {"type": "image/png", "name": "filename.png"} @model.set("selectedFile", file); - expect(@model.isValid()).toBeFalsy() + expect(@model.isValid()).toBeTruthy() it "can accept a file type when explicitly set", -> file = {"type": "image/png", "name": "filename.png"} diff --git a/cms/static/js/models/uploads.js b/cms/static/js/models/uploads.js index 41ee4abcdb..7a19b3c4bd 100644 --- a/cms/static/js/models/uploads.js +++ b/cms/static/js/models/uploads.js @@ -47,7 +47,8 @@ var FileUpload = Backbone.Model.extend({ return RegExp(('(?:.+)\\.(' + formats.join('|') + ')$'), 'i'); }; - return _.contains(attrs.mimeTypes, file.type) || + return (attrs.mimeTypes.length === 0 && attrs.fileFormats.length === 0) || + _.contains(attrs.mimeTypes, file.type) || getRegExp(attrs.fileFormats).test(file.name); }, // Return strings for the valid file types and extensions this diff --git a/cms/static/js/spec/video/file_uploader_editor_spec.js b/cms/static/js/spec/video/file_uploader_editor_spec.js new file mode 100644 index 0000000000..95b0913d07 --- /dev/null +++ b/cms/static/js/spec/video/file_uploader_editor_spec.js @@ -0,0 +1,176 @@ +define( + [ + 'jquery', 'underscore', 'js/spec_helpers/create_sinon', 'squire' + ], +function ($, _, create_sinon, Squire) { + 'use strict'; + describe('FileUploader', function () { + var FileUploaderTemplate = readFixtures( + 'metadata-file-uploader-entry.underscore' + ), + FileUploaderItemTemplate = readFixtures( + 'metadata-file-uploader-item.underscore' + ), + locator = 'locator', + feedbackTpl = readFixtures('system-feedback.underscore'), + modelStub = { + default_value: 'http://example.org/test_1', + display_name: 'File Upload', + explicitly_set: false, + field_name: 'file_upload', + help: 'Specifies the name for this component.', + type: 'FileUploader', + value: 'http://example.org/test_1' + }, + self, injector; + + var setValue = function (view, value) { + view.setValueInEditor(value); + view.updateModel(); + }; + + var createPromptSpy = function (name) { + var spy = jasmine.createSpyObj(name, ['constructor', 'show', 'hide']); + spy.constructor.andReturn(spy); + spy.show.andReturn(spy); + spy.extend = jasmine.createSpy().andReturn(spy.constructor); + + return spy; + }; + + beforeEach(function () { + self = this; + + this.addMatchers({ + assertValueInView: function(expected) { + var value = this.actual.getValueFromEditor(); + return this.env.equals_(value, expected); + }, + assertCanUpdateView: function (expected) { + var view = this.actual, + value; + + view.setValueInEditor(expected); + value = view.getValueFromEditor(); + + return this.env.equals_(value, expected); + }, + assertClear: function (modelValue) { + var env = this.env, + view = this.actual, + model = view.model; + + return model.getValue() === null && + env.equals_(model.getDisplayValue(), modelValue) && + env.equals_(view.getValueFromEditor(), modelValue); + }, + assertUpdateModel: function (originalValue, newValue) { + var env = this.env, + view = this.actual, + model = view.model, + expectOriginal; + + view.setValueInEditor(newValue); + expectOriginal = env.equals_(model.getValue(), originalValue); + view.updateModel(); + + return expectOriginal && + env.equals_(model.getValue(), newValue); + }, + verifyButtons: function (upload, download, index) { + var view = this.actual, + uploadBtn = view.$('.upload-setting'), + downloadBtn = view.$('.download-setting'); + + upload = upload ? uploadBtn.length : !uploadBtn.length; + download = download ? downloadBtn.length : !downloadBtn.length; + + return upload && download; + } + }); + + appendSetFixtures($(' -% for template_name in ["metadata-number-entry", "metadata-string-entry", "metadata-option-entry", "metadata-list-entry", "metadata-dict-entry"]: +% for template_name in ["metadata-number-entry", "metadata-string-entry", "metadata-option-entry", "metadata-list-entry", "metadata-dict-entry", "metadata-file-uploader-entry", "metadata-file-uploader-item"]: diff --git a/cms/templates/widgets/tabs/metadata-edit-tab.html b/cms/templates/widgets/tabs/metadata-edit-tab.html index 862dfd58d0..2861a9ffd0 100644 --- a/cms/templates/widgets/tabs/metadata-edit-tab.html +++ b/cms/templates/widgets/tabs/metadata-edit-tab.html @@ -8,7 +8,7 @@ <%static:include path="js/metadata-editor.underscore" /> -% for template_name in ["metadata-number-entry", "metadata-string-entry", "metadata-option-entry", "metadata-list-entry", "metadata-dict-entry"]: +% for template_name in ["metadata-number-entry", "metadata-string-entry", "metadata-option-entry", "metadata-list-entry", "metadata-dict-entry", "metadata-file-uploader-entry", "metadata-file-uploader-item"]: diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index 55b7f78a84..83aa6daa65 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -42,8 +42,7 @@ div.video { margin: 0; padding: 0; - .video-sources, - .video-tracks { + .video-download-button{ display: inline-block; vertical-align: top; margin: ($baseline*.75) ($baseline/2) 0 0; diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index cebb8dc30b..ed5afa2edd 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/common/lib/xmodule/xmodule/tests/test_video.py @@ -189,6 +189,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): + @@ -211,6 +212,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): 'start_time': datetime.timedelta(seconds=1), 'end_time': datetime.timedelta(seconds=60), 'track': 'http://www.example.com/track', + 'handout': 'http://www.example.com/handout', 'download_track': True, 'html5_sources': ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg'], 'data': '', @@ -229,6 +231,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): end_time="00:01:00"> + @@ -243,6 +246,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): 'start_time': datetime.timedelta(seconds=1), 'end_time': datetime.timedelta(seconds=60), 'track': 'http://www.example.com/track', + 'handout': 'http://www.example.com/handout', 'download_track': False, 'download_video': False, 'html5_sources': ['http://www.example.com/source.mp4'], @@ -273,6 +277,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): 'start_time': datetime.timedelta(seconds=0.0), 'end_time': datetime.timedelta(seconds=0.0), 'track': '', + 'handout': None, 'download_track': False, 'download_video': True, 'html5_sources': ['http://www.example.com/source.mp4'], @@ -326,6 +331,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): 'start_time': datetime.timedelta(seconds=0.0), 'end_time': datetime.timedelta(seconds=0.0), 'track': '', + 'handout': None, 'download_track': False, 'download_video': False, 'html5_sources': [], @@ -345,7 +351,8 @@ class VideoDescriptorImportTestCase(unittest.TestCase): show_captions="false" download_video="true" sub=""html5_subtitles"" - track=""http://download_track"" + track=""http://www.example.com/track"" + handout=""http://www.example.com/handout"" download_track="true" youtube_id_0_75=""OEoXaMPEzf65"" youtube_id_1_25=""OEoXaMPEzf125"" @@ -362,7 +369,8 @@ class VideoDescriptorImportTestCase(unittest.TestCase): 'show_captions': False, 'start_time': datetime.timedelta(seconds=0.0), 'end_time': datetime.timedelta(seconds=0.0), - 'track': 'http://download_track', + 'track': 'http://www.example.com/track', + 'handout': 'http://www.example.com/handout', 'download_track': True, 'download_video': True, 'html5_sources': ["source_1", "source_2"], @@ -386,6 +394,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase): 'start_time': datetime.timedelta(seconds=0.0), 'end_time': datetime.timedelta(seconds=0.0), 'track': '', + 'handout': None, 'download_track': False, 'download_video': False, 'html5_sources': [], @@ -509,6 +518,7 @@ class VideoExportTestCase(unittest.TestCase): desc.start_time = datetime.timedelta(seconds=1.0) desc.end_time = datetime.timedelta(seconds=60) desc.track = 'http://www.example.com/track' + desc.handout = 'http://www.example.com/handout' desc.download_track = True desc.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg'] desc.download_video = True @@ -520,6 +530,7 @@ class VideoExportTestCase(unittest.TestCase): + diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 27029dd327..11e3484b91 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -143,6 +143,7 @@ class VideoModule(VideoFields, VideoStudentViewHandlers, XModule): 'data_dir': getattr(self, 'data_dir', None), 'display_name': self.display_name_with_default, 'end': self.end_time.total_seconds(), + 'handout': self.handout, 'id': self.location.html_id(), 'show_captions': json.dumps(self.show_captions), 'sources': sources, @@ -248,6 +249,8 @@ class VideoDescriptor(VideoFields, VideoStudioViewHandlers, TabsEditingDescripto editable_fields['transcripts']['languages'] = languages editable_fields['transcripts']['type'] = 'VideoTranslations' editable_fields['transcripts']['urlRoot'] = self.runtime.handler_url(self, 'studio_transcript', 'translation').rstrip('/?') + editable_fields['handout']['type'] = 'FileUploader' + return editable_fields @classmethod @@ -320,6 +323,11 @@ class VideoDescriptor(VideoFields, VideoStudioViewHandlers, TabsEditingDescripto ele.set('src', self.track) xml.append(ele) + if self.handout: + ele = etree.Element('handout') + ele.set('src', self.handout) + xml.append(ele) + # sorting for easy testing of resulting xml for transcript_language in sorted(self.transcripts.keys()): ele = etree.Element('transcript') @@ -422,6 +430,10 @@ class VideoDescriptor(VideoFields, VideoStudioViewHandlers, TabsEditingDescripto if track is not None: field_data['track'] = track.get('src') + handout = xml.find('handout') + if handout is not None: + field_data['handout'] = handout.get('src') + transcripts = xml.findall('transcript') if transcripts: field_data['transcripts'] = {tr.get('language'): tr.get('src') for tr in transcripts} diff --git a/common/lib/xmodule/xmodule/video_module/video_xfields.py b/common/lib/xmodule/xmodule/video_module/video_xfields.py index 2bf5c96482..f70e5e2841 100644 --- a/common/lib/xmodule/xmodule/video_module/video_xfields.py +++ b/common/lib/xmodule/xmodule/video_module/video_xfields.py @@ -15,8 +15,9 @@ class VideoFields(object): default="Video", scope=Scope.settings ) + saved_video_position = RelativeTime( - help="Current position in the video", + help="Current position in the video.", scope=Scope.user_state, default=datetime.timedelta(seconds=0) ) @@ -105,13 +106,13 @@ class VideoFields(object): ) # Data format: {'de': 'german_translation', 'uk': 'ukrainian_translation'} transcripts = Dict( - help="Add additional transcripts in other languages", + help="Add additional transcripts in other languages.", display_name="Transcript Translations", scope=Scope.settings, default={} ) transcript_language = String( - help="Preferred language for transcript", + help="Preferred language for transcript.", display_name="Preferred language for transcript", scope=Scope.preferences, default="en" @@ -130,12 +131,18 @@ class VideoFields(object): scope=Scope.user_state, ) global_speed = Float( - help="Default speed in cases when speed wasn't explicitly for specific video", + help="Default speed in cases when speed wasn't explicitly for specific video.", scope=Scope.preferences, default=1.0 ) youtube_is_available = Boolean( - help="The availaibility of YouTube API for the user", + help="The availaibility of YouTube API for the user.", scope=Scope.user_info, default=True ) + + handout = String( + help="Upload a handout to accompany this video. Students can download the handout by clicking Download Handout under the video.", + display_name="Upload Handout", + scope=Scope.settings, + ) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index fc9e291909..6054b23153 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -41,6 +41,7 @@ class TestVideoYouTube(TestVideo): 'end': 3610.0, 'id': self.item_descriptor.location.html_id(), 'show_captions': 'true', + 'handout': None, 'sources': sources, 'speed': 'null', 'general_speed': 1.0, @@ -104,6 +105,7 @@ class TestVideoNonYouTube(TestVideo): 'ajax_url': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state', 'data_dir': getattr(self, 'data_dir', None), 'show_captions': 'true', + 'handout': None, 'display_name': u'A Name', 'end': 3610.0, 'id': self.item_descriptor.location.html_id(), @@ -203,6 +205,7 @@ class TestGetHtmlMethod(BaseTestXmodule): expected_context = { 'data_dir': getattr(self, 'data_dir', None), 'show_captions': 'true', + 'handout': None, 'display_name': u'A Name', 'end': 3610.0, 'id': None, @@ -324,6 +327,7 @@ class TestGetHtmlMethod(BaseTestXmodule): expected_context = { 'data_dir': getattr(self, 'data_dir', None), 'show_captions': 'true', + 'handout': None, 'display_name': u'A Name', 'end': 3610.0, 'id': None, @@ -469,6 +473,7 @@ class TestVideoDescriptorInitialization(BaseTestXmodule): 'options': [], }, 'transcripts': {}, + 'handout': {}, } ): metadata = { diff --git a/lms/templates/video.html b/lms/templates/video.html index b3e1a8a24f..9dfaa72443 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -107,12 +107,12 @@