feat: add escalation_email to course exam configuration

This commit adds an escalation email to course exam configurations. This value is used to specify who learners can contact in the event of issues with or questions about their exam attempts.

edx_exams/apps/api/serializers.py

@@ -7,7 +7,14 @@
 from edx_exams.apps.api.constants import ASSESSMENT_CONTROL_CODES
 from edx_exams.apps.core.api import get_exam_attempt_time_remaining, get_exam_url_path
 from edx_exams.apps.core.exam_types import EXAM_TYPES
-from edx_exams.apps.core.models import AssessmentControlResult, Exam, ExamAttempt, ProctoringProvider, User
+from edx_exams.apps.core.models import (
+    AssessmentControlResult,
+    CourseExamConfiguration,
+    Exam,
+    ExamAttempt,
+    ProctoringProvider,
+    User
+)
 
 
 class UserSerializer(serializers.ModelSerializer):
@@ -40,6 +47,67 @@ class Meta:
         fields = ['name', 'verbose_name', 'lti_configuration_id']
 
 
+class CourseExamConfigurationWriteSerializer(serializers.ModelSerializer):
+    """
+    Serializer for writing to the CourseExamConfiguration model.
+
+    We have separate read and write serializers because the read serializer uses the name field of the provider instance
+    to populate the provider serializer field by using the source attribute. The write serializer, on the other hand,
+    accepts the name as the value for the provider field. If we use the same serializer, deserialized data gets
+    represented as {'provider': {'name': <provider>}}, which makes reading the data less clear.
+    """
+    provider = serializers.CharField(allow_null=True)
+    # The escalation_email is a nullable field, but we require that clients send us an escalation_email.
+    escalation_email = serializers.EmailField(allow_null=True, allow_blank=False, required=True)
+
+    class Meta:
+        model = CourseExamConfiguration
+        fields = ['provider', 'escalation_email']
+
+    def validate_provider(self, value):
+        """
+        Validate that the provider value corresponds to an existing ProctoringProvider. If the value is None,
+        the value is valid.
+        """
+        if value is not None:
+            try:
+                ProctoringProvider.objects.get(name=value)
+            # This exception is handled by the Django Rest Framework, so we don't want to use "raise from e" and risk
+            # getting the stack trace included in client facing errors.
+            except ProctoringProvider.DoesNotExist:
+                raise serializers.ValidationError('Proctoring provider does not exist.') from None
+            return value
+
+        return value
+
+    def validate(self, attrs):
+        """
+        Validate that escalalation_email is None if the provider is None.
+        """
+        if attrs.get('provider') is None and attrs.get('escalation_email') is not None:
+            raise serializers.ValidationError('Escalation email must be None when provider is None.')
+
+        return attrs
+
+
+class CourseExamConfigurationReadSerializer(serializers.ModelSerializer):
+    """
+    Serializer for reading from the CourseExamConfiguration model
+
+    We have separate read and write serializers because the read serializer uses the name field of the provider instance
+    to populate the provider serializer field by using the source attribute. The write serializer, on the other hand,
+    accepts the name as the value for the provider field. If we use the same serializer, deserialized data gets
+    represented as {'provider': {'name': <provider>}}, which makes reading the data less clear.
+    """
+    provider = serializers.CharField(source='provider.name', allow_null=True)
+    # The escalation_email is a nullable field, but we require that clients send us an escalation_email.
+    escalation_email = serializers.EmailField(allow_null=True, allow_blank=False, required=True)
+
+    class Meta:
+        model = CourseExamConfiguration
+        fields = ['provider', 'escalation_email']
+
+
 class ExamSerializer(serializers.ModelSerializer):
     """
     Serializer for the Exam Model

edx_exams/apps/api/v1/tests/test_views.py

@@ -358,23 +358,46 @@ def test_course_staff_write_access(self):
         CourseStaffRole.objects.create(user=course_staff_user, course_id=self.course_id)
         response = self.patch_api(course_staff_user, {
             'provider': None,
+            'escalation_email': None,
         })
         self.assertEqual(204, response.status_code)
 
-    def test_patch_invalid_data(self):
+    @ddt.data(
+        {},
+        {'escalation_email': '[email protected]'},
+        {'provider': 'test-provider'}
+    )
+    def test_patch_invalid_data_missing_data(self, data):
+        """
+        Assert that endpoint returns 400 if any required parameter is missing
+        """
+        response = self.patch_api(self.user, data)
+        self.assertEqual(400, response.status_code)
+
+    def test_patch_invalid_data_escalation_email(self):
         """
