Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow course entitlements REST API to be filtered on course_uuid #32305

Merged
merged 3 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion common/djangoapps/entitlements/rest_api/v1/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class CourseEntitlementFilter(filters.FilterSet):

uuid = UUIDListFilter()
user = filters.CharFilter(field_name='user__username')
course_uuid = UUIDListFilter(field_name='course_uuid')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will query params character length be an issue if there is a large courseUUID list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

potentially, yes


class Meta:
model = CourseEntitlement
fields = ('uuid', 'user')
fields = ('uuid', 'user', 'course_uuid')
28 changes: 28 additions & 0 deletions common/djangoapps/entitlements/rest_api/v1/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,34 @@ def test_get_expired_entitlement_by_uuid(self):
results = response.data
assert results.get('expired_at')

def test_entitlements_filter_on_course_uuid(self):
"""
Verify that courses filtered properly on list of course_uuids.
"""
entitlements = CourseEntitlementFactory.create_batch(5, user=self.user)
course_uuids_to_filter_on = {
str(entitlement.course_uuid)
for entitlement in entitlements[0:3]
}
# Create a 2nd entitlement for one of the ones to filter on
CourseEntitlementFactory.create(user=self.user, course_uuid=str(entitlements[0].course_uuid))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not get it. Could you please elaborate on why are we creating another entry with the same course UUID?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted the test set to seem more realistic, because in the wild we can potentially encounter a response that has entitlements with the same course_uuid


url = reverse('entitlements_api:v1:entitlements-list')
url += f'?user={self.user.username}'
url += f'&course_uuid={",".join(course_uuids_to_filter_on)}'

response = self.client.get(
url,
content_type='application/json',
)
assert response.status_code == 200

results = response.data
assert results['count'] == 4
actual_course_uuids = {entitlement['course_uuid'] for entitlement in results['results']}
assert actual_course_uuids == course_uuids_to_filter_on
assert len(actual_course_uuids) == 3
shafqatfarhan marked this conversation as resolved.
Show resolved Hide resolved

def test_delete_and_revoke_entitlement(self):
course_entitlement = CourseEntitlementFactory.create()
url = reverse(self.ENTITLEMENTS_DETAILS_PATH, args=[str(course_entitlement.uuid)])
Expand Down