Skip to content

Commit

Permalink
fix: fixing removal behavior for memberships (#2082)
Browse files Browse the repository at this point in the history
* fix: adding show_removed var to endpoint

* fix: fixing removal behavior for memberships

* chore: version bump

* fix: additional fixes
  • Loading branch information
kiram15 authored Apr 24, 2024
1 parent 624db77 commit 061ffbe
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 7 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ Change Log
Unreleased
----------
* Nothing
[4.16.3]
--------
* fix: fixing the removal logic for EnterpriseGroupMemberships, adding optional flag

[4.16.2]
---------
Expand Down
2 changes: 1 addition & 1 deletion enterprise/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Your project description goes here.
"""

__version__ = "4.16.2"
__version__ = "4.16.3"
5 changes: 4 additions & 1 deletion enterprise/api/v1/views/enterprise_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ def get_learners(self, request, *args, **kwargs):
are `memberDetails`, `memberStatus`, and `recentAction`. Ordering can be reversed by supplying a `-` at the
beginning of the sorting value ie `-memberStatus`.
- ``page`` (int, optional): Which page of paginated data to return.
- ``show_removed`` (bool, optional): Include removed learners in the response.
Returns: Paginated list of learners that are associated with the enterprise group uuid::
Expand All @@ -137,6 +138,7 @@ def get_learners(self, request, *args, **kwargs):
"""
query_params = self.request.query_params.copy()
is_reversed = bool(query_params.get('is_reversed', False))
show_removed = bool(query_params.get('show_removed', False))

param_serializers = serializers.EnterpriseGroupLearnersRequestQuerySerializer(
data=query_params
Expand All @@ -155,7 +157,8 @@ def get_learners(self, request, *args, **kwargs):
members = group_object.get_all_learners(user_query,
sort_by,
desc_order=is_reversed,
pending_users_only=pending_users_only)
pending_users_only=pending_users_only,
fetch_removed=show_removed)

page = self.paginate_queryset(members)
serializer = serializers.EnterpriseGroupMembershipSerializer(page, many=True)
Expand Down
6 changes: 3 additions & 3 deletions enterprise/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4338,9 +4338,9 @@ def _get_explicit_group_members(self, user_query=None, fetch_removed=False, pend
"""
Fetch explicitly defined members of a group, indicated by an existing membership record
"""
members = self.members.select_related(
'enterprise_customer_user', 'pending_enterprise_customer_user'
)
# note: self.members doesn't seem to surface soft deleted items
members = EnterpriseGroupMembership.all_objects.filter(
group__uuid=self.uuid).select_related('enterprise_customer_user', 'pending_enterprise_customer_user')
if not fetch_removed:
members = members.filter(is_removed=False)
if user_query:
Expand Down
40 changes: 39 additions & 1 deletion tests/test_enterprise/api/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7629,6 +7629,44 @@ def test_list_learners_filtered(self):
'pending_learner_id'
) == pending_membership.pending_enterprise_customer_user.id

def test_list_removed_learners(self):
group = EnterpriseGroupFactory(
enterprise_customer=self.enterprise_customer,
applies_to_all_contexts=False,
)
memberships_to_delete = []
membership = EnterpriseGroupMembershipFactory(
group=group,
pending_enterprise_customer_user=None,
enterprise_customer_user__enterprise_customer=self.enterprise_customer,
)
memberships_to_delete.append(membership.enterprise_customer_user.user.email)

# first we remove the membership
remove_url = settings.TEST_SERVER + reverse(
'enterprise-group-remove-learners',
kwargs={'group_uuid': group.uuid},
)
request_data = {'learner_emails': memberships_to_delete}
response = self.client.post(remove_url, data=request_data)

# then we're checking if the filter works
removed_users_query_string = '?show_removed=true'
url = settings.TEST_SERVER + reverse(
'enterprise-group-learners',
kwargs={'group_uuid': group.uuid},
) + removed_users_query_string
response = self.client.get(url)
assert response.json().get('count') == 1

# but we're doing an extra check to make sure its not fetched normally
url = settings.TEST_SERVER + reverse(
'enterprise-group-learners',
kwargs={'group_uuid': group.uuid},
)
response = self.client.get(url)
assert response.json().get('count') == 0

def test_list_learners_sort_by(self):
"""
Test that the list learners endpoint can be sorted by 'recentAction', 'status', 'memberDetails'
Expand Down Expand Up @@ -7978,7 +8016,7 @@ def test_successful_assign_learners_to_group(self, mock_send_group_membership_in

def test_remove_learners_404(self):
"""
Test that the remove learners endpoint properly handles no finding the provided group
Test that the remove learners endpoint properly handles not finding the provided group
"""
url = settings.TEST_SERVER + reverse(
'enterprise-group-remove-learners',
Expand Down

0 comments on commit 061ffbe

Please sign in to comment.