diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index c49c4c5a3c..f91b188d32 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -2,12 +2,14 @@ """ Unit tests for video-related REST APIs. """ +from datetime import datetime import csv import ddt import json import dateutil.parser import re from StringIO import StringIO +import pytz from django.conf import settings from django.test.utils import override_settings @@ -16,7 +18,7 @@ from mock import Mock, patch from edxval.api import create_profile, create_video, get_video_info from contentstore.models import VideoUploadConfig -from contentstore.views.videos import KEY_EXPIRATION_IN_SECONDS, StatusDisplayStrings +from contentstore.views.videos import KEY_EXPIRATION_IN_SECONDS, StatusDisplayStrings, convert_video_status from contentstore.tests.utils import CourseTestCase from contentstore.utils import reverse_course_url from xmodule.modulestore.tests.factories import CourseFactory @@ -49,6 +51,7 @@ class VideoUploadTestMixin(object): # course ids for videos course_ids = [unicode(self.course.id), unicode(self.course2.id)] + created = datetime.now(pytz.utc) self.profiles = ["profile1", "profile2"] self.previous_uploads = [ @@ -59,6 +62,7 @@ class VideoUploadTestMixin(object): "status": "upload", "courses": course_ids, "encoded_videos": [], + "created": created }, { "edx_video_id": "test2", @@ -66,6 +70,7 @@ class VideoUploadTestMixin(object): "duration": 128.0, "status": "file_complete", "courses": course_ids, + "created": created, "encoded_videos": [ { "profile": "profile1", @@ -87,6 +92,7 @@ class VideoUploadTestMixin(object): "duration": 256.0, "status": "transcode_active", "courses": course_ids, + "created": created, "encoded_videos": [ { "profile": "profile1", @@ -105,6 +111,7 @@ class VideoUploadTestMixin(object): "duration": 3.14, "status": status, "courses": course_ids, + "created": created, "encoded_videos": [], } for status in ( @@ -184,7 +191,7 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): self.assertEqual(response_video[field], original_video[field]) self.assertEqual( response_video["status"], - StatusDisplayStrings.get(original_video["status"]) + convert_video_status(original_video) ) def test_get_html(self): @@ -442,6 +449,67 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): self._assert_video_removal(self.url, edx_video_id, 1) self._assert_video_removal(self.get_url_for_course_key(self.course2.id), edx_video_id, 0) + def test_convert_video_status(self): + """ + Verifies that convert_video_status works as expected. + """ + video = self.previous_uploads[0] + + # video status should be failed if it's in upload state for more than 24 hours + video['created'] = datetime(2016, 1, 1, 10, 10, 10, 0, pytz.UTC) + status = convert_video_status(video) + self.assertEqual(status, StatusDisplayStrings.get('upload_failed')) + + # `invalid_token` should be converted to `youtube_duplicate` + video['created'] = datetime.now(pytz.UTC) + video['status'] = 'invalid_token' + status = convert_video_status(video) + self.assertEqual(status, StatusDisplayStrings.get('youtube_duplicate')) + + # for all other status, there should not be any conversion + statuses = StatusDisplayStrings._STATUS_MAP.keys() # pylint: disable=protected-access + statuses.remove('invalid_token') + for status in statuses: + video['status'] = status + new_status = convert_video_status(video) + self.assertEqual(new_status, StatusDisplayStrings.get(status)) + + def assert_video_status(self, url, edx_video_id, status): + """ + Verifies that video with `edx_video_id` has `status` + """ + response = self.client.get_json(url) + self.assertEqual(response.status_code, 200) + videos = json.loads(response.content)["videos"] + for video in videos: + if video['edx_video_id'] == edx_video_id: + return self.assertEqual(video['status'], status) + + # Test should fail if video not found + self.assertEqual(True, False, 'Invalid edx_video_id') + + def test_video_status_update_request(self): + """ + Verifies that video status update request works as expected. + """ + url = self.get_url_for_course_key(self.course.id) + edx_video_id = 'test1' + + self.assert_video_status(url, edx_video_id, 'Uploading') + + response = self.client.post( + url, + json.dumps([{ + 'edxVideoId': edx_video_id, + 'status': 'upload_failed', + 'message': 'server down' + }]), + content_type="application/json" + ) + self.assertEqual(response.status_code, 204) + + self.assert_video_status(url, edx_video_id, 'Failed') + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_VIDEO_UPLOAD_PIPELINE": True}) @override_settings(VIDEO_UPLOAD_PIPELINE={"BUCKET": "test_bucket", "ROOT_PATH": "test_root"}) @@ -486,7 +554,7 @@ class VideoUrlsCsvTestCase(VideoUploadTestMixin, CourseTestCase): self.assertEqual(response_video["Duration"], str(original_video["duration"])) dateutil.parser.parse(response_video["Date Added"]) self.assertEqual(response_video["Video ID"], original_video["edx_video_id"]) - self.assertEqual(response_video["Status"], StatusDisplayStrings.get(original_video["status"])) + self.assertEqual(response_video["Status"], convert_video_status(original_video)) for profile in expected_profiles: response_profile_url = response_video["{} URL".format(profile)] original_encoded_for_profile = next( diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index df268c9796..74441197cf 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -1,6 +1,9 @@ """ Views related to the video upload feature """ +from datetime import datetime, timedelta +import logging + from boto import s3 import csv from uuid import uuid4 @@ -12,7 +15,14 @@ from django.utils.translation import ugettext as _, ugettext_noop from django.views.decorators.http import require_GET, require_http_methods import rfc6266 -from edxval.api import create_video, get_videos_for_course, SortDirection, VideoSortField, remove_video_for_course +from edxval.api import ( + create_video, + get_videos_for_course, + SortDirection, + VideoSortField, + remove_video_for_course, + update_video_status +) from opaque_keys.edx.keys import CourseKey from contentstore.models import VideoUploadConfig @@ -25,6 +35,8 @@ from .course import get_course_and_check_access __all__ = ["videos_handler", "video_encodings_download"] +LOGGER = logging.getLogger(__name__) + # Default expiration, in seconds, of one-time URLs used for uploading videos. KEY_EXPIRATION_IN_SECONDS = 86400 @@ -36,6 +48,9 @@ VIDEO_SUPPORTED_FILE_FORMATS = { VIDEO_UPLOAD_MAX_FILE_SIZE_GB = 5 +# maximum time for video to remain in upload state +MAX_UPLOAD_HOURS = 24 + class StatusDisplayStrings(object): """ @@ -49,11 +64,17 @@ class StatusDisplayStrings(object): _IN_PROGRESS = ugettext_noop("In Progress") # Translators: This is the status for a video that the servers have successfully processed _COMPLETE = ugettext_noop("Ready") + # Translators: This is the status for a video that is uploaded completely + _UPLOAD_COMPLETED = ugettext_noop("Uploaded") # Translators: This is the status for a video that the servers have failed to process _FAILED = ugettext_noop("Failed") + # Translators: This is the status for a video that is cancelled during upload by user + _CANCELLED = ugettext_noop("Cancelled") # Translators: This is the status for a video which has failed # due to being flagged as a duplicate by an external or internal CMS _DUPLICATE = ugettext_noop("Failed Duplicate") + # Translators: This is the status for a video which has duplicate token for youtube + _YOUTUBE_DUPLICATE = ugettext_noop("YouTube Duplicate") # Translators: This is the status for a video for which an invalid # processing token was provided in the course settings _INVALID_TOKEN = ugettext_noop("Invalid Token") @@ -69,9 +90,14 @@ class StatusDisplayStrings(object): "transcode_active": _IN_PROGRESS, "file_delivered": _COMPLETE, "file_complete": _COMPLETE, + "upload_completed": _UPLOAD_COMPLETED, "file_corrupt": _FAILED, "pipeline_error": _FAILED, + "upload_failed": _FAILED, + "s3_upload_failed": _FAILED, + "upload_cancelled": _CANCELLED, "duplicate": _DUPLICATE, + "youtube_duplicate": _YOUTUBE_DUPLICATE, "invalid_token": _INVALID_TOKEN, "imported": _IMPORTED, } @@ -115,6 +141,9 @@ def videos_handler(request, course_key_string, edx_video_id=None): remove_video_for_course(course_key_string, edx_video_id) return JsonResponse() else: + if is_status_update_request(request.json): + return send_video_status_update(request.json) + return videos_post(course, request) @@ -226,6 +255,36 @@ def _get_and_validate_course(course_key_string, user): return None +def convert_video_status(video): + """ + Convert status of a video. Status can be converted to one of the following: + + * FAILED if video is in `upload` state for more than 24 hours + * `YouTube Duplicate` if status is `invalid_token` + * user-friendly video status + """ + now = datetime.now(video['created'].tzinfo) + if video['status'] == 'upload' and (now - video['created']) > timedelta(hours=MAX_UPLOAD_HOURS): + new_status = 'upload_failed' + status = StatusDisplayStrings.get(new_status) + message = 'Video with id [%s] is still in upload after [%s] hours, setting status to [%s]' % ( + video['edx_video_id'], MAX_UPLOAD_HOURS, new_status + ) + send_video_status_update([ + { + 'edxVideoId': video['edx_video_id'], + 'status': new_status, + 'message': message + } + ]) + elif video['status'] == 'invalid_token': + status = StatusDisplayStrings.get('youtube_duplicate') + else: + status = StatusDisplayStrings.get(video['status']) + + return status + + def _get_videos(course): """ Retrieves the list of videos from VAL corresponding to this course. @@ -234,7 +293,7 @@ def _get_videos(course): # convert VAL's status to studio's Video Upload feature status. for video in videos: - video["status"] = StatusDisplayStrings.get(video["status"]) + video["status"] = convert_video_status(video) return videos @@ -386,3 +445,21 @@ def storage_service_key(bucket, file_name): file_name ) return s3.key.Key(bucket, key_name) + + +def send_video_status_update(updates): + """ + Update video status in edx-val. + """ + for update in updates: + update_video_status(update.get('edxVideoId'), update.get('status')) + LOGGER.info(update.get('message')) + + return JsonResponse() + + +def is_status_update_request(request_data): + """ + Returns True if `request_data` contains status update else False. + """ + return any('status' in update for update in request_data) diff --git a/cms/static/cms/js/spec/main.js b/cms/static/cms/js/spec/main.js index c8e1223780..8eee091914 100644 --- a/cms/static/cms/js/spec/main.js +++ b/cms/static/cms/js/spec/main.js @@ -4,6 +4,22 @@ (function(requirejs, requireSerial) { 'use strict'; + if (window) { + define('add-a11y-deps', + [ + 'underscore', + 'underscore.string', + 'edx-ui-toolkit/js/utils/html-utils', + 'edx-ui-toolkit/js/utils/string-utils' + ], function(_, str, HtmlUtils, StringUtils) { + window._ = _; + window._.str = str; + window.edx = window.edx || {}; + window.edx.HtmlUtils = HtmlUtils; + window.edx.StringUtils = StringUtils; + }); + } + var i, specHelpers, testFiles; requirejs.config({ @@ -169,6 +185,10 @@ return window.MathJax.Hub.Configured(); } }, + 'accessibility': { + exports: 'accessibility', + deps: ['add-a11y-deps'] + }, 'URI': { exports: 'URI' }, diff --git a/cms/static/js/models/active_video_upload.js b/cms/static/js/models/active_video_upload.js index 7252729c3f..ad0b4b56ee 100644 --- a/cms/static/js/models/active_video_upload.js +++ b/cms/static/js/models/active_video_upload.js @@ -21,7 +21,13 @@ define( defaults: { videoId: null, status: statusStrings.STATUS_QUEUED, - progress: 0 + progress: 0, + failureMessage: null + }, + + uploading: function() { + var status = this.get('status'); + return (this.get('progress') < 1) && ((status === statusStrings.STATUS_UPLOADING)); } }, statusStrings diff --git a/cms/static/js/spec/views/active_video_upload_list_spec.js b/cms/static/js/spec/views/active_video_upload_list_spec.js index cfb0dec156..35cfe5076c 100644 --- a/cms/static/js/spec/views/active_video_upload_list_spec.js +++ b/cms/static/js/spec/views/active_video_upload_list_spec.js @@ -4,18 +4,39 @@ define( 'jquery', 'js/models/active_video_upload', 'js/views/active_video_upload_list', + 'edx-ui-toolkit/js/utils/string-utils', 'common/js/spec_helpers/template_helpers', + 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', + 'accessibility', 'mock-ajax' ], - function($, ActiveVideoUpload, ActiveVideoUploadListView, TemplateHelpers) { + function($, ActiveVideoUpload, ActiveVideoUploadListView, StringUtils, TemplateHelpers, AjaxHelpers) { 'use strict'; - var concurrentUploadLimit = 2; + var concurrentUploadLimit = 2, + POST_URL = '/test/post/url', + VIDEO_ID = 'video101', + UPLOAD_STATUS = { + s3Fail: 's3_upload_failed', + fail: 'upload_failed', + success: 'upload_completed' + }, + makeUploadUrl, + getSentRequests, + verifyUploadViewInfo, + getStatusUpdateRequest, + verifyStatusUpdateRequest, + sendUploadPostResponse, + verifyA11YMessage, + verifyUploadPostRequest; describe('ActiveVideoUploadListView', function() { beforeEach(function() { - TemplateHelpers.installTemplate('active-video-upload', true); + setFixtures( + '
' + ); + TemplateHelpers.installTemplate('active-video-upload'); TemplateHelpers.installTemplate('active-video-upload-list'); - this.postUrl = '/test/post/url'; + this.postUrl = POST_URL; this.uploadButton = $('