Skip to content

Commit bb67094

Browse files
feat: add ordering, exam_type, and username to instructor API v2 endpoints (#38458)
- Support ?ordering= query param on special exam allowances and attempts list endpoints via in-memory sort (edx-proctoring returns plain lists, not querysets; see openedx/edx-proctoring#1320) - Add derived exam_type field (timed/proctored/practice) to special exam and attempt serializer responses - Return authenticated user's username in course metadata response - Make reason field optional and blank-safe on due date extension endpoint (BlockDueDateSerializerV2)
1 parent c53db77 commit bb67094

5 files changed

Lines changed: 297 additions & 31 deletions

File tree

lms/djangoapps/instructor/tests/test_api.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4581,6 +4581,24 @@ def test_change_due_date_v2_success(self):
45814581

45824582
assert get_extended_due(self.course, self.homework, self.user1) == due_date
45834583

4584+
def test_change_due_date_v2_without_reason(self):
4585+
"""Test that reason is optional — both omitted and blank are accepted."""
4586+
url = reverse('instructor_api_v2:change_due_date', kwargs={'course_id': str(self.course.id)})
4587+
base_payload = {
4588+
'email_or_username': self.user1.username,
4589+
'block_id': str(self.homework.location),
4590+
'due_datetime': '12/30/2013 00:00',
4591+
}
4592+
# Omitted reason
4593+
response = self.client.post(url, json.dumps(base_payload), content_type='application/json')
4594+
assert response.status_code == 200, response.content
4595+
4596+
# Blank reason
4597+
response = self.client.post(
4598+
url, json.dumps({**base_payload, 'reason': ''}), content_type='application/json'
4599+
)
4600+
assert response.status_code == 200, response.content
4601+
45844602
def test_change_due_date_v2_with_email(self):
45854603
"""Test due date change using email instead of username"""
45864604
url = reverse('instructor_api_v2:change_due_date', kwargs={'course_id': str(self.course.id)})

lms/djangoapps/instructor/tests/test_api_v2.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,9 @@ def test_get_course_metadata_as_instructor(self):
177177
assert 'studio_grading_url' in data
178178
assert 'admin_console_url' in data
179179

180+
# Verify current user's username is returned
181+
assert data['username'] == self.instructor.username
182+
180183
assert data['studio_grading_url'] == f'http://localhost:2001/authoring/course/{self.course.id}/settings/grading'
181184
assert data['admin_console_url'] == 'http://localhost:2025/admin-console/authz'
182185

@@ -214,12 +217,13 @@ def test_get_course_metadata_as_staff(self):
214217
self.client.force_authenticate(user=self.staff)
215218
response = self.client.get(self._get_url())
216219

217-
self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009
220+
assert response.status_code == status.HTTP_200_OK
218221
data = response.data
219-
self.assertEqual(data['course_id'], str(self.course_key)) # noqa: PT009
220-
self.assertIn('permissions', data) # noqa: PT009
222+
assert data['course_id'] == str(self.course_key)
223+
assert 'permissions' in data
221224
# Staff should have staff permission
222-
self.assertTrue(data['permissions']['staff']) # noqa: PT009
225+
assert data['permissions']['staff'] is True
226+
assert data['username'] == self.staff.username
223227

224228
def test_get_course_metadata_unauthorized(self):
225229
"""

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

Lines changed: 133 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
from django.conf import settings
88
from django.test.utils import override_settings
99
from django.urls import reverse
10+
from django.utils import timezone
1011
from edx_proctoring.api import (
1112
add_allowance_for_user,
1213
create_exam,
1314
create_exam_attempt,
1415
)
16+
from edx_proctoring.models import ProctoredExamStudentAttempt
1517
from rest_framework import status
1618
from rest_framework.test import APIClient
1719

@@ -84,18 +86,21 @@ def test_list_exams(self):
8486
assert timed['is_practice_exam'] is False
8587
assert timed['is_active'] is True
8688
assert timed['hide_after_due'] is False
89+
assert timed['exam_type'] == 'timed'
8790

8891
proctored = exams_by_name['Proctored Exam']
8992
assert proctored['id'] == self.proctored_exam_id
9093
assert proctored['time_limit_mins'] == 90
9194
assert proctored['is_proctored'] is True
9295
assert proctored['is_practice_exam'] is False
96+
assert proctored['exam_type'] == 'proctored'
9397

9498
practice = exams_by_name['Practice Exam']
9599
assert practice['id'] == self.practice_exam_id
96100
assert practice['time_limit_mins'] == 30
97101
assert practice['is_proctored'] is True
98102
assert practice['is_practice_exam'] is True
103+
assert practice['exam_type'] == 'practice'
99104

100105
def test_unauthenticated(self):
101106
self.client.force_authenticate(user=None)
@@ -215,6 +220,7 @@ def test_reset_no_attempts(self):
215220

216221
@override_settings(**PROCTORING_SETTINGS)
217222
@patch.dict(settings.FEATURES, {'ENABLE_SPECIAL_EXAMS': True})
223+
@ddt.ddt
218224
class SpecialExamAttemptsViewTest(ModuleStoreTestCase):
219225
"""Tests for GET /api/instructor/v2/courses/{course_key}/special_exams/{exam_id}/attempts"""
220226

@@ -226,46 +232,52 @@ def setUp(self):
226232
self.student = UserFactory(username='student1', email='student1@example.com')
227233
self.client.force_authenticate(user=self.instructor)
228234
self.course_id = str(self.course.id)
229-
self.exam_id = create_exam(
235+
236+
def _create_exam(self, is_proctored=False, is_practice_exam=False, content_suffix='exam1'):
237+
return create_exam(
230238
course_id=self.course_id,
231-
content_id='block-v1:test+test+test+type@sequential+block@exam1',
232-
exam_name='Midterm Exam',
239+
content_id=f'block-v1:test+test+test+type@sequential+block@{content_suffix}',
240+
exam_name='Test Exam',
233241
time_limit_mins=60,
234-
is_proctored=False,
242+
is_proctored=is_proctored,
243+
is_practice_exam=is_practice_exam,
235244
)
236245

237-
def _url(self, exam_id=None):
246+
def _url(self, exam_id):
238247
return reverse('instructor_api_v2:special_exam_attempts', kwargs={
239248
'course_id': self.course_id,
240-
'exam_id': exam_id or self.exam_id,
249+
'exam_id': exam_id,
241250
})
242251

243-
def test_list_attempts(self):
244-
create_exam_attempt(self.exam_id, self.student.id)
245-
response = self.client.get(self._url())
252+
@ddt.data(
253+
(False, False, 'timed'),
254+
(True, False, 'proctored'),
255+
(True, True, 'practice'),
256+
)
257+
@ddt.unpack
258+
def test_attempt_exam_type(self, is_proctored, is_practice_exam, expected_type):
259+
exam_id = self._create_exam(is_proctored=is_proctored, is_practice_exam=is_practice_exam)
260+
create_exam_attempt(exam_id, self.student.id)
261+
response = self.client.get(self._url(exam_id))
246262
assert response.status_code == status.HTTP_200_OK
247263
data = response.json()
248264
assert data['count'] == 1
249-
assert data['results'][0]['exam_id'] == self.exam_id
265+
assert data['results'][0]['exam_id'] == exam_id
266+
assert data['results'][0]['exam_type'] == expected_type
250267
assert data['results'][0]['user']['username'] == 'student1'
251268

252269
def test_list_attempts_filters_by_exam(self):
253270
"""Only attempts for the requested exam_id are returned."""
254-
other_exam_id = create_exam(
255-
course_id=self.course_id,
256-
content_id='block-v1:test+test+test+type@sequential+block@exam2',
257-
exam_name='Final Exam',
258-
time_limit_mins=120,
259-
is_proctored=False,
260-
)
261-
create_exam_attempt(self.exam_id, self.student.id)
271+
exam_id = self._create_exam(content_suffix='exam1')
272+
other_exam_id = self._create_exam(content_suffix='exam2')
273+
create_exam_attempt(exam_id, self.student.id)
262274
other_student = UserFactory(username='student2')
263275
create_exam_attempt(other_exam_id, other_student.id)
264276

265-
response = self.client.get(self._url())
277+
response = self.client.get(self._url(exam_id))
266278
data = response.json()
267279
assert data['count'] == 1
268-
assert data['results'][0]['exam_id'] == self.exam_id
280+
assert data['results'][0]['exam_id'] == exam_id
269281

270282

271283
@override_settings(**PROCTORING_SETTINGS)
@@ -459,6 +471,7 @@ def test_delete_allowance_missing_fields(self):
459471

460472
@override_settings(**PROCTORING_SETTINGS)
461473
@patch.dict(settings.FEATURES, {'ENABLE_SPECIAL_EXAMS': True})
474+
@ddt.ddt
462475
class CourseAllowancesViewTest(ModuleStoreTestCase):
463476
"""Tests for GET /api/instructor/v2/courses/{course_key}/special_exams/allowances"""
464477

@@ -543,9 +556,53 @@ def test_bulk_create_allowances_missing_fields(self):
543556
)
544557
assert response.status_code == status.HTTP_400_BAD_REQUEST
545558

559+
@ddt.data(
560+
'username',
561+
'user.username',
562+
'email',
563+
'user.email',
564+
'exam_name',
565+
'proctored_exam.exam_name',
566+
'allowance_type',
567+
'key',
568+
'value',
569+
)
570+
def test_sort_allowances(self, ordering):
571+
"""Verify all ordering fields are accepted and reverse correctly."""
572+
student2 = UserFactory(username='alice', email='alice@example.com')
573+
exam_id_2 = create_exam(
574+
course_id=self.course_id,
575+
content_id='block-v1:test+test+test+type@sequential+block@exam2',
576+
exam_name='AAA Exam',
577+
time_limit_mins=30,
578+
is_proctored=False,
579+
)
580+
add_allowance_for_user(self.exam_id, self.student.username, 'additional_time_granted', '30')
581+
add_allowance_for_user(exam_id_2, student2.username, 'review_policy_exception', '60')
582+
asc_response = self.client.get(self._url(), {'ordering': ordering})
583+
desc_response = self.client.get(self._url(), {'ordering': f'-{ordering}'})
584+
assert asc_response.status_code == status.HTTP_200_OK
585+
asc_results = asc_response.json()['results']
586+
desc_results = desc_response.json()['results']
587+
assert len(asc_results) == 2
588+
# All allowance fields differ between the two records, so order must reverse
589+
assert asc_results[0] == desc_results[1]
590+
assert asc_results[1] == desc_results[0]
591+
592+
def test_sort_allowances_descending(self):
593+
student2 = UserFactory(username='alice', email='alice@example.com')
594+
add_allowance_for_user(self.exam_id, self.student.username, 'additional_time_granted', '30')
595+
add_allowance_for_user(self.exam_id, student2.username, 'additional_time_granted', '60')
596+
response = self.client.get(self._url(), {'ordering': '-username'})
597+
assert response.status_code == status.HTTP_200_OK
598+
results = response.json()['results']
599+
assert results[0]['user']['username'] == 'student1'
600+
assert results[1]['user']['username'] == 'alice'
601+
546602

547603
@override_settings(**PROCTORING_SETTINGS)
548604
@patch.dict(settings.FEATURES, {'ENABLE_SPECIAL_EXAMS': True})
605+
@ddt.ddt
549606
class CourseExamAttemptsViewTest(ModuleStoreTestCase):
550607
"""Tests for GET /api/instructor/v2/courses/{course_key}/special_exams/attempts"""
551608

@@ -589,3 +646,59 @@ def test_search_attempts_no_match(self):
589646
response = self.client.get(self._url(), {'search': 'nonexistent'})
590647
assert response.status_code == status.HTTP_200_OK
591648
assert response.json()['count'] == 0
649+
650+
@ddt.data(
651+
'username',
652+
'user.username',
653+
'email',
654+
'user.email',
655+
'exam_name',
656+
'proctored_exam.exam_name',
657+
'time_limit',
658+
'proctored_exam.time_limit_mins',
659+
'type',
660+
'started_at',
661+
'start_time',
662+
'completed_at',
663+
'end_time',
664+
'status',
665+
)
666+
def test_sort_attempts(self, ordering):
667+
"""Verify all ordering fields produce reversed results for asc vs desc."""
668+
student2 = UserFactory(username='student2', email='student2@example.com')
669+
exam_id_2 = create_exam(
670+
course_id=self.course_id,
671+
content_id='block-v1:test+test+test+type@sequential+block@exam2',
672+
exam_name='Final Exam',
673+
time_limit_mins=120,
674+
is_proctored=True,
675+
)
676+
attempt_id_1 = create_exam_attempt(self.exam_id, self.student.id)
677+
create_exam_attempt(exam_id_2, student2.id)
678+
679+
# Give attempt 1 a distinct completed_at and status so all fields differ
680+
attempt = ProctoredExamStudentAttempt.objects.get(id=attempt_id_1)
681+
attempt.started_at = timezone.now() - timezone.timedelta(hours=1)
682+
attempt.completed_at = timezone.now()
683+
attempt.status = 'submitted'
684+
attempt.save()
685+
686+
asc_response = self.client.get(self._url(), {'ordering': ordering})
687+
desc_response = self.client.get(self._url(), {'ordering': f'-{ordering}'})
688+
assert asc_response.status_code == status.HTTP_200_OK
689+
assert desc_response.status_code == status.HTTP_200_OK
690+
asc_results = asc_response.json()['results']
691+
desc_results = desc_response.json()['results']
692+
assert len(asc_results) == 2
693+
assert asc_results[0] == desc_results[1]
694+
assert asc_results[1] == desc_results[0]
695+
696+
def test_sort_attempts_descending(self):
697+
student2 = UserFactory(username='student2', email='student2@example.com')
698+
create_exam_attempt(self.exam_id, self.student.id)
699+
create_exam_attempt(self.exam_id, student2.id)
700+
response = self.client.get(self._url(), {'ordering': '-username'})
701+
assert response.status_code == status.HTTP_200_OK
702+
results = response.json()['results']
703+
assert results[0]['user']['username'] == 'student2'
704+
assert results[1]['user']['username'] == 'student1'

0 commit comments

Comments
 (0)