Skip to content

Commit

Permalink
Merge pull request #159 from nelc/tehreem/add_username_filter_enrollm…
Browse files Browse the repository at this point in the history
…ent_api

fix: add usernames filter to enrollment API
  • Loading branch information
shadinaif authored Dec 4, 2024
2 parents 6995c87 + 8a21ab3 commit f111854
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 12 deletions.
15 changes: 13 additions & 2 deletions futurex_openedx_extensions/dashboard/details/learners.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -287,14 +288,24 @@ 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:
raise FXCodedException(
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)) |
Expand Down
6 changes: 6 additions & 0 deletions futurex_openedx_extensions/dashboard/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
)
Expand Down
32 changes: 22 additions & 10 deletions tests/test_dashboard/test_details/test_details_learners.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand All @@ -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 (
Expand Down
14 changes: 14 additions & 0 deletions tests/test_dashboard/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down

0 comments on commit f111854

Please sign in to comment.