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)