fix: Overwrite allowances in special exams instead of creating a new … (#38488)

Author: brianjbuck-wgu

lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py

@@ -12,6 +12,7 @@
     add_allowance_for_user,
     create_exam,
     create_exam_attempt,
+    get_allowances_for_course,
 )
 from edx_proctoring.models import ProctoredExamStudentAttempt
 from rest_framework import status
@@ -428,6 +429,33 @@ def test_update_allowance(self):
         assert response.status_code == status.HTTP_200_OK
         assert response.json()['results'][0]['success'] is True
 
+    def test_grant_allowance_replaces_different_key(self):
+        """Granting an allowance with a different key replaces the existing one (one per user+exam)."""
+        self.client.post(
+            self._url(),
+            data={
+                'user_ids': [self.student.username],
+                'allowance_type': 'additional_time_granted',
+                'value': '30',
+            },
+            format='json',
+        )
+        response = self.client.post(
+            self._url(),
+            data={
+                'user_ids': [self.student.username],
+                'allowance_type': 'review_policy_exception',
+                'value': 'special review',
+            },
+            format='json',
+        )
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()['results'][0]['success'] is True
+        allowances = get_allowances_for_course(self.course_id)
+        user_allowances = [a for a in allowances if a['user']['username'] == self.student.username]
+        assert len(user_allowances) == 1
+        assert user_allowances[0]['key'] == 'review_policy_exception'
+
     def test_delete_allowance(self):
         add_allowance_for_user(self.exam_id, self.student.username, 'additional_time_granted', '30')
         response = self.client.delete(
@@ -548,6 +576,26 @@ def test_bulk_create_allowances(self):
         assert len(data['results']) == 4
         assert all(r['success'] is True for r in data['results'])
 
+    def test_bulk_create_allowances_replaces_different_key(self):
+        """Bulk-creating an allowance with a different key replaces the existing one."""
+        add_allowance_for_user(self.exam_id, self.student.username, 'additional_time_granted', '30')
+        response = self.client.post(
+            self._url(),
+            data={
+                'exam_ids': [self.exam_id],
+                'user_ids': [self.student.username],
+                'allowance_type': 'review_policy_exception',
+                'value': 'special review',
+            },
+            format='json',
+        )
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()['results'][0]['success'] is True
+        allowances = get_allowances_for_course(self.course_id)
+        user_allowances = [a for a in allowances if a['user']['username'] == self.student.username]
+        assert len(user_allowances) == 1
+        assert user_allowances[0]['key'] == 'review_policy_exception'
+
     def test_bulk_create_allowances_missing_fields(self):
         response = self.client.post(
             self._url(),

lms/djangoapps/instructor/views/api_v2.py

@@ -43,6 +43,7 @@
     ProctoredBaseException,
     ProctoredExamNotFoundException,
 )
+from edx_proctoring.models import ProctoredExamStudentAllowance
 from edx_rest_framework_extensions.paginators import DefaultPagination
 from edx_when import api as edx_when_api
 from opaque_keys import InvalidKeyError
@@ -4233,6 +4234,23 @@ def patch(self, request, course_id):
         return Response(serializer.data, status=status.HTTP_200_OK)
 
 
+def add_or_replace_allowance_for_user(exam_id, username_or_email, key, value):
+    """
+    Add an allowance for a user on an exam, removing any existing allowance with a different key.
+
+    Enforces one allowance per user per exam regardless of allowance type. If the user already
+    has an allowance for this exam with a different key, it is removed before the new one is created.
+    """
+    user_id = get_user_by_username_or_email(username_or_email).id
+
+    with transaction.atomic():
+        for allowance in ProctoredExamStudentAllowance.get_allowances_for_user(exam_id, user_id):
+            if allowance.key != key:
+                remove_allowance_for_user(exam_id, user_id, allowance.key)
+
+        add_allowance_for_user(exam_id, username_or_email, key, value)
+
+
 class ExamAllowanceView(DeveloperErrorViewMixin, APIView):
     """
     Grant, update, or remove an allowance for a student on a proctored exam.
@@ -4289,17 +4307,17 @@ def post(self, request, course_id, exam_id):
 
         validated = serializer.validated_data
         results = []
-        for user_info in validated['user_ids']:
+        for username_or_email in validated['user_ids']:
             try:
-                add_allowance_for_user(
+                add_or_replace_allowance_for_user(
                     int(exam_id),
-                    user_info,
+                    username_or_email,
                     validated['allowance_type'],
                     validated['value'],
                 )
-                results.append({'identifier': user_info, 'success': True})
-            except ProctoredBaseException as err:
-                results.append({'identifier': user_info, 'success': False, 'error': str(err)})
+                results.append({'identifier': username_or_email, 'success': True})
+            except (ProctoredBaseException, User.DoesNotExist, User.MultipleObjectsReturned) as err:
+                results.append({'identifier': username_or_email, 'success': False, 'error': str(err)})
 
         return Response(
             {'allowance_type': validated['allowance_type'], 'results': results},
@@ -4471,17 +4489,24 @@ def post(self, request, course_id):
         validated = serializer.validated_data
         results = []
         for exam_id in validated['exam_ids']:
-            for user_info in validated['user_ids']:
+            for username_or_email in validated['user_ids']:
                 try:
-                    add_allowance_for_user(
+                    add_or_replace_allowance_for_user(
                         exam_id,
-                        user_info,
+                        username_or_email,
                         validated['allowance_type'],
                         validated['value'],
                     )
-                    results.append({'identifier': user_info, 'exam_id': exam_id, 'success': True})
-                except ProctoredBaseException as err:
-                    results.append({'identifier': user_info, 'exam_id': exam_id, 'success': False, 'error': str(err)})
+                    results.append({'identifier': username_or_email, 'exam_id': exam_id, 'success': True})
+                except (ProctoredBaseException, User.DoesNotExist, User.MultipleObjectsReturned) as err:
+                    results.append(
+                        {
+                            'identifier': username_or_email,
+                            'exam_id': exam_id,
+                            'success': False,
+                            'error': str(err)
+                        }
+                    )
 
         return Response({
             'allowance_type': validated['allowance_type'],