Merge pull request #2040 from openedx/asheehan-edx/ENT-8524

feat: adding management command to remove expired pending group memberships

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`.

enterprise/__init__.py

@@ -2,4 +2,4 @@
 Your project description goes here.
 """
 
-__version__ = "4.13.2"
+__version__ = "4.13.3"

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):

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

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()

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.'),
+        ),
+    ]

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:

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())
 

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],
         ])
 
 

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):

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)