-        Assert that endpoint returns 400 if provider is missing
+        Assert that endpoint returns 400 if provider is None but escalation_email is not
         """
-        data = {}
+        data = {'provider': None, 'escalation_email': '[email protected]'}
 
         response = self.patch_api(self.user, data)
         self.assertEqual(400, response.status_code)
 
-    def test_patch_invalid_provider(self):
+    def test_patch_invalid_data_invalid_escalation_email(self):
+        """
+        Assert that endpoint returns 400 if the escalation email is not a valid email.
+        """
+        data = {'provider': 'test_provider', 'escalation_email': 'test'}
+
+        response = self.patch_api(self.user, data)
+        self.assertEqual(400, response.status_code)
+
+    @ddt.data('nonexistent_provider', '')
+    def test_patch_invalid_provider(self, provider_name):
         """
         Assert endpoint returns 400 if provider is invalid
         """
-        data = {'provider': 'nonexistent_provider'}
+        data = {'provider': provider_name}
 
         response = self.patch_api(self.user, data)
         self.assertEqual(400, response.status_code)
@@ -392,14 +415,17 @@ def test_patch_config_update(self):
             verbose_name='testing_provider_2',
             lti_configuration_id='223456789'
         )
-        data = {'provider': provider.name}
+        escalation_email = '[email protected]'
+
+        data = {'provider': provider.name, 'escalation_email': escalation_email}
 
         response = self.patch_api(self.user, data)
         self.assertEqual(204, response.status_code)
         self.assertEqual(len(CourseExamConfiguration.objects.all()), 1)
 
         config = CourseExamConfiguration.get_configuration_for_course(self.course_id)
         self.assertEqual(config.provider, provider)
+        self.assertEqual(config.escalation_email, escalation_email)
 
     def test_patch_config_update_exams(self):
         """
@@ -441,12 +467,15 @@ def test_patch_config_update_exams(self):
         exams = Exam.objects.filter(course_id=self.course_id, is_active=True)
         self.assertEqual(2, len(exams))
 
-        data = {'provider': provider.name}
+        escalation_email = '[email protected]'
+
+        data = {'provider': provider.name, 'escalation_email': escalation_email}
         response = self.patch_api(self.user, data)
         self.assertEqual(204, response.status_code)
         self.assertEqual(len(CourseExamConfiguration.objects.all()), 1)
         config = CourseExamConfiguration.get_configuration_for_course(self.course_id)
         self.assertEqual(config.provider, provider)
+        self.assertEqual(config.escalation_email, escalation_email)
 
         exams = Exam.objects.filter(course_id=self.course_id, is_active=True)
         self.assertEqual(2, len(exams))
@@ -459,12 +488,13 @@ def test_patch_config_update_exams(self):
             self.assertEqual(self.test_provider, exam.provider)
 
         # updating to the same provider is a do nothing, no new exams
-        data = {'provider': provider.name}
+        data = {'provider': provider.name, 'escalation_email': escalation_email}
         response = self.patch_api(self.user, data)
         self.assertEqual(204, response.status_code)
         self.assertEqual(len(CourseExamConfiguration.objects.all()), 1)
         config = CourseExamConfiguration.get_configuration_for_course(self.course_id)
         self.assertEqual(config.provider, provider)
+        self.assertEqual(config.escalation_email, escalation_email)
 
         exams = Exam.objects.filter(course_id=self.course_id, is_active=True)
         self.assertEqual(2, len(exams))
@@ -477,12 +507,13 @@ def test_patch_config_update_exams(self):
             self.assertEqual(self.test_provider, exam.provider)
 
         # updating back to the original provider creates two new active exams, now 4 inactive
-        data = {'provider': self.test_provider.name}
+        data = {'provider': self.test_provider.name, 'escalation_email': '[email protected]'}
         response = self.patch_api(self.user, data)
         self.assertEqual(204, response.status_code)
         self.assertEqual(len(CourseExamConfiguration.objects.all()), 1)
         config = CourseExamConfiguration.get_configuration_for_course(self.course_id)
         self.assertEqual(config.provider, self.test_provider)
+        self.assertEqual(config.escalation_email, escalation_email)
 
         exams = Exam.objects.filter(course_id=self.course_id, is_active=True)
         self.assertEqual(2, len(exams))
@@ -496,27 +527,30 @@ def test_patch_config_create(self):
         """
         Test that config is created
         """
-        data = {'provider': 'test_provider'}
+        escalation_email = '[email protected]'
+        data = {'provider': 'test_provider', 'escalation_email': escalation_email}
 
         response = self.patch_api(self.user, data)
         self.assertEqual(204, response.status_code)
         self.assertEqual(len(CourseExamConfiguration.objects.all()), 1)
 
         config = CourseExamConfiguration.get_configuration_for_course(self.course_id)
         self.assertEqual(config.provider, self.test_provider)
+        self.assertEqual(config.escalation_email, escalation_email)
 
     def test_patch_null_provider(self):
         """
         Assert provider can be explicitly set to null
         """
