diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers.py b/cms/djangoapps/contentstore/rest_api/v1/serializers.py index f04baff1d3..a303f1c9f6 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers.py @@ -21,6 +21,7 @@ class LimitedProctoredExamSettingsSerializer(serializers.Serializer): enable_proctored_exams = serializers.BooleanField() proctoring_provider = serializers.CharField() proctoring_escalation_email = serializers.CharField(allow_blank=True) + create_zendesk_tickets = serializers.BooleanField(required=False) class ProctoredExamConfigurationSerializer(serializers.Serializer): 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 55a96ba4dc..f42518a443 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py +++ b/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py @@ -2,6 +2,8 @@ Unit tests for Contentstore views. """ +import ddt +from mock import patch from django.test.utils import override_settings from django.urls import reverse from opaque_keys.edx.keys import CourseKey @@ -116,6 +118,7 @@ class ProctoringExamSettingsGetTests(ProctoringExamSettingsTestMixin, ModuleStor assert response.data == self.get_expected_response_data(self.course, self.course_instructor) +@ddt.ddt class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleStoreTestCase, APITestCase): """ Tests for proctored exam settings POST """ @@ -267,7 +270,7 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto assert updated.enable_proctored_exams is False assert updated.proctoring_provider == 'null' - def test_403_if_instructor_request_includes_opting_out_or_zendesk(self): + def test_403_if_instructor_request_includes_opting_out(self): self.client.login(username=self.course_instructor, password=self.password) data = self.get_request_data() response = self.make_request(data=data) @@ -279,7 +282,7 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto 'proctortrack': {} }, ) - def test_200_for_instructor_request(self): + def test_200_for_instructor_request_compatibility(self): self.client.login(username=self.course_instructor, password=self.password) data = { 'proctored_exam_settings': { @@ -290,3 +293,45 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto } response = self.make_request(data=data) assert response.status_code == status.HTTP_200_OK + + @override_settings( + PROCTORING_BACKENDS={ + 'DEFAULT': 'null', + 'proctortrack': {}, + 'software_secure': {}, + }, + ) + @patch('logging.Logger.info') + @ddt.data( + ('proctortrack', False, False), + ('software_secure', True, False), + ('proctortrack', True, True), + ('software_secure', False, True), + ) + @ddt.unpack + def test_nonadmin_with_zendesk_ticket(self, proctoring_provider, create_zendesk_tickets, expect_log, logger_mock): + self.client.login(username=self.course_instructor, password=self.password) + data = { + 'proctored_exam_settings': { + 'enable_proctored_exams': True, + 'proctoring_provider': proctoring_provider, + 'proctoring_escalation_email': 'foo@bar.com', + 'create_zendesk_tickets': create_zendesk_tickets, + } + } + response = self.make_request(data=data) + assert response.status_code == status.HTTP_200_OK + if expect_log: + logger_string = ( + 'create_zendesk_tickets set to {ticket_value} but proctoring ' + 'provider is {provider} for course {course_id}. create_zendesk_tickets ' + 'should be updated for this course.'.format( + ticket_value=create_zendesk_tickets, + provider=proctoring_provider, + course_id=self.course.id + ) + ) + logger_mock.assert_any_call(logger_string) + + updated = modulestore().get_item(self.course.location) + assert updated.create_zendesk_tickets is create_zendesk_tickets diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index d99db18120..dc33590cd9 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -1549,14 +1549,7 @@ class CourseMetadataEditingTest(CourseTestCase): ) self.assertIn(field_name, test_model) - def test_create_zendesk_tickets_not_present_for_course_staff(self): - """ - Tests that create zendesk tickets field is filtered out when the user is not an edX staff member. - """ - test_model = CourseMetadata.fetch(self.fullcourse) - self.assertNotIn('create_zendesk_tickets', test_model) - - def test_validate_update_does_filter_out_create_zendesk_tickets_for_course_staff(self): + def test_validate_update_does_not_filter_out_create_zendesk_tickets_for_course_staff(self): """ Tests that create zendesk tickets field is not returned by validate_and_update_from_json method when the user is not an edX staff member. @@ -1570,9 +1563,9 @@ class CourseMetadataEditingTest(CourseTestCase): }, user=self.user ) - self.assertNotIn(field_name, test_model) + self.assertIn(field_name, test_model) - def test_update_from_json_does_filter_out_create_zendesk_tickets_for_course_staff(self): + def test_update_from_json_does_not_filter_out_create_zendesk_tickets_for_course_staff(self): """ Tests that create zendesk tickets field is not returned by update_from_json method when the user is not an edX staff member. @@ -1586,7 +1579,7 @@ class CourseMetadataEditingTest(CourseTestCase): }, user=self.user ) - self.assertNotIn(field_name, test_model) + self.assertIn(field_name, test_model) def _set_request_user_to_staff(self): """ diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 96d803f0bc..f00ca35287 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -4,22 +4,23 @@ Django module for Course Metadata class -- manages advanced settings and related from datetime import datetime +import logging import pytz -from crum import get_current_user from django.conf import settings from django.core.exceptions import ValidationError from django.utils.translation import ugettext as _ from xblock.fields import Scope from cms.djangoapps.contentstore import toggles -from common.djangoapps.student.roles import GlobalStaff 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 +LOGGER = logging.getLogger(__name__) + class CourseMetadata: ''' @@ -135,11 +136,6 @@ class CourseMetadata: if not COURSE_ENABLE_UNENROLLED_ACCESS_FLAG.is_enabled(course_key=course_key): exclude_list.append('course_visibility') - # Do not show "Create Zendesk Tickets For Suspicious Proctored Exam Attempts" in - # Studio Advanced Settings if the user is not edX staff. - if not GlobalStaff().has_user(get_current_user()): - exclude_list.append('create_zendesk_tickets') - # Do not show "Proctortrack Exam Escalation Contact" if Proctortrack is not # an available proctoring backend. if not settings.PROCTORING_BACKENDS or settings.PROCTORING_BACKENDS.get('proctortrack') is None: @@ -414,10 +410,15 @@ class CourseMetadata: else: escalation_email = descriptor.proctoring_escalation_email + if proctoring_provider_model: + proctoring_provider = proctoring_provider_model.get('value') + else: + proctoring_provider = descriptor.proctoring_provider + missing_escalation_email_msg = 'Provider \'{provider}\' requires an exam escalation contact.' - if proctoring_provider_model and proctoring_provider_model.get('value') == 'proctortrack': + if proctoring_provider_model and proctoring_provider == 'proctortrack': if not escalation_email: - message = missing_escalation_email_msg.format(provider=proctoring_provider_model.get('value')) + message = missing_escalation_email_msg.format(provider=proctoring_provider) errors.append({ 'key': 'proctoring_provider', 'message': message, @@ -426,16 +427,37 @@ class CourseMetadata: if ( escalation_email_model and not proctoring_provider_model and - descriptor.proctoring_provider == 'proctortrack' + proctoring_provider == 'proctortrack' ): if not escalation_email: - message = missing_escalation_email_msg.format(provider=descriptor.proctoring_provider) + message = missing_escalation_email_msg.format(provider=proctoring_provider) errors.append({ 'key': 'proctoring_escalation_email', 'message': message, 'model': escalation_email_model }) + # Check that Zendesk field is appropriate for the provider + zendesk_ticket_model = settings_dict.get('create_zendesk_tickets') + if zendesk_ticket_model: + create_zendesk_tickets = zendesk_ticket_model.get('value') + else: + create_zendesk_tickets = descriptor.create_zendesk_tickets + + if ( + (proctoring_provider == 'proctortrack' and create_zendesk_tickets) + or (proctoring_provider == 'software_secure' and not create_zendesk_tickets) + ): + LOGGER.info( + 'create_zendesk_tickets set to {ticket_value} but proctoring ' + 'provider is {provider} for course {course_id}. create_zendesk_tickets ' + 'should be updated for this course.'.format( + ticket_value=create_zendesk_tickets, + provider=proctoring_provider, + course_id=descriptor.id + ) + ) + return errors @staticmethod