From 2333eff57e7e55e7b83ff4bf9a2a3b6b06826a36 Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Mon, 27 Apr 2026 09:23:00 -0600 Subject: [PATCH 01/11] feat: Add exam type to special exams endpoints. --- .../tests/views/test_special_exams_api_v2.py | 47 +++++++++++-------- .../instructor/views/serializers_v2.py | 10 ++++ 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py index 73ad265d928d..0c04fc5ff369 100644 --- a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py +++ b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py @@ -215,6 +215,7 @@ def test_reset_no_attempts(self): @override_settings(**PROCTORING_SETTINGS) @patch.dict(settings.FEATURES, {'ENABLE_SPECIAL_EXAMS': True}) +@ddt.ddt class SpecialExamAttemptsViewTest(ModuleStoreTestCase): """Tests for GET /api/instructor/v2/courses/{course_key}/special_exams/{exam_id}/attempts""" @@ -226,46 +227,52 @@ def setUp(self): self.student = UserFactory(username='student1', email='student1@example.com') self.client.force_authenticate(user=self.instructor) self.course_id = str(self.course.id) - self.exam_id = create_exam( + + def _create_exam(self, is_proctored=False, is_practice_exam=False, content_suffix='exam1'): + return create_exam( course_id=self.course_id, - content_id='block-v1:test+test+test+type@sequential+block@exam1', - exam_name='Midterm Exam', + content_id=f'block-v1:test+test+test+type@sequential+block@{content_suffix}', + exam_name='Test Exam', time_limit_mins=60, - is_proctored=False, + is_proctored=is_proctored, + is_practice_exam=is_practice_exam, ) - def _url(self, exam_id=None): + def _url(self, exam_id): return reverse('instructor_api_v2:special_exam_attempts', kwargs={ 'course_id': self.course_id, - 'exam_id': exam_id or self.exam_id, + 'exam_id': exam_id, }) - def test_list_attempts(self): - create_exam_attempt(self.exam_id, self.student.id) - response = self.client.get(self._url()) + @ddt.data( + (False, False, 'timed'), + (True, False, 'proctored'), + (True, True, 'practice'), + ) + @ddt.unpack + def test_attempt_exam_type(self, is_proctored, is_practice_exam, expected_type): + exam_id = self._create_exam(is_proctored=is_proctored, is_practice_exam=is_practice_exam) + create_exam_attempt(exam_id, self.student.id) + response = self.client.get(self._url(exam_id)) assert response.status_code == status.HTTP_200_OK data = response.json() assert data['count'] == 1 - assert data['results'][0]['exam_id'] == self.exam_id + assert data['results'][0]['exam_id'] == exam_id + assert data['results'][0]['exam_type'] == expected_type assert data['results'][0]['user']['username'] == 'student1' def test_list_attempts_filters_by_exam(self): """Only attempts for the requested exam_id are returned.""" - other_exam_id = create_exam( - course_id=self.course_id, - content_id='block-v1:test+test+test+type@sequential+block@exam2', - exam_name='Final Exam', - time_limit_mins=120, - is_proctored=False, - ) - create_exam_attempt(self.exam_id, self.student.id) + exam_id = self._create_exam(content_suffix='exam1') + other_exam_id = self._create_exam(content_suffix='exam2') + create_exam_attempt(exam_id, self.student.id) other_student = UserFactory(username='student2') create_exam_attempt(other_exam_id, other_student.id) - response = self.client.get(self._url()) + response = self.client.get(self._url(exam_id)) data = response.json() assert data['count'] == 1 - assert data['results'][0]['exam_id'] == self.exam_id + assert data['results'][0]['exam_id'] == exam_id @override_settings(**PROCTORING_SETTINGS) diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index 89fa151716c5..703df4c239c0 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -1194,11 +1194,21 @@ class ExamAttemptSerializer(serializers.Serializer): id = serializers.IntegerField() user = ExamAttemptUserSerializer() exam_id = serializers.IntegerField(source='proctored_exam.id') + exam_type = serializers.SerializerMethodField() status = serializers.CharField() start_time = serializers.DateTimeField(source='started_at', allow_null=True, required=False) end_time = serializers.DateTimeField(source='completed_at', allow_null=True, required=False) allowed_time_limit_mins = serializers.IntegerField(allow_null=True, required=False) + def get_exam_type(self, obj): + """Derive exam type from proctored_exam flags.""" + exam = obj.get('proctored_exam', {}) + if exam.get('is_practice_exam'): + return 'practice' + if exam.get('is_proctored'): + return 'proctored' + return 'timed' + class ProctoringSettingsSerializer(serializers.Serializer): """Serializer for course proctoring configuration.""" From eedda0f935fff159a2bdc00c8ce787e060d321ab Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Mon, 27 Apr 2026 10:30:19 -0600 Subject: [PATCH 02/11] fix: Add ordering for special exams list endpoints --- .../tests/views/test_special_exams_api_v2.py | 54 ++++++++++++++ lms/djangoapps/instructor/views/api_v2.py | 71 +++++++++++++++++-- 2 files changed, 121 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py index 0c04fc5ff369..b49443c316ad 100644 --- a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py +++ b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py @@ -466,6 +466,7 @@ def test_delete_allowance_missing_fields(self): @override_settings(**PROCTORING_SETTINGS) @patch.dict(settings.FEATURES, {'ENABLE_SPECIAL_EXAMS': True}) +@ddt.ddt class CourseAllowancesViewTest(ModuleStoreTestCase): """Tests for GET /api/instructor/v2/courses/{course_key}/special_exams/allowances""" @@ -550,9 +551,36 @@ def test_bulk_create_allowances_missing_fields(self): ) assert response.status_code == status.HTTP_400_BAD_REQUEST + @ddt.data('username', 'email', 'exam_name', 'allowance_type', 'value') + def test_sort_allowances(self, ordering): + student2 = UserFactory(username='alice', email='alice@example.com') + exam_id_2 = create_exam( + course_id=self.course_id, + content_id='block-v1:test+test+test+type@sequential+block@exam2', + exam_name='AAA Exam', + time_limit_mins=30, + is_proctored=False, + ) + add_allowance_for_user(self.exam_id, self.student.username, 'additional_time_granted', '30') + add_allowance_for_user(exam_id_2, student2.username, 'review_policy_exception', '60') + response = self.client.get(self._url(), {'ordering': ordering}) + assert response.status_code == status.HTTP_200_OK + assert len(response.json()['results']) == 2 + + def test_sort_allowances_descending(self): + student2 = UserFactory(username='alice', email='alice@example.com') + add_allowance_for_user(self.exam_id, self.student.username, 'additional_time_granted', '30') + add_allowance_for_user(self.exam_id, student2.username, 'additional_time_granted', '60') + response = self.client.get(self._url(), {'ordering': '-username'}) + assert response.status_code == status.HTTP_200_OK + results = response.json()['results'] + assert results[0]['user']['username'] == 'student1' + assert results[1]['user']['username'] == 'alice' + @override_settings(**PROCTORING_SETTINGS) @patch.dict(settings.FEATURES, {'ENABLE_SPECIAL_EXAMS': True}) +@ddt.ddt class CourseExamAttemptsViewTest(ModuleStoreTestCase): """Tests for GET /api/instructor/v2/courses/{course_key}/special_exams/attempts""" @@ -596,3 +624,29 @@ def test_search_attempts_no_match(self): response = self.client.get(self._url(), {'search': 'nonexistent'}) assert response.status_code == status.HTTP_200_OK assert response.json()['count'] == 0 + + @ddt.data('username', 'exam_name', 'time_limit', 'type', 'started_at', 'completed_at', 'status') + def test_sort_attempts(self, ordering): + student2 = UserFactory(username='student2', email='student2@example.com') + exam_id_2 = create_exam( + course_id=self.course_id, + content_id='block-v1:test+test+test+type@sequential+block@exam2', + exam_name='Final Exam', + time_limit_mins=120, + is_proctored=True, + ) + create_exam_attempt(self.exam_id, self.student.id) + create_exam_attempt(exam_id_2, student2.id) + response = self.client.get(self._url(), {'ordering': ordering}) + assert response.status_code == status.HTTP_200_OK + assert len(response.json()['results']) == 2 + + def test_sort_attempts_descending(self): + student2 = UserFactory(username='student2', email='student2@example.com') + create_exam_attempt(self.exam_id, self.student.id) + create_exam_attempt(self.exam_id, student2.id) + response = self.client.get(self._url(), {'ordering': '-username'}) + assert response.status_code == status.HTTP_200_OK + results = response.json()['results'] + assert results[0]['user']['username'] == 'student2' + assert results[1]['user']['username'] == 'student1' diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 0469dc922706..97ebfd38a535 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -4366,6 +4366,38 @@ def delete(self, request, course_id, exam_id): ) +def _sort_in_memory(items, ordering): + """ + Sort a list of dicts by the given ordering param. + + Supports dotted paths (e.g. 'user.username') and descending with '-' prefix. + + Note: Sorting is done in Python because edx_proctoring's API functions + (get_all_exam_attempts, get_allowances_for_course) return pre-serialized + lists of dicts with no sorting parameter. Database-level sorting would + require changes to the edx-proctoring package: + https://github.com/openedx/edx-proctoring/issues/1320 + """ + if not ordering: + return items + descending = ordering.startswith('-') + field = ordering.lstrip('-') + + def sort_key(item): + value = item + for part in field.split('.'): + if isinstance(value, dict): + value = value.get(part) + else: + value = None + break + if value is None: + return '' + return value + + return sorted(items, key=sort_key, reverse=descending) + + class CourseAllowancesView(DeveloperErrorViewMixin, ListAPIView): """ List or bulk-create exam allowances for a course. @@ -4374,6 +4406,7 @@ class CourseAllowancesView(DeveloperErrorViewMixin, ListAPIView): GET /api/instructor/v2/courses/{course_id}/special_exams/allowances GET /api/instructor/v2/courses/{course_id}/special_exams/allowances?search=student1 + GET /api/instructor/v2/courses/{course_id}/special_exams/allowances?ordering=-value POST /api/instructor/v2/courses/{course_id}/special_exams/allowances """ @@ -4381,6 +4414,14 @@ class CourseAllowancesView(DeveloperErrorViewMixin, ListAPIView): permission_name = permissions.EXAM_RESULTS serializer_class = ExamAllowanceSerializer + ORDERING_FIELDS = { + 'username': 'user.username', + 'email': 'user.email', + 'exam_name': 'proctored_exam.exam_name', + 'allowance_type': 'key', + 'value': 'value', + } + def get_queryset(self): course_id = self.kwargs['course_id'] allowances = get_allowances_for_course(course_id) @@ -4391,6 +4432,11 @@ def get_queryset(self): if search in a.get('user', {}).get('username', '').lower() or search in a.get('user', {}).get('email', '').lower() ] + ordering = self.request.query_params.get('ordering', '') + field = ordering.lstrip('-') + if field in self.ORDERING_FIELDS: + prefix = '-' if ordering.startswith('-') else '' + allowances = _sort_in_memory(allowances, prefix + self.ORDERING_FIELDS[field]) return allowances def post(self, request, course_id): @@ -4423,12 +4469,13 @@ def post(self, request, course_id): class CourseExamAttemptsView(DeveloperErrorViewMixin, ListAPIView): """ - List all exam attempts across all exams in a course with optional search and pagination. + List all exam attempts across all exams in a course with optional search, sorting, and pagination. **Example Requests** GET /api/instructor/v2/courses/{course_id}/special_exams/attempts GET /api/instructor/v2/courses/{course_id}/special_exams/attempts?search=student1 + GET /api/instructor/v2/courses/{course_id}/special_exams/attempts?ordering=-started_at GET /api/instructor/v2/courses/{course_id}/special_exams/attempts?page=2&page_size=50 """ @@ -4436,10 +4483,26 @@ class CourseExamAttemptsView(DeveloperErrorViewMixin, ListAPIView): permission_name = permissions.EXAM_RESULTS serializer_class = ExamAttemptSerializer + ORDERING_FIELDS = { + 'username': 'user.username', + 'exam_name': 'proctored_exam.exam_name', + 'time_limit': 'proctored_exam.time_limit_mins', + 'type': 'proctored_exam.is_proctored', + 'started_at': 'started_at', + 'completed_at': 'completed_at', + 'status': 'status', + } + def get_queryset(self): course_id = self.kwargs['course_id'] search = self.request.query_params.get('search', '').strip() if search: - # get_filtered_exam_attempts does server-side filtering by username/email - return get_filtered_exam_attempts(course_id, search) - return get_all_exam_attempts(course_id) + attempts = get_filtered_exam_attempts(course_id, search) + else: + attempts = get_all_exam_attempts(course_id) + ordering = self.request.query_params.get('ordering', '') + field = ordering.lstrip('-') + if field in self.ORDERING_FIELDS: + prefix = '-' if ordering.startswith('-') else '' + attempts = _sort_in_memory(attempts, prefix + self.ORDERING_FIELDS[field]) + return attempts From 109d189af282e987ee48d8db6b689823a69951b3 Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Mon, 27 Apr 2026 10:53:34 -0600 Subject: [PATCH 03/11] fix: Allow sending blank reason in change course endpoint. --- lms/djangoapps/instructor/tests/test_api.py | 18 ++++++++++++++++++ .../instructor/views/serializers_v2.py | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 869ea0235a1f..c23793b82099 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -4581,6 +4581,24 @@ def test_change_due_date_v2_success(self): assert get_extended_due(self.course, self.homework, self.user1) == due_date + def test_change_due_date_v2_without_reason(self): + """Test that reason is optional — both omitted and blank are accepted.""" + url = reverse('instructor_api_v2:change_due_date', kwargs={'course_id': str(self.course.id)}) + base_payload = { + 'email_or_username': self.user1.username, + 'block_id': str(self.homework.location), + 'due_datetime': '12/30/2013 00:00', + } + # Omitted reason + response = self.client.post(url, json.dumps(base_payload), content_type='application/json') + assert response.status_code == 200, response.content + + # Blank reason + response = self.client.post( + url, json.dumps({**base_payload, 'reason': ''}), content_type='application/json' + ) + assert response.status_code == 200, response.content + def test_change_due_date_v2_with_email(self): """Test due date change using email instead of username""" url = reverse('instructor_api_v2:change_due_date', kwargs={'course_id': str(self.course.id)}) diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index 703df4c239c0..b9d2b55e0f22 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -530,7 +530,7 @@ class BlockDueDateSerializerV2(serializers.Serializer): block_id (str): The ID related to the block that needs the due date update. due_datetime (str): The new due date and time for the block. email_or_username (str): The email or username of the student whose access is being modified. - reason (str): Reason why updating this. + reason (str, optional): Reason why updating this. """ block_id = serializers.CharField() due_datetime = serializers.CharField() @@ -538,7 +538,7 @@ class BlockDueDateSerializerV2(serializers.Serializer): max_length=255, help_text="Email or username of user to change access" ) - reason = serializers.CharField(required=False) + reason = serializers.CharField(required=False, allow_blank=True, default='') def validate_email_or_username(self, value): """ From aa8bf4b51ab4de4eaa712f466c3cddffcb0f5f89 Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Mon, 27 Apr 2026 11:26:22 -0600 Subject: [PATCH 04/11] fix: Address Copilot feedback --- .../tests/views/test_special_exams_api_v2.py | 22 ++++++++++++++----- lms/djangoapps/instructor/views/api_v2.py | 6 ++++- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py index b49443c316ad..538f7ce2c877 100644 --- a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py +++ b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py @@ -553,6 +553,7 @@ def test_bulk_create_allowances_missing_fields(self): @ddt.data('username', 'email', 'exam_name', 'allowance_type', 'value') def test_sort_allowances(self, ordering): + """Verify all ordering fields are accepted and reverse correctly.""" student2 = UserFactory(username='alice', email='alice@example.com') exam_id_2 = create_exam( course_id=self.course_id, @@ -563,9 +564,15 @@ def test_sort_allowances(self, ordering): ) add_allowance_for_user(self.exam_id, self.student.username, 'additional_time_granted', '30') add_allowance_for_user(exam_id_2, student2.username, 'review_policy_exception', '60') - response = self.client.get(self._url(), {'ordering': ordering}) - assert response.status_code == status.HTTP_200_OK - assert len(response.json()['results']) == 2 + asc_response = self.client.get(self._url(), {'ordering': ordering}) + desc_response = self.client.get(self._url(), {'ordering': f'-{ordering}'}) + assert asc_response.status_code == status.HTTP_200_OK + asc_results = asc_response.json()['results'] + desc_results = desc_response.json()['results'] + assert len(asc_results) == 2 + # All allowance fields differ between the two records, so order must reverse + assert asc_results[0] == desc_results[1] + assert asc_results[1] == desc_results[0] def test_sort_allowances_descending(self): student2 = UserFactory(username='alice', email='alice@example.com') @@ -627,6 +634,7 @@ def test_search_attempts_no_match(self): @ddt.data('username', 'exam_name', 'time_limit', 'type', 'started_at', 'completed_at', 'status') def test_sort_attempts(self, ordering): + """Verify all ordering fields are accepted and return correct results.""" student2 = UserFactory(username='student2', email='student2@example.com') exam_id_2 = create_exam( course_id=self.course_id, @@ -637,9 +645,11 @@ def test_sort_attempts(self, ordering): ) create_exam_attempt(self.exam_id, self.student.id) create_exam_attempt(exam_id_2, student2.id) - response = self.client.get(self._url(), {'ordering': ordering}) - assert response.status_code == status.HTTP_200_OK - assert len(response.json()['results']) == 2 + # Verify ascending and descending both succeed + for prefix in ('', '-'): + response = self.client.get(self._url(), {'ordering': f'{prefix}{ordering}'}) + assert response.status_code == status.HTTP_200_OK + assert len(response.json()['results']) == 2 def test_sort_attempts_descending(self): student2 = UserFactory(username='student2', email='student2@example.com') diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 97ebfd38a535..ee203f46b54a 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -4391,9 +4391,13 @@ def sort_key(item): else: value = None break + # Return a type-consistent default so sorted() doesn't raise + # TypeError when comparing None with str/int/datetime values. if value is None: return '' - return value + if isinstance(value, bool): + return int(value) + return str(value) return sorted(items, key=sort_key, reverse=descending) From 02b4400663081c6ab6dade9773eaea80cf76bfc7 Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Mon, 27 Apr 2026 11:45:41 -0600 Subject: [PATCH 05/11] fix: More Copilot feedback --- lms/djangoapps/instructor/views/api_v2.py | 41 ++++++++++++++++++----- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index ee203f46b54a..16bf84048c33 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -4391,13 +4391,11 @@ def sort_key(item): else: value = None break - # Return a type-consistent default so sorted() doesn't raise - # TypeError when comparing None with str/int/datetime values. + # Return a tuple (is_none, value) so None values sort last + # and non-None values compare naturally within their type. if value is None: - return '' - if isinstance(value, bool): - return int(value) - return str(value) + return (1, '') + return (0, value) return sorted(items, key=sort_key, reverse=descending) @@ -4412,6 +4410,14 @@ class CourseAllowancesView(DeveloperErrorViewMixin, ListAPIView): GET /api/instructor/v2/courses/{course_id}/special_exams/allowances?search=student1 GET /api/instructor/v2/courses/{course_id}/special_exams/allowances?ordering=-value POST /api/instructor/v2/courses/{course_id}/special_exams/allowances + + **Query Parameters** + + search (optional): Filter by username or email. + ordering (optional): Sort by field. Prefix with '-' for descending. + Valid values: username, email, exam_name, allowance_type, value. + page (optional): Page number for pagination. + page_size (optional): Number of results per page. """ permission_classes = (IsAuthenticated, permissions.InstructorPermission) @@ -4481,6 +4487,14 @@ class CourseExamAttemptsView(DeveloperErrorViewMixin, ListAPIView): GET /api/instructor/v2/courses/{course_id}/special_exams/attempts?search=student1 GET /api/instructor/v2/courses/{course_id}/special_exams/attempts?ordering=-started_at GET /api/instructor/v2/courses/{course_id}/special_exams/attempts?page=2&page_size=50 + + **Query Parameters** + + search (optional): Filter by username or email. + ordering (optional): Sort by field. Prefix with '-' for descending. + Valid values: username, exam_name, time_limit, type, started_at, completed_at, status. + page (optional): Page number for pagination. + page_size (optional): Number of results per page. """ permission_classes = (IsAuthenticated, permissions.InstructorPermission) @@ -4491,12 +4505,20 @@ class CourseExamAttemptsView(DeveloperErrorViewMixin, ListAPIView): 'username': 'user.username', 'exam_name': 'proctored_exam.exam_name', 'time_limit': 'proctored_exam.time_limit_mins', - 'type': 'proctored_exam.is_proctored', 'started_at': 'started_at', 'completed_at': 'completed_at', 'status': 'status', } + def _get_exam_type(self, attempt): + """Derive exam type string for sorting purposes.""" + exam = attempt.get('proctored_exam', {}) + if exam.get('is_practice_exam'): + return 'practice' + if exam.get('is_proctored'): + return 'proctored' + return 'timed' + def get_queryset(self): course_id = self.kwargs['course_id'] search = self.request.query_params.get('search', '').strip() @@ -4506,7 +4528,10 @@ def get_queryset(self): attempts = get_all_exam_attempts(course_id) ordering = self.request.query_params.get('ordering', '') field = ordering.lstrip('-') - if field in self.ORDERING_FIELDS: + if field == 'type': + descending = ordering.startswith('-') + attempts = sorted(attempts, key=self._get_exam_type, reverse=descending) + elif field in self.ORDERING_FIELDS: prefix = '-' if ordering.startswith('-') else '' attempts = _sort_in_memory(attempts, prefix + self.ORDERING_FIELDS[field]) return attempts From 2c72798036133cad429ebe8f2b524cebdca622b3 Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Tue, 28 Apr 2026 10:26:08 -0600 Subject: [PATCH 06/11] fix: Add username to the course metadata endpoint --- lms/djangoapps/instructor/tests/test_api_v2.py | 4 ++++ lms/djangoapps/instructor/views/serializers_v2.py | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/lms/djangoapps/instructor/tests/test_api_v2.py b/lms/djangoapps/instructor/tests/test_api_v2.py index 84acf396b6b9..45df3b108c21 100644 --- a/lms/djangoapps/instructor/tests/test_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_api_v2.py @@ -177,6 +177,9 @@ def test_get_course_metadata_as_instructor(self): assert 'studio_grading_url' in data assert 'admin_console_url' in data + # Verify current user's username is returned + assert data['username'] == self.instructor.username + assert data['studio_grading_url'] == f'http://localhost:2001/authoring/course/{self.course.id}/settings/grading' assert data['admin_console_url'] == 'http://localhost:2025/admin-console/authz' @@ -220,6 +223,7 @@ def test_get_course_metadata_as_staff(self): self.assertIn('permissions', data) # noqa: PT009 # Staff should have staff permission self.assertTrue(data['permissions']['staff']) # noqa: PT009 + assert data['username'] == self.staff.username def test_get_course_metadata_unauthorized(self): """ diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index b9d2b55e0f22..e6c9c351a4ca 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -51,6 +51,7 @@ class CourseInformationSerializerV2(serializers.Serializer): enrollment statistics, permissions, and dashboard configuration. """ course_id = serializers.SerializerMethodField(help_text="Course run key") + username = serializers.SerializerMethodField(help_text="Username of the current authenticated user") display_name = serializers.SerializerMethodField(help_text="Course display name") org = serializers.SerializerMethodField(help_text="Organization identifier") course_number = serializers.SerializerMethodField(help_text="Course number") @@ -328,6 +329,10 @@ def get_course_id(self, data): """Get course ID as string.""" return str(data['course'].id) + def get_username(self, data): + """Get the username of the current authenticated user.""" + return data['user'].username + def get_display_name(self, data): """Get course display name.""" return data['course'].display_name From 3710e50558050d7b649920bea05aadc00e307a2a Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Tue, 28 Apr 2026 10:26:28 -0600 Subject: [PATCH 07/11] fix: Convert PT009 warnings in tests. --- lms/djangoapps/instructor/tests/test_api_v2.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api_v2.py b/lms/djangoapps/instructor/tests/test_api_v2.py index 45df3b108c21..6f52a41ddd35 100644 --- a/lms/djangoapps/instructor/tests/test_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_api_v2.py @@ -217,12 +217,12 @@ def test_get_course_metadata_as_staff(self): self.client.force_authenticate(user=self.staff) response = self.client.get(self._get_url()) - self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 + assert response.status_code == status.HTTP_200_OK data = response.data - self.assertEqual(data['course_id'], str(self.course_key)) # noqa: PT009 - self.assertIn('permissions', data) # noqa: PT009 + assert data['course_id'] == str(self.course_key) + assert 'permissions' in data # Staff should have staff permission - self.assertTrue(data['permissions']['staff']) # noqa: PT009 + assert data['permissions']['staff'] is True assert data['username'] == self.staff.username def test_get_course_metadata_unauthorized(self): From b8a53d2bfbe6ab2b874ba5c3adf74063bcc20c76 Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Wed, 29 Apr 2026 09:46:57 -0600 Subject: [PATCH 08/11] fix: Allow for dotted lookups in order by --- .../tests/views/test_special_exams_api_v2.py | 27 +++++++++++++++++-- lms/djangoapps/instructor/views/api_v2.py | 9 +++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py index 538f7ce2c877..ffeeb496050d 100644 --- a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py +++ b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py @@ -551,7 +551,17 @@ def test_bulk_create_allowances_missing_fields(self): ) assert response.status_code == status.HTTP_400_BAD_REQUEST - @ddt.data('username', 'email', 'exam_name', 'allowance_type', 'value') + @ddt.data( + 'username', + 'user.username', + 'email', + 'user.email', + 'exam_name', + 'proctored_exam.exam_name', + 'allowance_type', + 'key', + 'value', + ) def test_sort_allowances(self, ordering): """Verify all ordering fields are accepted and reverse correctly.""" student2 = UserFactory(username='alice', email='alice@example.com') @@ -632,7 +642,20 @@ def test_search_attempts_no_match(self): assert response.status_code == status.HTTP_200_OK assert response.json()['count'] == 0 - @ddt.data('username', 'exam_name', 'time_limit', 'type', 'started_at', 'completed_at', 'status') + @ddt.data( + 'username', + 'user.username', + 'email', + 'user.email', + 'exam_name', + 'proctored_exam.exam_name', + 'time_limit', + 'proctored_exam.time_limit_mins', + 'type', + 'started_at', + 'completed_at', + 'status', + ) def test_sort_attempts(self, ordering): """Verify all ordering fields are accepted and return correct results.""" student2 = UserFactory(username='student2', email='student2@example.com') diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 16bf84048c33..54b757ff195e 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -4426,9 +4426,13 @@ class CourseAllowancesView(DeveloperErrorViewMixin, ListAPIView): ORDERING_FIELDS = { 'username': 'user.username', + 'user.username': 'user.username', 'email': 'user.email', + 'user.email': 'user.email', 'exam_name': 'proctored_exam.exam_name', + 'proctored_exam.exam_name': 'proctored_exam.exam_name', 'allowance_type': 'key', + 'key': 'key', 'value': 'value', } @@ -4503,8 +4507,13 @@ class CourseExamAttemptsView(DeveloperErrorViewMixin, ListAPIView): ORDERING_FIELDS = { 'username': 'user.username', + 'user.username': 'user.username', + 'email': 'user.email', + 'user.email': 'user.email', 'exam_name': 'proctored_exam.exam_name', + 'proctored_exam.exam_name': 'proctored_exam.exam_name', 'time_limit': 'proctored_exam.time_limit_mins', + 'proctored_exam.time_limit_mins': 'proctored_exam.time_limit_mins', 'started_at': 'started_at', 'completed_at': 'completed_at', 'status': 'status', From a815adcf3d658820bec8ea98bfc5aa2bceac436e Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Wed, 29 Apr 2026 15:13:02 -0600 Subject: [PATCH 09/11] fix: Fix exam_type coming over the wire. --- .../tests/views/test_special_exams_api_v2.py | 3 +++ lms/djangoapps/instructor/views/serializers_v2.py | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py index ffeeb496050d..daec8c2e8d32 100644 --- a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py +++ b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py @@ -84,18 +84,21 @@ def test_list_exams(self): assert timed['is_practice_exam'] is False assert timed['is_active'] is True assert timed['hide_after_due'] is False + assert timed['exam_type'] == 'timed' proctored = exams_by_name['Proctored Exam'] assert proctored['id'] == self.proctored_exam_id assert proctored['time_limit_mins'] == 90 assert proctored['is_proctored'] is True assert proctored['is_practice_exam'] is False + assert proctored['exam_type'] == 'proctored' practice = exams_by_name['Practice Exam'] assert practice['id'] == self.practice_exam_id assert practice['time_limit_mins'] == 30 assert practice['is_proctored'] is True assert practice['is_practice_exam'] is True + assert practice['exam_type'] == 'practice' def test_unauthenticated(self): self.client.force_authenticate(user=None) diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index e6c9c351a4ca..1ecef007e9f0 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -1179,13 +1179,21 @@ class SpecialExamSerializer(serializers.Serializer): exam_name = serializers.CharField() time_limit_mins = serializers.IntegerField() due_date = serializers.DateTimeField(allow_null=True, required=False) - exam_type = serializers.CharField(required=False, default='') + exam_type = serializers.SerializerMethodField() is_proctored = serializers.BooleanField() is_practice_exam = serializers.BooleanField() is_active = serializers.BooleanField() hide_after_due = serializers.BooleanField() backend = serializers.CharField(allow_null=True, required=False) + def get_exam_type(self, obj): + """Derive exam type from proctoring flags.""" + if obj.get('is_practice_exam'): + return 'practice' + if obj.get('is_proctored'): + return 'proctored' + return 'timed' + class ExamAttemptUserSerializer(serializers.Serializer): """Serializer for user info within an exam attempt.""" From b9a6e0624a26f7699a39ecda997b88dd0caf0ae4 Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Thu, 30 Apr 2026 09:49:38 -0600 Subject: [PATCH 10/11] fix: Refactor PR feedback --- .../tests/views/test_special_exams_api_v2.py | 15 ++++++---- lms/djangoapps/instructor/views/api_v2.py | 13 ++++---- .../instructor/views/serializers_v2.py | 30 ++++++++++++------- 3 files changed, 35 insertions(+), 23 deletions(-) diff --git a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py index daec8c2e8d32..c79c760a3b32 100644 --- a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py +++ b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py @@ -671,11 +671,16 @@ def test_sort_attempts(self, ordering): ) create_exam_attempt(self.exam_id, self.student.id) create_exam_attempt(exam_id_2, student2.id) - # Verify ascending and descending both succeed - for prefix in ('', '-'): - response = self.client.get(self._url(), {'ordering': f'{prefix}{ordering}'}) - assert response.status_code == status.HTTP_200_OK - assert len(response.json()['results']) == 2 + # Verify ascending and descending return reversed order + asc_response = self.client.get(self._url(), {'ordering': ordering}) + desc_response = self.client.get(self._url(), {'ordering': f'-{ordering}'}) + assert asc_response.status_code == status.HTTP_200_OK + assert desc_response.status_code == status.HTTP_200_OK + asc_results = asc_response.json()['results'] + desc_results = desc_response.json()['results'] + assert len(asc_results) == 2 + assert asc_results[0] == desc_results[1] + assert asc_results[1] == desc_results[0] def test_sort_attempts_descending(self): student2 = UserFactory(username='student2', email='student2@example.com') diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 54b757ff195e..f0cedd6b1ae2 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -160,6 +160,7 @@ TaskStatusSerializer, ToggleCertificateGenerationSerializer, UnitExtensionSerializer, + derive_exam_type, ) from .tools import find_unit, get_units_with_due_date, keep_field_private, set_due_date_extension, title_or_url @@ -4515,18 +4516,16 @@ class CourseExamAttemptsView(DeveloperErrorViewMixin, ListAPIView): 'time_limit': 'proctored_exam.time_limit_mins', 'proctored_exam.time_limit_mins': 'proctored_exam.time_limit_mins', 'started_at': 'started_at', + 'start_time': 'started_at', 'completed_at': 'completed_at', + 'end_time': 'completed_at', 'status': 'status', } - def _get_exam_type(self, attempt): + @staticmethod + def _get_exam_type(attempt): """Derive exam type string for sorting purposes.""" - exam = attempt.get('proctored_exam', {}) - if exam.get('is_practice_exam'): - return 'practice' - if exam.get('is_proctored'): - return 'proctored' - return 'timed' + return derive_exam_type(attempt.get('proctored_exam', {})) def get_queryset(self): course_id = self.kwargs['course_id'] diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index 1ecef007e9f0..fbbadc57b970 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -1171,6 +1171,23 @@ def to_internal_value(self, data): return super().to_internal_value(data) +def derive_exam_type(exam_dict): + """ + Derive exam type string from proctoring flags. + + Args: + exam_dict: dict with 'is_proctored' and 'is_practice_exam' keys. + + Returns: + 'practice', 'proctored', or 'timed'. + """ + if exam_dict.get('is_practice_exam'): + return 'practice' + if exam_dict.get('is_proctored'): + return 'proctored' + return 'timed' + + class SpecialExamSerializer(serializers.Serializer): """Serializer for proctored/timed exam data from edx_proctoring.""" id = serializers.IntegerField() @@ -1188,11 +1205,7 @@ class SpecialExamSerializer(serializers.Serializer): def get_exam_type(self, obj): """Derive exam type from proctoring flags.""" - if obj.get('is_practice_exam'): - return 'practice' - if obj.get('is_proctored'): - return 'proctored' - return 'timed' + return derive_exam_type(obj) class ExamAttemptUserSerializer(serializers.Serializer): @@ -1215,12 +1228,7 @@ class ExamAttemptSerializer(serializers.Serializer): def get_exam_type(self, obj): """Derive exam type from proctored_exam flags.""" - exam = obj.get('proctored_exam', {}) - if exam.get('is_practice_exam'): - return 'practice' - if exam.get('is_proctored'): - return 'proctored' - return 'timed' + return derive_exam_type(obj.get('proctored_exam', {})) class ProctoringSettingsSerializer(serializers.Serializer): From fd5fda55c828a3049efbb382be621f07c311171d Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Thu, 30 Apr 2026 10:43:42 -0600 Subject: [PATCH 11/11] fix: Make sorting tests more complete. --- .../tests/views/test_special_exams_api_v2.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py index c79c760a3b32..699da0b9590c 100644 --- a/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py +++ b/lms/djangoapps/instructor/tests/views/test_special_exams_api_v2.py @@ -7,11 +7,13 @@ from django.conf import settings from django.test.utils import override_settings from django.urls import reverse +from django.utils import timezone from edx_proctoring.api import ( add_allowance_for_user, create_exam, create_exam_attempt, ) +from edx_proctoring.models import ProctoredExamStudentAttempt from rest_framework import status from rest_framework.test import APIClient @@ -656,11 +658,13 @@ def test_search_attempts_no_match(self): 'proctored_exam.time_limit_mins', 'type', 'started_at', + 'start_time', 'completed_at', + 'end_time', 'status', ) def test_sort_attempts(self, ordering): - """Verify all ordering fields are accepted and return correct results.""" + """Verify all ordering fields produce reversed results for asc vs desc.""" student2 = UserFactory(username='student2', email='student2@example.com') exam_id_2 = create_exam( course_id=self.course_id, @@ -669,9 +673,16 @@ def test_sort_attempts(self, ordering): time_limit_mins=120, is_proctored=True, ) - create_exam_attempt(self.exam_id, self.student.id) + attempt_id_1 = create_exam_attempt(self.exam_id, self.student.id) create_exam_attempt(exam_id_2, student2.id) - # Verify ascending and descending return reversed order + + # Give attempt 1 a distinct completed_at and status so all fields differ + attempt = ProctoredExamStudentAttempt.objects.get(id=attempt_id_1) + attempt.started_at = timezone.now() - timezone.timedelta(hours=1) + attempt.completed_at = timezone.now() + attempt.status = 'submitted' + attempt.save() + asc_response = self.client.get(self._url(), {'ordering': ordering}) desc_response = self.client.get(self._url(), {'ordering': f'-{ordering}'}) assert asc_response.status_code == status.HTTP_200_OK