user_groups: Use check_add_user_group instead in test cases.

"check_add_user_group" is a safer helper function than
"create_user_group" to use when creating user_groups. It does
error handling and notify the client with the appropriate event.

Note that the populate_db command still uses "create_user_group"
because we do not need to enqueue events at that point.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>

zerver/actions/user_groups.py

@@ -139,15 +139,16 @@ def check_add_user_group(
     realm: Realm,
     name: str,
     initial_members: List[UserProfile],
-    description: str,
+    description: str = "",
     *,
     acting_user: Optional[UserProfile],
-) -> None:
+) -> UserGroup:
     try:
         user_group = create_user_group(
             name, initial_members, realm, description=description, acting_user=acting_user
         )
         do_send_create_user_group_event(user_group, initial_members)
+        return user_group
     except django.db.utils.IntegrityError:
         raise JsonableError(_("User group '{}' already exists.").format(name))
 

zerver/tests/test_email_notifications.py

@@ -18,7 +18,7 @@ from django.utils.timezone import now as timezone_now
 from django_auth_ldap.config import LDAPSearch
 
 from zerver.actions.create_user import do_create_user
-from zerver.actions.user_groups import create_user_group
+from zerver.actions.user_groups import check_add_user_group
 from zerver.actions.user_settings import do_change_user_setting
 from zerver.actions.users import do_change_user_role
 from zerver.lib.email_notifications import (
@@ -868,11 +868,11 @@ class TestMissedMessages(ZulipTestCase):
         othello = self.example_user("othello")
         cordelia = self.example_user("cordelia")
 
-        hamlet_only = create_user_group(
-            "hamlet_only", [hamlet], get_realm("zulip"), acting_user=None
+        hamlet_only = check_add_user_group(
+            get_realm("zulip"), "hamlet_only", [hamlet], acting_user=None
         )
-        hamlet_and_cordelia = create_user_group(
-            "hamlet_and_cordelia", [hamlet, cordelia], get_realm("zulip"), acting_user=None
+        hamlet_and_cordelia = check_add_user_group(
+            get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
         )
 
         hamlet_only_message_id = self.send_stream_message(othello, "Denmark", "@*hamlet_only*")
@@ -909,8 +909,8 @@ class TestMissedMessages(ZulipTestCase):
         cordelia = self.example_user("cordelia")
         othello = self.example_user("othello")
 
-        hamlet_and_cordelia = create_user_group(
-            "hamlet_and_cordelia", [hamlet, cordelia], get_realm("zulip"), acting_user=None
+        hamlet_and_cordelia = check_add_user_group(
+            get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
         )
 
         user_group_mentioned_message_id = self.send_stream_message(
@@ -949,8 +949,8 @@ class TestMissedMessages(ZulipTestCase):
         cordelia = self.example_user("cordelia")
         othello = self.example_user("othello")
 
-        hamlet_and_cordelia = create_user_group(
-            "hamlet_and_cordelia", [hamlet, cordelia], get_realm("zulip"), acting_user=None
+        hamlet_and_cordelia = check_add_user_group(
+            get_realm("zulip"), "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
         )
 
         wildcard_mentioned_message_id = self.send_stream_message(othello, "Denmark", "@**all**")
@@ -1669,8 +1669,8 @@ class TestMissedMessages(ZulipTestCase):
         hamlet = self.example_user("hamlet")
         othello = self.example_user("othello")
         cordelia = self.example_user("cordelia")
-        large_user_group = create_user_group(
-            "large_user_group", [hamlet, othello, cordelia], get_realm("zulip"), acting_user=None
+        large_user_group = check_add_user_group(
+            get_realm("zulip"), "large_user_group", [hamlet, othello, cordelia], acting_user=None
         )
 
         # Do note that the event dicts for the missed messages are constructed by hand

zerver/tests/test_event_queue.py

@@ -8,7 +8,7 @@ from django.http import HttpRequest, HttpResponse
 from zerver.actions.message_send import internal_send_private_message
 from zerver.actions.muted_users import do_mute_user
 from zerver.actions.streams import do_change_subscription_property
-from zerver.actions.user_groups import create_user_group
+from zerver.actions.user_groups import check_add_user_group
 from zerver.actions.user_settings import do_change_user_setting
 from zerver.actions.user_topics import do_set_user_topic_visibility_policy
 from zerver.lib.cache import cache_delete, get_muting_users_cache_key
@@ -449,10 +449,10 @@ class MissedMessageHookTest(ZulipTestCase):
             )
 
     def test_user_group_mention(self) -> None:
-        hamlet_and_cordelia = create_user_group(
+        hamlet_and_cordelia = check_add_user_group(
+            self.cordelia.realm,
             "hamlet_and_cordelia",
             [self.user_profile, self.cordelia],
-            self.cordelia.realm,
             acting_user=None,
         )
         msg_id = self.send_stream_message(

zerver/tests/test_events.py

@@ -94,7 +94,6 @@ from zerver.actions.user_groups import (
     bulk_add_members_to_user_group,
     check_add_user_group,
     check_delete_user_group,
-    create_user_group,
     do_update_user_group_description,
     do_update_user_group_name,
     remove_members_from_user_group,
@@ -1321,8 +1320,8 @@ class NormalActionsTest(BaseAction):
 
         check_user_group_remove_members("events[0]", events[0])
 
-        api_design = create_user_group(
-            "api-design", [hamlet], hamlet.realm, description="API design team", acting_user=None
+        api_design = check_add_user_group(
+            hamlet.realm, "api-design", [hamlet], description="API design team", acting_user=None
         )
 
         # Test add subgroups

zerver/tests/test_markdown.py

@@ -16,7 +16,7 @@ from zerver.actions.alert_words import do_add_alert_words
 from zerver.actions.create_realm import do_create_realm
 from zerver.actions.realm_emoji import do_remove_realm_emoji
 from zerver.actions.realm_settings import do_set_realm_property
-from zerver.actions.user_groups import create_user_group
+from zerver.actions.user_groups import check_add_user_group
 from zerver.actions.user_settings import do_change_user_setting
 from zerver.actions.users import change_user_is_active
 from zerver.lib.alert_words import get_alert_word_automaton
@@ -2462,7 +2462,9 @@ class MarkdownTest(ZulipTestCase):
 
     def create_user_group_for_test(self, user_group_name: str) -> UserGroup:
         othello = self.example_user("othello")
-        return create_user_group(user_group_name, [othello], get_realm("zulip"), acting_user=None)
+        return check_add_user_group(
+            get_realm("zulip"), user_group_name, [othello], acting_user=None
+        )
 
     def test_user_group_mention_single(self) -> None:
         sender_user_profile = self.example_user("othello")

zerver/tests/test_notification_data.py

@@ -1,4 +1,4 @@
-from zerver.actions.user_groups import create_user_group
+from zerver.actions.user_groups import check_add_user_group
 from zerver.lib.mention import MentionBackend, MentionData
 from zerver.lib.notification_data import UserMessageNotificationsData, get_user_group_mentions_data
 from zerver.lib.test_classes import ZulipTestCase
@@ -275,9 +275,9 @@ class TestNotificationData(ZulipTestCase):
         cordelia = self.example_user("cordelia")
         realm = hamlet.realm
 
-        hamlet_only = create_user_group("hamlet_only", [hamlet], realm, acting_user=None)
-        hamlet_and_cordelia = create_user_group(
-            "hamlet_and_cordelia", [hamlet, cordelia], realm, acting_user=None
+        hamlet_only = check_add_user_group(realm, "hamlet_only", [hamlet], acting_user=None)
+        hamlet_and_cordelia = check_add_user_group(
+            realm, "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
         )
 
         mention_backend = MentionBackend(realm.id)

zerver/tests/test_push_notifications.py

@@ -25,7 +25,7 @@ from analytics.lib.counts import CountStat, LoggingCountStat
 from analytics.models import InstallationCount, RealmCount
 from zerver.actions.message_delete import do_delete_messages
 from zerver.actions.message_flags import do_mark_stream_messages_as_read, do_update_message_flags
-from zerver.actions.user_groups import create_user_group
+from zerver.actions.user_groups import check_add_user_group
 from zerver.actions.user_settings import do_regenerate_api_key
 from zerver.lib.avatar import absolute_avatar_url
 from zerver.lib.exceptions import JsonableError
@@ -1502,10 +1502,10 @@ class HandlePushNotificationTest(PushNotificationTest):
     ) -> None:
         othello = self.example_user("othello")
         cordelia = self.example_user("cordelia")
-        large_user_group = create_user_group(
+        large_user_group = check_add_user_group(
+            get_realm("zulip"),
             "large_user_group",
             [self.user_profile, othello, cordelia],
-            get_realm("zulip"),
             acting_user=None,
         )
 
@@ -1874,8 +1874,8 @@ class TestGetAPNsPayload(PushNotificationTest):
 
     def test_get_message_payload_apns_user_group_mention(self) -> None:
         user_profile = self.example_user("othello")
-        user_group = create_user_group(
-            "test_user_group", [user_profile], get_realm("zulip"), acting_user=None
+        user_group = check_add_user_group(
+            get_realm("zulip"), "test_user_group", [user_profile], acting_user=None
         )
         stream = Stream.objects.filter(name="Verona").get()
         message = self.get_message(Recipient.STREAM, stream.id, stream.realm_id)

zerver/tests/test_subs.py

@@ -33,7 +33,7 @@ from zerver.actions.streams import (
     do_change_stream_post_policy,
     do_deactivate_stream,
 )
-from zerver.actions.user_groups import add_subgroups_to_user_group, create_user_group
+from zerver.actions.user_groups import add_subgroups_to_user_group, check_add_user_group
 from zerver.actions.users import do_change_user_role, do_deactivate_user
 from zerver.lib.exceptions import JsonableError
 from zerver.lib.message import UnreadStreamInfo, aggregate_unread_data, get_raw_unread_data
@@ -2565,14 +2565,14 @@ class StreamAdminTest(ZulipTestCase):
 
     def test_can_remove_subscribers_group(self) -> None:
         realm = get_realm("zulip")
-        leadership_group = create_user_group(
+        leadership_group = check_add_user_group(
+            realm,
             "leadership",
             [self.example_user("iago"), self.example_user("shiva")],
-            realm,
             acting_user=None,
         )
-        managers_group = create_user_group(
-            "managers", [self.example_user("hamlet")], realm=realm, acting_user=None
+        managers_group = check_add_user_group(
+            realm, "managers", [self.example_user("hamlet")], acting_user=None
         )
         add_subgroups_to_user_group(managers_group, [leadership_group], acting_user=None)
         cordelia = self.example_user("cordelia")

zerver/tests/test_user_groups.py

@@ -6,7 +6,7 @@ import orjson
 from django.utils.timezone import now as timezone_now
 
 from zerver.actions.realm_settings import do_set_realm_property
-from zerver.actions.user_groups import create_user_group, promote_new_full_members
+from zerver.actions.user_groups import check_add_user_group, promote_new_full_members
 from zerver.lib.streams import ensure_stream
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import most_recent_usermessage
@@ -34,7 +34,7 @@ class UserGroupTestCase(ZulipTestCase):
         self, group_name: str, realm: Realm = get_realm("zulip")
     ) -> UserGroup:
         members = [self.example_user("othello")]
-        return create_user_group(group_name, members, realm, acting_user=None)
+        return check_add_user_group(realm, group_name, members, acting_user=None)
 
     def test_user_groups_in_realm_serialized(self) -> None:
         realm = get_realm("zulip")
@@ -43,7 +43,7 @@ class UserGroupTestCase(ZulipTestCase):
         membership = UserGroupMembership.objects.filter(user_group=user_group).values_list(
             "user_profile_id", flat=True
         )
-        empty_user_group = create_user_group("newgroup", [], realm, acting_user=None)
+        empty_user_group = check_add_user_group(realm, "newgroup", [], acting_user=None)
 
         user_groups = user_groups_in_realm_serialized(realm)
         self.assert_length(user_groups, 9)
@@ -293,8 +293,8 @@ class UserGroupAPITestCase(UserGroupTestCase):
         self.assert_json_error(result, "Invalid user group")
 
         lear_realm = get_realm("lear")
-        lear_test_group = create_user_group(
-            "test", [self.lear_user("cordelia")], lear_realm, acting_user=None
+        lear_test_group = check_add_user_group(
+            lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
         )
         result = self.client_patch(f"/json/user_groups/{lear_test_group.id}", info=params)
         self.assert_json_error(result, "Invalid user group")
@@ -303,8 +303,8 @@ class UserGroupAPITestCase(UserGroupTestCase):
         hamlet = self.example_user("hamlet")
         self.login_user(hamlet)
         realm = get_realm("zulip")
-        support_user_group = create_user_group("support", [hamlet], realm, acting_user=None)
-        marketing_user_group = create_user_group("marketing", [hamlet], realm, acting_user=None)
+        support_user_group = check_add_user_group(realm, "support", [hamlet], acting_user=None)
+        marketing_user_group = check_add_user_group(realm, "marketing", [hamlet], acting_user=None)
 
         params = {
             "name": marketing_user_group.name,
@@ -334,8 +334,8 @@ class UserGroupAPITestCase(UserGroupTestCase):
         self.assert_json_error(result, "Invalid user group")
 
         lear_realm = get_realm("lear")
-        lear_test_group = create_user_group(
-            "test", [self.lear_user("cordelia")], lear_realm, acting_user=None
+        lear_test_group = check_add_user_group(
+            lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
         )
         result = self.client_delete(f"/json/user_groups/{lear_test_group.id}")
         self.assert_json_error(result, "Invalid user group")
@@ -418,8 +418,8 @@ class UserGroupAPITestCase(UserGroupTestCase):
         for user in all_users:
             self.subscribe(user, stream_name)
 
-        create_user_group(
-            name=group_name, members=list(support_team), realm=realm, acting_user=None
+        check_add_user_group(
+            name=group_name, initial_members=list(support_team), realm=realm, acting_user=None
         )
 
         payload = dict(
@@ -837,10 +837,10 @@ class UserGroupAPITestCase(UserGroupTestCase):
         hamlet = self.example_user("hamlet")
         othello = self.example_user("othello")
 
-        leadership_group = create_user_group(
-            "leadership", [desdemona, iago, hamlet], realm, acting_user=None
+        leadership_group = check_add_user_group(
+            realm, "leadership", [desdemona, iago, hamlet], acting_user=None
         )
-        support_group = create_user_group("support", [hamlet, othello], realm, acting_user=None)
+        support_group = check_add_user_group(realm, "support", [hamlet, othello], acting_user=None)
 
         self.login("cordelia")
         # Non-admin and non-moderators who are not a member of group cannot add or remove subgroups.
@@ -905,8 +905,8 @@ class UserGroupAPITestCase(UserGroupTestCase):
         )
 
         lear_realm = get_realm("lear")
-        lear_test_group = create_user_group(
-            "test", [self.lear_user("cordelia")], lear_realm, acting_user=None
+        lear_test_group = check_add_user_group(
+            lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
         )
         result = self.client_post(f"/json/user_groups/{lear_test_group.id}/subgroups", info=params)
         self.assert_json_error(result, "Invalid user group")
@@ -940,7 +940,9 @@ class UserGroupAPITestCase(UserGroupTestCase):
 
         lear_realm = get_realm("lear")
         lear_cordelia = self.lear_user("cordelia")
-        lear_test_group = create_user_group("test", [lear_cordelia], lear_realm, acting_user=None)
+        lear_test_group = check_add_user_group(
+            lear_realm, "test", [lear_cordelia], acting_user=None
+        )
         result = self.client_get(
             f"/json/user_groups/{lear_test_group.id}/members/{lear_cordelia.id}"
         )
@@ -1000,8 +1002,8 @@ class UserGroupAPITestCase(UserGroupTestCase):
         self.assert_json_error(result, "Invalid user group")
 
         lear_realm = get_realm("lear")
-        lear_test_group = create_user_group(
-            "test", [self.lear_user("cordelia")], lear_realm, acting_user=None
+        lear_test_group = check_add_user_group(
+            lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
         )
         result = self.client_get(f"/json/user_groups/{lear_test_group.id}/members")
         self.assert_json_error(result, "Invalid user group")
@@ -1048,8 +1050,8 @@ class UserGroupAPITestCase(UserGroupTestCase):
         self.assert_json_error(result, "Invalid user group")
 
         lear_realm = get_realm("lear")
-        lear_test_group = create_user_group(
-            "test", [self.lear_user("cordelia")], lear_realm, acting_user=None
+        lear_test_group = check_add_user_group(
+            lear_realm, "test", [self.lear_user("cordelia")], acting_user=None
         )
         result = self.client_get(f"/json/user_groups/{lear_test_group.id}/subgroups")
         self.assert_json_error(result, "Invalid user group")