From 0d1c2afc2a411758e1018e5894c77cf6f9eaf5c3 Mon Sep 17 00:00:00 2001 From: Alison Langston <46360176+alangsto@users.noreply.github.com> Date: Mon, 22 Jul 2024 14:18:38 -0400 Subject: [PATCH] feat: add view for allowance creation (#291) --- .gitignore | 2 + edx_exams/apps/api/serializers.py | 22 +++++-- edx_exams/apps/api/v1/tests/test_views.py | 79 +++++++++++++++++++++-- edx_exams/apps/api/v1/views.py | 60 ++++++++++++++++- 4 files changed, 151 insertions(+), 12 deletions(-) diff --git a/.gitignore b/.gitignore index 24aefe00..11cfa2a8 100644 --- a/.gitignore +++ b/.gitignore @@ -96,3 +96,5 @@ docs/_build/ # Temp reports file reports/ + +requirements/private.txt diff --git a/edx_exams/apps/api/serializers.py b/edx_exams/apps/api/serializers.py index 431e3561..0048080d 100644 --- a/edx_exams/apps/api/serializers.py +++ b/edx_exams/apps/api/serializers.py @@ -289,13 +289,23 @@ class AllowanceSerializer(serializers.ModelSerializer): # directly from the Allowance Model id = serializers.IntegerField(required=False) - exam_id = serializers.IntegerField() - user_id = serializers.IntegerField() - extra_time_mins = serializers.IntegerField() + exam_id = serializers.IntegerField(required=True) + user_id = serializers.IntegerField(required=False) + extra_time_mins = serializers.IntegerField(required=True) # custom fields based on related models - username = serializers.CharField(source='user.username') - exam_name = serializers.CharField(source='exam.exam_name') + username = serializers.CharField(source='user.username', required=False) + exam_name = serializers.CharField(source='exam.exam_name', required=False) + email = serializers.CharField(source='user.email', required=False) + + # add custom validator, serializer should have either username or email + def validate(self, attrs): + """ + Verify that either username or email is provided + """ + if not (attrs.get('user', {}).get('username') or attrs.get('user', {}).get('email')): + raise serializers.ValidationError({'username': 'username or email must be provided'}) + return attrs class Meta: """ @@ -304,5 +314,5 @@ class Meta: model = ExamAttempt fields = ( - 'id', 'exam_id', 'user_id', 'extra_time_mins', 'username', 'exam_name' + 'id', 'exam_id', 'user_id', 'extra_time_mins', 'username', 'exam_name', 'email' ) diff --git a/edx_exams/apps/api/v1/tests/test_views.py b/edx_exams/apps/api/v1/tests/test_views.py index 3a7b088d..e683fa28 100644 --- a/edx_exams/apps/api/v1/tests/test_views.py +++ b/edx_exams/apps/api/v1/tests/test_views.py @@ -17,7 +17,14 @@ from edx_exams.apps.api.test_utils import ExamsAPITestCase from edx_exams.apps.core.exam_types import get_exam_type from edx_exams.apps.core.exceptions import ExamAttemptOnPastDueExam, ExamIllegalStatusTransition -from edx_exams.apps.core.models import CourseExamConfiguration, CourseStaffRole, Exam, ExamAttempt, ProctoringProvider +from edx_exams.apps.core.models import ( + CourseExamConfiguration, + CourseStaffRole, + Exam, + ExamAttempt, + ProctoringProvider, + StudentAllowance +) from edx_exams.apps.core.statuses import ExamAttemptStatus from edx_exams.apps.core.test_utils.factories import ( AssessmentControlResultFactory, @@ -1782,18 +1789,21 @@ def setUp(self): course_id=self.course_id, ) - def request_api(self, method, user, course_id): + def request_api(self, method, user, course_id, data=None): """ Helper function to make API request """ - assert method in ['get'] + assert method in ['get', 'post'] headers = self.build_jwt_headers(user) url = reverse( 'api:v1:course-allowances', kwargs={'course_id': course_id} ) - return getattr(self.client, method)(url, **headers) + if data: + return getattr(self.client, method)(url, json.dumps(data), **headers, content_type='application/json') + else: + return getattr(self.client, method)(url, **headers) def test_auth_required(self): """ @@ -1847,6 +1857,7 @@ def test_get_allowances(self): 'user_id': self.user.id, 'exam_name': self.exam.exam_name, 'username': self.user.username, + 'email': self.user.email, 'extra_time_mins': 30, }) self.assertDictEqual(response.data[1], { @@ -1855,6 +1866,7 @@ def test_get_allowances(self): 'user_id': self.user.id, 'exam_name': other_exam_in_course.exam_name, 'username': self.user.username, + 'email': self.user.email, 'extra_time_mins': 30, }) @@ -1865,3 +1877,62 @@ def test_get_empty_response(self): response = self.request_api('get', self.user, 'course-v1:edx+no+allowances') self.assertEqual(response.status_code, 200) self.assertEqual(response.data, []) + + def test_post_allowances(self): + """ + Test that the endpoint creates allowances for the given request data + """ + other_exam_in_course = ExamFactory.create(course_id=self.exam.course_id) + other_user = UserFactory() + StudentAllowanceFactory.create( + exam=self.exam, + user=self.user, + extra_time_mins=30, + ) + StudentAllowanceFactory.create( + exam=other_exam_in_course, + user=self.user, + extra_time_mins=30, + ) + + request_data = [ + {'exam_id': self.exam.id, 'username': self.user.username, 'extra_time_mins': 45}, + {'exam_id': other_exam_in_course.id, 'username': self.user.username, 'extra_time_mins': 45}, + {'exam_id': other_exam_in_course.id, 'email': other_user.email, 'extra_time_mins': 45}, + ] + + response = self.request_api('post', self.user, self.exam.course_id, data=request_data) + self.assertEqual(response.status_code, 200) + + course_allowances = StudentAllowance.objects.all() + self.assertEqual(len(course_allowances), 3) + + self.assertEqual(len(StudentAllowance.objects.filter(user_id=self.user.id)), 2) + self.assertEqual(len(StudentAllowance.objects.filter(extra_time_mins=45)), 3) + + def test_post_allowances_empty(self): + """ + Test that 400 response is returned for empty list + """ + response = self.request_api('post', self.user, self.exam.course_id, data=[]) + self.assertEqual(response.status_code, 400) + + def test_post_invalid_field_value(self): + """ + Test that 400 response is returned if serializer is invalid due to incorrect field type + """ + request_data = [ + {'exam_id': self.exam.id, 'username': self.user.username, 'extra_time_mins': 'yyyy'}, + ] + response = self.request_api('post', self.user, self.exam.course_id, data=request_data) + self.assertEqual(response.status_code, 400) + + def test_post_invalid_missing_user(self): + """ + Test that 400 response is returned if serializer is invalid due to missing required field + """ + request_data = [ + {'exam_id': self.exam.id, 'extra_time_mins': 45}, + ] + response = self.request_api('post', self.user, self.exam.course_id, data=request_data) + self.assertEqual(response.status_code, 400) diff --git a/edx_exams/apps/api/v1/views.py b/edx_exams/apps/api/v1/views.py index ce0e29f1..8f40e044 100644 --- a/edx_exams/apps/api/v1/views.py +++ b/edx_exams/apps/api/v1/views.py @@ -46,7 +46,14 @@ update_attempt_status ) from edx_exams.apps.core.exam_types import get_exam_type -from edx_exams.apps.core.models import CourseExamConfiguration, Exam, ExamAttempt, ProctoringProvider, StudentAllowance +from edx_exams.apps.core.models import ( + CourseExamConfiguration, + Exam, + ExamAttempt, + ProctoringProvider, + StudentAllowance, + User +) from edx_exams.apps.core.statuses import ExamAttemptStatus from edx_exams.apps.router.interop import get_active_exam_attempt @@ -790,13 +797,25 @@ def get(self, request, course_id, exam_id): class AllowanceView(ExamsAPIView): """ - Endpoint for getting allowances in a course + Endpoint for the StudentAllowance /exams/course_id/{course_id}/allowances Supports: HTTP GET: Returns a list of allowances for a course. + HTTP POST: + Create one or more allowances + + Expected POST data: [{ + "username": "test_user", + "exam_id": 1234, + "extra_time_mins": 30, + }] + **POST data Parameters** + * username OR email: username or email for which to create or update an allowance. + * exam_id: ID of the exam for which to create or update an allowance + * extra_time_mins: Extra time (in minutes) that a student is allotted for an exam. """ authentication_classes = (JwtAuthentication,) @@ -808,3 +827,40 @@ def get(self, request, course_id): """ allowances = StudentAllowance.get_allowances_for_course(course_id) return Response(AllowanceSerializer(allowances, many=True).data) + + def post(self, request, course_id): # pylint: disable=unused-argument + """ + HTTP POST handler. Creates allowances based on the given list. + """ + allowances = request.data + + serializer = AllowanceSerializer(data=allowances, many=True) + + if serializer.is_valid(): + # We expect the number of allowances in each request to be small. Should they increase, + # we should not query within the loop, and instead refactor this to optimize + # the DB calls. + allowance_objects = [ + StudentAllowance( + user=( + User.objects.get(username=allowance['username']) + if allowance.get('username') + else User.objects.get(email=allowance['email']) + ), + exam=Exam.objects.get(id=allowance['exam_id']), + extra_time_mins=allowance['extra_time_mins'] + ) + for allowance in allowances + ] + StudentAllowance.objects.bulk_create( + allowance_objects, + update_conflicts=True, + unique_fields=['user', 'exam'], + update_fields=['extra_time_mins'] + ) + + return Response(status=status.HTTP_200_OK) + else: + response_status = status.HTTP_400_BAD_REQUEST + data = {'detail': 'Invalid data', 'errors': serializer.errors} + return Response(status=response_status, data=data)