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 1c22114d76..252f36e392 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -3,6 +3,7 @@ import json import os import tarfile import shutil +import cgi from tempfile import mkdtemp from path import path @@ -27,7 +28,7 @@ from xmodule.modulestore import Location 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 593cf3dd6e..0cc67a7b5a 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 d0a8e76557..48c083cbf1 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 b972d7c8eb..4c6ddb5b51 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -4,6 +4,7 @@ from xmodule.xml_module import XmlDescriptor import logging import sys from xblock.core import String, Scope +from exceptions import SerializationError log = logging.getLogger(__name__) @@ -27,11 +28,11 @@ class RawDescriptor(XmlDescriptor, XMLEditingDescriptor): # 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 252f36e392..c2da87d0aa 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 0cc67a7b5a..3b5af8bf87 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 c2da87d0aa..bbaea74ce5 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 3b5af8bf87..dd494a532b 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 bbaea74ce5..2334c61b4c 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 dd494a532b..3356bea42b 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(); })