Merge pull request #2108 from openedx/asheehan-edx/ENT-8943-reviving-…

…removed-members

feat: allowing for group members to be revived by reassigning

CHANGELOG.rst

@@ -17,6 +17,10 @@ Unreleased
 ----------
 * nothing unreleased
 
+[4.18.6]
+--------
+* feat: allowing for group members to be revived by reassigning
+
 [4.18.5]
 --------
 * feat: force transmit content metadata if customer configs are modified

enterprise/__init__.py

@@ -2,4 +2,4 @@
 Your project description goes here.
 """
 
-__version__ = "4.18.5"
+__version__ = "4.18.6"

enterprise/api/v1/views/enterprise_group.py

@@ -210,14 +210,24 @@ def assign_learners(self, request, group_uuid):
         total_records_processed = 0
         total_existing_users_processed = 0
         total_new_users_processed = 0
+        user_emails_to_create = []
+        memberships_to_create = []
         for user_email_batch in utils.batch(learner_emails[: 1000], batch_size=200):
-            user_emails_to_create = []
-            memberships_to_create = []
-            # ecus: enterprise customer users
-            ecus = []
             # Gather all existing User objects associated with the email batch
             existing_users = User.objects.filter(email__in=user_email_batch)
 
+            # Revive any previously deleted membership records connected to ECUs containing related emails
+            previously_removed_ecu_learners = models.EnterpriseGroupMembership.all_objects.filter(
+                enterprise_customer_user__user_id__in=existing_users.values_list('id', flat=True),
+                is_removed=True,
+                group=group,
+            )
+            previously_removed_ecu_learners.update(
+                status=constants.GROUP_MEMBERSHIP_ACCEPTED_STATUS,
+                removed_at=None,
+                is_removed=False,
+            )
+
             # Build and create a list of EnterpriseCustomerUser objects for the emails of existing Users
             # Ignore conflicts in case any of the ent customer user objects already exist
             ecu_by_email = {
@@ -232,12 +242,10 @@ def assign_learners(self, request, group_uuid):
 
             # Fetch all ent customer users related to existing users provided by requester
             # whether they were created above or already existed
-            ecus.extend(
-                models.EnterpriseCustomerUser.objects.filter(
-                    user_id__in=existing_users.values_list('id', flat=True),
-                    enterprise_customer=customer,
-                )
-            )
+            ecus = models.EnterpriseCustomerUser.objects.filter(
+                user_id__in=existing_users.values_list('id', flat=True),
+                enterprise_customer=customer,
+            ).exclude(pk__in=previously_removed_ecu_learners.values_list('pk', flat=True))
 
             # Extend the list of emails that don't have User objects associated and need to be turned into
             # new PendingEnterpriseCustomerUser objects
@@ -257,6 +265,18 @@ def assign_learners(self, request, group_uuid):
 
         # Go over (in batches) all emails that don't have User objects
         for emails_to_create_batch in utils.batch(user_emails_to_create, batch_size=200):
+            # Revive any previously deleted membership records connected to PECUs containing related emails
+            previously_removed_pecu_learners = models.EnterpriseGroupMembership.all_objects.filter(
+                pending_enterprise_customer_user__user_email__in=emails_to_create_batch,
+                is_removed=True,
+                group=group,
+            )
+            previously_removed_pecu_learners.update(
+                status=constants.GROUP_MEMBERSHIP_PENDING_STATUS,
+                removed_at=None,
+                is_removed=False,
+            )
+
             # Create the PendingEnterpriseCustomerUser objects
             pecu_records = [
                 models.PendingEnterpriseCustomerUser(
@@ -269,7 +289,7 @@ def assign_learners(self, request, group_uuid):
             pecus = models.PendingEnterpriseCustomerUser.objects.filter(
                 user_email__in=emails_to_create_batch,
                 enterprise_customer=customer,
-            )
+            ).exclude(pk__in=previously_removed_pecu_learners.values_list("pk", flat=True))
             total_new_users_processed += len(pecus)
             # Extend the list of memberships that need to be created associated with the new pending users
             memberships_to_create.extend([

tests/test_enterprise/api/test_views.py

@@ -43,6 +43,8 @@
     ENTERPRISE_LEARNER_ROLE,
     ENTERPRISE_OPERATOR_ROLE,
     ENTERPRISE_REPORTING_CONFIG_ADMIN_ROLE,
+    GROUP_MEMBERSHIP_ACCEPTED_STATUS,
+    GROUP_MEMBERSHIP_PENDING_STATUS,
     PATHWAY_CUSTOMER_ADMIN_ENROLLMENT,
 )
 from enterprise.models import (
@@ -7999,6 +8001,53 @@ def test_assign_learners_to_group_with_multiple_enterprises(self):
         self.client.post(url, data=request_data)
         assert len(EnterpriseGroupMembership.objects.filter(group=self.group_2)) == 1
 
+    def test_assign_learners_revives_previously_removed_members(self):
+        """
+        Test that assigning learners to a group when the learner has already been removed as a member will revive the
+        membership
+        """
+        pending_membership = EnterpriseGroupMembershipFactory(
+            group=self.group_2,
+            pending_enterprise_customer_user=PendingEnterpriseCustomerUserFactory(),
+            enterprise_customer_user=None,
+        )
+        membership = EnterpriseGroupMembershipFactory(
+            group=self.group_2,
+            enterprise_customer_user=EnterpriseCustomerUserFactory(),
+            pending_enterprise_customer_user=None,
+        )
+
+        # Remove the memberships
+        remove_url = settings.TEST_SERVER + reverse(
+            'enterprise-group-remove-learners',
+            kwargs={'group_uuid': self.group_2.uuid},
+        )
+        request_data = {'learner_emails': [membership.member_email, pending_membership.member_email]}
+        self.client.post(remove_url, data=request_data)
+
+        membership.refresh_from_db()
+        pending_membership.refresh_from_db()
+        assert membership.is_removed
+        assert pending_membership.is_removed
+
+        # Recreate the memberships for the emails
+        assign_url = settings.TEST_SERVER + reverse(
+            'enterprise-group-assign-learners',
+            kwargs={'group_uuid': self.group_2.uuid},
+        )
+        request_data = {
+            'learner_emails': [membership.member_email, pending_membership.member_email],
+        }
+        self.client.post(assign_url, data=request_data)
+
+        # Assert the memberships have been revived
+        membership.refresh_from_db()
+        pending_membership.refresh_from_db()
+        assert not pending_membership.is_removed
+        assert not membership.is_removed
+        assert pending_membership.status == GROUP_MEMBERSHIP_PENDING_STATUS
+        assert membership.status == GROUP_MEMBERSHIP_ACCEPTED_STATUS
+
     @mock.patch('enterprise.tasks.send_group_membership_invitation_notification.delay', return_value=mock.MagicMock())
     def test_successful_assign_learners_to_group(self, mock_send_group_membership_invitation_notification):
         """
