From 2bc48590a4be452ed31cbf271e7977d7375d66d3 Mon Sep 17 00:00:00 2001 From: michaelroytman Date: Tue, 5 Dec 2023 13:20:41 -0500 Subject: [PATCH] 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 | 70 ++++++++++++- edx_exams/apps/api/v1/tests/test_views.py | 56 ++++++++--- edx_exams/apps/api/v1/views.py | 55 +++++------ edx_exams/apps/core/admin.py | 4 +- edx_exams/apps/core/api.py | 49 +++++++++- edx_exams/apps/core/email.py | 12 ++- ...ourseexamconfiguration_escalation_email.py | 18 ++++ edx_exams/apps/core/models.py | 96 ++++++++++++------ .../email/proctoring_attempt_verified.html | 2 +- edx_exams/apps/core/test_utils/factories.py | 2 + edx_exams/apps/core/tests/test_api.py | 97 +++++++++++++++++- edx_exams/apps/core/tests/test_email.py | 33 +++++++ edx_exams/apps/core/tests/test_models.py | 98 ++++++++++++++++++- 13 files changed, 509 insertions(+), 83 deletions(-) create mode 100644 edx_exams/apps/core/migrations/0022_courseexamconfiguration_escalation_email.py diff --git a/edx_exams/apps/api/serializers.py b/edx_exams/apps/api/serializers.py index b26d04b8..92ae1330 100644 --- a/edx_exams/apps/api/serializers.py +++ b/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': }}, 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': }}, 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 diff --git a/edx_exams/apps/api/v1/tests/test_views.py b/edx_exams/apps/api/v1/tests/test_views.py index f996fdc0..ba2db127 100644 --- a/edx_exams/apps/api/v1/tests/test_views.py +++ b/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': 'test@example.com'}, + {'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': 'test@example.com'} 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,7 +415,9 @@ def test_patch_config_update(self): verbose_name='testing_provider_2', lti_configuration_id='223456789' ) - data = {'provider': provider.name} + escalation_email = 'test@example.com' + + data = {'provider': provider.name, 'escalation_email': escalation_email} response = self.patch_api(self.user, data) self.assertEqual(204, response.status_code) @@ -400,6 +425,7 @@ def test_patch_config_update(self): 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 = 'test@example.com' + + 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': 'test@example.com'} 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,7 +527,8 @@ def test_patch_config_create(self): """ Test that config is created """ - data = {'provider': 'test_provider'} + escalation_email = 'test@example.com' + data = {'provider': 'test_provider', 'escalation_email': escalation_email} response = self.patch_api(self.user, data) self.assertEqual(204, response.status_code) @@ -504,12 +536,13 @@ def test_patch_config_create(self): 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) @@ -517,6 +550,7 @@ def test_patch_null_provider(self): 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): """ diff --git a/edx_exams/apps/api/v1/views.py b/edx_exams/apps/api/v1/views.py index 5b10979f..7ef35561 100644 --- a/edx_exams/apps/api/v1/views.py +++ b/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': 'test@example.com', } HTTP PATCH Creates or updates a CourseExamConfiguration. Expected PATCH data: { 'provider': 'test_provider', + 'escalation_email': 'test@example.com', } **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): diff --git a/edx_exams/apps/core/admin.py b/edx_exams/apps/core/admin.py index b726721d..78a57fcb 100644 --- a/edx_exams/apps/core/admin.py +++ b/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',) diff --git a/edx_exams/apps/core/api.py b/edx_exams/apps/core/api.py index ef02fe2d..02730cf0 100644 --- a/edx_exams/apps/core/api.py +++ b/edx_exams/apps/core/api.py @@ -17,7 +17,7 @@ ExamDoesNotExist, ExamIllegalStatusTransition ) -from edx_exams.apps.core.models import Exam, ExamAttempt +from edx_exams.apps.core.models import CourseExamConfiguration, Exam, ExamAttempt, ProctoringProvider from edx_exams.apps.core.signals.signals import ( emit_exam_attempt_errored_event, emit_exam_attempt_rejected_event, @@ -141,7 +141,8 @@ def update_attempt_status(attempt_id, to_status): attempt_obj.status = to_status attempt_obj.save() - send_attempt_status_email(attempt_obj) + escalation_email = get_escalation_email(exam_id) + send_attempt_status_email(attempt_obj, escalation_email) return attempt_id @@ -422,3 +423,47 @@ def is_exam_passed_due(exam): due_date = dateparse.parse_datetime(due_date) return due_date <= datetime.now(pytz.UTC) return False + + +def get_escalation_email(exam_id): + """ + Return contact details for the course exam configuration. These details describe who learners should reach out to + for support with proctored exams. + + Parameters: + * exam_id: the ID representing the exam + + Returns: + * escalation_email: the escalation_email registered to the course in which the exam is configured, if there is + one; else, None. + """ + exam_obj = Exam.get_exam_by_id(exam_id) + + try: + course_config = CourseExamConfiguration.objects.get(course_id=exam_obj.course_id) + except CourseExamConfiguration.DoesNotExist: + return None + else: + return course_config.escalation_email + + +def create_or_update_course_exam_configuration(course_id, provider_name, escalation_email): + """ + Create or update the exam configuration for a course specified by course_id. If the course exam configuration + does not yet exist, create one with the provider set to the provider associated with the provider_name and the + escalation_email set to the escalation_email. + + Parameters: + * course_id: the ID representing the course + * provider_name: the name of the proctoring provider + * escalation_email: the escalation email + """ + if provider_name is not None: + provider = ProctoringProvider.objects.get(name=provider_name) + else: + # If the provider is set to None, then we must clear the escalation_email, + # even if a non-null value is provided. + escalation_email = None + provider = None + + CourseExamConfiguration.create_or_update(course_id, provider, escalation_email) diff --git a/edx_exams/apps/core/email.py b/edx_exams/apps/core/email.py index 1ceef53e..478190f2 100644 --- a/edx_exams/apps/core/email.py +++ b/edx_exams/apps/core/email.py @@ -13,7 +13,7 @@ log = logging.getLogger(__name__) -def send_attempt_status_email(attempt): +def send_attempt_status_email(attempt, escalation_email=None): """ Send email for attempt status if necessary """ @@ -35,13 +35,21 @@ def send_attempt_status_email(attempt): email_template = loader.get_template(email_template) course_url = f'{settings.LEARNING_MICROFRONTEND_URL}/course/{exam.course_id}' - contact_url = f'{settings.LMS_ROOT_URL}/support/contact_us' + + # If the course has a proctoring escalation email set, then use that rather than edX Support. + if escalation_email: + contact_url = f'mailto:{escalation_email}' + contact_url_text = escalation_email + else: + contact_url = f'{settings.LMS_ROOT_URL}/support/contact_us' + contact_url_text = contact_url email_subject = f'Proctored exam {exam.exam_name} for user {attempt.user.username}' body = email_template.render({ 'exam_name': exam.exam_name, 'course_url': course_url, 'contact_url': contact_url, + 'contact_url_text': contact_url_text, }) email = EmailMessage( diff --git a/edx_exams/apps/core/migrations/0022_courseexamconfiguration_escalation_email.py b/edx_exams/apps/core/migrations/0022_courseexamconfiguration_escalation_email.py new file mode 100644 index 00000000..f16bb682 --- /dev/null +++ b/edx_exams/apps/core/migrations/0022_courseexamconfiguration_escalation_email.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.23 on 2023-12-07 20:12 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0021_alter_exam_exam_types'), + ] + + operations = [ + migrations.AddField( + model_name='courseexamconfiguration', + name='escalation_email', + field=models.EmailField(max_length=254, null=True), + ), + ] diff --git a/edx_exams/apps/core/models.py b/edx_exams/apps/core/models.py index 7478f7a5..821abbd5 100644 --- a/edx_exams/apps/core/models.py +++ b/edx_exams/apps/core/models.py @@ -325,6 +325,8 @@ class CourseExamConfiguration(TimeStampedModel): allow_opt_out = models.BooleanField(default=False) + escalation_email = models.EmailField(null=True, blank=False) + class Meta: """ Meta class for this Django model """ db_table = 'exams_courseexamconfiguration' @@ -343,7 +345,7 @@ def get_configuration_for_course(cls, course_id): @classmethod @transaction.atomic - def create_or_update(cls, provider, course_id): + def create_or_update(cls, course_id, provider, escalation_email): """ Helper method that decides whether to update existing or create new config. @@ -351,40 +353,72 @@ def create_or_update(cls, provider, course_id): existing exams. """ provider_name = provider.name if provider else None + + # If the provider is set to None, then we must clear the escalation_email, regardless of the value provided. + if provider is None: + escalation_email = None + existing_config = CourseExamConfiguration.get_configuration_for_course(course_id) if existing_config: - if existing_config.provider == provider: - # nothing to be done - log.info(f'Course exam configuration course_id={course_id} already has provider={provider_name}') - return - count = cls.update_course_config_provider(existing_config, provider) - log.info(f'Updated course exam configuration course_id={course_id} ' - + f'to provider={provider_name} and recreated {count} exams') + existing_provider = existing_config.provider + + cls.update_course_config(existing_config, provider, escalation_email) + + # If the provider is updated for a course, all existing exams have to be retired + # and duplicates made with the new provider. + if provider != existing_provider: + count = cls._sync_exams_with_new_provider(existing_config.course_id, provider) + log.info(f'Updated course exam configuration course_id={course_id} ' + + f'to provider={provider_name} and recreated {count} exams') else: - CourseExamConfiguration.objects.create(course_id=course_id, provider=provider) - log.info(f'Created course exam configuration course_id={course_id}, provider={provider_name}') + CourseExamConfiguration.objects.create( + course_id=course_id, + escalation_email=escalation_email, + provider=provider, + ) + log.info(f'Created course exam configuration course_id={course_id}, provider={provider_name}, ' + + f'escalation_email={escalation_email}') @classmethod - def update_course_config_provider(cls, existing_config, new_provider): + def update_course_config(cls, existing_config, new_provider, escalation_email): """ - If the provider is updated for a course, all existing exams have to be retired - and duplicates made with the new provider. + Update the provider and escalation_email fields of an instance of a CourseExamConfiguration model represented + by the existing_config parameter. + + Parameters: + * existing_config: an instance of the CourseExamConfiguration model + * new_provider: an instance of the ProctoringProvider model; the provider to be set + * escalation_email: a string representing an email address; the escalation_email to be set + """ + existing_config.provider = new_provider + existing_config.escalation_email = escalation_email + + existing_config.save() + + @classmethod + def _sync_exams_with_new_provider(cls, course_id, new_provider): + """ + For a particular course represented by the course_id argument, duplicate all the exams in the course with the + new proctoring provider. Set the originale exams to inactive and create new active exams with the new + proctoring provider and with all other fields of the original exams. + + Parameters: + * course_id: a string representing the course ID + * provider: an instance of the ProctoringProvider model """ - with transaction.atomic(): - exams = Exam.objects.filter(course_id=existing_config.course_id, is_active=True) - - existing_config.provider = new_provider - existing_config.save() - - # we could bulk update, but that would avoid any django save/update hooks - # that might be added to these objects later and the number of exams per course - # will not be high enough to worry about - for exam in exams: - # set the original inactive - exam.is_active = False - exam.save() - # use the original to stamp out an active duplicate with the new provider - exam.pk = None - exam.is_active = True - exam.provider = new_provider - exam.save() + exams = Exam.objects.filter(course_id=course_id, is_active=True) + + # we could bulk update, but that would avoid any django save/update hooks + # that might be added to these objects later and the number of exams per course + # will not be high enough to worry about + for exam in exams: + # set the original inactive + exam.is_active = False + exam.save() + # use the original to stamp out an active duplicate with the new provider + exam.pk = None + exam.is_active = True + exam.provider = new_provider + exam.save() + + return len(exams) diff --git a/edx_exams/apps/core/templates/email/proctoring_attempt_verified.html b/edx_exams/apps/core/templates/email/proctoring_attempt_verified.html index 175ba8d5..a8b5fa4b 100644 --- a/edx_exams/apps/core/templates/email/proctoring_attempt_verified.html +++ b/edx_exams/apps/core/templates/email/proctoring_attempt_verified.html @@ -21,7 +21,7 @@ {% blocktrans %} If you have any questions about your results, you can reach out at - {{ contact_url }} + {{ contact_url_text }} . {% endblocktrans %} {% endblock %} diff --git a/edx_exams/apps/core/test_utils/factories.py b/edx_exams/apps/core/test_utils/factories.py index 4ec17ca1..d778ed4b 100644 --- a/edx_exams/apps/core/test_utils/factories.py +++ b/edx_exams/apps/core/test_utils/factories.py @@ -66,6 +66,7 @@ class Meta: course_id = 'course-v1:edX+Test+Test_Course' provider = factory.SubFactory(ProctoringProviderFactory) allow_opt_out = False + escalation_email = factory.Sequence('escalation{}@example.com'.format) class ExamFactory(DjangoModelFactory): @@ -83,6 +84,7 @@ class Meta: exam_type = 'proctored' time_limit_mins = 30 due_date = datetime.datetime.now() + datetime.timedelta(days=1) + is_active = True class ExamAttemptFactory(DjangoModelFactory): diff --git a/edx_exams/apps/core/tests/test_api.py b/edx_exams/apps/core/tests/test_api.py index fc156053..b449e8bc 100644 --- a/edx_exams/apps/core/tests/test_api.py +++ b/edx_exams/apps/core/tests/test_api.py @@ -17,10 +17,12 @@ 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_attempt_for_user_with_attempt_number_and_resource_id, get_current_exam_attempt, + get_escalation_email, get_exam_attempt_time_remaining, get_exam_by_content_id, get_exam_url_path, @@ -36,7 +38,13 @@ ) from edx_exams.apps.core.models import Exam, ExamAttempt from edx_exams.apps.core.statuses import ExamAttemptStatus -from edx_exams.apps.core.test_utils.factories import ExamAttemptFactory, ExamFactory, UserFactory +from edx_exams.apps.core.test_utils.factories import ( + CourseExamConfigurationFactory, + ExamAttemptFactory, + ExamFactory, + ProctoringProviderFactory, + UserFactory +) test_start_time = datetime(2023, 11, 4, 11, 5, 23) test_time_limit_mins = 30 @@ -882,3 +890,90 @@ def test_not_passed_due_no_due_date(self): exam = {'due_date': None} self.assertFalse(is_exam_passed_due(exam)) + + +class TestGetEscalationEmail(ExamsAPITestCase): + """ + Tests for the get_escalation_email API function. + """ + def setUp(self): + super().setUp() + + self.exam = ExamFactory( + provider=self.test_provider, + ) + + def test_get_escalation_email(self): + """ + Test that the escalation is returned correctly when a course is configured to use exams. + """ + expected_escalation_email = 'test@example.com' + + CourseExamConfigurationFactory( + course_id=self.exam.course_id, + provider=self.test_provider, + escalation_email=expected_escalation_email, + ) + + escalation_email = get_escalation_email(self.exam.id) + + self.assertEqual(escalation_email, expected_escalation_email) + + def test_get_escalation_email_no_configuration(self): + """ + Test that None is returned correctly when a course is not configured to use an exam. + """ + self.assertEqual(get_escalation_email(self.exam.id), None) + + +@ddt.ddt +class TestCreateOrUpdateCourseExamConfiguration(ExamsAPITestCase): + """ + Tests for the create_or_update_course_exam_configuration API function. + + This API function is primarily a wrapper around the CourseExamConfiguration.create_or_update method, so the + majority of testing of that functionality is in the corresponding test file for that model. + """ + def setUp(self): + super().setUp() + + self.exam = ExamFactory( + provider=self.test_provider, + ) + + @ddt.data('test@example.com', '') + def test_create_or_update_course_exam_configuration_none_provider(self, escalation_email): + """ + Test that the create_or_update_course_exam_configuration function correctly sets the provider to None and that + the function disregards any provided escalation_email, because it must be set to None. + """ + expected_provider = None + + config = CourseExamConfigurationFactory( + course_id=self.exam.course_id, + provider=self.test_provider, + ) + + create_or_update_course_exam_configuration(self.exam.course_id, expected_provider, escalation_email) + + config.refresh_from_db() + + self.assertEqual(config.provider, expected_provider) + self.assertEqual(config.escalation_email, None) + + def test_create_or_update_course_exam_configuration_provider(self): + """ + Test that the create_or_update_course_exam_configuration function correctly sets the provider to None. + """ + expected_provider = ProctoringProviderFactory() + + config = CourseExamConfigurationFactory( + course_id=self.exam.course_id, + provider=self.test_provider, + ) + + create_or_update_course_exam_configuration(self.exam.course_id, expected_provider.name, config.escalation_email) + + config.refresh_from_db() + + self.assertEqual(config.provider, expected_provider) diff --git a/edx_exams/apps/core/tests/test_email.py b/edx_exams/apps/core/tests/test_email.py index 27484c9a..a45990b0 100644 --- a/edx_exams/apps/core/tests/test_email.py +++ b/edx_exams/apps/core/tests/test_email.py @@ -1,13 +1,16 @@ """ Test email notifications for attempt status change """ +from itertools import product import ddt import mock +from django.conf import settings from django.core import mail from django.test import TestCase from edx_exams.apps.core.api import update_attempt_status +from edx_exams.apps.core.models import CourseExamConfiguration from edx_exams.apps.core.test_utils.factories import ExamAttemptFactory, ExamFactory, UserFactory @@ -51,6 +54,36 @@ def test_send_email(self, status, expected_message): self.assertIn(self.started_attempt.user.email, mail.outbox[0].to) self.assertIn(expected_message, self._normalize_whitespace(mail.outbox[0].body)) + @ddt.idata( + product( + ('verified', 'rejected'), + (True, False), + ) + ) + @ddt.unpack + def test_send_email_contact_url(self, status, has_escalation_email): + """ + Test correct correct contact URL is included in emails for sent for statuses that trigger an email. + """ + if has_escalation_email: + contact_url = 'test@example.com' + CourseExamConfiguration.objects.create( + course_id=self.proctored_exam.course_id, + escalation_email=contact_url, + ) + else: + contact_url = f'{settings.LMS_ROOT_URL}/support/contact_us' + + update_attempt_status(self.started_attempt.id, status) + self.assertEqual(len(mail.outbox), 1) + + email_body = self._normalize_whitespace(mail.outbox[0].body) + + self.assertIn(contact_url, email_body) + + if has_escalation_email: + self.assertIn(f'mailto:{contact_url}', email_body) + @mock.patch('edx_exams.apps.core.email.log.error') def test_send_email_failure(self, mock_log_error): """ diff --git a/edx_exams/apps/core/tests/test_models.py b/edx_exams/apps/core/tests/test_models.py index d509d996..9b7c0808 100644 --- a/edx_exams/apps/core/tests/test_models.py +++ b/edx_exams/apps/core/tests/test_models.py @@ -4,7 +4,12 @@ from django_dynamic_fixture import G from social_django.models import UserSocialAuth -from edx_exams.apps.core.models import User +from edx_exams.apps.core.models import CourseExamConfiguration, Exam, User +from edx_exams.apps.core.test_utils.factories import ( + CourseExamConfigurationFactory, + ExamFactory, + ProctoringProviderFactory +) class UserTests(TestCase): @@ -37,3 +42,94 @@ def test_get_full_name(self): user = G(User, full_name=full_name, first_name=first_name, last_name=last_name) self.assertEqual(user.get_full_name(), full_name) + + +class CourseExamConfigurationTests(TestCase): + """ + CourseExamConfiguration model tests. + """ + + def setUp(self): + super().setUp() + + self.escalation_email = 'test1@example.com' + self.config = CourseExamConfigurationFactory() + + for _ in range(3): + ExamFactory(provider=self.config.provider) + + def test_create_or_update_no_provider_change(self): + old_provider = self.config.provider + + CourseExamConfiguration.create_or_update( + self.config.course_id, + self.config.provider, + self.escalation_email, + ) + + self.config.refresh_from_db() + + # Assert that no new model instances were created. + num_configs = CourseExamConfiguration.objects.count() + self.assertEqual(num_configs, 1) + + self.assertEqual(self.config.provider, old_provider) + self.assertEqual(self.config.escalation_email, self.escalation_email) + + def test_create_or_update_no_provider(self): + CourseExamConfiguration.create_or_update( + self.config.course_id, + None, + self.escalation_email, + ) + + self.config.refresh_from_db() + + # Assert that no new model instances were created. + num_configs = CourseExamConfiguration.objects.count() + self.assertEqual(num_configs, 1) + + self.assertEqual(self.config.provider, None) + self.assertEqual(self.config.escalation_email, None) + + def test_create_or_update_provider_change_and_sync(self): + other_provider = ProctoringProviderFactory() + + previous_exams = set(Exam.objects.all()) + + CourseExamConfiguration.create_or_update( + self.config.course_id, + other_provider, + self.escalation_email, + ) + + all_exams = set(Exam.objects.all()) + new_exams = all_exams - previous_exams + + self.assertEqual(previous_exams <= all_exams, True) + self.assertEqual(new_exams <= all_exams, True) + self.assertEqual(new_exams.isdisjoint(previous_exams), True) + + for exam in previous_exams: + exam.refresh_from_db() + self.assertEqual(exam.is_active, False) + + for exam in new_exams: + self.assertEqual(exam.is_active, True) + self.assertEqual(exam.provider, other_provider) + + def test_create_or_update_new_config(self): + other_course_id = 'course-v1:edX+Test+Test_Course2' + CourseExamConfiguration.create_or_update( + other_course_id, + self.config.provider, + self.escalation_email, + ) + + # Assert that one new model instance was created. + num_configs = CourseExamConfiguration.objects.count() + self.assertEqual(num_configs, 2) + + new_config = CourseExamConfiguration.objects.get(course_id=other_course_id) + self.assertEqual(new_config.provider, self.config.provider) + self.assertEqual(new_config.escalation_email, self.escalation_email)