From 556213659bfe76e5ff3a9776b9fc57bcd373f0bf Mon Sep 17 00:00:00 2001 From: Qubad786 Date: Fri, 30 Jun 2017 13:21:05 +0500 Subject: [PATCH] Add waffle switch to toggle video thumbnail feature. --- .../contentstore/views/tests/test_videos.py | 39 +++++++++++++++++-- cms/djangoapps/contentstore/views/videos.py | 16 ++++++-- .../js/spec/views/video_thumbnail_spec.js | 23 ++++++++--- cms/static/js/views/previous_video_upload.js | 22 +++++++---- .../js/views/previous_video_upload_list.js | 6 ++- .../js/previous-video-upload-list.underscore | 2 + .../js/previous-video-upload.underscore | 2 + 7 files changed, 89 insertions(+), 21 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index f3f8a68cd2..500291155f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -6,6 +6,7 @@ import csv import json import re from datetime import datetime +from functools import wraps from StringIO import StringIO import dateutil.parser @@ -21,11 +22,10 @@ from contentstore.models import VideoUploadConfig from contentstore.tests.utils import CourseTestCase from contentstore.utils import reverse_course_url from contentstore.views.videos import ( - KEY_EXPIRATION_IN_SECONDS, - StatusDisplayStrings, - convert_video_status, _get_default_video_image_url, - validate_video_image + validate_video_image, + VIDEO_IMAGE_UPLOAD_ENABLED, + WAFFLE_SWITCHES, ) from contentstore.views.videos import KEY_EXPIRATION_IN_SECONDS, StatusDisplayStrings, convert_video_status from xmodule.modulestore.tests.factories import CourseFactory @@ -33,6 +33,24 @@ from xmodule.modulestore.tests.factories import CourseFactory from openedx.core.djangoapps.profile_images.tests.helpers import make_image_file +def override_switch(switch, active): + """ + Overrides the given waffle switch to `active` boolean. + + Arguments: + switch(str): switch name + active(bool): A boolean representing (to be overridden) value + """ + def decorate(function): + @wraps(function) + def inner(*args, **kwargs): + with WAFFLE_SWITCHES.override(switch, active=active): + function(*args, **kwargs) + return inner + + return decorate + + class VideoUploadTestBase(object): """ Test cases for the video upload feature @@ -576,6 +594,16 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase): self.assertIn('error', response) self.assertEqual(response['error'], error_message) + @override_switch(VIDEO_IMAGE_UPLOAD_ENABLED, False) + def test_video_image_upload_disabled(self): + """ + Tests the video image upload when the feature is disabled. + """ + video_image_upload_url = self.get_url_for_course_key(self.course.id, {'edx_video_id': 'test_vid_id'}) + response = self.client.post(video_image_upload_url, {'file': 'dummy_file'}, format='multipart') + self.assertEqual(response.status_code, 404) + + @override_switch(VIDEO_IMAGE_UPLOAD_ENABLED, True) def test_video_image(self): """ Test video image is saved. @@ -597,6 +625,7 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase): self.assertNotEqual(image_url1, image_url2) + @override_switch(VIDEO_IMAGE_UPLOAD_ENABLED, True) def test_video_image_no_file(self): """ Test that an error error message is returned if upload request is incorrect. @@ -625,6 +654,7 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase): error = validate_video_image(image_file) self.assertEquals(error, 'This image file is corrupted.') + @override_switch(VIDEO_IMAGE_UPLOAD_ENABLED, True) def test_no_video_image(self): """ Test image url is set to None if no video image. @@ -800,6 +830,7 @@ class VideoImageTestCase(VideoUploadTestBase, CourseTestCase): ) ) @ddt.unpack + @override_switch(VIDEO_IMAGE_UPLOAD_ENABLED, True) def test_video_image_validation_message(self, image_data, error_message): """ Test video image validation gives proper error message. diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index c7b4fdd1f2..964eb7089e 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -2,10 +2,7 @@ Views related to the video upload feature """ from contextlib import closing -from datetime import datetime, timedelta -import logging -from boto import s3 import csv import logging from datetime import datetime, timedelta @@ -31,6 +28,7 @@ from edxval.api import ( update_video_image ) from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.waffle_utils import WaffleSwitchNamespace from contentstore.models import VideoUploadConfig from contentstore.utils import reverse_course_url @@ -44,6 +42,12 @@ __all__ = ['videos_handler', 'video_encodings_download', 'video_images_handler'] LOGGER = logging.getLogger(__name__) +# Waffle switches namespace for videos +WAFFLE_NAMESPACE = 'videos' +WAFFLE_SWITCHES = WaffleSwitchNamespace(name=WAFFLE_NAMESPACE) + +# Waffle switch for enabling/disabling video image upload feature +VIDEO_IMAGE_UPLOAD_ENABLED = 'video_image_upload_enabled' # Default expiration, in seconds, of one-time URLs used for uploading videos. KEY_EXPIRATION_IN_SECONDS = 86400 @@ -210,6 +214,11 @@ def validate_video_image(image_file): @login_required @require_POST def video_images_handler(request, course_key_string, edx_video_id=None): + + # respond with a 404 if image upload is not enabled. + if not WAFFLE_SWITCHES.is_enabled(VIDEO_IMAGE_UPLOAD_ENABLED): + return HttpResponseNotFound() + if 'file' not in request.FILES: return JsonResponse({'error': _(u'No file provided for video image')}, status=400) @@ -428,6 +437,7 @@ def videos_index_html(course): 'video_supported_file_formats': VIDEO_SUPPORTED_FILE_FORMATS.keys(), 'video_upload_max_file_size': VIDEO_UPLOAD_MAX_FILE_SIZE_GB, 'video_image_settings': { + 'video_image_upload_enabled': WAFFLE_SWITCHES.is_enabled(VIDEO_IMAGE_UPLOAD_ENABLED), 'max_size': settings.VIDEO_IMAGE_SETTINGS['VIDEO_IMAGE_MAX_BYTES'], 'min_size': settings.VIDEO_IMAGE_SETTINGS['VIDEO_IMAGE_MIN_BYTES'], 'max_width': settings.VIDEO_IMAGE_MAX_WIDTH, diff --git a/cms/static/js/spec/views/video_thumbnail_spec.js b/cms/static/js/spec/views/video_thumbnail_spec.js index ff903b13d5..237f20c7ee 100644 --- a/cms/static/js/spec/views/video_thumbnail_spec.js +++ b/cms/static/js/spec/views/video_thumbnail_spec.js @@ -29,11 +29,12 @@ define( /** * Creates a list of video records. * + * @param {Boolean} videoImageUploadEnabled Boolean indicating if the feature is enabled. * @param {Object} modelData Model data for video records. * @param {Integer} numVideos Number of video elements to create. * @param {Integer} videoViewIndex Index of video on which videoThumbnailView would be based. */ - createVideoListView = function(modelData, numVideos, videoViewIndex) { + createVideoListView = function(videoImageUploadEnabled, modelData, numVideos, videoViewIndex) { var modelData = modelData || {}, // eslint-disable-line no-redeclare numVideos = numVideos || 1, // eslint-disable-line no-redeclare videoViewIndex = videoViewIndex || 0, // eslint-disable-line no-redeclare, @@ -58,12 +59,17 @@ define( min_size: VIDEO_IMAGE_MIN_BYTES, max_width: VIDEO_IMAGE_MAX_WIDTH, max_height: VIDEO_IMAGE_MAX_HEIGHT, - supported_file_formats: VIDEO_IMAGE_SUPPORTED_FILE_FORMATS + supported_file_formats: VIDEO_IMAGE_SUPPORTED_FILE_FORMATS, + video_image_upload_enabled: videoImageUploadEnabled } }); $videoListEl = videoListView.render().$el; - videoThumbnailView = videoListView.itemViews[videoViewIndex].videoThumbnailView; - $videoThumbnailEl = videoThumbnailView.render().$el; + + if (videoImageUploadEnabled) { + videoThumbnailView = videoListView.itemViews[videoViewIndex].videoThumbnailView; + $videoThumbnailEl = videoThumbnailView.render().$el; + } + return videoListView; }; @@ -113,7 +119,12 @@ define( setFixtures('
'); TemplateHelpers.installTemplate('video-thumbnail'); TemplateHelpers.installTemplate('previous-video-upload-list'); - createVideoListView(); + createVideoListView(true); + }); + + it('Verifies that the ThumbnailView is not initialized on disabling the feature', function() { + createVideoListView(false); + expect(videoListView.itemViews[0].videoThumbnailView).toEqual(undefined); }); it('renders as expected', function() { @@ -122,7 +133,7 @@ define( }); it('does not show duration if not available', function() { - createVideoListView({duration: 0}); + createVideoListView(true, {duration: 0}); expect($videoThumbnailEl.find('.thumbnail-wrapper .video-duration')).not.toExist(); }); diff --git a/cms/static/js/views/previous_video_upload.js b/cms/static/js/views/previous_video_upload.js index e2b293187a..0e7c288f84 100644 --- a/cms/static/js/views/previous_video_upload.js +++ b/cms/static/js/views/previous_video_upload.js @@ -19,16 +19,21 @@ define( initialize: function(options) { this.template = HtmlUtils.template(previousVideoUploadTemplate); this.videoHandlerUrl = options.videoHandlerUrl; - this.videoThumbnailView = new VideoThumbnailView({ - model: this.model, - imageUploadURL: options.videoImageUploadURL, - defaultVideoImageURL: options.defaultVideoImageURL, - videoImageSettings: options.videoImageSettings - }); + this.videoImageUploadEnabled = options.videoImageSettings.video_image_upload_enabled; + + if (this.videoImageUploadEnabled) { + this.videoThumbnailView = new VideoThumbnailView({ + model: this.model, + imageUploadURL: options.videoImageUploadURL, + defaultVideoImageURL: options.defaultVideoImageURL, + videoImageSettings: options.videoImageSettings + }); + } }, render: function() { var renderedAttributes = { + videoImageUploadEnabled: this.videoImageUploadEnabled, created: DateUtils.renderDate(this.model.get('created')), status: this.model.get('status') }; @@ -38,7 +43,10 @@ define( _.extend({}, this.model.attributes, renderedAttributes) ) ); - this.videoThumbnailView.setElement(this.$('.thumbnail-col')).render(); + + if (this.videoImageUploadEnabled) { + this.videoThumbnailView.setElement(this.$('.thumbnail-col')).render(); + } return this; }, diff --git a/cms/static/js/views/previous_video_upload_list.js b/cms/static/js/views/previous_video_upload_list.js index c842a2a3c8..c09b3a0dde 100644 --- a/cms/static/js/views/previous_video_upload_list.js +++ b/cms/static/js/views/previous_video_upload_list.js @@ -9,6 +9,7 @@ define( initialize: function(options) { this.template = this.loadTemplate('previous-video-upload-list'); this.encodingsDownloadUrl = options.encodingsDownloadUrl; + this.videoImageUploadEnabled = options.videoImageSettings.video_image_upload_enabled; this.itemViews = this.collection.map(function(model) { return new PreviousVideoUploadView({ videoImageUploadURL: options.videoImageUploadURL, @@ -23,7 +24,10 @@ define( render: function() { var $el = this.$el, $tabBody; - $el.html(this.template({encodingsDownloadUrl: this.encodingsDownloadUrl})); + $el.html(this.template({ + encodingsDownloadUrl: this.encodingsDownloadUrl, + videoImageUploadEnabled: this.videoImageUploadEnabled + })); $tabBody = $el.find('.js-table-body'); _.each(this.itemViews, function(view) { $tabBody.append(view.render().$el); diff --git a/cms/templates/js/previous-video-upload-list.underscore b/cms/templates/js/previous-video-upload-list.underscore index 1661210ab8..393dd95301 100644 --- a/cms/templates/js/previous-video-upload-list.underscore +++ b/cms/templates/js/previous-video-upload-list.underscore @@ -8,7 +8,9 @@
+ <% if (videoImageUploadEnabled) { %>
<%- gettext("Thumbnail") %>
+ <% } %>
<%- gettext("Name") %>
<%- gettext("Date Added") %>
<%- gettext("Video ID") %>
diff --git a/cms/templates/js/previous-video-upload.underscore b/cms/templates/js/previous-video-upload.underscore index dd18fc2a50..077292e9a0 100644 --- a/cms/templates/js/previous-video-upload.underscore +++ b/cms/templates/js/previous-video-upload.underscore @@ -1,5 +1,7 @@
+ <% if (videoImageUploadEnabled) { %>
+ <% } %>
<%- client_video_id %>
<%- created %>
<%- edx_video_id %>