From 8a21ab3e3b890dc607db6bbd453d6bc93dde3ffc Mon Sep 17 00:00:00 2001 From: Tehreem Sadat Date: Tue, 3 Dec 2024 22:22:04 +1300 Subject: [PATCH] fix: add usernames filter to enrollment API --- .../dashboard/details/learners.py | 15 +++++++-- futurex_openedx_extensions/dashboard/views.py | 6 ++++ .../test_details/test_details_learners.py | 32 +++++++++++++------ tests/test_dashboard/test_views.py | 14 ++++++++ 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/futurex_openedx_extensions/dashboard/details/learners.py b/futurex_openedx_extensions/dashboard/details/learners.py index 4ad176d..d1737e9 100644 --- a/futurex_openedx_extensions/dashboard/details/learners.py +++ b/futurex_openedx_extensions/dashboard/details/learners.py @@ -265,10 +265,11 @@ def get_learner_info_queryset( return queryset -def get_learners_enrollments_queryset( +def get_learners_enrollments_queryset( # pylint: disable=too-many-arguments fx_permission_info: dict, user_ids: list = None, course_ids: list = None, + usernames: list = None, search_text: str | None = None, include_staff: bool = False, ) -> QuerySet: @@ -287,6 +288,7 @@ def get_learners_enrollments_queryset( fx_permission_info=fx_permission_info, include_staff=include_staff, ) + user_filter = Q() if user_ids: invalid_user_ids = [user_id for user_id in user_ids if not isinstance(user_id, int)] if invalid_user_ids: @@ -294,7 +296,16 @@ def get_learners_enrollments_queryset( code=FXExceptionCodes.INVALID_INPUT, message=f'Invalid user ids: {invalid_user_ids}', ) - accessible_users = accessible_users.filter(id__in=user_ids) + user_filter |= Q(id__in=user_ids) + if usernames: + invalid_usernames = [username for username in usernames if not isinstance(username, str)] + if invalid_usernames: + raise FXCodedException( + code=FXExceptionCodes.INVALID_INPUT, + message=f'Invalid usernames: {invalid_usernames}', + ) + user_filter |= Q(username__in=usernames) + accessible_users = accessible_users.filter(user_filter) accessible_courses = CourseOverview.objects.filter( Q(id__in=get_partial_access_course_ids(fx_permission_info)) | diff --git a/futurex_openedx_extensions/dashboard/views.py b/futurex_openedx_extensions/dashboard/views.py index 92b904e..54ed0cd 100644 --- a/futurex_openedx_extensions/dashboard/views.py +++ b/futurex_openedx_extensions/dashboard/views.py @@ -456,16 +456,22 @@ def get_queryset(self, *args: Any, **kwargs: Any) -> QuerySet: """Get the list of learners for a course""" course_ids = self.request.query_params.get('course_ids', '') user_ids = self.request.query_params.get('user_ids', '') + usernames = self.request.query_params.get('usernames', '') course_ids_list = [ course.strip() for course in course_ids.split(',') ] if course_ids else None user_ids_list = [ int(user.strip()) for user in user_ids.split(',') if user.strip().isdigit() ] if user_ids else None + usernames_list = [ + username.strip() for username in usernames.split(',') + ] if usernames else None + return get_learners_enrollments_queryset( fx_permission_info=self.request.fx_permission_info, user_ids=user_ids_list, course_ids=course_ids_list, + usernames=usernames_list, search_text=self.request.query_params.get('search_text'), include_staff=self.request.query_params.get('include_staff', '0') == '1', ) diff --git a/tests/test_dashboard/test_details/test_details_learners.py b/tests/test_dashboard/test_details/test_details_learners.py index d920bdf..ee8f317 100644 --- a/tests/test_dashboard/test_details/test_details_learners.py +++ b/tests/test_dashboard/test_details/test_details_learners.py @@ -226,20 +226,24 @@ def test_get_learners_enrollments_queryset_annotations( @pytest.mark.django_db @pytest.mark.parametrize( - 'filter_user_ids, filter_course_ids, expected_error, expected_count, usecase', [ - (None, None, '', 6, 'No filter'), - ([21], None, '', 3, 'only user_ids filter'), - ([], ['course-v1:ORG1+5+5'], '', 2, 'only course_ids filter'), - ([15, 29], ['course-v1:ORG1+5+5'], '', 1, 'both course_ids and user_ids filter'), - ([15, 29, 'not-int', 0.2], [], 'Invalid user ids: [\'not-int\', 0.2]', 0, 'invalid user ids.'), - ([15, 29], ['course-v1:ORG1+5+5', 'invalid'], 'Invalid course ID format: invalid', 0, ''), + 'filter_user_ids, filter_course_ids, filter_usernames, expected_error, expected_count, usecase', [ + (None, None, None, '', 6, 'No filter'), + ([21], None, None, '', 3, 'only user_ids filter'), + (None, None, ['user21'], '', 3, 'only usernames filter'), + ([21], None, ['user29'], '', 4, 'user_ids and usernames filter'), + ([], ['course-v1:ORG1+5+5'], [], '', 2, 'only course_ids filter'), + ([15, 29], ['course-v1:ORG1+5+5'], [], '', 1, 'both course_ids and user_ids filter'), + ([15], ['course-v1:ORG1+5+5'], ['user21'], '', 2, 'course_ids, usernames and user_ids filter'), + ([15, 29, 'not-int', 0.2], [], [], 'Invalid user ids: [\'not-int\', 0.2]', 0, 'invalid user ids.'), + ([15, 29], ['course-v1:ORG1+5+5', 'invalid'], [], 'Invalid course ID format: invalid', 0, 'invalid course_ids'), + ([], [], [11], 'Invalid usernames: [11]', 0, 'invalid usernames'), ] ) @patch('futurex_openedx_extensions.dashboard.details.learners.get_permitted_learners_queryset') @patch('futurex_openedx_extensions.dashboard.details.learners.get_partial_access_course_ids') def test_get_learners_enrollments_queryset_for_course_ids_and_user_ids_filter( mocked_partial_course_ids, mocked_permitted_learners, - filter_user_ids, filter_course_ids, + filter_user_ids, filter_course_ids, filter_usernames, expected_error, expected_count, usecase, fx_permission_info, ): # pylint: disable=too-many-arguments, unused-argument """Test get_learners_by_course_queryset result for course_ids and user_ids filtration.""" @@ -254,16 +258,24 @@ def test_get_learners_enrollments_queryset_for_course_ids_and_user_ids_filter( queryset = get_learners_enrollments_queryset( fx_permission_info=fx_permission_info, course_ids=filter_course_ids, - user_ids=filter_user_ids + user_ids=filter_user_ids, + usernames=filter_usernames, ) assert str(exc_info.value) == expected_error else: queryset = get_learners_enrollments_queryset( fx_permission_info=fx_permission_info, course_ids=filter_course_ids, - user_ids=filter_user_ids + user_ids=filter_user_ids, + usernames=filter_usernames, ) expected_user_ids = filter_user_ids or fake_accessible_user_ids + if filter_usernames: + expected_user_ids.extend( + get_user_model().objects.filter( + username__in=filter_usernames + ).values_list('id', flat=True) + ) expected_course_ids = filter_course_ids or fake_accessible_course_ids assert queryset.count() == expected_count, f'unexpected enrollment queryset count for case: {usecase}' assert not ( diff --git a/tests/test_dashboard/test_views.py b/tests/test_dashboard/test_views.py index f546b54..0c92cb0 100644 --- a/tests/test_dashboard/test_views.py +++ b/tests/test_dashboard/test_views.py @@ -813,6 +813,20 @@ def test_success(self): self.assertEqual(response.data['results'][0]['user_id'], user_id) self.assertEqual(response.data['results'][0]['course_id'], course_id) + def test_success_for_user_ids_and_usernames(self): + """Verify that the view returns the correct response""" + self.login_user(self.staff_user) + response = self.client.get(self.url, data={ + 'user_ids': 15, + 'usernames': 'user21, user15', + }) + self.assertEqual(response.status_code, http_status.HTTP_200_OK) + self.assertEqual(response.data['count'], 6) + self.assertEqual( + set(user_data['user_id'] for user_data in response.data['results']), + {15, 21} + ) + class MockClickhouseQuery: """Mock ClickhouseQuery"""