user_groups: Improve bulk membership change logic.

Adds support for bulk-adjusting a single user's membership in multiple
user groups in a single transaction in the low-level actions
functions, for future use by work on #9957.

zerver/actions/user_groups.py

@@ -151,13 +151,13 @@ def update_users_in_full_members_system_group(
     new_full_member_ids = [user["id"] for user in new_full_members]
 
     if len(old_full_members) > 0:
-        remove_members_from_user_group(
-            full_members_system_group, old_full_member_ids, acting_user=acting_user
+        bulk_remove_members_from_user_groups(
+            [full_members_system_group], old_full_member_ids, acting_user=acting_user
         )
 
     if len(new_full_members) > 0:
-        bulk_add_members_to_user_group(
-            full_members_system_group, new_full_member_ids, acting_user=acting_user
+        bulk_add_members_to_user_groups(
+            [full_members_system_group], new_full_member_ids, acting_user=acting_user
         )
 
 
@@ -269,12 +269,21 @@ def do_send_user_group_members_update_event(
 
 
 @transaction.atomic(savepoint=False)
-def bulk_add_members_to_user_group(
-    user_group: UserGroup, user_profile_ids: List[int], *, acting_user: Optional[UserProfile]
+def bulk_add_members_to_user_groups(
+    user_groups: List[UserGroup],
+    user_profile_ids: List[int],
+    *,
+    acting_user: Optional[UserProfile],
 ) -> None:
+    # All intended callers of this function involve a single user
+    # being added to one or more groups, or many users being added to
+    # a single group; but it's easy enough for the implementation to
+    # support both.
+
     memberships = [
         UserGroupMembership(user_group_id=user_group.id, user_profile_id=user_id)
         for user_id in user_profile_ids
+        for user_group in user_groups
     ]
     UserGroupMembership.objects.bulk_create(memberships)
     now = timezone_now()
@@ -288,17 +297,27 @@ def bulk_add_members_to_user_group(
             acting_user=acting_user,
         )
         for user_id in user_profile_ids
+        for user_group in user_groups
     )
 
+    for user_group in user_groups:
         do_send_user_group_members_update_event("add_members", user_group, user_profile_ids)
 
 
 @transaction.atomic(savepoint=False)
-def remove_members_from_user_group(
-    user_group: UserGroup, user_profile_ids: List[int], *, acting_user: Optional[UserProfile]
+def bulk_remove_members_from_user_groups(
+    user_groups: List[UserGroup],
+    user_profile_ids: List[int],
+    *,
+    acting_user: Optional[UserProfile],
 ) -> None:
+    # All intended callers of this function involve a single user
+    # being added to one or more groups, or many users being added to
+    # a single group; but it's easy enough for the implementation to
+    # support both.
+
     UserGroupMembership.objects.filter(
-        user_group_id=user_group.id, user_profile_id__in=user_profile_ids
+        user_group__in=user_groups, user_profile_id__in=user_profile_ids
     ).delete()
     now = timezone_now()
     RealmAuditLog.objects.bulk_create(
@@ -311,8 +330,10 @@ def remove_members_from_user_group(
             acting_user=acting_user,
         )
         for user_id in user_profile_ids
+        for user_group in user_groups
     )
 
+    for user_group in user_groups:
         do_send_user_group_members_update_event("remove_members", user_group, user_profile_ids)
 
 

zerver/tests/test_audit_log.py

@@ -47,12 +47,12 @@ from zerver.actions.streams import (
 )
 from zerver.actions.user_groups import (
     add_subgroups_to_user_group,
-    bulk_add_members_to_user_group,
+    bulk_add_members_to_user_groups,
+    bulk_remove_members_from_user_groups,
     check_add_user_group,
     do_change_user_group_permission_setting,
     do_update_user_group_description,
     do_update_user_group_name,
-    remove_members_from_user_group,
     remove_subgroups_from_user_group,
 )
 from zerver.actions.user_settings import (
@@ -1202,7 +1202,7 @@ class TestRealmAuditLog(ZulipTestCase):
         now = timezone_now()
         user_group = check_add_user_group(hamlet.realm, "foo", [], acting_user=None)
 
-        bulk_add_members_to_user_group(user_group, [hamlet.id, cordelia.id], acting_user=hamlet)
+        bulk_add_members_to_user_groups([user_group], [hamlet.id, cordelia.id], acting_user=hamlet)
         audit_log_entries = RealmAuditLog.objects.filter(
             acting_user=hamlet,
             realm=hamlet.realm,
@@ -1214,7 +1214,7 @@ class TestRealmAuditLog(ZulipTestCase):
         self.assertEqual(audit_log_entries[0].modified_user, hamlet)
         self.assertEqual(audit_log_entries[1].modified_user, cordelia)
 
-        remove_members_from_user_group(user_group, [hamlet.id], acting_user=hamlet)
+        bulk_remove_members_from_user_groups([user_group], [hamlet.id], acting_user=hamlet)
         audit_log_entries = RealmAuditLog.objects.filter(
             acting_user=hamlet,
             realm=hamlet.realm,

zerver/tests/test_events.py

@@ -102,13 +102,13 @@ from zerver.actions.submessage import do_add_submessage
 from zerver.actions.typing import check_send_typing_notification, do_send_stream_typing_notification
 from zerver.actions.user_groups import (
     add_subgroups_to_user_group,
-    bulk_add_members_to_user_group,
+    bulk_add_members_to_user_groups,
+    bulk_remove_members_from_user_groups,
     check_add_user_group,
     check_delete_user_group,
     do_change_user_group_permission_setting,
     do_update_user_group_description,
     do_update_user_group_name,
-    remove_members_from_user_group,
     remove_subgroups_from_user_group,
 )
 from zerver.actions.user_settings import (
@@ -1461,14 +1461,14 @@ class NormalActionsTest(BaseAction):
         # Test add members
         hamlet = self.example_user("hamlet")
         events = self.verify_action(
-            lambda: bulk_add_members_to_user_group(backend, [hamlet.id], acting_user=None)
+            lambda: bulk_add_members_to_user_groups([backend], [hamlet.id], acting_user=None)
         )
         check_user_group_add_members("events[0]", events[0])
 
         # Test remove members
         hamlet = self.example_user("hamlet")
         events = self.verify_action(
-            lambda: remove_members_from_user_group(backend, [hamlet.id], acting_user=None)
+            lambda: bulk_remove_members_from_user_groups([backend], [hamlet.id], acting_user=None)
         )
 
         check_user_group_remove_members("events[0]", events[0])

zerver/views/user_groups.py

@@ -9,13 +9,13 @@ from django.utils.translation import override as override_language
 from zerver.actions.message_send import do_send_messages, internal_prep_private_message
 from zerver.actions.user_groups import (
     add_subgroups_to_user_group,
-    bulk_add_members_to_user_group,
+    bulk_add_members_to_user_groups,
+    bulk_remove_members_from_user_groups,
     check_add_user_group,
     check_delete_user_group,
     do_change_user_group_permission_setting,
     do_update_user_group_description,
     do_update_user_group_name,
-    remove_members_from_user_group,
     remove_subgroups_from_user_group,
 )
 from zerver.decorator import require_member_or_admin, require_user_group_edit_permission
@@ -258,7 +258,7 @@ def add_members_to_group_backend(
             )
 
     member_user_ids = [member_user.id for member_user in member_users]
-    bulk_add_members_to_user_group(user_group, member_user_ids, acting_user=user_profile)
+    bulk_add_members_to_user_groups([user_group], member_user_ids, acting_user=user_profile)
     notify_for_user_group_subscription_changes(
         acting_user=user_profile,
         recipient_users=member_users,
@@ -285,7 +285,7 @@ def remove_members_from_group_backend(
             )
 
     user_profile_ids = [user.id for user in user_profiles]
-    remove_members_from_user_group(user_group, user_profile_ids, acting_user=user_profile)
+    bulk_remove_members_from_user_groups([user_group], user_profile_ids, acting_user=user_profile)
     notify_for_user_group_subscription_changes(
         acting_user=user_profile,
         recipient_users=user_profiles,