Skip to content

Commit

Permalink
Merge pull request #2114 from openedx/asheehan-edx/remove-all-group-m…
Browse files Browse the repository at this point in the history
…embers

feat: allowing for group members removal endpoint to support remove all
  • Loading branch information
alex-sheehan-edx authored May 30, 2024
2 parents fac7adb + 306f2ab commit 3138559
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 30 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ Unreleased
----------
* nothing unreleased

[4.19.6]
--------
* feat: allowing for group members removal endpoint to support remove all

[4.19.5]
--------
* feat: replaced unencrypted user credentials from view and test
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.19.5"
__version__ = "4.19.6"
5 changes: 4 additions & 1 deletion enterprise/api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1761,7 +1761,10 @@ class EnterpriseGroupRequestDataSerializer(serializers.Serializer):
act_by_date = serializers.DateTimeField(required=False, allow_null=True)
learner_emails = serializers.ListField(
child=serializers.EmailField(required=True),
allow_empty=False)
allow_empty=False,
required=False,
)
remove_all = serializers.BooleanField(required=False, default=False)


class EnterpriseGroupLearnersRequestQuerySerializer(serializers.Serializer):
Expand Down
86 changes: 60 additions & 26 deletions enterprise/api/v1/views/enterprise_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,45 @@
User = auth.get_user_model()


def remove_group_membership_records(group, catalog_uuid=None, user_emails=None):
"""
Helper method to take a queryset of membership records, notify associated users of removal from the group,
and soft delete the data
params:
- group (EnterpriseGroup record, required): the Django ORM record related to the group from which members will be
removed
- catalog_uuid (UUID string, optional): Catalog UUID string used to email learners informing them of their removal.
If no catalog is supplied, email reminders will not be sent.
- user_emails (array[email strings], optional): List of user emails. If supplied, this method will remove
membership records related to ecu and pecu objects containing provided emails.
"""
records_to_delete = models.EnterpriseGroupMembership.available_objects.filter(group=group)
if user_emails:
existing_users = User.objects.filter(email__in=user_emails).values_list("id", flat=True)
ecu_in_q = Q(enterprise_customer_user__user_id__in=existing_users)
pecu_in_q = Q(pending_enterprise_customer_user__user_email__in=user_emails)
records_to_delete = records_to_delete.filter((ecu_in_q | pecu_in_q))

records_to_delete_uuids = [record.uuid for record in records_to_delete]
records_to_delete.delete()
for records_to_delete_uuids_batch in utils.batch(records_to_delete_uuids, batch_size=200):
send_group_membership_removal_notification.delay(
group.enterprise_customer.uuid,
records_to_delete_uuids_batch,
catalog_uuid
)
# Woohoo! Records removed! Now to update the soft deleted records
deleted_records = models.EnterpriseGroupMembership.all_objects.filter(
uuid__in=records_to_delete_uuids,
)
deleted_records.update(
status=constants.GROUP_MEMBERSHIP_REMOVED_STATUS,
removed_at=localized_utcnow()
)
return len(deleted_records)


class EnterpriseGroupViewSet(EnterpriseReadWriteModelViewSet):
"""
API views for the ``enterprise-group`` API endpoint.
Expand Down Expand Up @@ -216,6 +255,10 @@ def assign_learners(self, request, group_uuid):
act_by_date = param_serializer.validated_data.get('act_by_date')
catalog_uuid = param_serializer.validated_data.get('catalog_uuid')
learner_emails = param_serializer.validated_data.get('learner_emails', [])

if not learner_emails:
return Response({'learner_emails': ['This field is required.']}, status=400)

total_records_processed = 0
total_existing_users_processed = 0
total_new_users_processed = 0
Expand Down Expand Up @@ -351,40 +394,31 @@ def remove_learners(self, request, group_uuid):
"""
try:
group = self.get_queryset().get(uuid=group_uuid)
customer = group.enterprise_customer
except models.EnterpriseGroup.DoesNotExist as exc:
raise Http404 from exc
param_serializer = serializers.EnterpriseGroupRequestDataSerializer(data=request.data)
param_serializer.is_valid(raise_exception=True)

catalog_uuid = param_serializer.validated_data.get('catalog_uuid')
learner_emails = param_serializer.validated_data.get('learner_emails')
learner_emails = param_serializer.validated_data.get('learner_emails', [])
remove_all = param_serializer.validated_data.get('remove_all')

