Skip to content

Commit

Permalink
fix: allow courses to render with invalid proctoring provider (#35430)
Browse files Browse the repository at this point in the history
  • Loading branch information
alangsto authored Sep 9, 2024
1 parent 9df4a34 commit e498d20
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 6 deletions.
10 changes: 8 additions & 2 deletions cms/djangoapps/models/settings/course_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,10 @@ def update_from_json(cls, block, jsondict, user, filter_tabs=True):
try:
val = model['value']
if hasattr(block, key) and getattr(block, key) != val:
key_values[key] = block.fields[key].from_json(val)
if key == 'proctoring_provider':
key_values[key] = block.fields[key].from_json(val, validate_providers=True)
else:
key_values[key] = block.fields[key].from_json(val)
except (TypeError, ValueError) as err:
raise ValueError(_("Incorrect format for field '{name}'. {detailed_message}").format( # lint-amnesty, pylint: disable=raise-missing-from
name=model['display_name'], detailed_message=str(err)))
Expand Down Expand Up @@ -253,7 +256,10 @@ def validate_and_update_from_json(cls, block, jsondict, user, filter_tabs=True):
try:
val = model['value']
if hasattr(block, key) and getattr(block, key) != val:
key_values[key] = block.fields[key].from_json(val)
if key == 'proctoring_provider':
key_values[key] = block.fields[key].from_json(val, validate_providers=True)
else:
key_values[key] = block.fields[key].from_json(val)
except (TypeError, ValueError, ValidationError) as err:
did_validate = False
errors.append({'key': key, 'message': str(err), 'model': model})
Expand Down
5 changes: 3 additions & 2 deletions xmodule/course_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ class ProctoringProvider(String):
and default that pulls from edx platform settings.
"""

def from_json(self, value):
def from_json(self, value, validate_providers=False):
"""
Return ProctoringProvider as full featured Python type. Perform validation on the provider
and include any inherited values from the platform default.
Expand All @@ -237,7 +237,8 @@ def from_json(self, value):
if settings.FEATURES.get('ENABLE_PROCTORED_EXAMS'):
# Only validate the provider value if ProctoredExams are enabled on the environment
# Otherwise, the passed in provider does not matter. We should always return default
self._validate_proctoring_provider(value)
if validate_providers:
self._validate_proctoring_provider(value)
value = self._get_proctoring_value(value)
return value
else:
Expand Down
17 changes: 15 additions & 2 deletions xmodule/tests/test_course_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,14 +542,27 @@ def test_from_json_with_invalid_provider(self, proctored_exams_setting_enabled):
with override_settings(FEATURES=FEATURES_WITH_PROCTORED_EXAMS):
if proctored_exams_setting_enabled:
with pytest.raises(InvalidProctoringProvider) as context_manager:
self.proctoring_provider.from_json(provider)
self.proctoring_provider.from_json(provider, validate_providers=True)
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
else:
provider_value = self.proctoring_provider.from_json(provider)
provider_value = self.proctoring_provider.from_json(provider, validate_providers=True)
assert provider_value == self.proctoring_provider.default

def test_from_json_validate_providers(self):
"""
Test that an invalid provider is ignored if validate providers is set to false
"""
provider = 'invalid-provider'

FEATURES_WITH_PROCTORED_EXAMS = settings.FEATURES.copy()
FEATURES_WITH_PROCTORED_EXAMS['ENABLE_PROCTORED_EXAMS'] = True

with override_settings(FEATURES=FEATURES_WITH_PROCTORED_EXAMS):
provider_value = self.proctoring_provider.from_json(provider, validate_providers=False)
assert provider_value == provider

def test_from_json_adds_platform_default_for_missing_provider(self):
"""
Test that a value with no provider will inherit the default provider
Expand Down

0 comments on commit e498d20

Please sign in to comment.