@@ -8008,8 +8057,8 @@ def test_successful_assign_learners_to_group(self, mock_send_group_membership_in
             'enterprise-group-assign-learners',
             kwargs={'group_uuid': self.group_2.uuid},
         )
-        existing_emails = [UserFactory().email for _ in range(10)]
-        new_emails = [f"email_{x}@example.com" for x in range(10)]
+        existing_emails = [UserFactory(email=f"ayylmao{x}@example.com").email for x in range(400)]
+        new_emails = [f"email_{x}@example.com" for x in range(400)]
         act_by_date = datetime.now(pytz.UTC)
         catalog_uuid = uuid.uuid4()
         request_data = {
@@ -8019,24 +8068,38 @@ def test_successful_assign_learners_to_group(self, mock_send_group_membership_in
         }
         response = self.client.post(url, data=request_data)
         assert response.status_code == 201
-        assert response.data == {'records_processed': 20, 'new_learners': 10, 'existing_learners': 10}
+        assert response.data == {'records_processed': 800, 'new_learners': 400, 'existing_learners': 400}
         assert len(
             EnterpriseGroupMembership.objects.filter(
                 group=self.group_2,
                 pending_enterprise_customer_user__isnull=True
             )
-        ) == 10
+        ) == 400
         assert len(
             EnterpriseGroupMembership.objects.filter(
                 group=self.group_2,
                 enterprise_customer_user__isnull=True
             )
-        ) == 10
-        assert mock_send_group_membership_invitation_notification.call_count == 1
-        group_uuids = list(reversed(list(
-            EnterpriseGroupMembership.objects.filter(group=self.group_2).values_list('uuid', flat=True))))
-        mock_send_group_membership_invitation_notification.assert_has_calls([
-            mock.call(self.enterprise_customer.uuid, group_uuids, act_by_date, catalog_uuid)], any_order=True)
+        ) == 400
+
+        # Batch size for sending membership invitation notifications is 200, 800 total records means 4 iterations
+        group_uuids = list(
+            reversed(
+                list(EnterpriseGroupMembership.objects.filter(group=self.group_2).values_list('uuid', flat=True))
+            )
+        )
+        assert mock_send_group_membership_invitation_notification.call_count == len(group_uuids) / 200
+
+        for x in range(int(len(group_uuids) / 200)):
+            mock_send_group_membership_invitation_notification.assert_has_calls(
+                [mock.call(
+                    self.enterprise_customer.uuid,
+                    group_uuids[(x * 200):((x + 1) * 200)],
+                    act_by_date,
+                    catalog_uuid
+                )],
+                any_order=True,
+            )
 
     def test_remove_learners_404(self):
         """