From 57de9b2ac6228d1cd41d2603b15f46b5be0cb509 Mon Sep 17 00:00:00 2001 From: Zachary Hancock Date: Mon, 27 Jul 2020 09:23:03 -0400 Subject: [PATCH] POST proctored exam settings (#24597) --- .../rest_api/v1/tests/test_views.py | 221 ++++++++++++++---- .../contentstore/rest_api/v1/urls.py | 2 +- .../contentstore/rest_api/v1/views.py | 142 ++++++++--- .../models/settings/course_metadata.py | 16 +- 4 files changed, 296 insertions(+), 85 deletions(-) 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 d3677e3b79..daa109609b 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py +++ b/cms/djangoapps/contentstore/rest_api/v1/tests/test_views.py @@ -3,38 +3,41 @@ Unit tests for Contentstore views. """ from django.urls import reverse +from django.test.utils import override_settings from rest_framework import status from rest_framework.test import APITestCase from opaque_keys.edx.keys import CourseKey +from contentstore.config.waffle import ENABLE_PROCTORING_PROVIDER_OVERRIDES from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory, InstructorFactory +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from student.tests.factories import UserFactory -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -class ProctoringExamSettingsGetTests(SharedModuleStoreTestCase, APITestCase): - """ Tests for proctored exam settings GETs """ - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.course_key = CourseKey.from_string('course-v1:edX+ToyX+Toy_Course') - cls.other_course_key = CourseKey.from_string('course-v1:edX+ToyX_Other_Course+Toy_Course') - cls.course = cls.create_course_from_course_key(cls.course_key) - cls.other_course = cls.create_course_from_course_key(cls.other_course_key) - cls.password = 'password' - cls.student = UserFactory.create(username='student', password=cls.password) - cls.global_staff = GlobalStaffFactory(username='global-staff', password=cls.password) - cls.course_instructor = InstructorFactory( +class ProctoringExamSettingsTestMixin(): + """ setup for proctored exam settings tests """ + def setUp(self): + super().setUp() + self.course_key = CourseKey.from_string('course-v1:edX+ToyX+Toy_Course') + self.other_course_key = CourseKey.from_string('course-v1:edX+ToyX_Other_Course+Toy_Course') + self.course = self.create_course_from_course_key(self.course_key) + self.other_course = self.create_course_from_course_key(self.other_course_key) + self.password = 'password' + self.student = UserFactory.create(username='student', password=self.password) + self.global_staff = GlobalStaffFactory(username='global-staff', password=self.password) + self.course_instructor = InstructorFactory( username='instructor', - password=cls.password, - course_key=cls.course.id, + password=self.password, + course_key=self.course.id, ) - cls.other_course_instructor = InstructorFactory( + self.other_course_instructor = InstructorFactory( username='other-course-instructor', - password=cls.password, - course_key=cls.other_course.id, + password=self.password, + course_key=self.other_course.id, ) def tearDown(self): @@ -49,13 +52,42 @@ class ProctoringExamSettingsGetTests(SharedModuleStoreTestCase, APITestCase): run=course_key.run ) + def make_request(self, course_id=None, data=None): + raise NotImplementedError + def get_url(self, course_key): return reverse( 'cms.djangoapps.contentstore:v1:proctored_exam_settings', kwargs={'course_id': course_key} ) - def get_expected_response_data(self, course, user): + def test_403_if_student(self): + self.client.login(username=self.student.username, password=self.password) + response = self.make_request() + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_403_if_instructor_in_another_course(self): + self.client.login( + username=self.other_course_instructor.username, + password=self.password + ) + response = self.make_request() + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_404_no_course_module(self): + course_id = 'course-v1:edX+ToyX_Nonexistent_Course+Toy_Course' + self.client.login(username=self.global_staff, password=self.password) + response = self.make_request(course_id=course_id) + assert response.status_code == status.HTTP_404_NOT_FOUND + assert response.data == { + 'detail': 'Course with course_id {} does not exist.'.format(course_id) + } + + +class ProctoringExamSettingsGetTests(ProctoringExamSettingsTestMixin, ModuleStoreTestCase, APITestCase): + """ Tests for proctored exam settings GETs """ + @classmethod + def get_expected_response_data(cls, course, user): return { 'proctored_exam_settings': { 'enable_proctored_exams': course.enable_proctored_exams, @@ -69,39 +101,140 @@ class ProctoringExamSettingsGetTests(SharedModuleStoreTestCase, APITestCase): 'is_staff': user.is_staff, } - def test_403_if_student(self): - self.client.login(username=self.student.username, password=self.password) - url = self.get_url(self.course.id) - response = self.client.get(url) - assert response.status_code == status.HTTP_403_FORBIDDEN - - def test_403_if_instructor_in_another_course(self): - self.client.login( - username=self.other_course_instructor.username, - password=self.password - ) - url = self.get_url(self.course.id) - response = self.client.get(url) - assert response.status_code == status.HTTP_403_FORBIDDEN + def make_request(self, course_id=None, data=None): + course_id = course_id if course_id else self.course.id + url = self.get_url(course_id) + return self.client.get(url) def test_200_global_staff(self): self.client.login(username=self.global_staff.username, password=self.password) - url = self.get_url(self.course.id) - response = self.client.get(url) + response = self.make_request() assert response.status_code == status.HTTP_200_OK assert response.data == self.get_expected_response_data(self.course, self.global_staff) def test_200_course_instructor(self): self.client.login(username=self.course_instructor.username, password=self.password) - url = self.get_url(self.course.id) - response = self.client.get(url) + response = self.make_request() assert response.status_code == status.HTTP_200_OK assert response.data == self.get_expected_response_data(self.course, self.course_instructor) - def test_404_no_course_module(self): - course_id = 'course-v1:edX+ToyX_Nonexistent_Course+Toy_Course' - self.client.login(username=self.global_staff, password=self.password) + +class ProctoringExamSettingsPostTests(ProctoringExamSettingsTestMixin, ModuleStoreTestCase, APITestCase): + """ Tests for proctored exam settings POST """ + + @classmethod + def get_request_data( # pylint: disable=missing-function-docstring + cls, + enable_proctored_exams=False, + allow_proctoring_opt_out=True, + proctoring_provider='null', + proctoring_escalation_email='example@edx.org', + create_zendesk_tickets=True, + ): + return { + 'proctored_exam_settings': { + 'enable_proctored_exams': enable_proctored_exams, + 'allow_proctoring_opt_out': allow_proctoring_opt_out, + 'proctoring_provider': proctoring_provider, + 'proctoring_escalation_email': proctoring_escalation_email, + 'create_zendesk_tickets': create_zendesk_tickets, + } + } + + def make_request(self, course_id=None, data=None): + course_id = course_id if course_id else self.course.id url = self.get_url(course_id) - response = self.client.get(url) - assert response.status_code == status.HTTP_404_NOT_FOUND - assert response.data == 'Course with course_id {} does not exist.'.format(course_id) + if data is None: + data = self.get_request_data() + return self.client.post(url, data, format='json') + + @override_settings( + PROCTORING_BACKENDS={ + 'DEFAULT': 'null', + 'proctortrack': {} + }, + ) + @override_waffle_flag(ENABLE_PROCTORING_PROVIDER_OVERRIDES, True) + def test_update_exam_settings_200(self): + self.client.login(username=self.global_staff.username, password=self.password) + data = self.get_request_data( + enable_proctored_exams=True, + proctoring_provider='proctortrack', + proctoring_escalation_email='foo@bar.com', + ) + 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': 'proctortrack', + 'proctoring_escalation_email': 'foo@bar.com', + '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 == 'proctortrack' + assert updated.proctoring_escalation_email == 'foo@bar.com' + + def test_update_exam_settings_excluded_field(self): + """ + Excluded settings in POST data should not be updated + """ + self.client.login(username=self.global_staff.username, password=self.password) + data = self.get_request_data( + proctoring_escalation_email='foo@bar.com', + ) + 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': False, + 'allow_proctoring_opt_out': True, + 'proctoring_provider': 'null', + 'proctoring_escalation_email': None, + 'create_zendesk_tickets': True, + } + }) + + # excluded course settings are not updated + updated = modulestore().get_item(self.course.location) + assert updated.proctoring_escalation_email is None + + @override_settings( + PROCTORING_BACKENDS={ + 'DEFAULT': 'null', + 'test_proctoring_provider': {} + }, + ) + @override_waffle_flag(ENABLE_PROCTORING_PROVIDER_OVERRIDES, True) + def test_update_exam_settings_invalid_value(self): + self.client.login(username=self.global_staff.username, password=self.password) + data = self.get_request_data( + enable_proctored_exams=True, + proctoring_provider='notvalidprovider', + ) + 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, notvalidprovider, is not a valid provider. ' + 'Please select from one of [\'test_proctoring_provider\'].\"]' + ) + }] + }) + + # 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/urls.py b/cms/djangoapps/contentstore/rest_api/v1/urls.py index 14ffd948bb..6b78e302cf 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -10,7 +10,7 @@ app_name = 'v1' urlpatterns = [ re_path( r'^proctored_exam_settings/{}$'.format(COURSE_ID_PATTERN), - views.proctored_exam_settings, + views.ProctoredExamSettingsView.as_view(), name="proctored_exam_settings" ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v1/views.py b/cms/djangoapps/contentstore/rest_api/v1/views.py index d9dc1e72b2..292142eefa 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views.py @@ -1,9 +1,11 @@ "Contentstore Views" +import copy from opaque_keys.edx.keys import CourseKey from rest_framework import status -from rest_framework.decorators import api_view +from rest_framework.exceptions import NotFound from rest_framework.response import Response +from rest_framework.views import APIView from common.lib.xmodule.xmodule.course_module import get_available_providers from contentstore.views.course import get_course_and_check_access @@ -11,18 +13,20 @@ from models.settings.course_metadata import CourseMetadata from openedx.core.lib.api.view_utils import view_auth_classes from xmodule.modulestore.django import modulestore -from contentstore.rest_api.v1.serializers import ProctoredExamConfigurationSerializer +from contentstore.rest_api.v1.serializers import ( + ProctoredExamConfigurationSerializer, + ProctoredExamSettingsSerializer, +) -@api_view(['GET']) @view_auth_classes() -def proctored_exam_settings(request, course_id): +class ProctoredExamSettingsView(APIView): """ A view for retrieving information about proctored exam settings for a course. Path: ``/api/contentstore/v1/proctored_exam_settings/{course_id}`` - Accepts: [GET] + Accepts: [GET, POST] ------------------------------------------------------------------------------------ GET @@ -58,45 +62,111 @@ def proctored_exam_settings(request, course_id): "course_start_date": "2013-02-05T05:00:00Z", "is_staff": true } + + ------------------------------------------------------------------------------------ + POST + ------------------------------------------------------------------------------------ + + **Returns** + + * 200: OK - Proctored exam settings saved. + * 400: Bad Request - Unable to save requested settings. + * 401: The requesting user is not authenticated. + * 403: The requesting user lacks access to the course. + * 404: The requested course does not exist. + + **Response** + + In the case of a 200 response code, the response will echo the updated proctored + exam settings data. """ - course_key = CourseKey.from_string(course_id) - with modulestore().bulk_operations(course_key): - if request.method == 'GET': - course_module = get_course_and_check_access(course_key, request.user) - - if not course_module: - return Response( - 'Course with course_id {} does not exist.'.format(course_id), - status=status.HTTP_404_NOT_FOUND - ) + PROCTORED_EXAM_SETTINGS_KEYS = [ + 'enable_proctored_exams', + 'allow_proctoring_opt_out', + 'proctoring_provider', + 'proctoring_escalation_email', + 'create_zendesk_tickets', + ] + def get(self, request, course_id): + """ GET handler """ + with modulestore().bulk_operations(CourseKey.from_string(course_id)): + course_module = self._get_and_validate_course_access(request.user, course_id) course_metadata = CourseMetadata().fetch_all(course_module) + proctored_exam_settings = self._get_proctored_exam_setting_values(course_metadata) + data = {} - # specify only the advanced settings we want to return - proctored_exam_settings_advanced_settings_keys = [ - 'enable_proctored_exams', - 'allow_proctoring_opt_out', - 'proctoring_provider', - 'proctoring_escalation_email', - 'create_zendesk_tickets', - 'start' - ] - proctored_exam_settings_data = { - setting_key: setting_value.get('value') - for (setting_key, setting_value) in course_metadata.items() - if setting_key in proctored_exam_settings_advanced_settings_keys - } - - data['proctored_exam_settings'] = proctored_exam_settings_data + data['proctored_exam_settings'] = proctored_exam_settings data['available_proctoring_providers'] = get_available_providers() - - # move start key:value out of proctored_exam_settings dictionary and change key - data['course_start_date'] = proctored_exam_settings_data['start'] - del data['proctored_exam_settings']['start'] - + data['course_start_date'] = course_metadata['start'].get('value') data['is_staff'] = request.user.is_staff serializer = ProctoredExamConfigurationSerializer(data) return Response(serializer.data) + + def post(self, request, course_id): + """ POST handler """ + exam_config = ProctoredExamSettingsSerializer(data=request.data.get('proctored_exam_settings', {})) + exam_config.is_valid(raise_exception=True) + with modulestore().bulk_operations(CourseKey.from_string(course_id)): + course_module = self._get_and_validate_course_access(request.user, course_id) + course_metadata = CourseMetadata().fetch_all(course_module) + + models_to_update = {} + for setting_key, value in exam_config.data.items(): + model = course_metadata.get(setting_key) + if model: + models_to_update[setting_key] = copy.deepcopy(model) + models_to_update[setting_key]['value'] = value + + # validate data formats and update the course module object + is_valid, errors, updated_data = CourseMetadata.validate_and_update_from_json( + course_module, + models_to_update, + user=request.user, + ) + + if not is_valid: + error_messages = [{error.get('key'): error.get('message')} for error in errors] + return Response( + {'detail': error_messages}, + status=status.HTTP_400_BAD_REQUEST + ) + + # save to mongo + modulestore().update_item(course_module, request.user.id) + + # 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) + return Response({ + 'proctored_exam_settings': serializer.data + }) + + @classmethod + def _get_proctored_exam_setting_values(cls, course_metadata): + return { + setting_key: course_metadata[setting_key].get('value') + for setting_key in cls.PROCTORED_EXAM_SETTINGS_KEYS + } + + @staticmethod + def _get_and_validate_course_access(user, course_id): + """ + Check if course_id exists and is accessible by the user. + + Returns a course_module object + """ + course_key = CourseKey.from_string(course_id) + course_module = get_course_and_check_access(course_key, user) + + if not course_module: + raise NotFound( + 'Course with course_id {} does not exist.'.format(course_id) + ) + + return course_module diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 7fb075b2ac..214d7debf8 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -252,7 +252,7 @@ class CourseMetadata(object): key_values[key] = descriptor.fields[key].from_json(val) except (TypeError, ValueError, ValidationError) as err: did_validate = False - errors.append({'message': text_type(err), 'model': model}) + errors.append({'key': key, 'message': text_type(err), 'model': model}) proctoring_errors = cls._validate_proctoring_settings(descriptor, filtered_dict, user) if proctoring_errors: @@ -302,7 +302,7 @@ class CourseMetadata(object): 'The proctoring provider cannot be modified after a course has started.' ' Contact {support_email} for assistance' ).format(support_email=settings.PARTNER_SUPPORT_EMAIL or 'support') - errors.append({'message': message, 'model': proctoring_provider_model}) + errors.append({'key': 'proctoring_provider', 'message': message, 'model': proctoring_provider_model}) # Require a valid escalation email if Proctortrack is chosen as the proctoring provider # This requirement will be disabled until release of the new exam settings view @@ -317,7 +317,11 @@ class CourseMetadata(object): if proctoring_provider_model and proctoring_provider_model.get('value') == 'proctortrack': if not escalation_email: message = missing_escalation_email_msg.format(provider=proctoring_provider_model.get('value')) - errors.append({'message': message, 'model': proctoring_provider_model}) + errors.append({ + 'key': 'proctoring_provider', + 'message': message, + 'model': proctoring_provider_model + }) if ( escalation_email_model and not proctoring_provider_model and @@ -325,7 +329,11 @@ class CourseMetadata(object): ): if not escalation_email: message = missing_escalation_email_msg.format(provider=descriptor.proctoring_provider) - errors.append({'message': message, 'model': escalation_email_model}) + errors.append({ + 'key': 'proctoring_escalation_email', + 'message': message, + 'model': escalation_email_model + }) return errors