Commit ab7f4140 by muhammad-ammar

Update error handling for Video Uploads

Tnl-4777
parent 74a0d769
...@@ -2,12 +2,14 @@ ...@@ -2,12 +2,14 @@
""" """
Unit tests for video-related REST APIs. Unit tests for video-related REST APIs.
""" """
from datetime import datetime
import csv import csv
import ddt import ddt
import json import json
import dateutil.parser import dateutil.parser
import re import re
from StringIO import StringIO from StringIO import StringIO
import pytz
from django.conf import settings from django.conf import settings
from django.test.utils import override_settings from django.test.utils import override_settings
...@@ -16,7 +18,7 @@ from mock import Mock, patch ...@@ -16,7 +18,7 @@ from mock import Mock, patch
from edxval.api import create_profile, create_video, get_video_info from edxval.api import create_profile, create_video, get_video_info
from contentstore.models import VideoUploadConfig 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.tests.utils import CourseTestCase
from contentstore.utils import reverse_course_url from contentstore.utils import reverse_course_url
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
...@@ -49,6 +51,7 @@ class VideoUploadTestMixin(object): ...@@ -49,6 +51,7 @@ class VideoUploadTestMixin(object):
# course ids for videos # course ids for videos
course_ids = [unicode(self.course.id), unicode(self.course2.id)] course_ids = [unicode(self.course.id), unicode(self.course2.id)]
created = datetime.now(pytz.utc)
self.profiles = ["profile1", "profile2"] self.profiles = ["profile1", "profile2"]
self.previous_uploads = [ self.previous_uploads = [
...@@ -59,6 +62,7 @@ class VideoUploadTestMixin(object): ...@@ -59,6 +62,7 @@ class VideoUploadTestMixin(object):
"status": "upload", "status": "upload",
"courses": course_ids, "courses": course_ids,
"encoded_videos": [], "encoded_videos": [],
"created": created
}, },
{ {
"edx_video_id": "test2", "edx_video_id": "test2",
...@@ -66,6 +70,7 @@ class VideoUploadTestMixin(object): ...@@ -66,6 +70,7 @@ class VideoUploadTestMixin(object):
"duration": 128.0, "duration": 128.0,
"status": "file_complete", "status": "file_complete",
"courses": course_ids, "courses": course_ids,
"created": created,
"encoded_videos": [ "encoded_videos": [
{ {
"profile": "profile1", "profile": "profile1",
...@@ -87,6 +92,7 @@ class VideoUploadTestMixin(object): ...@@ -87,6 +92,7 @@ class VideoUploadTestMixin(object):
"duration": 256.0, "duration": 256.0,
"status": "transcode_active", "status": "transcode_active",
"courses": course_ids, "courses": course_ids,
"created": created,
"encoded_videos": [ "encoded_videos": [
{ {
"profile": "profile1", "profile": "profile1",
...@@ -105,6 +111,7 @@ class VideoUploadTestMixin(object): ...@@ -105,6 +111,7 @@ class VideoUploadTestMixin(object):
"duration": 3.14, "duration": 3.14,
"status": status, "status": status,
"courses": course_ids, "courses": course_ids,
"created": created,
"encoded_videos": [], "encoded_videos": [],
} }
for status in ( for status in (
...@@ -184,7 +191,7 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): ...@@ -184,7 +191,7 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase):
self.assertEqual(response_video[field], original_video[field]) self.assertEqual(response_video[field], original_video[field])
self.assertEqual( self.assertEqual(
response_video["status"], response_video["status"],
StatusDisplayStrings.get(original_video["status"]) convert_video_status(original_video)
) )
def test_get_html(self): def test_get_html(self):
...@@ -442,6 +449,67 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase): ...@@ -442,6 +449,67 @@ class VideosHandlerTestCase(VideoUploadTestMixin, CourseTestCase):
self._assert_video_removal(self.url, edx_video_id, 1) 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) 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}) @patch.dict("django.conf.settings.FEATURES", {"ENABLE_VIDEO_UPLOAD_PIPELINE": True})
@override_settings(VIDEO_UPLOAD_PIPELINE={"BUCKET": "test_bucket", "ROOT_PATH": "test_root"}) @override_settings(VIDEO_UPLOAD_PIPELINE={"BUCKET": "test_bucket", "ROOT_PATH": "test_root"})
...@@ -486,7 +554,7 @@ class VideoUrlsCsvTestCase(VideoUploadTestMixin, CourseTestCase): ...@@ -486,7 +554,7 @@ class VideoUrlsCsvTestCase(VideoUploadTestMixin, CourseTestCase):
self.assertEqual(response_video["Duration"], str(original_video["duration"])) self.assertEqual(response_video["Duration"], str(original_video["duration"]))
dateutil.parser.parse(response_video["Date Added"]) dateutil.parser.parse(response_video["Date Added"])
self.assertEqual(response_video["Video ID"], original_video["edx_video_id"]) 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: for profile in expected_profiles:
response_profile_url = response_video["{} URL".format(profile)] response_profile_url = response_video["{} URL".format(profile)]
original_encoded_for_profile = next( original_encoded_for_profile = next(
......
""" """
Views related to the video upload feature Views related to the video upload feature
""" """
from datetime import datetime, timedelta
import logging
from boto import s3 from boto import s3
import csv import csv
from uuid import uuid4 from uuid import uuid4
...@@ -12,7 +15,14 @@ from django.utils.translation import ugettext as _, ugettext_noop ...@@ -12,7 +15,14 @@ from django.utils.translation import ugettext as _, ugettext_noop
from django.views.decorators.http import require_GET, require_http_methods from django.views.decorators.http import require_GET, require_http_methods
import rfc6266 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 opaque_keys.edx.keys import CourseKey
from contentstore.models import VideoUploadConfig from contentstore.models import VideoUploadConfig
...@@ -25,6 +35,8 @@ from .course import get_course_and_check_access ...@@ -25,6 +35,8 @@ from .course import get_course_and_check_access
__all__ = ["videos_handler", "video_encodings_download"] __all__ = ["videos_handler", "video_encodings_download"]
LOGGER = logging.getLogger(__name__)
# Default expiration, in seconds, of one-time URLs used for uploading videos. # Default expiration, in seconds, of one-time URLs used for uploading videos.
KEY_EXPIRATION_IN_SECONDS = 86400 KEY_EXPIRATION_IN_SECONDS = 86400
...@@ -36,6 +48,9 @@ VIDEO_SUPPORTED_FILE_FORMATS = { ...@@ -36,6 +48,9 @@ VIDEO_SUPPORTED_FILE_FORMATS = {
VIDEO_UPLOAD_MAX_FILE_SIZE_GB = 5 VIDEO_UPLOAD_MAX_FILE_SIZE_GB = 5
# maximum time for video to remain in upload state
MAX_UPLOAD_HOURS = 24
class StatusDisplayStrings(object): class StatusDisplayStrings(object):
""" """
...@@ -49,11 +64,17 @@ class StatusDisplayStrings(object): ...@@ -49,11 +64,17 @@ class StatusDisplayStrings(object):
_IN_PROGRESS = ugettext_noop("In Progress") _IN_PROGRESS = ugettext_noop("In Progress")
# Translators: This is the status for a video that the servers have successfully processed # Translators: This is the status for a video that the servers have successfully processed
_COMPLETE = ugettext_noop("Ready") _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 # Translators: This is the status for a video that the servers have failed to process
_FAILED = ugettext_noop("Failed") _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 # Translators: This is the status for a video which has failed
# due to being flagged as a duplicate by an external or internal CMS # due to being flagged as a duplicate by an external or internal CMS
_DUPLICATE = ugettext_noop("Failed Duplicate") _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 # Translators: This is the status for a video for which an invalid
# processing token was provided in the course settings # processing token was provided in the course settings
_INVALID_TOKEN = ugettext_noop("Invalid Token") _INVALID_TOKEN = ugettext_noop("Invalid Token")
...@@ -69,9 +90,14 @@ class StatusDisplayStrings(object): ...@@ -69,9 +90,14 @@ class StatusDisplayStrings(object):
"transcode_active": _IN_PROGRESS, "transcode_active": _IN_PROGRESS,
"file_delivered": _COMPLETE, "file_delivered": _COMPLETE,
"file_complete": _COMPLETE, "file_complete": _COMPLETE,
"upload_completed": _UPLOAD_COMPLETED,
"file_corrupt": _FAILED, "file_corrupt": _FAILED,
"pipeline_error": _FAILED, "pipeline_error": _FAILED,
"upload_failed": _FAILED,
"s3_upload_failed": _FAILED,
"upload_cancelled": _CANCELLED,
"duplicate": _DUPLICATE, "duplicate": _DUPLICATE,
"youtube_duplicate": _YOUTUBE_DUPLICATE,
"invalid_token": _INVALID_TOKEN, "invalid_token": _INVALID_TOKEN,
"imported": _IMPORTED, "imported": _IMPORTED,
} }
...@@ -115,6 +141,9 @@ def videos_handler(request, course_key_string, edx_video_id=None): ...@@ -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) remove_video_for_course(course_key_string, edx_video_id)
return JsonResponse() return JsonResponse()
else: else:
if is_status_update_request(request.json):
return send_video_status_update(request.json)
return videos_post(course, request) return videos_post(course, request)
...@@ -226,6 +255,36 @@ def _get_and_validate_course(course_key_string, user): ...@@ -226,6 +255,36 @@ def _get_and_validate_course(course_key_string, user):
return None 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): def _get_videos(course):
""" """
Retrieves the list of videos from VAL corresponding to this course. Retrieves the list of videos from VAL corresponding to this course.
...@@ -234,7 +293,7 @@ def _get_videos(course): ...@@ -234,7 +293,7 @@ def _get_videos(course):
# convert VAL's status to studio's Video Upload feature status. # convert VAL's status to studio's Video Upload feature status.
for video in videos: for video in videos:
video["status"] = StatusDisplayStrings.get(video["status"]) video["status"] = convert_video_status(video)
return videos return videos
...@@ -386,3 +445,21 @@ def storage_service_key(bucket, file_name): ...@@ -386,3 +445,21 @@ def storage_service_key(bucket, file_name):
file_name file_name
) )
return s3.key.Key(bucket, key_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)
...@@ -4,6 +4,22 @@ ...@@ -4,6 +4,22 @@
(function(requirejs, requireSerial) { (function(requirejs, requireSerial) {
'use strict'; '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; var i, specHelpers, testFiles;
requirejs.config({ requirejs.config({
...@@ -169,6 +185,10 @@ ...@@ -169,6 +185,10 @@
return window.MathJax.Hub.Configured(); return window.MathJax.Hub.Configured();
} }
}, },
'accessibility': {
exports: 'accessibility',
deps: ['add-a11y-deps']
},
'URI': { 'URI': {
exports: 'URI' exports: 'URI'
}, },
......
...@@ -21,7 +21,13 @@ define( ...@@ -21,7 +21,13 @@ define(
defaults: { defaults: {
videoId: null, videoId: null,
status: statusStrings.STATUS_QUEUED, 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 statusStrings
......
define( define(
['js/models/active_video_upload', 'js/views/baseview'], ['underscore', 'js/models/active_video_upload', 'js/views/baseview', 'common/js/components/views/feedback_prompt'],
function(ActiveVideoUpload, BaseView) { function(_, ActiveVideoUpload, BaseView, PromptView) {
'use strict'; 'use strict';
var STATUS_CLASSES = [ var STATUS_CLASSES = [
...@@ -13,15 +13,20 @@ define( ...@@ -13,15 +13,20 @@ define(
tagName: 'li', tagName: 'li',
className: 'active-video-upload', className: 'active-video-upload',
events: {
'click a.more-details-action': 'showUploadFailureMessage'
},
initialize: function() { initialize: function() {
this.template = this.loadTemplate('active-video-upload'); this.template = this.loadTemplate('active-video-upload');
this.listenTo(this.model, 'change', this.render); this.listenTo(this.model, 'change', this.render);
}, },
render: function() { render: function() {
var $el = this.$el; var $el = this.$el,
status;
$el.html(this.template(this.model.attributes)); $el.html(this.template(this.model.attributes));
var status = this.model.get('status'); status = this.model.get('status');
_.each( _.each(
STATUS_CLASSES, STATUS_CLASSES,
function(statusClass) { function(statusClass) {
...@@ -29,6 +34,21 @@ define( ...@@ -29,6 +34,21 @@ define(
} }
); );
return this; return this;
},
showUploadFailureMessage: function() {
return new PromptView.Warning({
title: gettext('Your file could not be uploaded'),
message: this.model.get('failureMessage'),
actions: {
primary: {
text: gettext('Close'),
click: function(prompt) {
return prompt.hide();
}
}
}
}).show();
} }
}); });
......
...@@ -65,11 +65,15 @@ ...@@ -65,11 +65,15 @@
font-size: 90%; font-size: 90%;
} }
.video-detail-status { .video-detail-status, .more-details-action {
@include font-size(12); @include font-size(12);
@include line-height(12); @include line-height(12);
} }
.more-details-action, .upload-failure {
display: none;
}
.video-detail-progress { .video-detail-progress {
-webkit-appearance: none; -webkit-appearance: none;
-moz-appearance: none; -moz-appearance: none;
...@@ -105,7 +109,7 @@ ...@@ -105,7 +109,7 @@
} }
&.error { &.error {
.video-detail-status { .video-upload-status {
color: $color-error; color: $color-error;
} }
...@@ -117,10 +121,20 @@ ...@@ -117,10 +121,20 @@
.video-detail-progress::-moz-progress-bar { .video-detail-progress::-moz-progress-bar {
background-color: $color-error; background-color: $color-error;
} }
.more-details-action, .upload-failure {
display: inline-block;
color: $color-error;
}
.more-details-action {
margin-top: ($baseline/5);
float: right;
}
} }
&.success { &.success {
.video-detail-status { .video-upload-status {
color: $color-ready; color: $color-ready;
} }
} }
......
<h4 class="video-detail-name"><%- fileName %></h4> <h4 class="video-detail-name"><%- fileName %></h4>
<progress class="video-detail-progress" value="<%= progress %>"></progress> <progress class="video-detail-progress" value="<%= progress %>"></progress>
<p class="video-detail-status"><%- gettext(status) %></p> <div class="video-upload-status">
<span class="icon alert-icon fa fa-warning upload-failure" aria-hidden="true"></span>
<span class="video-detail-status"><%- gettext(status) %></span>
<% if (failureMessage) { %>
<a href="#" class="more-details-action">
<%- gettext("Read More") %>
<span class="sr"><%- gettext("details about the failure") %></span>
</a>
<% } %>
</div>
...@@ -80,7 +80,7 @@ git+https://github.com/edx/edx-ora2.git@1.1.13#egg=ora2==1.1.13 ...@@ -80,7 +80,7 @@ git+https://github.com/edx/edx-ora2.git@1.1.13#egg=ora2==1.1.13
-e git+https://github.com/edx/edx-submissions.git@1.1.4#egg=edx-submissions==1.1.4 -e git+https://github.com/edx/edx-submissions.git@1.1.4#egg=edx-submissions==1.1.4
git+https://github.com/edx/ease.git@release-2015-07-14#egg=ease==0.1.3 git+https://github.com/edx/ease.git@release-2015-07-14#egg=ease==0.1.3
git+https://github.com/edx/i18n-tools.git@v0.3.2#egg=i18n-tools==v0.3.2 git+https://github.com/edx/i18n-tools.git@v0.3.2#egg=i18n-tools==v0.3.2
git+https://github.com/edx/edx-val.git@0.0.11#egg=edxval==0.0.11 git+https://github.com/edx/edx-val.git@0.0.12#egg=edxval==0.0.12
git+https://github.com/pmitros/RecommenderXBlock.git@v1.1#egg=recommender-xblock==1.1 git+https://github.com/pmitros/RecommenderXBlock.git@v1.1#egg=recommender-xblock==1.1
git+https://github.com/solashirai/crowdsourcehinter.git@518605f0a95190949fe77bd39158450639e2e1dc#egg=crowdsourcehinter-xblock==0.1 git+https://github.com/solashirai/crowdsourcehinter.git@518605f0a95190949fe77bd39158450639e2e1dc#egg=crowdsourcehinter-xblock==0.1
-e git+https://github.com/pmitros/RateXBlock.git@367e19c0f6eac8a5f002fd0f1559555f8e74bfff#egg=rate-xblock -e git+https://github.com/pmitros/RateXBlock.git@367e19c0f6eac8a5f002fd0f1559555f8e74bfff#egg=rate-xblock
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment