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 5b752ccb8a..f36d9e4738 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py +++ b/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py @@ -7,6 +7,7 @@ from mock import patch from django.conf import settings from django.test.utils import override_settings from django.urls import reverse +from edx_toggles.toggles.testutils import override_waffle_flag from opaque_keys.edx.keys import CourseKey from rest_framework import status from rest_framework.test import APITestCase @@ -14,6 +15,7 @@ from rest_framework.test import APITestCase from common.djangoapps.student.tests.factories import GlobalStaffFactory from common.djangoapps.student.tests.factories import InstructorFactory from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.course_apps.toggles import EXAMS_IDA from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -118,6 +120,46 @@ class ProctoringExamSettingsGetTests(ProctoringExamSettingsTestMixin, ModuleStor assert response.status_code == status.HTTP_200_OK assert response.data == self.get_expected_response_data(self.course, self.course_instructor) + @override_waffle_flag(EXAMS_IDA, active=False) + def test_providers_with_disabled_lti(self): + self.client.login(username=self.course_instructor.username, password=self.password) + response = self.make_request() + assert response.status_code == status.HTTP_200_OK + + # expected data should not include lti_external value + expected_data = { + 'proctored_exam_settings': { + 'enable_proctored_exams': self.course.enable_proctored_exams, + 'allow_proctoring_opt_out': self.course.allow_proctoring_opt_out, + 'proctoring_provider': self.course.proctoring_provider, + 'proctoring_escalation_email': self.course.proctoring_escalation_email, + 'create_zendesk_tickets': self.course.create_zendesk_tickets, + }, + 'course_start_date': '2030-01-01T00:00:00Z', + 'available_proctoring_providers': ['null'], + } + assert response.data == expected_data + + @override_waffle_flag(EXAMS_IDA, active=True) + def test_providers_with_enabled_lti(self): + self.client.login(username=self.course_instructor.username, password=self.password) + response = self.make_request() + assert response.status_code == status.HTTP_200_OK + + # expected data should include lti_external value + expected_data = { + 'proctored_exam_settings': { + 'enable_proctored_exams': self.course.enable_proctored_exams, + 'allow_proctoring_opt_out': self.course.allow_proctoring_opt_out, + 'proctoring_provider': self.course.proctoring_provider, + 'proctoring_escalation_email': self.course.proctoring_escalation_email, + 'create_zendesk_tickets': self.course.create_zendesk_tickets, + }, + 'course_start_date': '2030-01-01T00:00:00Z', + 'available_proctoring_providers': ['lti_external', 'null'], + } + assert response.data == expected_data + @ddt.ddt class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleStoreTestCase, APITestCase): @@ -339,3 +381,59 @@ class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleSto updated = modulestore().get_item(self.course.location) assert updated.create_zendesk_tickets is create_zendesk_tickets + + @override_waffle_flag(EXAMS_IDA, active=True) + def test_200_for_lti_provider(self): + self.client.login(username=self.global_staff.username, password=self.password) + PROCTORED_EXAMS_ENABLED_FEATURES = settings.FEATURES + PROCTORED_EXAMS_ENABLED_FEATURES['ENABLE_PROCTORED_EXAMS'] = True + with override_settings(FEATURES=PROCTORED_EXAMS_ENABLED_FEATURES): + data = self.get_request_data( + enable_proctored_exams=True, + proctoring_provider='lti_external', + ) + response = self.make_request(data=data) + + # response is correct + assert response.status_code == status.HTTP_200_OK + + self.assertDictEqual(response.data, { + 'proctored_exam_settings': { + 'enable_proctored_exams': True, + 'allow_proctoring_opt_out': True, + 'proctoring_provider': 'lti_external', + 'proctoring_escalation_email': None, + 'create_zendesk_tickets': True, + } + }) + + # course settings have been updated + updated = modulestore().get_item(self.course.location) + assert updated.enable_proctored_exams is True + assert updated.proctoring_provider == 'lti_external' + + @override_waffle_flag(EXAMS_IDA, active=False) + def test_400_for_disabled_lti(self): + self.client.login(username=self.global_staff.username, password=self.password) + PROCTORED_EXAMS_ENABLED_FEATURES = settings.FEATURES + PROCTORED_EXAMS_ENABLED_FEATURES['ENABLE_PROCTORED_EXAMS'] = True + with override_settings(FEATURES=PROCTORED_EXAMS_ENABLED_FEATURES): + data = self.get_request_data( + enable_proctored_exams=True, + proctoring_provider='lti_external', + ) + response = self.make_request(data=data) + + # response is correct + assert response.status_code == status.HTTP_400_BAD_REQUEST + self.assertDictEqual(response.data, { + 'detail': [{ + 'proctoring_provider': "The selected proctoring provider, lti_external, is not a valid provider. " + "Please select from one of ['null']." + }] + }) + + # course settings have been updated + updated = modulestore().get_item(self.course.location) + assert updated.enable_proctored_exams is False + assert updated.proctoring_provider == 'null' diff --git a/cms/djangoapps/contentstore/rest_api/v1/views.py b/cms/djangoapps/contentstore/rest_api/v1/views.py index 4ec2371a2e..f56a6287ab 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views.py @@ -10,6 +10,7 @@ from rest_framework.views import APIView from cms.djangoapps.contentstore.views.course import get_course_and_check_access from cms.djangoapps.models.settings.course_metadata import CourseMetadata from xmodule.course_module import get_available_providers # lint-amnesty, pylint: disable=wrong-import-order +from openedx.core.djangoapps.course_apps.toggles import exams_ida_enabled from openedx.core.lib.api.view_utils import view_auth_classes from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -98,9 +99,14 @@ class ProctoredExamSettingsView(APIView): data = {} data['proctored_exam_settings'] = proctored_exam_settings - data['available_proctoring_providers'] = get_available_providers() data['course_start_date'] = course_metadata['start'].get('value') + available_providers = get_available_providers() + if not exams_ida_enabled(CourseKey.from_string(course_id)): + available_providers.remove('lti_external') + + data['available_proctoring_providers'] = available_providers + serializer = ProctoredExamConfigurationSerializer(data) return Response(serializer.data) @@ -147,8 +153,8 @@ class ProctoredExamSettingsView(APIView): # merge updated settings with all existing settings. # do this because fields that could not be modified are excluded from the result course_metadata = {**course_metadata, **updated_data} - updated_setttings = self._get_proctored_exam_setting_values(course_metadata) - serializer = ProctoredExamSettingsSerializer(updated_setttings) + updated_settings = self._get_proctored_exam_setting_values(course_metadata) + serializer = ProctoredExamSettingsSerializer(updated_settings) return Response({ 'proctored_exam_settings': serializer.data }) diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 143b7a6d58..9dea9868d9 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -14,9 +14,11 @@ from xblock.fields import Scope from cms.djangoapps.contentstore import toggles from common.djangoapps.xblock_django.models import XBlockStudioConfigurationFlag +from openedx.core.djangoapps.course_apps.toggles import exams_ida_enabled from openedx.core.djangoapps.discussions.config.waffle_utils import legacy_discussion_experience_enabled from openedx.core.lib.teams_config import TeamsetType from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG +from xmodule.course_module import get_available_providers # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import InvalidProctoringProvider # lint-amnesty, pylint: disable=wrong-import-order @@ -254,9 +256,21 @@ class CourseMetadata: val = model['value'] if hasattr(descriptor, key) and getattr(descriptor, key) != val: key_values[key] = descriptor.fields[key].from_json(val) - except (InvalidProctoringProvider, TypeError, ValueError, ValidationError) as err: + except (TypeError, ValueError, ValidationError) as err: did_validate = False errors.append({'key': key, 'message': str(err), 'model': model}) + except InvalidProctoringProvider as err: + # LTI is automatically considered a proctoring provider, so it will be included in the error message + # Because we cannot pass course context to the exception, we need to check if the LTI provider + # should actually be available to the course + err_message = str(err) + if not exams_ida_enabled(descriptor.id): + available_providers = get_available_providers() + available_providers.remove('lti_external') + err_message = str(InvalidProctoringProvider(val, available_providers)) + + did_validate = False + errors.append({'key': key, 'message': err_message, 'model': model}) team_setting_errors = cls.validate_team_settings(filtered_dict) if team_setting_errors: @@ -408,6 +422,15 @@ class CourseMetadata: ).format(support_email=settings.PARTNER_SUPPORT_EMAIL or 'support') errors.append({'key': 'proctoring_provider', 'message': message, 'model': proctoring_provider_model}) + # check that a user should actually be able to update the provider to lti, which + # should only be allowed if the exams IDA is enabled for a course + available_providers = get_available_providers() + updated_provider = settings_dict.get('proctoring_provider', {}).get('value') + if updated_provider == 'lti_external' and not exams_ida_enabled(descriptor.id): + available_providers.remove('lti_external') + error = InvalidProctoringProvider('lti_external', available_providers) + errors.append({'key': 'proctoring_provider', 'message': str(error), 'model': proctoring_provider_model}) + enable_proctoring_model = settings_dict.get('enable_proctored_exams') if enable_proctoring_model: enable_proctoring = enable_proctoring_model.get('value') diff --git a/xmodule/course_module.py b/xmodule/course_module.py index d6afd01829..b1f09b57a4 100644 --- a/xmodule/course_module.py +++ b/xmodule/course_module.py @@ -267,6 +267,7 @@ def get_available_providers(): # lint-amnesty, pylint: disable=missing-function ) available_providers = [provider for provider in proctoring_backend_settings if provider != 'DEFAULT'] + available_providers.append('lti_external') available_providers.sort() return available_providers diff --git a/xmodule/tests/test_course_module.py b/xmodule/tests/test_course_module.py index 93a42b4f97..1e1657db57 100644 --- a/xmodule/tests/test_course_module.py +++ b/xmodule/tests/test_course_module.py @@ -494,7 +494,7 @@ class ProctoringProviderTestCase(unittest.TestCase): throws a ValueError with the correct error message. """ provider = 'invalid-provider' - allowed_proctoring_providers = ['mock', 'mock_proctoring_without_rules'] + allowed_proctoring_providers = xmodule.course_module.get_available_providers() FEATURES_WITH_PROCTORED_EXAMS = settings.FEATURES.copy() FEATURES_WITH_PROCTORED_EXAMS['ENABLE_PROCTORED_EXAMS'] = proctored_exams_setting_enabled