Skip to content

E
edx-platform
Commit 31980150 by jkarni
Options

Merge pull request #1043 from edx/jkarni/feature/import-feedback 

Jkarni/feature/import feedback
parents faf796b5 bb009249
Showing with 486 additions and 101 deletions
CHANGELOG.rst
...@@ -20,6 +20,8 @@ registration. ...@@ -20,6 +20,8 @@ registration.
Studio: Switched to loading Javascript using require.js Studio: Switched to loading Javascript using require.js
Studio: Better feedback during the course import process
LMS: Add split testing functionality for internal use. LMS: Add split testing functionality for internal use.
CMS: Add edit_course_tabs management command, providing a primitive CMS: Add edit_course_tabs management command, providing a primitive
......
cms/djangoapps/contentstore/tests/test_import_export.py
...@@ -7,6 +7,8 @@ import tarfile ...@@ -7,6 +7,8 @@ import tarfile
import tempfile import tempfile
import copy import copy
from path import path from path import path
import json
import logging
from uuid import uuid4 from uuid import uuid4
from pymongo import MongoClient from pymongo import MongoClient
...@@ -20,6 +22,7 @@ from xmodule.contentstore.django import _CONTENTSTORE ...@@ -20,6 +22,7 @@ from xmodule.contentstore.django import _CONTENTSTORE
TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex
log = logging.getLogger(__name__)
@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE)
class ImportTestCase(CourseTestCase): class ImportTestCase(CourseTestCase):
...@@ -84,6 +87,17 @@ class ImportTestCase(CourseTestCase): ...@@ -84,6 +87,17 @@ class ImportTestCase(CourseTestCase):
"course-data": [btar] "course-data": [btar]
}) })
self.assertEquals(resp.status_code, 415) self.assertEquals(resp.status_code, 415)
# Check that `import_status` returns the appropriate stage (i.e., the
# stage at which import failed).
status_url = reverse("import_status", kwargs={
'org': self.course.location.org,
'course': self.course.location.course,
'name': os.path.split(self.bad_tar)[1],
})
resp_status = self.client.get(status_url)
log.debug(str(self.client.session["import_status"]))
self.assertEquals(json.loads(resp_status.content)["ImportStatus"], 2)
def test_with_coursexml(self): def test_with_coursexml(self):
""" """
...@@ -183,3 +197,14 @@ class ImportTestCase(CourseTestCase): ...@@ -183,3 +197,14 @@ class ImportTestCase(CourseTestCase):
try_tar(self._symlink_tar()) try_tar(self._symlink_tar())
try_tar(self._outside_tar()) try_tar(self._outside_tar())
try_tar(self._outside_tar2()) try_tar(self._outside_tar2())
# Check that `import_status` returns the appropriate stage (i.e.,
# either 3, indicating all previous steps are completed, or 0,
# indicating no upload in progress)
status_url = reverse("import_status", kwargs={
'org': self.course.location.org,
'course': self.course.location.course,
'name': os.path.split(self.good_tar)[1],
})
resp_status = self.client.get(status_url)
import_status = json.loads(resp_status.content)["ImportStatus"]
self.assertIn(import_status, (0, 3))
cms/djangoapps/contentstore/views/import_export.py
...@@ -9,7 +9,6 @@ import shutil ...@@ -9,7 +9,6 @@ import shutil
import re import re
from tempfile import mkdtemp from tempfile import mkdtemp
from path import path from path import path
from contextlib import contextmanager
from django.conf import settings from django.conf import settings
from django.http import HttpResponse from django.http import HttpResponse
...@@ -18,8 +17,9 @@ from django_future.csrf import ensure_csrf_cookie ...@@ -18,8 +17,9 @@ from django_future.csrf import ensure_csrf_cookie
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.core.servers.basehttp import FileWrapper from django.core.servers.basehttp import FileWrapper
from django.core.files.temp import NamedTemporaryFile from django.core.files.temp import NamedTemporaryFile
from django.views.decorators.http import require_http_methods
from django.core.exceptions import SuspiciousOperation from django.core.exceptions import SuspiciousOperation
from django.views.decorators.http import require_http_methods, require_GET
from django.utils.translation import ugettext as _
from mitxmako.shortcuts import render_to_response from mitxmako.shortcuts import render_to_response
from auth.authz import create_all_course_groups from auth.authz import create_all_course_groups
...@@ -36,7 +36,7 @@ from util.json_request import JsonResponse ...@@ -36,7 +36,7 @@ from util.json_request import JsonResponse
from extract_tar import safetar_extractall from extract_tar import safetar_extractall
__all__ = ['import_course', 'generate_export_course', 'export_course'] __all__ = ['import_course', 'import_status', 'generate_export_course', 'export_course']
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -55,20 +55,6 @@ def import_course(request, org, course, name): ...@@ -55,20 +55,6 @@ def import_course(request, org, course, name):
""" """
location = get_location_and_verify_access(request, org, course, name) location = get_location_and_verify_access(request, org, course, name)
@contextmanager
def wfile(filename, dirname):
"""
A with-context that creates `filename` on entry and removes it on exit.
`filename` is truncted on creation. Additionally removes dirname on
exit.
"""
open(filename, "w").close()
try:
yield filename
finally:
os.remove(filename)
shutil.rmtree(dirname)
if request.method == 'POST': if request.method == 'POST':
data_root = path(settings.GITHUB_REPO_ROOT) data_root = path(settings.GITHUB_REPO_ROOT)
...@@ -78,7 +64,10 @@ def import_course(request, org, course, name): ...@@ -78,7 +64,10 @@ def import_course(request, org, course, name):
filename = request.FILES['course-data'].name filename = request.FILES['course-data'].name
if not filename.endswith('.tar.gz'): if not filename.endswith('.tar.gz'):
return JsonResponse( return JsonResponse(
{'ErrMsg': 'We only support uploading a .tar.gz file.'}, {
'ErrMsg': _('We only support uploading a .tar.gz file.'),
'Stage': 1
},
status=415 status=415
) )
temp_filepath = course_dir / filename temp_filepath = course_dir / filename
...@@ -112,7 +101,10 @@ def import_course(request, org, course, name): ...@@ -112,7 +101,10 @@ def import_course(request, org, course, name):
size size
) )
return JsonResponse( return JsonResponse(
{'ErrMsg': 'File upload corrupted. Please try again'}, {
'ErrMsg': _('File upload corrupted. Please try again'),
'Stage': 1
},
status=409 status=409
) )
# The last request sometimes comes twice. This happens because # The last request sometimes comes twice. This happens because
...@@ -145,15 +137,15 @@ def import_course(request, org, course, name): ...@@ -145,15 +137,15 @@ def import_course(request, org, course, name):
else: # This was the last chunk. else: # This was the last chunk.
# 'Lock' with status info. # Use sessions to keep info about import progress
status_file = data_root / (course + filename + ".lock") session_status = request.session.setdefault("import_status", {})
key = org + course + filename
session_status[key] = 1
request.session.modified = True
# Do everything from now on in a with-context, to be sure we've # Do everything from now on in a try-finally block to make sure
# properly cleaned up. # everything is properly cleaned up.
with wfile(status_file, course_dir): try:
with open(status_file, 'w+') as sf:
sf.write("Extracting")
tar_file = tarfile.open(temp_filepath) tar_file = tarfile.open(temp_filepath)
try: try:
...@@ -162,17 +154,18 @@ def import_course(request, org, course, name): ...@@ -162,17 +154,18 @@ def import_course(request, org, course, name):
return JsonResponse( return JsonResponse(
{ {
'ErrMsg': 'Unsafe tar file. Aborting import.', 'ErrMsg': 'Unsafe tar file. Aborting import.',
'SuspiciousFileOperationMsg': exc.args[0] 'SuspiciousFileOperationMsg': exc.args[0],
'Stage': 1
}, },
status=400 status=400
) )
finally:
tar_file.close()
with open(status_file, 'w+') as sf: session_status[key] = 2
sf.write("Verifying") request.session.modified = True
# find the 'course.xml' file # find the 'course.xml' file
dirpath = None
def get_all_files(directory): def get_all_files(directory):
""" """
For each file in the directory, yield a 2-tuple of (file-name, For each file in the directory, yield a 2-tuple of (file-name,
...@@ -199,7 +192,10 @@ def import_course(request, org, course, name): ...@@ -199,7 +192,10 @@ def import_course(request, org, course, name):
if not dirpath: if not dirpath:
return JsonResponse( return JsonResponse(
{'ErrMsg': 'Could not find the course.xml file in the package.'}, {
'ErrMsg': _('Could not find the course.xml file in the package.'),
'Stage': 2
},
status=415 status=415
) )
...@@ -221,12 +217,25 @@ def import_course(request, org, course, name): ...@@ -221,12 +217,25 @@ def import_course(request, org, course, name):
logging.debug('new course at {0}'.format(course_items[0].location)) logging.debug('new course at {0}'.format(course_items[0].location))
with open(status_file, 'w') as sf: session_status[key] = 3
sf.write("Updating course") request.session.modified = True
create_all_course_groups(request.user, course_items[0].location) create_all_course_groups(request.user, course_items[0].location)
logging.debug('created all course groups at {0}'.format(course_items[0].location)) logging.debug('created all course groups at {0}'.format(course_items[0].location))
# Send errors to client with stage at which error occured.
except Exception as exception: # pylint: disable=W0703
return JsonResponse(
{
'ErrMsg': str(exception),
'Stage': session_status[key]
},
status=400
)
finally:
shutil.rmtree(course_dir)
return JsonResponse({'Status': 'OK'}) return JsonResponse({'Status': 'OK'})
else: else:
course_module = modulestore().get_item(location) course_module = modulestore().get_item(location)
...@@ -241,6 +250,29 @@ def import_course(request, org, course, name): ...@@ -241,6 +250,29 @@ def import_course(request, org, course, name):
}) })
@require_GET
@ensure_csrf_cookie
@login_required
def import_status(request, org, course, name):
"""
Returns an integer corresponding to the status of a file import. These are:
0 : No status info found (import done or upload still in progress)
1 : Extracting file
2 : Validating.
3 : Importing to mongo
"""
try:
session_status = request.session["import_status"]
status = session_status[org + course + name]
except KeyError:
status = 0
return JsonResponse({"ImportStatus": status})
@ensure_csrf_cookie @ensure_csrf_cookie
@login_required @login_required
def generate_export_course(request, org, course, name): def generate_export_course(request, org, course, name):
......
cms/envs/common.py
...@@ -250,6 +250,25 @@ PIPELINE_CSS = { ...@@ -250,6 +250,25 @@ PIPELINE_CSS = {
# test_order: Determines the position of this chunk of javascript on # test_order: Determines the position of this chunk of javascript on
# the jasmine test page # the jasmine test page
PIPELINE_JS = { PIPELINE_JS = {
'main': {
'source_filenames': sorted(
rooted_glob(COMMON_ROOT / 'static/', 'coffee/src/**/*.js') +
rooted_glob(PROJECT_ROOT / 'static/', 'coffee/src/**/*.js')
) + ['js/hesitate.js', 'js/base.js', 'js/views/feedback.js',
'js/models/course.js',
'js/models/section.js', 'js/views/section.js',
'js/models/metadata_model.js', 'js/views/metadata_editor_view.js',
'js/models/uploads.js', 'js/views/uploads.js',
'js/models/textbook.js', 'js/views/textbook.js',
'js/src/utility.js',
'js/models/settings/course_grading_policy.js',
'js/models/asset.js', 'js/models/assets.js',
'js/views/assets.js',
'js/views/import.js',
'js/views/assets_view.js', 'js/views/asset_view.js'],
'output_filename': 'js/cms-application.js',
'test_order': 0
},
'module-js': { 'module-js': {
'source_filenames': ( 'source_filenames': (
rooted_glob(COMMON_ROOT / 'static/', 'xmodule/descriptors/js/*.js') + rooted_glob(COMMON_ROOT / 'static/', 'xmodule/descriptors/js/*.js') +
......
cms/static/js/base.js
...@@ -127,10 +127,10 @@ $(document).ready(function() { ...@@ -127,10 +127,10 @@ $(document).ready(function() {
$('.sync-date').bind('click', syncReleaseDate); $('.sync-date').bind('click', syncReleaseDate);
// import form setup // import form setup
$('.import .file-input').bind('change', showImportSubmit); $('.view-import .file-input').bind('change', showImportSubmit);
$('.import .choose-file-button, .import .choose-file-button-inline').bind('click', function(e) { $('.view-import .choose-file-button, .view-import .choose-file-button-inline').bind('click', function(e) {
e.preventDefault(); e.preventDefault();
$('.import .file-input').click(); $('.view-import .file-input').click();
}); });
$('.new-course-button').bind('click', addNewCourse); $('.new-course-button').bind('click', addNewCourse);
...@@ -227,7 +227,7 @@ function showImportSubmit(e) { ...@@ -227,7 +227,7 @@ function showImportSubmit(e) {
$('.error-block').hide(); $('.error-block').hide();
$('.file-name').html($(this).val().replace('C:\\fakepath\\', '')); $('.file-name').html($(this).val().replace('C:\\fakepath\\', ''));
$('.file-name-block').show(); $('.file-name-block').show();
$('.import .choose-file-button').hide(); $('.view-import .choose-file-button').hide();
$('.submit-button').show(); $('.submit-button').show();
$('.progress').show(); $('.progress').show();
} else { } else {
......
cms/static/js/views/import.js 0 → 100644
/**
* Course import-related js.
*/
define(
["jquery", "underscore", "gettext"],
function($, _, gettext) {
"use strict";
/********** Private functions ************************************************/
/**
* Toggle the spin on the progress cog.
* @param {boolean} isSpinning Turns cog spin on if true, off otherwise.
*/
var updateCog = function (elem, isSpinning) {
var cogI = elem.find('i.icon-cog');
if (isSpinning) { cogI.addClass("icon-spin");}
else { cogI.removeClass("icon-spin");}
};
/**
* Manipulate the DOM to reflect current status of upload.
* @param {int} stageNo Current stage.
*/
var updateStage = function (stageNo){
var all = $('ol.status-progress').children();
var prevList = all.slice(0, stageNo);
_.map(prevList, function (elem){
$(elem).
removeClass("is-not-started").
removeClass("is-started").
addClass("is-complete");
updateCog($(elem), false);
});
var curList = all.eq(stageNo);
curList.removeClass("is-not-started").addClass("is-started");
updateCog(curList, true);
};
/**
* Check for import status updates every `timemout` milliseconds, and update
* the page accordingly.
* @param {string} url Url to call for status updates.
* @param {int} timeout Number of milliseconds to wait in between ajax calls
* for new updates.
* @param {int} stage Starting stage.
*/
var getStatus = function (url, timeout, stage) {
var currentStage = stage || 0;
if (CourseImport.stopGetStatus) { return ;}
updateStage(currentStage);
if (currentStage == 3 ) { return ;}
var time = timeout || 1000;
$.getJSON(url,
function (data) {
setTimeout(function () {
getStatus(url, time, data.ImportStatus);
}, time);
}
);
};
/********** Public functions *************************************************/
var CourseImport = {
/**
* Whether to stop sending AJAX requests for updates on the import
* progress.
*/
stopGetStatus: false,
/**
* Update DOM to set all stages as not-started (for retrying an upload that
* failed).
*/
clearImportDisplay: function () {
var all = $('ol.status-progress').children();
_.map(all, function (elem){
$(elem).removeClass("is-complete").
removeClass("is-started").
removeClass("has-error").
addClass("is-not-started");
$(elem).find('p.error').remove(); // remove error messages
$(elem).find('p.copy').show();
updateCog($(elem), false);
});
this.stopGetStatus = false;
},
/**
* Update DOM to set all stages as complete, and stop asking for status
* updates.
*/
displayFinishedImport: function () {
this.stopGetStatus = true;
var all = $('ol.status-progress').children();
_.map(all, function (elem){
$(elem).
removeClass("is-not-started").
removeClass("is-started").
addClass("is-complete");
updateCog($(elem), false);
});
},
/**
* Entry point for server feedback. Makes status list visible and starts
* sending requests to the server for status updates.
* @param {string} url The url to send Ajax GET requests for updates.
*/
startServerFeedback: function (url){
this.stopGetStatus = false;
$('div.wrapper-status').removeClass('is-hidden');
$('.status-info').show();
getStatus(url, 500, 0);
},
/**
* Give error message at the list element that corresponds to the stage
* where the error occurred.
* @param {int} stageNo Stage of import process at which error occured.
* @param {string} msg Error message to display.
*/
stageError: function (stageNo, msg) {
var all = $('ol.status-progress').children();
// Make all stages up to, and including, the error stage 'complete'.
var prevList = all.slice(0, stageNo + 1);
_.map(prevList, function (elem){
$(elem).
removeClass("is-not-started").
removeClass("is-started").
addClass("is-complete");
updateCog($(elem), false);
});
var message = msg || gettext("There was an error with the upload");
var elem = $('ol.status-progress').children().eq(stageNo);
elem.removeClass('is-started').addClass('has-error');
elem.find('p.copy').hide().after("<p class='copy error'>" + message + "</p>");
}
};
return CourseImport;
});
cms/static/sass/views/_import.scss
...@@ -3,109 +3,264 @@ ...@@ -3,109 +3,264 @@
.view-import { .view-import {
.import-overview { .content-primary, .content-supplementary {
@extend %ui-window; @include box-sizing(border-box);
@include clearfix;
padding: 30px 40px;
}
.description {
float: left; float: left;
width: 62%;
margin-right: 3%;
font-size: 14px;
h2 {
font-weight: 700;
font-size: 19px;
margin-bottom: 20px;
} }
strong { .content-primary {
font-weight: 700; width: flex-grid(9,12);
margin-right: flex-gutter();
} }
p + p { .content-supplementary {
margin-top: 20px; width: flex-grid(3,12);
}
} }
// UI: import form
.import-form { .import-form {
float: left;
width: 35%;
padding: 25px 30px 35px;
@include box-sizing(border-box); @include box-sizing(border-box);
border: 1px solid $mediumGrey; @extend %ui-window;
border-radius: 3px; padding: $baseline ($baseline*1.5) ($baseline*1.5) ($baseline*1.5);
background: $lightGrey;
text-align: center;
h2 { > .title {
margin-bottom: 30px; @extend %t-title4;
font-size: 26px;
font-weight: 300;
} }
.file-name-block, .file-name-block,
.error-block { .error-block {
display: none; display: none;
margin-bottom: 15px; margin-bottom: $baseline;
font-size: 13px;
} }
.error-block { .error-block {
color: $error-red; color: $error-red;
} }
.status-block { .file-input {
display: none; display: none;
font-size: 13px;
} }
.choose-file-button {
@include blue-button;
padding: 10px 50px 11px;
font-size: 17px;
} }
.choose-file-button-inline { // ====================
// UI: default
.action-choose-file {
@extend %btn-primary-blue;
@extend %t-action1;
display: block; display: block;
margin: $baseline 0;
padding: ($baseline*0.75) $baseline;
} }
.file-input { // ====================
display: none;
// UI: elem - file selection
.wrapper-file-name {
@extend %ui-well;
margin: $baseline 0;
padding: $baseline ($baseline*1.5);
background-color: $gray-l4;
.title {
@extend %t-copy-lead1;
overflow-x: hidden;
text-overflow: ellipsis;
margin-bottom: 0;
.label {
margin-right: ($baseline/2);
} }
.file-name {
font-weight: bold;
}
}
// artifact styling needed for current page behavior logic
.submit-button { .submit-button {
@include orange-button; @extend %btn-primary-green;
@extend %t-action1;
display: none; display: none;
max-width: 100%; margin-top: ($baseline*0.75);
padding: 8px 20px 10px; width: 100%;
white-space: normal; padding: ($baseline*0.75) $baseline;
} }
} }
.progress-bar { // ====================
// UI: upload progress
.wrapper-status {
@include transition(opacity $tmg-f2 ease-in-out 0);
opacity: 1.0;
// STATE: hidden
&.is-hidden {
opacity: 0.0;
display: none; display: none;
width: 350px; }
height: 30px;
margin: 30px auto 10px;
border: 1px solid $blue;
&.loaded { > .title {
border-color: #66b93d; @extend %t-title4;
margin-bottom: $baseline;
border-bottom: 1px solid $gray-l3;
padding-bottom: ($baseline/2);
}
// elem - progress list
.list-progress {
width: flex-grid(9, 9);
.status-visual {
position: relative;
float: left;
width: flex-grid(1,9);
*[class^="icon-"] {
@include transition(opacity $tmg-f1 ease-in-out 0);
@include font-size(22);
position: absolute;
top: ($baseline/2);
left: $baseline;
}
}
.status-detail {
float: left;
width: flex-grid(8,9);
margin-left: ($baseline*3);
.title {
@extend %t-title5;
font-weight: 600;
}
.copy {
@extend %t-copy-base;
color: $gray-l2;
}
}
.item-progresspoint {
@include clearfix();
@include transition(opacity $tmg-f1 ease-in-out 0);
margin-bottom: $baseline;
border-bottom: 1px solid $gray-l4;
padding-bottom: $baseline;
&:last-child {
margin-bottom: 0;
border-bottom: none;
padding-bottom: 0;
}
// CASE: has actions
&.has-actions {
.status-detail {
width: flex-grid(5,9);
}
.list-actions {
display: none;
width: flex-grid(3,9);
float: right;
margin-left: flex-gutter();
text-align: right;
.progress-fill { .action-primary {
background: #66b93d; @extend %btn-primary-blue;
} }
} }
} }
.progress-fill { // TYPE: success
width: 0%; &.item-progresspoint-success {
height: 30px;
background: $blue; }
color: #fff;
line-height: 48px;
// STATE: not started
&.is-not-started {
opacity: 0.5;
.icon-warning-sign {
visibility: hidden;
opacity: 0.0;
}
.icon-cog {
visibility: visible;
opacity: 1.0;
}
.icon-check {
opacity: 0.3;
}
}
// STATE: started
&.is-started {
.icon-warning-sign {
visibility: hidden;
opacity: 0.0;
}
.icon-cog {
visibility: visible;
opacity: 1.0;
}
}
// STATE: completed
&.is-complete {
.icon-cog {
visibility: visible;
opacity: 1.0;
}
.icon-warning-sign {
visibility: hidden;
opacity: 0.0;
}
*[class^="icon-"] {
color: $green;
}
.status-detail .title {
color: $green;
}
.list-actions {
display: block;
}
}
// STATE: error
&.has-error {
.icon-cog {
visibility: hidden;
opacity: 0.0;
}
.icon-warning-sign {
visibility: visible;
opacity: 1.0;
}
*[class^="icon-"] {
color: $red;
}
.status-detail .title, .status-detail .copy {
color: $red;
}
}
}
}
} }
} }
cms/urls.py
...@@ -29,6 +29,8 @@ urlpatterns = ('', # nopep8 ...@@ -29,6 +29,8 @@ urlpatterns = ('', # nopep8
'contentstore.views.course_index', name='course_index'), 'contentstore.views.course_index', name='course_index'),
url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/import/(?P<name>[^/]+)$', url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/import/(?P<name>[^/]+)$',
'contentstore.views.import_course', name='import_course'), 'contentstore.views.import_course', name='import_course'),
url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/import_status/(?P<name>[^/]+)$',
'contentstore.views.import_status', name='import_status'),
url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/export/(?P<name>[^/]+)$', url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/export/(?P<name>[^/]+)$',
'contentstore.views.export_course', name='export_course'), 'contentstore.views.export_course', name='export_course'),
......
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