Skip to content

Commit

Permalink
prevent errors for escalation email if proctoring is disabled
Browse files Browse the repository at this point in the history 
fix for quality

fix for python tests

fixed bug that prevented updating fields

fixed tests

fixed other tests
  • Loading branch information
@alangsto
alangsto committed Mar 29, 2021
1 parent dc581d5 commit 8c761ae
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 28 deletions.
82 changes: 80 additions & 2 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1349,8 +1349,10 @@ def test_validate_update_requires_escalation_email_for_proctortrack(self, includ
if include_blank_email:
json_data["proctoring_escalation_email"] = {"value": ""}

course = CourseFactory.create()
CourseMetadata.update_from_dict({"enable_proctored_exams": True}, course, self.user)
did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json(
self.course,
course,
json_data,
user=self.user
)
Expand Down Expand Up @@ -1390,7 +1392,11 @@ def test_validate_update_does_not_require_escalation_email_by_default(self):
)
def test_validate_update_cannot_unset_escalation_email_when_proctortrack_is_provider(self):
course = CourseFactory.create()
CourseMetadata.update_from_dict({"proctoring_provider": 'proctortrack'}, course, self.user)
CourseMetadata.update_from_dict(
{"proctoring_provider": 'proctortrack', "enable_proctored_exams": True},
course,
self.user
)
did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json(
course,
{
Expand Down Expand Up @@ -1426,6 +1432,78 @@ def test_validate_update_set_proctortrack_provider_with_valid_escalation_email(s
self.assertIn('proctoring_provider', test_model)
self.assertIn('proctoring_escalation_email', test_model)

@override_settings(
PROCTORING_BACKENDS={
'DEFAULT': 'proctortrack',
'proctortrack': {}
}
)
def test_validate_update_disable_proctoring_with_no_escalation_email(self):
course = CourseFactory.create()
CourseMetadata.update_from_dict(
{"proctoring_provider": 'proctortrack', "proctoring_escalation_email": '', "enable_proctored_exams": True},
course,
self.user
)
did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json(
course,
{
"enable_proctored_exams": {"value": False},
},
user=self.user
)
self.assertTrue(did_validate)
self.assertEqual(len(errors), 0)
self.assertIn('enable_proctored_exams', test_model)

@override_settings(
PROCTORING_BACKENDS={
'DEFAULT': 'proctortrack',
'proctortrack': {}
}
)
def test_validate_update_disable_proctoring_and_change_escalation_email(self):
did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json(
self.course,
{
"proctoring_provider": {"value": "proctortrack"},
"proctoring_escalation_email": {"value": ""},
"enable_proctored_exams": {"value": False},
},
user=self.user
)
self.assertTrue(did_validate)
self.assertEqual(len(errors), 0)
self.assertIn('proctoring_provider', test_model)
self.assertIn('proctoring_escalation_email', test_model)
self.assertIn('enable_proctored_exams', test_model)

@override_settings(
PROCTORING_BACKENDS={
'DEFAULT': 'proctortrack',
'proctortrack': {}
}
)
def test_validate_update_disabled_proctoring_and_unset_escalation_email(self):
course = CourseFactory.create()
CourseMetadata.update_from_dict(
{"proctoring_provider": 'proctortrack', "enable_proctored_exams": False},
course,
self.user
)
did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json(
course,
{
"proctoring_escalation_email": {"value": ""},
},
user=self.user
)
self.assertTrue(did_validate)
self.assertEqual(len(errors), 0)
self.assertIn('proctoring_provider', test_model)
self.assertIn('proctoring_escalation_email', test_model)
self.assertIn('enable_proctored_exams', test_model)

def test_create_zendesk_tickets_present_for_edx_staff(self):
"""
Tests that create zendesk tickets field is not filtered out when the user is an edX staff member.
Expand Down
2 changes: 2 additions & 0 deletions cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ def test_exam_settings_alert_with_exam_settings_enabled(self, page_handler):
# create an error by setting proctortrack as the provider and not setting
# the (required) escalation contact
self.course.proctoring_provider = 'proctortrack'
self.course.enable_proctored_exams = True
self.save_course()

url = reverse_course_url(page_handler, self.course.id)
Expand Down Expand Up @@ -131,6 +132,7 @@ def test_exam_settings_alert_with_exam_settings_disabled(self, page_handler):
# create an error by setting proctortrack as the provider and not setting
# the (required) escalation contact
self.course.proctoring_provider = 'proctortrack'
self.course.enable_proctored_exams = True
self.save_course()

url = reverse_course_url(page_handler, self.course.id)
Expand Down
59 changes: 33 additions & 26 deletions cms/djangoapps/models/settings/course_metadata.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,34 +399,41 @@ def validate_proctoring_settings(cls, descriptor, settings_dict, user):
).format(support_email=settings.PARTNER_SUPPORT_EMAIL or 'support')
errors.append({'key': 'proctoring_provider', 'message': message, 'model': proctoring_provider_model})

# Require a valid escalation email if Proctortrack is chosen as the proctoring provider
escalation_email_model = settings_dict.get('proctoring_escalation_email')
if escalation_email_model:
escalation_email = escalation_email_model.get('value')
enable_proctoring_model = settings_dict.get('enable_proctored_exams')
if enable_proctoring_model:
enable_proctoring = enable_proctoring_model.get('value')
else:
escalation_email = descriptor.proctoring_escalation_email

missing_escalation_email_msg = 'Provider \'{provider}\' requires an exam escalation contact.'
if proctoring_provider_model and proctoring_provider_model.get('value') == 'proctortrack':
if not escalation_email:
message = missing_escalation_email_msg.format(provider=proctoring_provider_model.get('value'))
errors.append({
'key': 'proctoring_provider',
'message': message,
'model': proctoring_provider_model
})
enable_proctoring = descriptor.enable_proctored_exams

if (
escalation_email_model and not proctoring_provider_model and
descriptor.proctoring_provider == 'proctortrack'
):
if not escalation_email:
message = missing_escalation_email_msg.format(provider=descriptor.proctoring_provider)
errors.append({
'key': 'proctoring_escalation_email',
'message': message,
'model': escalation_email_model
})
if enable_proctoring:
# Require a valid escalation email if Proctortrack is chosen as the proctoring provider
escalation_email_model = settings_dict.get('proctoring_escalation_email')
if escalation_email_model:
escalation_email = escalation_email_model.get('value')
else:
escalation_email = descriptor.proctoring_escalation_email

missing_escalation_email_msg = 'Provider \'{provider}\' requires an exam escalation contact.'
if proctoring_provider_model and proctoring_provider_model.get('value') == 'proctortrack':
if not escalation_email:
message = missing_escalation_email_msg.format(provider=proctoring_provider_model.get('value'))
errors.append({
'key': 'proctoring_provider',
'message': message,
'model': proctoring_provider_model
})

if (
escalation_email_model and not proctoring_provider_model and
descriptor.proctoring_provider == 'proctortrack'
):
if not escalation_email:
message = missing_escalation_email_msg.format(provider=descriptor.proctoring_provider)
errors.append({
'key': 'proctoring_escalation_email',
'message': message,
'model': escalation_email_model
})

return errors

Expand Down

0 comments on commit 8c761ae

Please sign in to comment.