Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions cms/djangoapps/contentstore/tests/test_course_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -1423,30 +1423,6 @@ 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={

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it's nice to remove no longer used unit tests, but I believe you should add unit test for the added warning messages on course settings.

'DEFAULT': 'test_proctoring_provider',
'proctortrack': {}
}
)
def test_validate_update_escalation_email_not_requirement_disabled(self):
"""
Tests the escalation email is not required if 'ENABLED_EXAM_SETTINGS_HTML_VIEW'
setting is not set to True
"""
json_data = {
"proctoring_provider": {"value": 'proctortrack'},
}
did_validate, errors, test_model = CourseMetadata.validate_and_update_from_json(
self.course,
json_data,
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)

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
11 changes: 10 additions & 1 deletion cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,10 @@ def course_index(request, course_key):

course_authoring_microfrontend_url = get_proctored_exam_settings_url(course_module)

# gather any errors in the currently stored proctoring settings.
advanced_dict = CourseMetadata.fetch(course_module)
proctoring_errors = CourseMetadata.validate_proctoring_settings(course_module, advanced_dict, request.user)

return render_to_response('course_outline.html', {
'language_code': request.LANGUAGE_CODE,
'context_course': course_module,
Expand All @@ -684,6 +688,8 @@ def course_index(request, course_key):
) if current_action else None,
'frontend_app_publisher_url': frontend_app_publisher_url,
'course_authoring_microfrontend_url': course_authoring_microfrontend_url,
'advance_settings_url': reverse_course_url('advanced_settings_handler', course_module.id),
'proctoring_errors': proctoring_errors,
})


Expand Down Expand Up @@ -1341,13 +1347,16 @@ def advanced_settings_handler(request, course_key_string):

course_authoring_microfrontend_url = get_proctored_exam_settings_url(course_module)

# gather any errors in the currently stored proctoring settings.
proctoring_errors = CourseMetadata.validate_proctoring_settings(course_module, advanced_dict, request.user)

return render_to_response('settings_advanced.html', {
'context_course': course_module,
'advanced_dict': advanced_dict,
'advanced_settings_url': reverse_course_url('advanced_settings_handler', course_key),
'publisher_enabled': publisher_enabled,
'course_authoring_microfrontend_url': course_authoring_microfrontend_url,

'proctoring_errors': proctoring_errors,
})
elif 'application/json' in request.META.get('HTTP_ACCEPT', ''):
if request.method == 'GET':
Expand Down
95 changes: 95 additions & 0 deletions cms/djangoapps/contentstore/views/tests/test_exam_settings_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"""

import ddt
import lxml

from django.conf import settings
from django.test.utils import override_settings
Expand Down Expand Up @@ -35,6 +36,15 @@ def setUp(self):
super(TestExamSettingsView, self).setUp()
self.reset_urls()

@staticmethod
def _get_exam_settings_alert_text(raw_html_content):
""" Get text content of alert banner """
parsed_html = lxml.html.fromstring(raw_html_content)
alert_nodes = parsed_html.find_class('exam-settings-alert')
assert len(alert_nodes) == 1
alert_node = alert_nodes[0]
return alert_node.text_content()

@override_settings(FEATURES=FEATURES_WITH_EXAM_SETTINGS_DISABLED)
@ddt.data(
"certificates_list_handler",
Expand Down Expand Up @@ -70,3 +80,88 @@ def test_view_with_exam_settings_enabled(self, handler):
resp = self.client.get(outline_url, HTTP_ACCEPT='text/html')
self.assertEqual(resp.status_code, 200)
self.assertContains(resp, 'Proctored Exam Settings')

@override_settings(
PROCTORING_BACKENDS={
'DEFAULT': 'test_proctoring_provider',
'proctortrack': {}
},
FEATURES=FEATURES_WITH_EXAM_SETTINGS_ENABLED,
)
@ddt.data(
"advanced_settings_handler",
"course_handler",
)
def test_exam_settings_alert_with_exam_settings_enabled(self, page_handler):
"""
An alert should appear if current exam settings are invalid.
The exam settings alert should direct users to the exam settings page.
"""
# create an error by setting proctortrack as the provider and not setting
# the (required) escalation contact
self.course.proctoring_provider = 'proctortrack'
self.save_course()

url = reverse_course_url(page_handler, self.course.id)
resp = self.client.get(url, HTTP_ACCEPT='text/html')
alert_text = self._get_exam_settings_alert_text(resp.content)
assert (
'This course has proctored exam settings that are incomplete or invalid.'
in alert_text
)
assert (
'To update these settings go to the Proctored Exam Settings page.'
in alert_text
)

@override_settings(
PROCTORING_BACKENDS={
'DEFAULT': 'test_proctoring_provider',
'proctortrack': {}
},
FEATURES=FEATURES_WITH_EXAM_SETTINGS_DISABLED,
)
@ddt.data(
"advanced_settings_handler",
"course_handler",
)
def test_exam_settings_alert_with_exam_settings_disabled(self, page_handler):
"""
An alert should appear if current exam settings are invalid.
The exam settings alert should direct users to the advanced settings page
if the exam settings feature is not available.
"""
# create an error by setting proctortrack as the provider and not setting
# the (required) escalation contact
self.course.proctoring_provider = 'proctortrack'
self.save_course()

url = reverse_course_url(page_handler, self.course.id)
resp = self.client.get(url, HTTP_ACCEPT='text/html')
alert_text = self._get_exam_settings_alert_text(resp.content)
assert (
'This course has proctored exam settings that are incomplete or invalid.'
in alert_text
)
self.maxDiff = None
if page_handler == 'advanced_settings_handler':
assert (
'You will be unable to make changes until the following settings are updated on the page below.'
in alert_text
)
else:
assert 'To update these settings go to the Advanced Settings page.' in alert_text

@ddt.data(
"advanced_settings_handler",
"course_handler",
)
def test_exam_settings_alert_not_shown(self, page_handler):
"""
Alert should not be visible if no proctored exam setting error exists
"""
url = reverse_course_url(page_handler, self.course.id)
resp = self.client.get(url, HTTP_ACCEPT='text/html')
parsed_html = lxml.html.fromstring(resp.content)
alert_nodes = parsed_html.find_class('exam-settings-alert')
assert len(alert_nodes) == 0
60 changes: 29 additions & 31 deletions cms/djangoapps/models/settings/course_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def validate_and_update_from_json(cls, descriptor, jsondict, user, filter_tabs=T
did_validate = False
errors.append({'key': key, 'message': text_type(err), 'model': model})

proctoring_errors = cls._validate_proctoring_settings(descriptor, filtered_dict, user)
proctoring_errors = cls.validate_proctoring_settings(descriptor, filtered_dict, user)
if proctoring_errors:
errors = errors + proctoring_errors
did_validate = False
Expand All @@ -273,7 +273,7 @@ def update_from_dict(cls, key_values, descriptor, user, save=True):
return cls.fetch(descriptor)

@classmethod
def _validate_proctoring_settings(cls, descriptor, settings_dict, user):
def validate_proctoring_settings(cls, descriptor, settings_dict, user):
"""
Verify proctoring settings

