diff --git a/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py b/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py index c70327da1d..46d5f9d90f 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py +++ b/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py @@ -256,10 +256,8 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto assert response.status_code == status.HTTP_400_BAD_REQUEST self.assertDictEqual(response.data, { 'detail': [{ - 'proctoring_provider': ( - '[\"The selected proctoring provider, notvalidprovider, is not a valid provider. ' - 'Please select from one of [\'test_proctoring_provider\'].\"]' - ) + 'proctoring_provider': "The selected proctoring provider, notvalidprovider, is not a valid provider. " + "Please select from one of ['test_proctoring_provider']." }] }) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 1ad50c68e6..525f371440 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -58,7 +58,7 @@ from xmodule.course_module import CourseFields from xmodule.exceptions import SerializationError from xmodule.modulestore import COURSE_ROOT, LIBRARY_ROOT from xmodule.modulestore.django import modulestore -from xmodule.modulestore.exceptions import DuplicateCourseError, ItemNotFoundError +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 @@ -612,9 +612,12 @@ 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) - LOGGER.exception(f'{log_prefix}: Unknown error while importing course {msg}') + status_msg = _('Unknown error while importing course.') + if isinstance(exception, InvalidProctoringProvider): + status_msg = msg + LOGGER.exception(f'{log_prefix}: Unknown error while importing course {str(exception)}') if self.status.state != UserTaskStatus.FAILED: - self.status.fail(_('Unknown error while importing course.')) + self.status.fail(status_msg) monitor_import_failure(courselike_key, current_step, exception=exception) finally: if course_dir.isdir(): diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index bdbce74c09..96d803f0bc 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -18,6 +18,7 @@ from common.djangoapps.xblock_django.models import XBlockStudioConfigurationFlag from openedx.core.lib.teams_config import TeamsetType from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import InvalidProctoringProvider class CourseMetadata: @@ -245,7 +246,7 @@ class CourseMetadata: val = model['value'] if hasattr(descriptor, key) and getattr(descriptor, key) != val: key_values[key] = descriptor.fields[key].from_json(val) - except (TypeError, ValueError, ValidationError) as err: + except (InvalidProctoringProvider, TypeError, ValueError, ValidationError) as err: did_validate = False errors.append({'key': key, 'message': str(err), 'model': model}) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 54af7e7297..ec03d786b4 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -27,6 +27,7 @@ from xmodule.seq_module import SequenceBlock from xmodule.tabs import CourseTabList, InvalidTabsException from .fields import Date +from .modulestore.exceptions import InvalidProctoringProvider log = logging.getLogger(__name__) @@ -210,17 +211,9 @@ class ProctoringProvider(String): Return ProctoringProvider as full featured Python type. Perform validation on the provider and include any inherited values from the platform default. """ - errors = [] value = super().from_json(value) - - provider_errors = self._validate_proctoring_provider(value) - errors.extend(provider_errors) - - if errors: - raise ValueError(errors) - + self._validate_proctoring_provider(value) value = self._get_proctoring_value(value) - return value def _get_proctoring_value(self, value): @@ -240,21 +233,10 @@ class ProctoringProvider(String): specified, and it is not one of the providers configured at the platform level, return a list of error messages to the caller. """ - errors = [] - available_providers = get_available_providers() if value is not None and value not in available_providers: - errors.append( - _('The selected proctoring provider, {proctoring_provider}, is not a valid provider. ' - 'Please select from one of {available_providers}.') - .format( - proctoring_provider=value, - available_providers=available_providers - ) - ) - - return errors + raise InvalidProctoringProvider(value, available_providers) @property def default(self): diff --git a/common/lib/xmodule/xmodule/modulestore/exceptions.py b/common/lib/xmodule/xmodule/modulestore/exceptions.py index f03b416042..0f29ce6cfd 100644 --- a/common/lib/xmodule/xmodule/modulestore/exceptions.py +++ b/common/lib/xmodule/xmodule/modulestore/exceptions.py @@ -106,3 +106,21 @@ class InvalidBranchSetting(Exception): super().__init__(f"Invalid branch: expected {expected_setting} but got {actual_setting}") # lint-amnesty, pylint: disable=line-too-long, super-with-arguments self.expected_setting = expected_setting self.actual_setting = actual_setting + + +class InvalidProctoringProvider(Exception): + """ + Error with selected proctoring provider raised when the provided is unknown. + """ + + def __init__(self, proctoring_provider, available_providers): + super().__init__() + self.proctoring_provider = proctoring_provider + self.available_providers = available_providers + + def __str__(self, *args, **kwargs): + """ + Print details about error + """ + return f"The selected proctoring provider, {self.proctoring_provider}, is not a valid provider. " \ + f"Please select from one of {self.available_providers}." diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 06724aeec2..cb40f58aa4 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -381,17 +381,18 @@ class XMLModuleStore(ModuleStoreReadBase): errorlog.tracker(msg) self.errored_courses[course_dir] = errorlog monitor_import_failure(target_course_id, 'Updating', exception=exc) - - if course_descriptor is None: - pass - elif isinstance(course_descriptor, ErrorBlock): - # Didn't load course. Instead, save the errors elsewhere. - self.errored_courses[course_dir] = errorlog - else: - self.courses[course_dir] = course_descriptor - course_descriptor.parent = None - course_id = self.id_from_descriptor(course_descriptor) - self._course_errors[course_id] = errorlog + raise exc + finally: + if course_descriptor is None: + pass + elif isinstance(course_descriptor, ErrorBlock): + # Didn't load course. Instead, save the errors elsewhere. + self.errored_courses[course_dir] = errorlog + else: + self.courses[course_dir] = course_descriptor + course_descriptor.parent = None + course_id = self.id_from_descriptor(course_descriptor) + self._course_errors[course_id] = errorlog def __str__(self): ''' diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py index 54361635a4..dbe49300aa 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_module.py +++ b/common/lib/xmodule/xmodule/tests/test_course_module.py @@ -19,6 +19,7 @@ from xblock.runtime import DictKeyValueStore, KvsFieldData from openedx.core.lib.teams_config import TeamsConfig, DEFAULT_COURSE_RUN_MAX_TEAM_SIZE import xmodule.course_module from xmodule.modulestore.xml import ImportSystem, XMLModuleStore +from xmodule.modulestore.exceptions import InvalidProctoringProvider ORG = 'test_org' COURSE = 'test_course' @@ -450,11 +451,11 @@ class ProctoringProviderTestCase(unittest.TestCase): provider = 'invalid-provider' allowed_proctoring_providers = ['mock', 'mock_proctoring_without_rules'] - with pytest.raises(ValueError) as context_manager: + with pytest.raises(InvalidProctoringProvider) as context_manager: self.proctoring_provider.from_json(provider) - assert context_manager.value.args[0] ==\ - [f'The selected proctoring provider, {provider},' - f' is not a valid provider. Please select from one of {allowed_proctoring_providers}.'] + expected_error = f'The selected proctoring provider, {provider}, is not a valid provider. ' \ + f'Please select from one of {allowed_proctoring_providers}.' + assert str(context_manager.value) == expected_error def test_from_json_adds_platform_default_for_missing_provider(self): """