diff --git a/edx_exams/apps/api/v1/tests/test_views.py b/edx_exams/apps/api/v1/tests/test_views.py index e3f0b2ce..6be0f561 100644 --- a/edx_exams/apps/api/v1/tests/test_views.py +++ b/edx_exams/apps/api/v1/tests/test_views.py @@ -1900,6 +1900,18 @@ def test_delete_not_found(self): response = self.request_api('delete', self.user, self.exam.course_id, allowance_id=19) self.assertEqual(response.status_code, 404) + def test_delete_not_allowed(self): + """ + Test that 404 is returned if the allowance is not for the authorized course + """ + other_course_id = 'course-v1:edx+another+course' + allowance = StudentAllowanceFactory.create( + exam=ExamFactory.create(course_id=other_course_id), + user=self.user, + ) + response = self.request_api('delete', self.user, self.exam.course_id, allowance_id=allowance.id) + self.assertEqual(response.status_code, 404) + def test_post_allowances(self): """ Test that the endpoint creates allowances for the given request data @@ -1958,3 +1970,18 @@ def test_post_invalid_missing_user(self): ] response = self.request_api('post', self.user, self.exam.course_id, data=request_data) self.assertEqual(response.status_code, 400) + + def test_post_unauthorized_course(self): + """ + Test that an error is returned when atttempting to create an allowance for + an exam in a different course. + """ + other_course_id = 'course-v1:edx+another+course' + other_course_exam = ExamFactory.create(course_id=other_course_id) + + request_data = [ + {'exam_id': self.exam.id, 'username': self.user.username, 'extra_time_mins': 45}, + {'exam_id': other_course_exam.id, 'username': self.user.username, '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 00645bb5..20451c05 100644 --- a/edx_exams/apps/api/v1/views.py +++ b/edx_exams/apps/api/v1/views.py @@ -828,14 +828,12 @@ def get(self, request, course_id): allowances = StudentAllowance.get_allowances_for_course(course_id) return Response(AllowanceSerializer(allowances, many=True).data) - def delete(self, request, course_id, allowance_id): # pylint: disable=unused-argument + def delete(self, request, course_id, allowance_id): """ HTTP DELETE handler. Deletes all allowances for a course. - - TODO: both this and the POST endpoint should be limiting operations by course_id """ try: - StudentAllowance.objects.get(id=allowance_id).delete() + StudentAllowance.objects.get(id=allowance_id, exam__course_id=course_id).delete() except StudentAllowance.DoesNotExist: return Response( status=status.HTTP_404_NOT_FOUND, @@ -844,7 +842,7 @@ def delete(self, request, course_id, allowance_id): # pylint: disable=unused-ar return Response(status=status.HTTP_204_NO_CONTENT) - def post(self, request, course_id): # pylint: disable=unused-argument + def post(self, request, course_id): """ HTTP POST handler. Creates allowances based on the given list. """ @@ -856,18 +854,25 @@ def post(self, request, course_id): # pylint: disable=unused-argument # 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'] + try: + 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'], course_id=course_id), + extra_time_mins=allowance['extra_time_mins'] + ) + for allowance in allowances + ] + except Exam.DoesNotExist: + return Response( + status=status.HTTP_400_BAD_REQUEST, + data={'detail': 'Exam does not exist'} ) - for allowance in allowances - ] + StudentAllowance.objects.bulk_create( allowance_objects, update_conflicts=True,