records_deleted = 0
for user_email_batch in utils.batch(learner_emails[: 1000], batch_size=200):
existing_users = User.objects.filter(email__in=user_email_batch).values_list("id", flat=True)
group_q = Q(group=group)
ecu_in_q = Q(enterprise_customer_user__user_id__in=existing_users)
pecu_in_q = Q(pending_enterprise_customer_user__user_email__in=user_email_batch)
records_to_delete = models.EnterpriseGroupMembership.objects.filter(
group_q & (ecu_in_q | pecu_in_q),
)
records_deleted += len(records_to_delete)
records_to_delete_uuids = [record.uuid for record in records_to_delete]
records_to_delete.delete()
for records_to_delete_uuids_batch in utils.batch(records_to_delete_uuids, batch_size=200):
send_group_membership_removal_notification.delay(
customer.uuid,
records_to_delete_uuids_batch,
catalog_uuid)
# Woohoo! Records removed! Now to update the soft deleted records
deleted_records = models.EnterpriseGroupMembership.all_objects.filter(
uuid__in=records_to_delete_uuids,
)
deleted_records.update(
status=constants.GROUP_MEMBERSHIP_REMOVED_STATUS,
removed_at=localized_utcnow()
if bool(remove_all) == bool(learner_emails):
return Response("Must supply `remove_all` or `learner_email` but not both", status=400)

if remove_all:
records_deleted = remove_group_membership_records(
group=group,
catalog_uuid=catalog_uuid
)
else:
records_deleted = 0
for user_email_batch in utils.batch(learner_emails[: 1000], batch_size=200):
records_deleted += remove_group_membership_records(
group=group,
catalog_uuid=catalog_uuid,
user_emails=user_email_batch,
)
data = {
'records_deleted': records_deleted,
}
Expand Down
34 changes: 32 additions & 2 deletions tests/test_enterprise/api/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8361,7 +8361,7 @@ def test_remove_learners_404(self):
)
assert self.client.post(url).status_code == 404

def test_remove_learners_requires_learner_emails(self):
def test_remove_learners_requires_learner_emails_or_remove_all(self):
"""
Test that the remove learners endpoint requires a POST body param: `learner_emails`
"""
Expand All @@ -8371,7 +8371,7 @@ def test_remove_learners_requires_learner_emails(self):
)
response = self.client.post(url)
assert response.status_code == 400
assert response.json() == {'learner_emails': ['This field is required.']}
assert response.json() == 'Must supply `remove_all` or `learner_email` but not both'

def test_patch_with_bad_request_customer_to_change_to(self):
"""
Expand Down Expand Up @@ -8402,6 +8402,36 @@ def test_patch_with_bad_request_customer_to_change_to(self):
response = self.client.patch(url, data=request_data)
assert response.status_code == 401

@mock.patch('enterprise.tasks.send_group_membership_removal_notification.delay', return_value=mock.MagicMock())
def test_successful_remove_all_learners_from_group(self, mock_send_group_membership_removal_notification):
"""
Test that both existing and new learners in groups are properly removed by the remove_learners endpoint
"""
url = settings.TEST_SERVER + reverse(
'enterprise-group-remove-learners',
kwargs={'group_uuid': self.group_2.uuid},
)
existing_emails = []
memberships_to_delete = []
for _ in range(10):
membership = EnterpriseGroupMembershipFactory(group=self.group_2)
memberships_to_delete.append(membership)
existing_emails.append(membership.enterprise_customer_user.user.email)
catalog_uuid = uuid.uuid4()
request_data = {'remove_all': True, 'catalog_uuid': catalog_uuid}
response = self.client.post(url, data=request_data)
assert response.data == {'records_deleted': 10}
mock_send_group_membership_removal_notification.assert_called_once_with(
self.enterprise_customer.uuid,
[membership.uuid for membership in reversed(memberships_to_delete)],
catalog_uuid,
)
for membership in memberships_to_delete:
assert EnterpriseGroupMembership.all_objects.get(pk=membership.pk).status == 'removed'
assert EnterpriseGroupMembership.all_objects.get(pk=membership.pk).removed_at
with self.assertRaises(EnterpriseGroupMembership.DoesNotExist):
EnterpriseGroupMembership.objects.get(pk=membership.pk)

@mock.patch('enterprise.tasks.send_group_membership_removal_notification.delay', return_value=mock.MagicMock())
def test_successful_remove_learners_from_group(self, mock_send_group_membership_removal_notification):
"""
Expand Down

0 comments on commit 3138559

Please sign in to comment.