diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7585f8a96f..a0b7dafd9a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,10 @@ Change Log Unreleased ---------- +[4.13.3] +--------- +* feat: adding management command to remove expired pending group memberships + [4.13.2] --------- * feat: add a waffle flag for enterprise groups feature @@ -23,7 +27,6 @@ Unreleased --------- * feat: adding soft delete functionality for groups and group memberships - [4.13.0] --------- * feat: add Waffle-based `enterprise_features` to the `EnterpriseCustomerUserViewSet`. diff --git a/enterprise/__init__.py b/enterprise/__init__.py index 8f23b4aaf2..e09e5a3e5d 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "4.13.2" +__version__ = "4.13.3" diff --git a/enterprise/api/v1/serializers.py b/enterprise/api/v1/serializers.py index 7bfafbc5f9..5bdab56b1f 100644 --- a/enterprise/api/v1/serializers.py +++ b/enterprise/api/v1/serializers.py @@ -692,7 +692,23 @@ def get_enterprise_group(self, obj): """ Return the enterprise group membership for this enterprise customer user. """ - return obj.memberships.select_related('group').all() + related_customer = obj.enterprise_customer + # Find any groups that have ``applies_to_all_contexts`` set to True that are connected to the customer + # that's related to the customer associated with this customer user record. + all_context_groups = models.EnterpriseGroup.objects.filter( + enterprise_customer=related_customer, + applies_to_all_contexts=True + ).values_list('uuid', flat=True) + enterprise_groups_from_memberships = obj.memberships.select_related('group').all().values_list( + 'group', + flat=True + ) + # Combine both sets of group UUIDs + group_uuids = set(enterprise_groups_from_memberships) + for group in all_context_groups: + group_uuids.add(group) + + return list(group_uuids) class EnterpriseCustomerUserWriteSerializer(serializers.ModelSerializer): diff --git a/enterprise/api/v1/views/enterprise_group.py b/enterprise/api/v1/views/enterprise_group.py index 4ab5368e0e..00c3bbc38b 100644 --- a/enterprise/api/v1/views/enterprise_group.py +++ b/enterprise/api/v1/views/enterprise_group.py @@ -119,8 +119,32 @@ def get_learners(self, *args, **kwargs): group_uuid = kwargs.get('group_uuid') try: - learner_list = self.get_queryset().get(uuid=group_uuid).members.all() - page = self.paginate_queryset(learner_list) + group_object = self.get_queryset().get(uuid=group_uuid) + if group_object.applies_to_all_contexts: + members = [] + customer_users = models.EnterpriseCustomerUser.objects.filter( + enterprise_customer=group_object.enterprise_customer, + active=True, + ) + pending_customer_users = models.PendingEnterpriseCustomerUser.objects.filter( + enterprise_customer=group_object.enterprise_customer, + ) + for ent_user in customer_users: + members.append(models.EnterpriseGroupMembership( + uuid=None, + enterprise_customer_user=ent_user, + group=group_object, + )) + for pending_user in pending_customer_users: + members.append(models.EnterpriseGroupMembership( + uuid=None, + pending_enterprise_customer_user=pending_user, + group=group_object, + )) + page = self.paginate_queryset(members) + else: + learner_list = group_object.members.all() + page = self.paginate_queryset(learner_list) serializer = serializers.EnterpriseGroupMembershipSerializer(page, many=True) response = self.get_paginated_response(serializer.data) return response diff --git a/enterprise/management/commands/remove_expired_pending_group_memberships.py b/enterprise/management/commands/remove_expired_pending_group_memberships.py new file mode 100644 index 0000000000..ffbaee5098 --- /dev/null +++ b/enterprise/management/commands/remove_expired_pending_group_memberships.py @@ -0,0 +1,47 @@ +""" +Management command for ensuring any pending group membership, ie memberships associated with a pending enterprise user, +are removed after 90 days. +""" + +import logging +from datetime import timedelta + +from django.core.management.base import BaseCommand + +from enterprise.models import EnterpriseCustomer, EnterpriseGroupMembership +from enterprise.utils import localized_utcnow + +log = logging.getLogger(__name__) + + +class Command(BaseCommand): + """ + Management command for ensuring any pending group membership, ie memberships associated with a pending enterprise + user, are removed after 90 days. Optionally supply a ``--enterprise_customer`` arg to only run this command on + a singular customer. + + Example usage: + $ ./manage.py remove_expired_pending_group_memberships + """ + help = 'Removes pending group memberships if they are older than 90 days.' + + def add_arguments(self, parser): + parser.add_argument("-e", "--enterprise_customer") + + def handle(self, *args, **options): + queryset = EnterpriseGroupMembership.objects.all() + if enterprise_arg := options.get("enterprise_customer"): + try: + enterprise_customer = EnterpriseCustomer.objects.get(uuid=enterprise_arg) + queryset = queryset.filter(group__enterprise_customer=enterprise_customer) + except EnterpriseCustomer.DoesNotExist as exc: + log.exception(f'Enterprise Customer: {enterprise_arg} not found') + raise exc + expired_memberships = queryset.filter( + enterprise_customer_user=None, + pending_enterprise_customer_user__isnull=False, + created__lte=localized_utcnow() - timedelta(days=90) + ) + for membership in expired_memberships: + membership.pending_enterprise_customer_user.delete() + expired_memberships.delete() diff --git a/enterprise/migrations/0202_enterprisegroup_applies_to_all_contexts_and_more.py b/enterprise/migrations/0202_enterprisegroup_applies_to_all_contexts_and_more.py new file mode 100644 index 0000000000..3a3b0261de --- /dev/null +++ b/enterprise/migrations/0202_enterprisegroup_applies_to_all_contexts_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.10 on 2024-03-01 17:35 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('enterprise', '0201_auto_20240227_2227'), + ] + + operations = [ + migrations.AddField( + model_name='enterprisegroup', + name='applies_to_all_contexts', + field=models.BooleanField(default=False, help_text='When enabled, all learners connected to the org will be considered a member.', verbose_name='Set group membership to the entire org of learners.'), + ), + migrations.AddField( + model_name='historicalenterprisegroup', + name='applies_to_all_contexts', + field=models.BooleanField(default=False, help_text='When enabled, all learners connected to the org will be considered a member.', verbose_name='Set group membership to the entire org of learners.'), + ), + ] diff --git a/enterprise/models.py b/enterprise/models.py index 8b70f88c47..79323f0119 100644 --- a/enterprise/models.py +++ b/enterprise/models.py @@ -4233,6 +4233,14 @@ class EnterpriseGroup(TimeStampedModel, SoftDeletableModel): related_name='groups', on_delete=models.deletion.CASCADE ) + applies_to_all_contexts = models.BooleanField( + verbose_name="Set group membership to the entire org of learners.", + default=False, + help_text=_( + "When enabled, all learners connected to the org will be considered a member." + ) + ) + history = HistoricalRecords() class Meta: diff --git a/test_utils/factories.py b/test_utils/factories.py index e988a1cc1a..379ad0e908 100644 --- a/test_utils/factories.py +++ b/test_utils/factories.py @@ -2,6 +2,7 @@ Factoryboy factories. """ +from random import randint from uuid import UUID import factory @@ -244,7 +245,7 @@ class Meta: model = User email = factory.LazyAttribute(lambda x: FAKER.email()) - username = factory.LazyAttribute(lambda x: FAKER.user_name()) + username = factory.LazyAttribute(lambda x: FAKER.user_name() + str(randint(1, 10000))) first_name = factory.LazyAttribute(lambda x: FAKER.first_name()) last_name = factory.LazyAttribute(lambda x: FAKER.last_name()) is_staff = False @@ -1114,6 +1115,7 @@ class Meta: model = EnterpriseGroup uuid = factory.LazyAttribute(lambda x: UUID(FAKER.uuid4())) + applies_to_all_contexts = False enterprise_customer = factory.SubFactory(EnterpriseCustomerFactory) name = factory.LazyAttribute(lambda x: FAKER.company()) diff --git a/tests/test_enterprise/api/test_serializers.py b/tests/test_enterprise/api/test_serializers.py index 4f84058626..37c115b1b0 100644 --- a/tests/test_enterprise/api/test_serializers.py +++ b/tests/test_enterprise/api/test_serializers.py @@ -310,7 +310,22 @@ def test_group_membership(self): serializer = EnterpriseCustomerUserReadOnlySerializer(self.enterprise_customer_user_1) assert len(serializer.data['enterprise_group']) == 1 - assert serializer.data['enterprise_group'][0].uuid == membership.uuid + assert serializer.data['enterprise_group'][0] == membership.group.uuid + + def test_group_membership_when_applies_to_all_contexts(self): + """ + Test that when a group has ``applies_to_all_contexts`` set to True, that group is included in the enterprise + customer user serializer data when there is an associated via an enterprise customer object. + """ + enterprise_group = factories.EnterpriseGroupFactory( + enterprise_customer=self.enterprise_customer_1, + applies_to_all_contexts=True, + ) + serializer = EnterpriseCustomerUserReadOnlySerializer(self.enterprise_customer_user_1) + # Assert the enterprise customer user serializer found the group + assert serializer.data.get('enterprise_group') == [enterprise_group.uuid] + # Assert the group has no memberships that could be read by the serializer + assert not enterprise_group.members.all() def test_multi_group_membership(self): """ @@ -330,9 +345,9 @@ def test_multi_group_membership(self): serializer = EnterpriseCustomerUserReadOnlySerializer(self.enterprise_customer_user_1) assert len(serializer.data['enterprise_group']) == 2 - assert sorted([membership.uuid, membership_2.uuid]) == sorted([ - serializer.data['enterprise_group'][0].uuid, - serializer.data['enterprise_group'][1].uuid, + assert sorted([membership.group.uuid, membership_2.group.uuid]) == sorted([ + serializer.data['enterprise_group'][0], + serializer.data['enterprise_group'][1], ]) diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index df0f2daf0c..c08a05d97c 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -7668,6 +7668,28 @@ def test_remove_learners_from_group_only_removes_from_specified_group(self): EnterpriseGroupMembership.objects.get(pk=membership_to_remove.pk) assert EnterpriseGroupMembership.objects.get(pk=existing_membership.pk) + def test_group_applies_to_all_contexts_learner_list(self): + """ + Test that hitting the enterprise-group `/learners/` endpoint for a group that has ``applies_to_all_contexts`` + will return all learners in the group's org regardless of what membership records exist. + """ + new_group = EnterpriseGroupFactory(applies_to_all_contexts=True) + new_user = EnterpriseCustomerUserFactory( + user_id=self.user.id, enterprise_customer=new_group.enterprise_customer, + active=True + ) + pending_user = PendingEnterpriseCustomerUserFactory( + enterprise_customer=new_group.enterprise_customer, + ) + url = settings.TEST_SERVER + reverse( + 'enterprise-group-learners', + kwargs={'group_uuid': new_group.uuid}, + ) + response = self.client.get(url) + results = response.json().get('results') + for result in results: + assert (result.get('pending_learner_id') == pending_user.id) or (result.get('learner_id') == new_user.id) + @mark.django_db class TestEnterpriseCustomerSsoConfigurationViewSet(APITest): diff --git a/tests/test_enterprise/management/test_remove_expired_pending_group_memberships.py b/tests/test_enterprise/management/test_remove_expired_pending_group_memberships.py new file mode 100644 index 0000000000..e414875108 --- /dev/null +++ b/tests/test_enterprise/management/test_remove_expired_pending_group_memberships.py @@ -0,0 +1,112 @@ +""" +Tests for the django management command `remove_expired_pending_group_memberships`. +""" +from datetime import timedelta + +from pytest import mark + +from django.core.management import call_command +from django.test import TestCase + +from enterprise import models +from enterprise.utils import localized_utcnow +from test_utils import factories + + +@mark.django_db +class RemoveExpiredPendingGroupMembershipsCommandTests(TestCase): + """ + Test command `remove_expired_pending_group_memberships`. + """ + command = 'remove_expired_pending_group_memberships' + + def test_specifying_a_customer_limits_command_scope(self): + """ + Test that if the command is passed an optional ``--enterprise_customer`` arg, it will limit the scope of + queryable objects to just that customer's memberships + """ + # Target membership that should be removed because it has a pending user and is over 90 days old + group_to_remove_from = factories.EnterpriseGroupFactory() + membership_to_remove = factories.EnterpriseGroupMembershipFactory( + group=group_to_remove_from, + enterprise_customer_user=None, + ) + membership_to_remove.created = localized_utcnow() - timedelta(days=91) + membership_to_remove.save() + + # A membership that is older than 90 days but connected to a different customer + membership_to_keep = factories.EnterpriseGroupMembershipFactory( + enterprise_customer_user=None, + ) + membership_to_keep.created = localized_utcnow() - timedelta(days=91) + membership_to_keep.save() + + call_command(self.command, enterprise_customer=str(group_to_remove_from.enterprise_customer.uuid)) + + assert not models.EnterpriseGroupMembership.all_objects.filter(pk=membership_to_remove.pk) + assert not models.PendingEnterpriseCustomerUser.objects.filter( + pk=membership_to_remove.pending_enterprise_customer_user.pk + ) + + assert models.EnterpriseGroupMembership.all_objects.filter(pk=membership_to_keep.pk) + assert models.PendingEnterpriseCustomerUser.objects.filter( + pk=membership_to_keep.pending_enterprise_customer_user.pk + ) + + # Sanity check + call_command(self.command) + assert not models.EnterpriseGroupMembership.all_objects.filter(pk=membership_to_keep.pk) + assert not models.PendingEnterpriseCustomerUser.objects.filter( + pk=membership_to_keep.pending_enterprise_customer_user.pk + ) + + def test_removing_old_records(self): + """ + Test that the command properly hard deletes membership records and pending enterprise customer user records + """ + # Target membership that should be removed because it has a pending user and is over 90 days old + membership_to_remove = factories.EnterpriseGroupMembershipFactory( + enterprise_customer_user=None, + ) + membership_to_remove.created = localized_utcnow() - timedelta(days=91) + membership_to_remove.save() + + # A membership that is older than 90 days but has a realized enterprise customer user + old_membership_to_keep = factories.EnterpriseGroupMembershipFactory( + pending_enterprise_customer_user=None, + ) + old_membership_to_keep.created = localized_utcnow() - timedelta(days=91) + old_membership_to_keep.save() + + # A membership that has a pending user but has not reached the 90 days cutoff + new_pending_membership = factories.EnterpriseGroupMembershipFactory( + enterprise_customer_user=None, + ) + new_pending_membership.created = localized_utcnow() + + # Sanity check, a membership that is younger than 90 days and has a realized enterprise customer user + membership = factories.EnterpriseGroupMembershipFactory( + pending_enterprise_customer_user=None, + ) + membership.created = localized_utcnow() + membership.save() + + call_command(self.command) + + # Assert that memberships and pending customers are removed + assert not models.EnterpriseGroupMembership.all_objects.filter(pk=membership_to_remove.pk) + assert not models.PendingEnterpriseCustomerUser.objects.filter( + pk=membership_to_remove.pending_enterprise_customer_user.pk + ) + + # Assert that expected memberships and users are kept + assert models.EnterpriseGroupMembership.all_objects.filter(pk=old_membership_to_keep.pk) + assert models.EnterpriseCustomerUser.objects.filter(pk=old_membership_to_keep.enterprise_customer_user.pk) + + assert models.EnterpriseGroupMembership.all_objects.filter(pk=new_pending_membership.pk) + assert models.PendingEnterpriseCustomerUser.objects.filter( + pk=new_pending_membership.pending_enterprise_customer_user.pk + ) + + assert models.EnterpriseGroupMembership.all_objects.filter(pk=membership.pk) + assert models.EnterpriseCustomerUser.objects.filter(pk=membership.enterprise_customer_user.pk)