From fcaf3e6329f464e4a5f82168b65ee4e3b289eead Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 5 Aug 2013 23:08:35 -0400 Subject: [PATCH 1/6] give some debug message regarding why export might fail --- cms/djangoapps/contentstore/views/assets.py | 31 +++++++++++++++++---- cms/templates/export.html | 19 +++++++++++++ common/lib/xmodule/xmodule/exceptions.py | 10 +++++++ common/lib/xmodule/xmodule/raw_module.py | 5 ++-- 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 1c22114d76ba..252f36e3924f 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -3,6 +3,7 @@ import os import tarfile import shutil +import cgi from tempfile import mkdtemp from path import path @@ -27,7 +28,7 @@ from xmodule.contentstore.content import StaticContent from xmodule.util.date_utils import get_default_time_display from xmodule.modulestore import InvalidLocationError -from xmodule.exceptions import NotFoundError +from xmodule.exceptions import NotFoundError, SerializationError from .access import get_location_and_verify_access from util.json_request import JsonResponse @@ -336,16 +337,34 @@ def generate_export_course(request, org, course, name): the course """ location = get_location_and_verify_access(request, org, course, name) - + course_module = modulestore().get_item(location) loc = Location(location) export_file = NamedTemporaryFile(prefix=name + '.', suffix=".tar.gz") root_dir = path(mkdtemp()) - # export out to a tempdir - logging.debug('root = {0}'.format(root_dir)) - - export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name, modulestore()) + try: + export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name, modulestore()) + except SerializationError, e: + failed_item = modulestore().get_item(e.location) + parent_locs = modulestore().get_parent_locations(failed_item.location, None) + if len(parent_locs) > 0: + parent = modulestore().get_item(parent_locs[0]) + parent_info = "Parent Display Name: {0}
Parent Identifier: {1}".format(parent.display_name, parent.location.name) + else: + parent_info = '' + return render_to_response('export.html', { + 'context_course': course_module, + 'successful_import_redirect_url': '', + 'err_msg': "A courseware module has failed to convert to XML. Details:
Module Type: {0}
Display Name: {1}
Identifier: {2}
{3}". + format(failed_item.location.category, failed_item.display_name, failed_item.location.name, parent_info) + }) + except Exception, e: + return render_to_response('export.html', { + 'context_course': course_module, + 'successful_import_redirect_url': '', + 'err_msg': str(e) + }) logging.debug('tar file being generated at {0}'.format(export_file.name)) tar_file = tarfile.open(name=export_file.name, mode='w:gz') diff --git a/cms/templates/export.html b/cms/templates/export.html index 593cf3dd6ea4..0cc67a7b5ac3 100644 --- a/cms/templates/export.html +++ b/cms/templates/export.html @@ -6,6 +6,24 @@ <%block name="title">${_("Course Export")} <%block name="bodyclass">is-signedin course tools export +<%block name="jsextra"> + % if err_msg: + + %endif + + <%block name="content">
@@ -18,6 +36,7 @@

+

${_("About Exporting Courses")}

diff --git a/common/lib/xmodule/xmodule/exceptions.py b/common/lib/xmodule/xmodule/exceptions.py index d0a8e76557ed..48c083cbf11e 100644 --- a/common/lib/xmodule/xmodule/exceptions.py +++ b/common/lib/xmodule/xmodule/exceptions.py @@ -13,6 +13,7 @@ class ProcessingError(Exception): ''' pass + class InvalidVersionError(Exception): """ Tried to save an item with a location that a store cannot support (e.g., draft version @@ -21,3 +22,12 @@ class InvalidVersionError(Exception): def __init__(self, location): super(InvalidVersionError, self).__init__() self.location = location + + +class SerializationError(Exception): + """ + Thrown when a module cannot be exported to XML + """ + def __init__(self, location, msg): + super(SerializationError, self).__init__(msg) + self.location = location diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index b972d7c8eb3a..4c6ddb5b5124 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -4,6 +4,7 @@ import logging import sys from xblock.core import String, Scope +from exceptions import SerializationError log = logging.getLogger(__name__) @@ -27,11 +28,11 @@ def definition_to_xml(self, resource_fs): # re-raise lines = self.data.split('\n') line, offset = err.position - msg = ("Unable to create xml for problem {loc}. " + msg = ("Unable to create xml for module {loc}. " "Context: '{context}'".format( context=lines[line - 1][offset - 40:offset + 40], loc=self.location)) - raise Exception, msg, sys.exc_info()[2] + raise SerializationError(self.location, msg) class EmptyDataRawDescriptor(XmlDescriptor, XMLEditingDescriptor): From 69c34a65b14727375d6f75c1d7ef190dc69d695c Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 7 Aug 2013 22:09:08 -0400 Subject: [PATCH 2/6] switch the notification to be a prompt and allow for the user to go to the edit unit page which contains the module in error. Otherwise, present the raw exception message and allow user to go to the course outline page. --- cms/djangoapps/contentstore/views/assets.py | 34 ++++++++++--- cms/templates/export.html | 56 +++++++++++++++++---- 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 252f36e3924f..c2da87d0aa78 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -346,24 +346,42 @@ def generate_export_course(request, org, course, name): try: export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name, modulestore()) except SerializationError, e: - failed_item = modulestore().get_item(e.location) - parent_locs = modulestore().get_parent_locations(failed_item.location, None) + failed_item = modulestore().get_instance(course_module.location.course_id, e.location) + parent_locs = modulestore().get_parent_locations(failed_item.location, course_module.location.course_id) + unit = None if len(parent_locs) > 0: parent = modulestore().get_item(parent_locs[0]) - parent_info = "Parent Display Name: {0}
Parent Identifier: {1}".format(parent.display_name, parent.location.name) - else: - parent_info = '' + if parent.location.category == 'vertical': + unit = parent + return render_to_response('export.html', { 'context_course': course_module, 'successful_import_redirect_url': '', - 'err_msg': "A courseware module has failed to convert to XML. Details:
Module Type: {0}
Display Name: {1}
Identifier: {2}
{3}". - format(failed_item.location.category, failed_item.display_name, failed_item.location.name, parent_info) + 'in_err': True, + 'raw_err_msg': str(e), + 'failed_module': failed_item, + 'unit': unit, + 'edit_unit_url': reverse('edit_unit', kwargs={ + 'location': parent.location + }), + 'course_home_url': reverse('course_index', kwargs={ + 'org': org, + 'course': course, + 'name': name + }) }) except Exception, e: return render_to_response('export.html', { 'context_course': course_module, 'successful_import_redirect_url': '', - 'err_msg': str(e) + 'in_err': True, + 'unit': None, + 'raw_err_msg': str(e), + 'course_home_url': reverse('course_index', kwargs={ + 'org': org, + 'course': course, + 'name': name + }) }) logging.debug('tar file being generated at {0}'.format(export_file.name)) diff --git a/cms/templates/export.html b/cms/templates/export.html index 0cc67a7b5ac3..3b5af8bf87e6 100644 --- a/cms/templates/export.html +++ b/cms/templates/export.html @@ -7,17 +7,55 @@ <%block name="bodyclass">is-signedin course tools export <%block name="jsextra"> - % if err_msg: + % if in_err: From 40545441264a26deb007110ab99ba55034a7cdf0 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 7 Aug 2013 22:26:00 -0400 Subject: [PATCH 3/6] handle exceptions inside the outer exception handling --- cms/djangoapps/contentstore/views/assets.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index c2da87d0aa78..bbaea74ce5a4 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -346,13 +346,20 @@ def generate_export_course(request, org, course, name): try: export_to_xml(modulestore('direct'), contentstore(), loc, root_dir, name, modulestore()) except SerializationError, e: - failed_item = modulestore().get_instance(course_module.location.course_id, e.location) - parent_locs = modulestore().get_parent_locations(failed_item.location, course_module.location.course_id) unit = None - if len(parent_locs) > 0: - parent = modulestore().get_item(parent_locs[0]) - if parent.location.category == 'vertical': - unit = parent + failed_item = None + parent = None + try: + failed_item = modulestore().get_instance(course_module.location.course_id, e.location) + parent_locs = modulestore().get_parent_locations(failed_item.location, course_module.location.course_id) + + if len(parent_locs) > 0: + parent = modulestore().get_item(parent_locs[0]) + if parent.location.category == 'vertical': + unit = parent + except: + # if we have a nested exception, then we'll show the more generic error message + pass return render_to_response('export.html', { 'context_course': course_module, @@ -363,7 +370,7 @@ def generate_export_course(request, org, course, name): 'unit': unit, 'edit_unit_url': reverse('edit_unit', kwargs={ 'location': parent.location - }), + }) if parent else '', 'course_home_url': reverse('course_index', kwargs={ 'org': org, 'course': course, From 487ae964e4e011027adf9c1d79bbbdb2f95e2ded Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 8 Aug 2013 15:58:23 -0400 Subject: [PATCH 4/6] implement PR feedback --- cms/templates/export.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cms/templates/export.html b/cms/templates/export.html index 3b5af8bf87e6..dd494a532b8c 100644 --- a/cms/templates/export.html +++ b/cms/templates/export.html @@ -12,7 +12,7 @@ $(document).ready(function() { %if unit: - dialog = new CMS.Views.Prompt({ + var dialog = new CMS.Views.Prompt({ title: gettext('There has been an error with your export.'), message: gettext("There has been a failure to export to XML at least one component. It is recommended that you go to the edit page and repair the error before attempting another export. Please check that all components on the page are valid and do not display any error messages."), intent: "error", @@ -25,7 +25,7 @@ } }, secondary: { - text: gettext('cancel'), + text: gettext('Cancel'), click: function(view) { view.hide(); } @@ -33,9 +33,9 @@ } }); % else: - var msg = gettext("

There has been a failure to export your course to XML. Unfortunately, we do not have specific enough information to assist you in identifying the failed component. It is recommended that you inspect your courseware to identify any components in error and try again.

The raw error message is:

"); + var msg = "

" + gettext("There has been a failure to export your course to XML. Unfortunately, we do not have specific enough information to assist you in identifying the failed component. It is recommended that you inspect your courseware to identify any components in error and try again.") + "

" + gettext("The raw error message is:") + "

"; msg = msg + "${raw_err_msg}"; - dialog = new CMS.Views.Prompt({ + var dialog = new CMS.Views.Prompt({ title: gettext('There has been an error with your export.'), message: msg, intent: "error", @@ -48,7 +48,7 @@ } }, secondary: { - text: gettext('cancel'), + text: gettext('Cancel'), click: function(view) { view.hide(); } From 32d92d97e63b16ebb42a330ffe204270a52c89f1 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 8 Aug 2013 20:36:39 -0400 Subject: [PATCH 5/6] get_item -> get_instance --- cms/djangoapps/contentstore/views/assets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index bbaea74ce5a4..2334c61b4c8d 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -337,7 +337,7 @@ def generate_export_course(request, org, course, name): the course """ location = get_location_and_verify_access(request, org, course, name) - course_module = modulestore().get_item(location) + course_module = modulestore().get_instance(location.course_id, location) loc = Location(location) export_file = NamedTemporaryFile(prefix=name + '.', suffix=".tar.gz") From 5b4c15a57d1ffdbba0b5aea6184822971c950aca Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Fri, 9 Aug 2013 10:01:23 -0400 Subject: [PATCH 6/6] Studio: revises export prompt control copy --- cms/templates/export.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cms/templates/export.html b/cms/templates/export.html index dd494a532b8c..3356bea42b68 100644 --- a/cms/templates/export.html +++ b/cms/templates/export.html @@ -13,19 +13,19 @@ %if unit: var dialog = new CMS.Views.Prompt({ - title: gettext('There has been an error with your export.'), + title: gettext('There has been an error while exporting.'), message: gettext("There has been a failure to export to XML at least one component. It is recommended that you go to the edit page and repair the error before attempting another export. Please check that all components on the page are valid and do not display any error messages."), intent: "error", actions: { primary: { - text: gettext('Yes, allow me to fix the failed component'), + text: gettext('Correct failed component'), click: function(view) { view.hide(); document.location = "${edit_unit_url}" } }, secondary: { - text: gettext('Cancel'), + text: gettext('Return to Export'), click: function(view) { view.hide(); } @@ -54,7 +54,7 @@ } } } - }); + }); %endif dialog.show(); })