Expand All @@ -299,35 +299,33 @@ def _validate_proctoring_settings(cls, descriptor, settings_dict, user):
errors.append({'key': 'proctoring_provider', 'message': message, 'model': proctoring_provider_model})

# Require a valid escalation email if Proctortrack is chosen as the proctoring provider
# This requirement will be disabled until release of the new exam settings view
if settings.FEATURES.get('ENABLE_EXAM_SETTINGS_HTML_VIEW'):
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
})
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
42 changes: 42 additions & 0 deletions cms/templates/course_outline.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<%!
import logging
import six
from six.moves.urllib.parse import quote

from cms.djangoapps.contentstore.config.waffle_utils import should_show_checklists_quality
from util.date_utils import get_default_time_display
Expand Down Expand Up @@ -114,6 +115,47 @@ <h2 class="title title-3 warning-heading-text">${_("This course uses features th
</div>
</div>
%endif

%if proctoring_errors:
<div class="wrapper wrapper-alert wrapper-alert-error is-shown">
<div class="alert announcement has-actions">
<span class="feedback-symbol fa fa-warning" aria-hidden="true"></span>

<div class="exam-settings-alert copy">
<h2 class="title title-3">${_("This course has proctored exam settings that are incomplete or invalid.")}</h2>
<p>
% if course_authoring_microfrontend_url:
<% url_encoded_course_id = quote(six.text_type(context_course.id).encode('utf-8'), safe='') %>
${Text(_("To update these settings go to the {link_start}Proctored Exam Settings page{link_end}.")).format(
link_start=HTML('<a href="{course_authoring_microfrontend_url}/proctored-exam-settings/{url_encoded_course_id}">').format(
course_authoring_microfrontend_url=course_authoring_microfrontend_url,
url_encoded_course_id=url_encoded_course_id,
),
link_end=HTML("</a>")
)}
% else:
${Text(_("To update these settings go to the {link_start}Advanced Settings page{link_end}.")).format(
link_start=HTML('<a href="{advance_settings_url}">').format(advance_settings_url=advance_settings_url),
link_end=HTML("</a>")
)}
% endif
</p>
<div class="errors-list">
<nav class="nav-related" aria-label="${_('Proctoring Settings Errors')}">
<ul>
% for error in proctoring_errors:
<li class="nav-item">
<h3 class="title title-4">${error.get('model', {}).get('display_name')}</h3>
<p>${error.get('message')}</p>
</li>
% endfor
</ul>
</nav>
</div>
</div>
</div>
</div>
%endif

</%block>

Expand Down
40 changes: 40 additions & 0 deletions cms/templates/settings_advanced.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,46 @@
});
</%block>

<%block name="page_alert">
%if proctoring_errors:
<div class="wrapper wrapper-alert wrapper-alert-error is-shown">
<div class="alert announcement has-actions">
<span class="feedback-symbol fa fa-warning" aria-hidden="true"></span>

<div class="exam-settings-alert copy">
<h2 class="title title-3">${_("This course has proctored exam settings that are incomplete or invalid.")}</h2>
<p>
% if course_authoring_microfrontend_url:
<% url_encoded_course_id = quote(six.text_type(context_course.id).encode('utf-8'), safe='') %>
${Text(_("You will be unable to make changes until the errors are resolved. To update these settings go to the {link_start}Proctored Exam Settings page{link_end}.")).format(
link_start=HTML('<a href="{course_authoring_microfrontend_url}/proctored-exam-settings/{url_encoded_course_id}">').format(
course_authoring_microfrontend_url=course_authoring_microfrontend_url,
url_encoded_course_id=url_encoded_course_id,
),
link_end=HTML("</a>")
)}
% else:
${_("You will be unable to make changes until the following settings are updated on the page below.")}
% endif
</p>
<div class="errors-list">
<nav class="nav-related" aria-label="${_('Proctoring Settings Errors')}">
<ul>
% for error in proctoring_errors:
<li class="nav-item">
<h3 class="title title-4">${error.get('model', {}).get('display_name')}</h3>
<p>${error.get('message')}</p>
</li>
% endfor
</ul>
</nav>
</div>
</div>
</div>
</div>
%endif
</%block>

<%block name="content">
<div class="wrapper-mast wrapper">
<header class="mast has-subtitle">
Expand Down