From d8f4638c01b209f6e0b05971ba8ad0c1c2a1dd5e Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Thu, 26 Mar 2026 12:37:15 -0600 Subject: [PATCH 1/2] feat: Refactor enrollments endpoint to v2 GET with ListAPIView and django-filter --- lms/djangoapps/instructor/tests/test_api.py | 10 +- .../instructor/tests/test_api_v2.py | 222 ++++++++- .../tests/test_enrollment_list_api.py | 468 ++++++++++++++++++ lms/djangoapps/instructor/views/api.py | 129 ++++- lms/djangoapps/instructor/views/api_urls.py | 6 + lms/djangoapps/instructor/views/api_v2.py | 79 +++ lms/djangoapps/instructor/views/filters.py | 51 ++ lms/djangoapps/instructor/views/serializer.py | 12 + .../instructor/views/serializers_v2.py | 27 + 9 files changed, 999 insertions(+), 5 deletions(-) create mode 100644 lms/djangoapps/instructor/tests/test_enrollment_list_api.py create mode 100644 lms/djangoapps/instructor/views/filters.py diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 4d9f6f83b9e8..898b64f3e9b3 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -2425,7 +2425,10 @@ def test_list_course_role_members_staff(self): 'first_name': self.other_staff.first_name, 'last_name': self.other_staff.last_name, } - ] + ], + 'count': 1, + 'num_pages': 1, + 'current_page': 1, } res_json = json.loads(response.content.decode('utf-8')) assert res_json == expected @@ -2440,7 +2443,10 @@ def test_list_course_role_members_beta(self): # check response content expected = { 'course_id': str(self.course.id), - 'beta': [] + 'beta': [], + 'count': 0, + 'num_pages': 0, + 'current_page': 1, } res_json = json.loads(response.content.decode('utf-8')) assert res_json == expected diff --git a/lms/djangoapps/instructor/tests/test_api_v2.py b/lms/djangoapps/instructor/tests/test_api_v2.py index fd6505a2bf5d..7f4055f91037 100644 --- a/lms/djangoapps/instructor/tests/test_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_api_v2.py @@ -17,7 +17,7 @@ from rest_framework.test import APIClient, APITestCase from edx_when.api import set_dates_for_course, set_date_for_block -from common.djangoapps.student.roles import CourseDataResearcherRole, CourseInstructorRole +from common.djangoapps.student.roles import CourseBetaTesterRole, CourseDataResearcherRole, CourseInstructorRole from common.djangoapps.student.tests.factories import ( AdminFactory, CourseEnrollmentFactory, @@ -1830,3 +1830,223 @@ def test_extension_data_structure(self, mock_title_or_url, mock_get_units, mock_ self.assertIsInstance(extension['email'], str) self.assertIsInstance(extension['unit_title'], str) self.assertIsInstance(extension['unit_location'], str) + + +class CourseEnrollmentsViewTest(SharedModuleStoreTestCase): + """Tests for the CourseEnrollmentsView v2 GET endpoint.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.client = APIClient() + self.instructor = InstructorFactory(course_key=self.course.id) + self.url = reverse( + 'instructor_api_v2:course_enrollments', + kwargs={'course_id': str(self.course.id)} + ) + + self.enrolled_users = [] + for i in range(30): + user = UserFactory( + username=f'student_{i}', + email=f'student{i}@example.com', + first_name=f'Student{i}', + last_name=f'Learner{i}' + ) + CourseEnrollmentFactory( + user=user, + course_id=self.course.id, + is_active=True + ) + self.enrolled_users.append(user) + + # Inactive enrollments should not appear + for i in range(5): + user = UserFactory( + username=f'inactive_{i}', + email=f'inactive{i}@example.com' + ) + CourseEnrollmentFactory( + user=user, + course_id=self.course.id, + is_active=False + ) + + def test_unauthenticated_returns_401(self): + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_student_returns_403(self): + student = UserFactory() + self.client.force_authenticate(user=student) + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_default_pagination(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + data = response.data + self.assertEqual(data['course_id'], str(self.course.id)) + self.assertEqual(data['count'], 30) + self.assertEqual(data['num_pages'], 3) + self.assertEqual(data['current_page'], 1) + self.assertIn('next', data) + self.assertIsNone(data['previous']) + self.assertIn('results', data) + # DefaultPagination page_size=10 + self.assertEqual(len(data['results']), 10) + + def test_custom_pagination(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'page': 1, 'page_size': 15}) + data = response.data + self.assertEqual(data['count'], 30) + self.assertEqual(data['num_pages'], 2) + self.assertEqual(data['current_page'], 1) + self.assertEqual(len(data['results']), 15) + + def test_second_page(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'page': 2, 'page_size': 10}) + data = response.data + self.assertEqual(data['current_page'], 2) + self.assertEqual(len(data['results']), 10) + self.assertIsNotNone(data['previous']) + + def test_last_page_partial(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'page': 3, 'page_size': 10}) + data = response.data + self.assertEqual(data['current_page'], 3) + self.assertEqual(len(data['results']), 10) + self.assertIsNone(data['next']) + + def test_search_by_username(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'search': 'student_2', 'page_size': 100}) + data = response.data + # Matches student_2, student_20..student_29 = 11 + self.assertEqual(data['count'], 11) + for user in data['results']: + self.assertIn('student_2', user['username']) + + def test_search_by_email(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'search': 'student7@example.com'}) + data = response.data + self.assertEqual(data['count'], 1) + self.assertEqual(data['results'][0]['email'], 'student7@example.com') + + def test_search_case_insensitive(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'search': 'STUDENT_5'}) + data = response.data + self.assertEqual(data['count'], 1) + self.assertEqual(data['results'][0]['username'], 'student_5') + + def test_search_no_results(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'search': 'nonexistent'}) + data = response.data + self.assertEqual(data['count'], 0) + self.assertEqual(len(data['results']), 0) + + def test_excludes_inactive_enrollments(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'search': 'inactive'}) + data = response.data + self.assertEqual(data['count'], 0) + + def test_invalid_page_returns_404(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'page': 999}) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_ordered_by_username(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'page_size': 5}) + data = response.data + usernames = [u['username'] for u in data['results']] + self.assertEqual(usernames, sorted(usernames)) + + def test_staff_can_access(self): + staff = StaffFactory(course_key=self.course.id) + self.client.force_authenticate(user=staff) + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_includes_mode_field(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'page_size': 1}) + enrollment = response.data['results'][0] + self.assertIn('mode', enrollment) + + def test_includes_full_name_field(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'page_size': 1}) + enrollment = response.data['results'][0] + self.assertIn('full_name', enrollment) + + def test_includes_is_beta_tester_field(self): + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'page_size': 100}) + for enrollment in response.data['results']: + self.assertIn('is_beta_tester', enrollment) + self.assertFalse(enrollment['is_beta_tester']) + + def test_beta_tester_flag_true(self): + beta_role = CourseBetaTesterRole(self.course.id) + target_user = self.enrolled_users[0] + beta_role.add_users(target_user) + + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'search': target_user.username}) + data = response.data + self.assertEqual(data['count'], 1) + self.assertTrue(data['results'][0]['is_beta_tester']) + + def test_filter_beta_testers_only(self): + beta_role = CourseBetaTesterRole(self.course.id) + beta_users = self.enrolled_users[:3] + for user in beta_users: + beta_role.add_users(user) + + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'is_beta_tester': 'true', 'page_size': 100}) + data = response.data + self.assertEqual(data['count'], 3) + for enrollment in data['results']: + self.assertTrue(enrollment['is_beta_tester']) + + def test_filter_non_beta_testers_only(self): + beta_role = CourseBetaTesterRole(self.course.id) + beta_users = self.enrolled_users[:3] + for user in beta_users: + beta_role.add_users(user) + + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, {'is_beta_tester': 'false', 'page_size': 100}) + data = response.data + self.assertEqual(data['count'], 27) + for enrollment in data['results']: + self.assertFalse(enrollment['is_beta_tester']) + + def test_filter_beta_testers_with_search(self): + beta_role = CourseBetaTesterRole(self.course.id) + beta_users = self.enrolled_users[:5] + for user in beta_users: + beta_role.add_users(user) + + self.client.force_authenticate(user=self.instructor) + response = self.client.get(self.url, { + 'is_beta_tester': 'true', + 'search': self.enrolled_users[0].username, + }) + data = response.data + self.assertEqual(data['count'], 1) + self.assertTrue(data['results'][0]['is_beta_tester']) diff --git a/lms/djangoapps/instructor/tests/test_enrollment_list_api.py b/lms/djangoapps/instructor/tests/test_enrollment_list_api.py new file mode 100644 index 000000000000..722b06ea2c25 --- /dev/null +++ b/lms/djangoapps/instructor/tests/test_enrollment_list_api.py @@ -0,0 +1,468 @@ +""" +Unit tests for instructor API enrollment list endpoints with search and pagination. +""" +import json +from django.urls import reverse +from common.djangoapps.student.roles import CourseBetaTesterRole +from common.djangoapps.student.tests.factories import ( + CourseEnrollmentFactory, + InstructorFactory, + UserFactory +) +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase + + +class TestListCourseRoleMembersWithPagination(SharedModuleStoreTestCase, LoginEnrollmentTestCase): + """ + Test the list_course_role_members endpoint with search and pagination functionality. + """ + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.instructor = InstructorFactory(course_key=self.course.id) + self.client.login(username=self.instructor.username, password=self.TEST_PASSWORD) + self.url = reverse('list_course_role_members', kwargs={'course_id': str(self.course.id)}) + + # Create beta testers for testing + self.beta_testers = [] + beta_role = CourseBetaTesterRole(self.course.id) + for i in range(25): + user = UserFactory( + username=f'beta_user_{i}', + email=f'beta{i}@example.com', + first_name=f'Beta{i}', + last_name=f'Tester{i}' + ) + beta_role.add_users(user) + self.beta_testers.append(user) + + def test_list_beta_testers_without_pagination(self): + """Test listing beta testers without pagination parameters (backward compatibility).""" + response = self.client.post(self.url, {'rolename': 'beta'}) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['course_id'] == str(self.course.id) + assert 'beta' in res_json + assert res_json['count'] == 25 + assert res_json['num_pages'] == 2 # 25 items with default page_size of 20 + assert res_json['current_page'] == 1 + assert len(res_json['beta']) == 20 # First page with default page_size + + def test_list_beta_testers_with_pagination(self): + """Test listing beta testers with pagination.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 25 + assert res_json['num_pages'] == 3 # 25 items / 10 per page + assert res_json['current_page'] == 1 + assert len(res_json['beta']) == 10 + + def test_list_beta_testers_second_page(self): + """Test listing beta testers on second page.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'page': 2, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['current_page'] == 2 + assert len(res_json['beta']) == 10 + + def test_list_beta_testers_last_page(self): + """Test listing beta testers on last page with partial results.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'page': 3, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['current_page'] == 3 + assert len(res_json['beta']) == 5 # Last page has 5 items + + def test_list_beta_testers_beyond_last_page(self): + """Test requesting a page beyond the last page returns empty results.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'page': 10, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['current_page'] == 10 + assert len(res_json['beta']) == 0 + + def test_list_beta_testers_search_by_username(self): + """Test searching beta testers by username.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'search': 'beta_user_1', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + # Should match beta_user_1, beta_user_10-19 (11 total) + assert res_json['count'] == 11 + assert len(res_json['beta']) == 10 # First page + for user in res_json['beta']: + assert 'beta_user_1' in user['username'] + + def test_list_beta_testers_search_by_email(self): + """Test searching beta testers by email.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'search': 'beta5@example.com', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 1 + assert len(res_json['beta']) == 1 + assert res_json['beta'][0]['email'] == 'beta5@example.com' + + def test_list_beta_testers_search_by_first_name(self): + """Test searching beta testers by first name.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'search': 'Beta2', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + # Should match Beta2, Beta20-24 (6 total) + assert res_json['count'] == 6 + for user in res_json['beta']: + assert 'Beta2' in user['first_name'] + + def test_list_beta_testers_search_case_insensitive(self): + """Test that search is case-insensitive.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'search': 'BETA_USER_3', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 1 + assert res_json['beta'][0]['username'] == 'beta_user_3' + + def test_list_beta_testers_search_no_results(self): + """Test searching with no matching results.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'search': 'nonexistent', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 0 + assert len(res_json['beta']) == 0 + + def test_list_beta_testers_empty_search(self): + """Test that empty search returns all results.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'search': '', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 25 + + def test_list_beta_testers_max_page_size(self): + """Test that page_size is capped at maximum.""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'page': 1, + 'page_size': 100 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert len(res_json['beta']) == 25 # All results fit in max page size + + def test_list_beta_testers_invalid_page_size(self): + """Test with invalid page_size (should fail validation).""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'page': 1, + 'page_size': 0 + }) + assert response.status_code == 400 + + def test_list_beta_testers_invalid_page(self): + """Test with invalid page number (should fail validation).""" + response = self.client.post(self.url, { + 'rolename': 'beta', + 'page': 0, + 'page_size': 10 + }) + assert response.status_code == 400 + + +class TestListCourseEnrollments(SharedModuleStoreTestCase, LoginEnrollmentTestCase): + """ + Test the list_course_enrollments endpoint with search and pagination functionality. + """ + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.instructor = InstructorFactory(course_key=self.course.id) + self.client.login(username=self.instructor.username, password=self.TEST_PASSWORD) + self.url = reverse('list_course_enrollments', kwargs={'course_id': str(self.course.id)}) + + # Create enrollments for testing + self.enrolled_users = [] + for i in range(30): + user = UserFactory( + username=f'student_{i}', + email=f'student{i}@example.com', + first_name=f'Student{i}', + last_name=f'Learner{i}' + ) + CourseEnrollmentFactory( + user=user, + course_id=self.course.id, + is_active=True + ) + self.enrolled_users.append(user) + + # Create some inactive enrollments (should not be included) + for i in range(5): + user = UserFactory( + username=f'inactive_{i}', + email=f'inactive{i}@example.com' + ) + CourseEnrollmentFactory( + user=user, + course_id=self.course.id, + is_active=False + ) + + def test_list_enrollments_without_pagination(self): + """Test listing enrollments without pagination parameters.""" + response = self.client.post(self.url, {}) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['course_id'] == str(self.course.id) + assert 'enrollments' in res_json + # 30 active student enrollments (InstructorFactory does not create an enrollment) + assert res_json['count'] == 30 + assert res_json['num_pages'] == 2 # 30 items with default page_size of 20 + assert res_json['current_page'] == 1 + assert len(res_json['enrollments']) == 20 + + def test_list_enrollments_with_pagination(self): + """Test listing enrollments with pagination.""" + response = self.client.post(self.url, { + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 30 + assert res_json['num_pages'] == 3 # 30 items / 10 per page = 3 pages + assert res_json['current_page'] == 1 + assert len(res_json['enrollments']) == 10 + + def test_list_enrollments_second_page(self): + """Test listing enrollments on second page.""" + response = self.client.post(self.url, { + 'page': 2, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['current_page'] == 2 + assert len(res_json['enrollments']) == 10 + + def test_list_enrollments_last_page(self): + """Test listing enrollments on last page with partial results.""" + response = self.client.post(self.url, { + 'page': 3, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['current_page'] == 3 + assert len(res_json['enrollments']) == 10 # Last page has 10 items + + def test_list_enrollments_search_by_username(self): + """Test searching enrollments by username.""" + response = self.client.post(self.url, { + 'search': 'student_2', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + # Should match student_2, student_20-29 (11 total) + assert res_json['count'] == 11 + for user in res_json['enrollments']: + assert 'student_2' in user['username'] + + def test_list_enrollments_search_by_email(self): + """Test searching enrollments by email.""" + response = self.client.post(self.url, { + 'search': 'student7@example.com', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 1 + assert res_json['enrollments'][0]['email'] == 'student7@example.com' + + def test_list_enrollments_search_by_first_name(self): + """Test searching enrollments by first name.""" + response = self.client.post(self.url, { + 'search': 'Student1', + 'page': 1, + 'page_size': 20 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + # Should match Student1, Student10-19 (11 total) + assert res_json['count'] == 11 + + def test_list_enrollments_search_case_insensitive(self): + """Test that search is case-insensitive.""" + response = self.client.post(self.url, { + 'search': 'STUDENT_5', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 1 + assert res_json['enrollments'][0]['username'] == 'student_5' + + def test_list_enrollments_search_no_results(self): + """Test searching with no matching results.""" + response = self.client.post(self.url, { + 'search': 'nonexistent', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert res_json['count'] == 0 + assert len(res_json['enrollments']) == 0 + + def test_list_enrollments_excludes_inactive(self): + """Test that inactive enrollments are not included.""" + response = self.client.post(self.url, { + 'search': 'inactive', + 'page': 1, + 'page_size': 10 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + # Should not find any inactive enrollments + assert res_json['count'] == 0 + + def test_list_enrollments_empty_course(self): + """Test listing enrollments for a course with no enrollments.""" + empty_course = CourseFactory.create() + empty_instructor = InstructorFactory(course_key=empty_course.id) + self.client.login(username=empty_instructor.username, password=self.TEST_PASSWORD) + + url = reverse('list_course_enrollments', kwargs={'course_id': str(empty_course.id)}) + response = self.client.post(url, {}) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + # InstructorFactory does not create an enrollment, so empty course has 0 + assert res_json['count'] == 0 + assert len(res_json['enrollments']) == 0 + + def test_list_enrollments_max_page_size(self): + """Test that page_size is capped at maximum.""" + response = self.client.post(self.url, { + 'page': 1, + 'page_size': 100 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + assert len(res_json['enrollments']) == 30 # All results fit in max page size + + def test_list_enrollments_invalid_page_size(self): + """Test with invalid page_size (should fail validation).""" + response = self.client.post(self.url, { + 'page': 1, + 'page_size': 0 + }) + assert response.status_code == 400 + + def test_list_enrollments_invalid_page(self): + """Test with invalid page number (should fail validation).""" + response = self.client.post(self.url, { + 'page': -1, + 'page_size': 10 + }) + assert response.status_code == 400 + + def test_list_enrollments_permission_required(self): + """Test that non-instructor users cannot access the endpoint.""" + student = UserFactory() + self.client.login(username=student.username, password=self.TEST_PASSWORD) + + response = self.client.post(self.url, {}) + assert response.status_code == 403 + + def test_list_enrollments_ordered_by_username(self): + """Test that enrollments are ordered by username.""" + response = self.client.post(self.url, { + 'page': 1, + 'page_size': 5 + }) + assert response.status_code == 200 + + res_json = json.loads(response.content.decode('utf-8')) + usernames = [user['username'] for user in res_json['enrollments']] + # Check that usernames are in alphabetical order + assert usernames == sorted(usernames) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 09b91d362b28..4eb7b80c7a09 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -22,6 +22,7 @@ from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist, PermissionDenied, ValidationError from django.core.validators import validate_email from django.db import IntegrityError, transaction +from django.db.models import Q from django.http import QueryDict, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound from django.shortcuts import redirect from django.urls import reverse @@ -103,6 +104,7 @@ BlockDueDateSerializer, CertificateSerializer, CertificateStatusesSerializer, + EnrollmentListSerializer, ForumRoleNameSerializer, ListInstructorTaskInputSerializer, ModifyAccessSerializer, @@ -1050,6 +1052,11 @@ class ListCourseRoleMembersView(APIView): rolename is one of ['instructor', 'staff', 'beta', 'ccx_coach'] + Supports optional search and pagination parameters: + - search: Filter users by username, email, first_name, or last_name + - page: Page number (default: 1) + - page_size: Number of results per page (default: 20, max: 100) + Returns JSON of the form { "course_id": "some/course/id", "staff": [ @@ -1059,7 +1066,10 @@ class ListCourseRoleMembersView(APIView): "first_name": "Joe", "last_name": "Shmoe", } - ] + ], + "count": 10, + "num_pages": 1, + "current_page": 1 } """ permission_classes = (IsAuthenticated, permissions.InstructorPermission) @@ -1087,13 +1097,128 @@ def post(self, request, course_id): role_serializer = RoleNameSerializer(data=request.data) role_serializer.is_valid(raise_exception=True) rolename = role_serializer.data['rolename'] + search = role_serializer.data.get('search', '').strip() + page = role_serializer.data.get('page', 1) + page_size = role_serializer.data.get('page_size', 20) users = list_with_level(course.id, rolename) - serializer = UserSerializer(users, many=True) + + # Apply search filter + if search: + users = [ + user for user in users + if search.lower() in user.username.lower() + or search.lower() in user.email.lower() + or search.lower() in (user.first_name or '').lower() + or search.lower() in (user.last_name or '').lower() + ] + + # Calculate pagination + total_count = len(users) + num_pages = (total_count + page_size - 1) // page_size if page_size > 0 else 1 + start_idx = (page - 1) * page_size + end_idx = start_idx + page_size + paginated_users = users[start_idx:end_idx] + + serializer = UserSerializer(paginated_users, many=True) response_payload = { 'course_id': str(course_id), rolename: serializer.data, + 'count': total_count, + 'num_pages': num_pages, + 'current_page': page, + } + + return Response(response_payload, status=status.HTTP_200_OK) + + +class ListCourseEnrollmentsView(APIView): + """ + View to list all enrollments (learners/students) for a specific course. + Requires the user to have instructor access. + + Supports optional search and pagination parameters: + - search: Filter users by username, email, first_name, or last_name + - page: Page number (default: 1) + - page_size: Number of results per page (default: 20, max: 100) + + Returns JSON of the form { + "course_id": "some/course/id", + "enrollments": [ + { + "username": "student1", + "email": "student1@example.org", + "first_name": "Jane", + "last_name": "Doe", + } + ], + "count": 100, + "num_pages": 5, + "current_page": 1 + } + """ + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.VIEW_ENROLLMENTS + + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): + """ + Handles POST request to list course enrollments. + + Args: + request (HttpRequest): The request object containing user data. + course_id (str): The ID of the course to list enrollments for. + + Returns: + Response: A Response object containing the list of enrollments or an error message. + + Raises: + Http404: If the course does not exist. + """ + course_key = CourseKey.from_string(course_id) + course = get_course_with_access( + request.user, 'staff', course_key, depth=None + ) + + enrollment_serializer = EnrollmentListSerializer(data=request.data) + enrollment_serializer.is_valid(raise_exception=True) + search = enrollment_serializer.data.get('search', '').strip() + page = enrollment_serializer.data.get('page', 1) + page_size = enrollment_serializer.data.get('page_size', 20) + + # Get all active enrollments for the course + enrollments = CourseEnrollment.objects.filter( + course_id=course_key, + is_active=True + ).select_related('user').order_by('user__username') + + # Apply search filter + if search: + enrollments = enrollments.filter( + Q(user__username__icontains=search) | + Q(user__email__icontains=search) | + Q(user__first_name__icontains=search) | + Q(user__last_name__icontains=search) + ) + + # Calculate pagination + total_count = enrollments.count() + num_pages = (total_count + page_size - 1) // page_size if page_size > 0 else 1 + start_idx = (page - 1) * page_size + end_idx = start_idx + page_size + paginated_enrollments = enrollments[start_idx:end_idx] + + # Extract user data from enrollments + users = [enrollment.user for enrollment in paginated_enrollments] + serializer = UserSerializer(users, many=True) + + response_payload = { + 'course_id': str(course_key), + 'enrollments': serializer.data, + 'count': total_count, + 'num_pages': num_pages, + 'current_page': page, } return Response(response_payload, status=status.HTTP_200_OK) diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index b61120e58c13..f2a19ddef492 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -66,12 +66,18 @@ api_v2.ORASummaryView.as_view(), name='ora_summary' ), + re_path( + rf'^courses/{COURSE_ID_PATTERN}/enrollments$', + api_v2.CourseEnrollmentsView.as_view(), + name='course_enrollments' + ), ] urlpatterns = [ path('students_update_enrollment', api.StudentsUpdateEnrollmentView.as_view(), name='students_update_enrollment'), path('register_and_enroll_students', api.RegisterAndEnrollStudents.as_view(), name='register_and_enroll_students'), path('list_course_role_members', api.ListCourseRoleMembersView.as_view(), name='list_course_role_members'), + path('list_course_enrollments', api.ListCourseEnrollmentsView.as_view(), name='list_course_enrollments'), path('modify_access', api.ModifyAccess.as_view(), name='modify_access'), path('bulk_beta_modify_access', api.BulkBetaModifyAccess.as_view(), name='bulk_beta_modify_access'), path('get_problem_responses', api.GetProblemResponses.as_view(), name='get_problem_responses'), diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index dd399dcb064a..8664507ac4e3 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -21,6 +21,7 @@ from django.utils.html import strip_tags from django.utils.translation import gettext as _ from django.views.decorators.cache import cache_control +from django_filters.rest_framework import DjangoFilterBackend from edx_when import api as edx_when_api from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey @@ -34,6 +35,8 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.roles import CourseBetaTesterRole from common.djangoapps.util.json_request import JsonResponseBadRequest from openedx.core.djangoapps.course_groups.cohorts import is_course_cohorted from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers @@ -52,10 +55,12 @@ from lms.djangoapps.instructor_task.tasks_helper.utils import upload_csv_file_to_report_store from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin from openedx.core.lib.courses import get_course_by_id +from .filters import CourseEnrollmentFilter from .serializers_v2 import ( InstructorTaskListSerializer, CourseInformationSerializerV2, BlockDueDateSerializerV2, + CourseEnrollmentSerializerV2, UnitExtensionSerializer, ORASerializer, ORASummarySerializer, @@ -1097,3 +1102,77 @@ def get(self, request, *args, **kwargs): serializer = self.get_serializer(items) return Response(serializer.data) + + +class CourseEnrollmentsView(DeveloperErrorViewMixin, ListAPIView): + """ + List all active enrollments for a course with optional search, filtering, and pagination. + + **Example Requests** + + GET /api/instructor/v2/courses/{course_id}/enrollments + GET /api/instructor/v2/courses/{course_id}/enrollments?search=john + GET /api/instructor/v2/courses/{course_id}/enrollments?is_beta_tester=true + GET /api/instructor/v2/courses/{course_id}/enrollments?page=2&page_size=50 + + **Response Values** + + { + "course_id": "course-v1:edX+DemoX+Demo_Course", + "count": 150, + "num_pages": 15, + "current_page": 1, + "start": 0, + "next": "http://example.com/api/instructor/v2/courses/.../enrollments?page=2", + "previous": null, + "results": [ + { + "username": "learner1", + "full_name": "Jane Doe", + "email": "jane@example.com", + "mode": "audit", + "is_beta_tester": false + }, + ... + ] + } + + **Parameters** + + course_id: Course key for the course. + search (optional): Filter by username, email, first name, or last name. + is_beta_tester (optional): Filter by beta tester status (true/false). + page (optional): Page number for pagination. + page_size (optional): Number of results per page (default: 10, max: 100). + + **Returns** + + * 200: OK - Returns paginated list of active enrollments + * 401: Unauthorized - User is not authenticated + * 403: Forbidden - User lacks instructor permissions + """ + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.VIEW_ENROLLMENTS + serializer_class = CourseEnrollmentSerializerV2 + filter_backends = [DjangoFilterBackend] + filterset_class = CourseEnrollmentFilter + + def get_queryset(self): + course_key = CourseKey.from_string(self.kwargs['course_id']) + return CourseEnrollment.objects.filter( + course_id=course_key, + is_active=True + ).select_related('user', 'user__profile').order_by('user__username') + + def get_serializer_context(self): + context = super().get_serializer_context() + course_key = CourseKey.from_string(self.kwargs['course_id']) + context['beta_tester_ids'] = set( + CourseBetaTesterRole(course_key).users_with_role().values_list('id', flat=True) + ) + return context + + def list(self, request, *args, **kwargs): + response = super().list(request, *args, **kwargs) + response.data['course_id'] = self.kwargs['course_id'] + return response diff --git a/lms/djangoapps/instructor/views/filters.py b/lms/djangoapps/instructor/views/filters.py new file mode 100644 index 000000000000..85de9ccf3806 --- /dev/null +++ b/lms/djangoapps/instructor/views/filters.py @@ -0,0 +1,51 @@ +""" +Filters for the Instructor API v2. +""" + +from django.db.models import Q +from django_filters import rest_framework as filters +from opaque_keys.edx.keys import CourseKey + +from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.roles import CourseBetaTesterRole + + +class CourseEnrollmentFilter(filters.FilterSet): + """ + FilterSet for filtering course enrollments. + + Supports filtering by: + - search: case-insensitive partial match on username, email, first name, or last name + - is_beta_tester: filter by beta tester role membership + """ + search = filters.CharFilter(method='filter_search', label='Search') + is_beta_tester = filters.BooleanFilter(method='filter_is_beta_tester', label='Is Beta Tester') + + class Meta: + model = CourseEnrollment + fields = ['search', 'is_beta_tester'] + + def _get_course_key(self): + """Extract the course key from the view's URL kwargs.""" + return CourseKey.from_string(self.request.parser_context['kwargs']['course_id']) + + def filter_search(self, queryset, name, value): + """Filter enrollments by username, email, first name, or last name.""" + if not value: + return queryset + return queryset.filter( + Q(user__username__icontains=value) + | Q(user__email__icontains=value) + | Q(user__first_name__icontains=value) + | Q(user__last_name__icontains=value) + ) + + def filter_is_beta_tester(self, queryset, name, value): + """Filter enrollments by beta tester role membership.""" + course_key = self._get_course_key() + beta_tester_ids = set( + CourseBetaTesterRole(course_key).users_with_role().values_list('id', flat=True) + ) + if value: + return queryset.filter(user__id__in=beta_tester_ids) + return queryset.exclude(user__id__in=beta_tester_ids) diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 9b83acdf6156..229a49dc9849 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -29,6 +29,9 @@ class RoleNameSerializer(serializers.Serializer): # pylint: disable=abstract-me """ rolename = serializers.CharField(help_text=_("Role name")) + search = serializers.CharField(required=False, allow_blank=True, help_text=_("Search term")) + page = serializers.IntegerField(required=False, min_value=1, help_text=_("Page number")) + page_size = serializers.IntegerField(required=False, min_value=1, max_value=100, help_text=_("Page size")) def validate_rolename(self, value): """ @@ -45,6 +48,15 @@ class Meta: fields = ['username', 'email', 'first_name', 'last_name'] +class EnrollmentListSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Serializer for enrollment list request parameters. + """ + search = serializers.CharField(required=False, allow_blank=True, help_text=_("Search term")) + page = serializers.IntegerField(required=False, min_value=1, help_text=_("Page number")) + page_size = serializers.IntegerField(required=False, min_value=1, max_value=100, help_text=_("Page size")) + + class UniqueStudentIdentifierSerializer(serializers.Serializer): """ Serializer for identifying unique_student. diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index c56c153d29c3..09d6357bf7bc 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -33,8 +33,10 @@ from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR from xmodule.modulestore.django import modulestore + from .tools import get_student_from_identifier, parse_datetime, DashboardError + log = logging.getLogger(__name__) @@ -563,3 +565,28 @@ class ORASummarySerializer(serializers.Serializer): waiting = serializers.IntegerField() staff = serializers.IntegerField() final_grade_received = serializers.IntegerField() + + +class CourseEnrollmentSerializerV2(serializers.Serializer): + """ + Serializer for course enrollment data. + + Serializes CourseEnrollment instances with derived fields for + the user's full name and beta tester status. + """ + username = serializers.CharField(source='user.username') + full_name = serializers.SerializerMethodField() + email = serializers.EmailField(source='user.email') + mode = serializers.CharField() + is_beta_tester = serializers.SerializerMethodField() + + def get_full_name(self, enrollment): + """Get the user's full name from their profile.""" + user = enrollment.user + profile = getattr(user, 'profile', None) + return profile.name if profile else '' + + def get_is_beta_tester(self, enrollment): + """Check if the user is a beta tester for this course.""" + beta_tester_ids = self.context.get('beta_tester_ids', set()) + return enrollment.user_id in beta_tester_ids From a5c01c54c6af01021a037d1783950c1e12dd99e2 Mon Sep 17 00:00:00 2001 From: Brian Buck Date: Wed, 1 Apr 2026 07:41:35 -0600 Subject: [PATCH 2/2] fix: PR feedback --- lms/djangoapps/instructor/tests/test_api.py | 2 +- .../tests/test_enrollment_list_api.py | 2 +- lms/djangoapps/instructor/views/api.py | 22 ++++++++++--------- lms/djangoapps/instructor/views/api_v2.py | 2 +- .../views/{filters.py => filters_v2.py} | 2 +- lms/djangoapps/instructor/views/serializer.py | 18 ++++++++------- 6 files changed, 26 insertions(+), 22 deletions(-) rename lms/djangoapps/instructor/views/{filters.py => filters_v2.py} (95%) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 898b64f3e9b3..1d517fa60774 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -2445,7 +2445,7 @@ def test_list_course_role_members_beta(self): 'course_id': str(self.course.id), 'beta': [], 'count': 0, - 'num_pages': 0, + 'num_pages': 1, 'current_page': 1, } res_json = json.loads(response.content.decode('utf-8')) diff --git a/lms/djangoapps/instructor/tests/test_enrollment_list_api.py b/lms/djangoapps/instructor/tests/test_enrollment_list_api.py index 722b06ea2c25..1f342ee2b03c 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment_list_api.py +++ b/lms/djangoapps/instructor/tests/test_enrollment_list_api.py @@ -313,7 +313,7 @@ def test_list_enrollments_second_page(self): assert len(res_json['enrollments']) == 10 def test_list_enrollments_last_page(self): - """Test listing enrollments on last page with partial results.""" + """Test listing enrollments on last page.""" response = self.client.post(self.url, { 'page': 3, 'page_size': 10 diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 4eb7b80c7a09..c02e4ff79307 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1105,17 +1105,18 @@ def post(self, request, course_id): # Apply search filter if search: + search_lower = search.lower() users = [ user for user in users - if search.lower() in user.username.lower() - or search.lower() in user.email.lower() - or search.lower() in (user.first_name or '').lower() - or search.lower() in (user.last_name or '').lower() + if search_lower in user.username.lower() + or search_lower in user.email.lower() + or search_lower in (user.first_name or '').lower() + or search_lower in (user.last_name or '').lower() ] # Calculate pagination total_count = len(users) - num_pages = (total_count + page_size - 1) // page_size if page_size > 0 else 1 + num_pages = max(1, (total_count + page_size - 1) // page_size) if page_size > 0 else 1 start_idx = (page - 1) * page_size end_idx = start_idx + page_size paginated_users = users[start_idx:end_idx] @@ -1177,7 +1178,8 @@ def post(self, request, course_id): Http404: If the course does not exist. """ course_key = CourseKey.from_string(course_id) - course = get_course_with_access( + # Verify the user has staff-level access to the course; raises Http404 if not. + get_course_with_access( request.user, 'staff', course_key, depth=None ) @@ -1204,13 +1206,13 @@ def post(self, request, course_id): # Calculate pagination total_count = enrollments.count() - num_pages = (total_count + page_size - 1) // page_size if page_size > 0 else 1 + num_pages = max(1, (total_count + page_size - 1) // page_size) if page_size > 0 else 1 start_idx = (page - 1) * page_size end_idx = start_idx + page_size paginated_enrollments = enrollments[start_idx:end_idx] # Extract user data from enrollments - users = [enrollment.user for enrollment in paginated_enrollments] + users = [enr.user for enr in paginated_enrollments] serializer = UserSerializer(users, many=True) response_payload = { @@ -2301,7 +2303,7 @@ def post(self, request, course_id): ) except NotImplementedError as exc: return HttpResponseBadRequest(str(exc)) - except ItemNotFoundError as exc: + except ItemNotFoundError: return HttpResponseBadRequest(f"{module_state_key} not found") elif all_students: @@ -2313,7 +2315,7 @@ def post(self, request, course_id): ) except NotImplementedError as exc: return HttpResponseBadRequest(str(exc)) - except ItemNotFoundError as exc: + except ItemNotFoundError: return HttpResponseBadRequest(f"{module_state_key} not found") else: return HttpResponseBadRequest() diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 8664507ac4e3..28966425b56f 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -55,7 +55,7 @@ from lms.djangoapps.instructor_task.tasks_helper.utils import upload_csv_file_to_report_store from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin from openedx.core.lib.courses import get_course_by_id -from .filters import CourseEnrollmentFilter +from .filters_v2 import CourseEnrollmentFilter from .serializers_v2 import ( InstructorTaskListSerializer, CourseInformationSerializerV2, diff --git a/lms/djangoapps/instructor/views/filters.py b/lms/djangoapps/instructor/views/filters_v2.py similarity index 95% rename from lms/djangoapps/instructor/views/filters.py rename to lms/djangoapps/instructor/views/filters_v2.py index 85de9ccf3806..3ed83e5f9fab 100644 --- a/lms/djangoapps/instructor/views/filters.py +++ b/lms/djangoapps/instructor/views/filters_v2.py @@ -27,7 +27,7 @@ class Meta: def _get_course_key(self): """Extract the course key from the view's URL kwargs.""" - return CourseKey.from_string(self.request.parser_context['kwargs']['course_id']) + return CourseKey.from_string(self.request.resolver_match.kwargs['course_id']) def filter_search(self, queryset, name, value): """Filter enrollments by username, email, first name, or last name.""" diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 229a49dc9849..9e0603be6eb2 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -23,16 +23,21 @@ from .tools import get_student_from_identifier -class RoleNameSerializer(serializers.Serializer): # pylint: disable=abstract-method +class PaginatedSearchSerializer(serializers.Serializer): # pylint: disable=abstract-method """ - Serializer that describes the response of the problem response report generation API. + Base serializer for paginated list requests with optional search. """ - - rolename = serializers.CharField(help_text=_("Role name")) search = serializers.CharField(required=False, allow_blank=True, help_text=_("Search term")) page = serializers.IntegerField(required=False, min_value=1, help_text=_("Page number")) page_size = serializers.IntegerField(required=False, min_value=1, max_value=100, help_text=_("Page size")) + +class RoleNameSerializer(PaginatedSearchSerializer): # pylint: disable=abstract-method + """ + Serializer for role member list requests. + """ + rolename = serializers.CharField(help_text=_("Role name")) + def validate_rolename(self, value): """ Check that the rolename is valid. @@ -48,13 +53,10 @@ class Meta: fields = ['username', 'email', 'first_name', 'last_name'] -class EnrollmentListSerializer(serializers.Serializer): # pylint: disable=abstract-method +class EnrollmentListSerializer(PaginatedSearchSerializer): # pylint: disable=abstract-method """ Serializer for enrollment list request parameters. """ - search = serializers.CharField(required=False, allow_blank=True, help_text=_("Search term")) - page = serializers.IntegerField(required=False, min_value=1, help_text=_("Page number")) - page_size = serializers.IntegerField(required=False, min_value=1, max_value=100, help_text=_("Page size")) class UniqueStudentIdentifierSerializer(serializers.Serializer):