Moved Import error messages to single place (#27331)

This commit is contained in:
Ahtisham Shahid
2021-04-15 12:25:04 +05:00
committed by GitHub
parent 70ab50181a
commit 58231c4262
3 changed files with 38 additions and 19 deletions

View File

@@ -0,0 +1,19 @@
"""
User facing error messages during course import and export
"""
from django.utils.translation import ugettext as _
COURSE_ALREADY_EXIST = _('Aborting import because a course with this id: {} already exists.')
COURSE_PERMISSION_DENIED = _('Permission denied. You do not have write access to this course.')
ERROR_WHILE_READING = _('Error while reading {}. Check file for XML errors.')
FAILED_TO_IMPORT_MODULE = _('Failed to import module: {} at location: {}')
FILE_MISSING = _('Could not find the {0} file in the package.')
FILE_NOT_FOUND = _('Uploaded Tar file not found. Try again.')
INVALID_FILE_TYPE = _('We only support uploading a .tar.gz file.')
LIBRARY_ALREADY_EXIST = _('Aborting import since a library with this id already exists.')
PERMISSION_DENIED = _('Permission denied')
UNKNOWN_ERROR_IN_IMPORT = _('Unknown error while importing course.')
UNKNOWN_ERROR_IN_UNPACKING = _('An Unknown error occurred during the unpacking step.')
UNKNOWN_USER_ID = _('Unknown User ID: {0}')
UNSAFE_TAR_FILE = _('Unsafe tar file. Aborting import.')
USER_PERMISSION_DENIED = _('User permission denied.')

View File

@@ -22,7 +22,6 @@ from django.core.exceptions import SuspiciousOperation
from django.core.files import File
from django.test import RequestFactory
from django.utils.text import get_valid_filename
from django.utils.translation import ugettext as _
from edx_django_utils.monitoring import (
set_code_owner_attribute,
set_code_owner_attribute_from_module,
@@ -62,7 +61,7 @@ from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import DuplicateCourseError, ItemNotFoundError, InvalidProctoringProvider
from xmodule.modulestore.xml_exporter import export_course_to_xml, export_library_to_xml
from xmodule.modulestore.xml_importer import import_course_from_xml, import_library_from_xml
import cms.djangoapps.contentstore.errors as UserErrors
from .outlines import update_outline_from_modulestore
from .utils import course_import_olx_validation_is_enabled
@@ -276,11 +275,11 @@ def export_olx(self, user_id, course_key_string, language):
user = User.objects.get(pk=user_id)
except User.DoesNotExist:
with translation_language(language):
self.status.fail(_('Unknown User ID: {0}').format(user_id))
self.status.fail(UserErrors.UNKNOWN_USER_ID.format(user_id))
return
if not has_course_author_access(user, courselike_key):
with translation_language(language):
self.status.fail(_('Permission denied'))
self.status.fail(UserErrors.PERMISSION_DENIED)
return
if isinstance(courselike_key, LibraryLocator):
@@ -421,7 +420,7 @@ def import_olx(self, user_id, course_key_string, archive_path, archive_name, lan
return User.objects.get(pk=user_id)
except User.DoesNotExist as exc:
with translation_language(language):
self.status.fail(_('User permission denied.'))
self.status.fail(UserErrors.USER_PERMISSION_DENIED)
LOGGER.error(f'{log_prefix}: Unknown User: {user_id}')
monitor_import_failure(courselike_key, current_step, exception=exc)
return
@@ -432,7 +431,7 @@ def import_olx(self, user_id, course_key_string, archive_path, archive_name, lan
if not has_access:
message = f'User permission denied: {user.username}'
with translation_language(language):
self.status.fail(_('Permission denied. You do not have write access to this course.'))
self.status.fail(UserErrors.COURSE_PERMISSION_DENIED)
LOGGER.error(f'{log_prefix}: {message}')
monitor_import_failure(courselike_key, current_step, message=message)
return has_access
@@ -444,7 +443,7 @@ def import_olx(self, user_id, course_key_string, archive_path, archive_name, lan
if not file_is_valid:
message = f'Unsupported file {archive_name}'
with translation_language(language):
self.status.fail(_('We only support uploading a .tar.gz file.'))
self.status.fail(UserErrors.INVALID_FILE_TYPE)
LOGGER.error(f'{log_prefix}: {message}')
monitor_import_failure(courselike_key, current_step, message=message)
return file_is_valid
@@ -456,7 +455,7 @@ def import_olx(self, user_id, course_key_string, archive_path, archive_name, lan
if not archive_path_exists:
message = f'Uploaded file {archive_path} not found'
with translation_language(language):
self.status.fail(_('Uploaded Tar file not found. Try again.'))
self.status.fail(UserErrors.FILE_NOT_FOUND)
LOGGER.error(f'{log_prefix}: {message}')
monitor_import_failure(courselike_key, current_step, message=message)
return archive_path_exists
@@ -486,9 +485,9 @@ def import_olx(self, user_id, course_key_string, archive_path, archive_name, lan
dirpath = get_dir_for_filename(course_dir, root_name)
if not dirpath:
message = f'Could not find the {root_name} file in the package.'
message = UserErrors.FILE_MISSING.format(root_name)
with translation_language(language):
self.status.fail(_('Could not find the {0} file in the package.').format(root_name))
self.status.fail(message)
LOGGER.error(f'{log_prefix}: {message}')
monitor_import_failure(courselike_key, current_step, message=message)
return
@@ -561,7 +560,7 @@ def import_olx(self, user_id, course_key_string, archive_path, archive_name, lan
shutil.rmtree(course_dir)
LOGGER.info(f'{log_prefix}: Temp data cleared')
self.status.fail(_('An Unknown error occurred during the unpacking step.'))
self.status.fail(UserErrors.UNKNOWN_ERROR_IN_UNPACKING)
LOGGER.exception(f'{log_prefix}: Unknown error while unpacking', exc_info=True)
monitor_import_failure(courselike_key, current_step, exception=exception)
return
@@ -573,7 +572,7 @@ def import_olx(self, user_id, course_key_string, archive_path, archive_name, lan
safetar_extractall(tar_file, (course_dir + '/'))
except SuspiciousOperation as exc:
with translation_language(language):
self.status.fail(_('Unsafe tar file. Aborting import.'))
self.status.fail(UserErrors.UNSAFE_TAR_FILE)
LOGGER.error(f'{log_prefix}: Unsafe tar file')
monitor_import_failure(courselike_key, current_step, exception=exc)
return
@@ -616,7 +615,7 @@ def import_olx(self, user_id, course_key_string, archive_path, archive_name, lan
set_custom_attribute('course_import_completed', True)
except Exception as exception: # pylint: disable=broad-except
msg = str(exception)
status_msg = _('Unknown error while importing course.')
status_msg = UserErrors.UNKNOWN_ERROR_IN_IMPORT
if isinstance(exception, InvalidProctoringProvider):
status_msg = msg
LOGGER.exception(f'{log_prefix}: Unknown error while importing course {str(exception)}')

View File

@@ -30,7 +30,6 @@ import re
from abc import abstractmethod
import xblock
from django.utils.translation import ugettext as _
from lxml import etree
from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import LibraryLocator
@@ -39,6 +38,8 @@ from xblock.core import XBlockMixin
from xblock.fields import Reference, ReferenceList, ReferenceValueDict, Scope
from xblock.runtime import DictKeyValueStore, KvsFieldData
import cms.djangoapps.contentstore.errors as UserErrors
from common.djangoapps.util.monitoring import monitor_import_failure
from xmodule.assetstore import AssetMetadata
from xmodule.contentstore.content import StaticContent
@@ -366,7 +367,7 @@ class ImportManager:
logging.exception(f'Course import {course_id}: Error while parsing {assets_filename}.')
if self.raise_on_failure: # lint-amnesty, pylint: disable=no-else-raise
if self.status:
self.status.fail(_('Error while reading {}. Check file for XML errors.').format(assets_filename))
self.status.fail(UserErrors.ERROR_WHILE_READING).format(assets_filename)
raise
else:
return
@@ -486,7 +487,7 @@ class ImportManager:
)
if self.status:
self.status.fail(
_('Failed to import module: {} at location: {}').format(
UserErrors.FAILED_TO_IMPORT_MODULE.format(
child.display_name, child.location
)
)
@@ -515,7 +516,7 @@ class ImportManager:
log.error(msg)
if self.status:
self.status.fail(
_('Failed to import module: {} at location: {}').format(
UserErrors.FAILED_TO_IMPORT_MODULE.format(
leftover.display_name, leftover.location
)
)
@@ -607,7 +608,7 @@ class CourseImportManager(ImportManager):
)
if self.status:
self.status.fail(
_('Aborting import because a course with this id: {} already exists.').format(dest_id)
UserErrors.COURSE_ALREADY_EXIST.format(dest_id)
)
raise
@@ -719,7 +720,7 @@ class LibraryImportManager(ImportManager):
"since it collides with an existing one", dest_id
)
if self.status:
self.status.fail(_('Aborting import since a library with this id already exists.'))
self.status.fail(UserErrors.LIBRARY_ALREADY_EXIST)
raise
return dest_id, runtime