-        data = {'provider': None}
+        data = {'provider': None, 'escalation_email': None}
 
         response = self.patch_api(self.user, data)
         self.assertEqual(204, response.status_code)
         self.assertEqual(len(CourseExamConfiguration.objects.all()), 1)
 
         config = CourseExamConfiguration.get_configuration_for_course(self.course_id)
         self.assertEqual(config.provider, None)
+        self.assertEqual(config.escalation_email, None)
 
     def test_get_config(self):
         """

edx_exams/apps/api/v1/views.py

@@ -17,6 +17,8 @@
 
 from edx_exams.apps.api.permissions import CourseStaffOrReadOnlyPermissions, CourseStaffUserPermissions
 from edx_exams.apps.api.serializers import (
+    CourseExamConfigurationReadSerializer,
+    CourseExamConfigurationWriteSerializer,
     ExamSerializer,
     InstructorViewAttemptSerializer,
     ProctoringProviderSerializer,
@@ -26,6 +28,7 @@
 from edx_exams.apps.core.api import (
     check_if_exam_timed_out,
     create_exam_attempt,
+    create_or_update_course_exam_configuration,
     get_active_attempt_for_user,
     get_attempt_by_id,
     get_course_exams,
@@ -227,15 +230,19 @@ class CourseExamConfigurationsView(ExamsAPIView):
     **Returns**
     {
         'provider': 'test_provider',
+        'escalation_email': '[email protected]',
     }
 
     HTTP PATCH
     Creates or updates a CourseExamConfiguration.
     Expected PATCH data: {
         'provider': 'test_provider',
+        'escalation_email': '[email protected]',
     }
     **PATCH data Parameters**
-        * name: This is the name of the proctoring provider.
+        * provider: This is the name of the selected proctoring provider for the course.
+        * escalation_email: This is the email to which learners should send emails to escalate problems for the course.
+
     **Exceptions**
         * HTTP_400_BAD_REQUEST
     """
@@ -246,46 +253,32 @@ class CourseExamConfigurationsView(ExamsAPIView):
     def get(self, request, course_id):
         """
         Get exam configuration for a course
-
-        TODO: This view should use a serializer to ensure the read/write bodies are the same
-        once more fields are added.
         """
         try:
-            provider = CourseExamConfiguration.objects.get(course_id=course_id).provider
-        except ObjectDoesNotExist:
-            provider = None
+            configuration = CourseExamConfiguration.objects.get(course_id=course_id)
+        except CourseExamConfiguration.DoesNotExist:
+            # If configuration is set to None, then the provider is serialized to the empty string instead of None.
+            configuration = {}
 
-        return Response({
-            'provider': provider.name if provider else None
-        })
+        serializer = CourseExamConfigurationReadSerializer(configuration)
+        return Response(serializer.data)
 
     def patch(self, request, course_id):
         """
         Create/update course exam configuration.
         """
-        error = None
+        serializer = CourseExamConfigurationWriteSerializer(data=request.data)
 
-        # check that proctoring provider is in request
-        if 'provider' not in request.data:
-            error = {'detail': 'No proctoring provider name in request.'}
-        elif request.data.get('provider') is None:
-            provider = None
-        else:
-            try:
-                provider = ProctoringProvider.objects.get(name=request.data['provider'])
-            # return 400 if proctoring provider does not exist
-            except ObjectDoesNotExist:
-                error = {'detail': 'Proctoring provider does not exist.'}
-
-        if not error:
-            CourseExamConfiguration.create_or_update(provider, course_id)
-            response_status = status.HTTP_204_NO_CONTENT
-            data = {}
+        if serializer.is_valid():
+            validated_data = serializer.validated_data
+            create_or_update_course_exam_configuration(
+                course_id,
+                validated_data['provider'],
+                validated_data['escalation_email'],
+            )
+            return Response({}, status=status.HTTP_204_NO_CONTENT)
         else:
-            response_status = status.HTTP_400_BAD_REQUEST
-            data = error
-
-        return Response(status=response_status, data=data)
+            return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
 
 
 class ProctoringProvidersView(ListAPIView):

edx_exams/apps/core/admin.py

@@ -70,9 +70,9 @@ def change_view(self, request, object_id, form_url='', extra_context=None):
 @admin.register(CourseExamConfiguration)
 class CourseExamConfigurationAdmin(admin.ModelAdmin):
     """ Admin configuration for the Course Exam Configuration model """
-    list_display = ('course_id', 'provider', 'allow_opt_out')
+    list_display = ('course_id', 'provider', 'allow_opt_out', 'escalation_email')
     readonly_fields = ('course_id', 'provider')
-    search_fields = ('course_id', 'provider__name', 'allow_opt_out')
+    search_fields = ('course_id', 'provider__name', 'allow_opt_out', 'escalation_email')
     ordering = ('course_id',)