groups: Allow setting anonymous group to settings while creation.

This commit adds support to set can_mention_group setting to
anonymous group while group creation.

api_docs/changelog.md

@@ -26,6 +26,9 @@ format used by the Zulip server that they are interacting with.
   /register`](/api/register-queue): `can_mention_group` field can now
   either be an ID of a named user group with the permission, or an
   object describing the set of users and groups with the permission.
+* [`POST /user_groups/create`](/api/create-user-group): The
+  `can_mention_group` parameter can now either be an ID of a named
+  user group or an object describing a set of users and groups.
 
 **Feature level 257**:
 

zerver/actions/user_groups.py

@@ -8,6 +8,7 @@ from django.utils.translation import gettext as _
 
 from zerver.lib.exceptions import JsonableError
 from zerver.lib.user_groups import (
+    get_group_setting_value_for_api,
     get_role_based_system_groups_dict,
     set_defaults_for_group_settings,
 )
@@ -175,7 +176,7 @@ def do_send_create_user_group_event(
             id=user_group.id,
             is_system_group=user_group.is_system_group,
             direct_subgroup_ids=[direct_subgroup.id for direct_subgroup in direct_subgroups],
-            can_mention_group=user_group.can_mention_group_id,
+            can_mention_group=get_group_setting_value_for_api(user_group.can_mention_group),
         ),
     )
     send_event(user_group.realm, event, active_user_ids(user_group.realm_id))

zerver/lib/event_schema.py

@@ -1790,6 +1790,18 @@ update_message_flags_remove_event = event_dict_type(
 check_update_message_flags_remove = make_checker(update_message_flags_remove_event)
 
 
+group_setting_type = UnionType(
+    [
+        int,
+        DictType(
+            required_keys=[
+                ("direct_members", ListType(int)),
+                ("direct_subgroups", ListType(int)),
+            ]
+        ),
+    ]
+)
+
 group_type = DictType(
     required_keys=[
         ("id", int),
@@ -1798,7 +1810,7 @@ group_type = DictType(
         ("direct_subgroup_ids", ListType(int)),
         ("description", str),
         ("is_system_group", bool),
-        ("can_mention_group", int),
+        ("can_mention_group", group_setting_type),
     ]
 )
 

zerver/lib/user_groups.py

@@ -241,8 +241,10 @@ def update_or_create_user_group_for_setting(
     direct_members: List[int],
     direct_subgroups: List[int],
     current_setting_value: Optional[UserGroup],
-) -> UserGroup:  # nocoverage
+) -> UserGroup:
-    if current_setting_value is not None and not hasattr(current_setting_value, "named_user_group"):
+    if current_setting_value is not None and not hasattr(
+        current_setting_value, "named_user_group"
+    ):  # nocoverage
         # We do not create a new group if the setting was already set
         # to an anonymous group. The memberships of existing group
         # itself are updated.
@@ -287,16 +289,16 @@ def access_user_group_for_setting(
 
     # The API would not allow passing the setting parameter as a Dict
     # if require_system_group is true for a setting.
-    assert permission_configuration.require_system_group is False  # nocoverage
+    assert permission_configuration.require_system_group is False
 
     user_group = update_or_create_user_group_for_setting(
         user_profile.realm,
         setting_user_group.direct_members,
         setting_user_group.direct_subgroups,
         current_setting_value,
-    )  # nocoverage
+    )
 
-    return user_group  # nocoverage
+    return user_group
 
 
 def check_user_group_name(group_name: str) -> str:

zerver/openapi/zulip.yaml

@@ -18389,19 +18389,24 @@ paths:
                     type: integer
                   example: [1, 2, 3, 4]
                 can_mention_group:
-                  description: |
+                  allOf:
-                    ID of the user group whose members are allowed to mention the new
+                    - description: |
-                    user group.
+                        Either the ID of a named user group that has permission to
+                        mention the group, or an object describing the set of users
+                        and groups who have permission mention the new group.
 
-                    This setting cannot be set to `"role:internet"` and `"role:owners"`
+                        This setting cannot be set to `"role:internet"` and
-                    system groups.
+                        `"role:owners"` system groups.
 
-                    **Changes**: Before Zulip 8.0 (feature level 198),
+                        **Changes**: Before Zulip 9.0 (feature level 258), the
-                    the `can_mention_group` setting was named `can_mention_group_id`.
+                        `can_mention_group` setting could only be set to an integer.
 
-                    New in Zulip 8.0 (feature level 191). Previously, groups
+                        **Changes**: Before Zulip 8.0 (feature level 198),
-                    could be mentioned if and only if they were not system groups.
+                        the `can_mention_group` setting was named `can_mention_group_id`.
-                  type: integer
+
+                        New in Zulip 8.0 (feature level 191). Previously, groups
+                        could be mentioned if and only if they were not system groups.
+                    - $ref: "#/components/schemas/CanMentionGroup"
                   example: 11
               required:
                 - name

zerver/tests/test_events.py

@@ -216,6 +216,7 @@ from zerver.lib.test_helpers import (
 from zerver.lib.timestamp import convert_to_UTC, datetime_to_timestamp
 from zerver.lib.topic import TOPIC_NAME
 from zerver.lib.types import ProfileDataElementUpdateDict
+from zerver.lib.user_groups import AnonymousSettingGroupDict
 from zerver.models import (
     Attachment,
     CustomProfileField,
@@ -231,6 +232,7 @@ from zerver.models import (
     RealmUserDefault,
     Service,
     Stream,
+    UserGroup,
     UserMessage,
     UserPresence,
     UserProfile,
@@ -1696,6 +1698,34 @@ class NormalActionsTest(BaseAction):
                 self.user_profile.realm, "backend", [othello], "Backend team", acting_user=None
             )
         check_user_group_add("events[0]", events[0])
+        everyone_group = NamedUserGroup.objects.get(
+            name=SystemGroups.EVERYONE, realm=self.user_profile.realm, is_system_group=True
+        )
+        self.assertEqual(events[0]["group"]["can_mention_group"], everyone_group.id)
+
+        moderators_group = NamedUserGroup.objects.get(
+            name=SystemGroups.MODERATORS, realm=self.user_profile.realm, is_system_group=True
+        )
+        user_group = UserGroup.objects.create(realm=self.user_profile.realm)
+        user_group.direct_members.set([othello])
+        user_group.direct_subgroups.set([moderators_group])
+
+        with self.verify_action() as events:
+            check_add_user_group(
+                self.user_profile.realm,
+                "frontend",
+                [othello],
+                "",
+                {"can_mention_group": user_group},
+                acting_user=None,
+            )
+        check_user_group_add("events[0]", events[0])
+        self.assertEqual(
+            events[0]["group"]["can_mention_group"],
+            AnonymousSettingGroupDict(
+                direct_members=[othello.id], direct_subgroups=[moderators_group.id]
+            ),
+        )
 
         # Test name update
         backend = NamedUserGroup.objects.get(name="backend")
@@ -1710,9 +1740,6 @@ class NormalActionsTest(BaseAction):
         check_user_group_update("events[0]", events[0], "description")
 
         # Test can_mention_group setting update
-        moderators_group = NamedUserGroup.objects.get(
-            name="role:moderators", realm=self.user_profile.realm, is_system_group=True
-        )
         with self.verify_action() as events:
             do_change_user_group_permission_setting(
                 backend, "can_mention_group", moderators_group, acting_user=None

zerver/tests/test_user_groups.py

@@ -434,6 +434,47 @@ class UserGroupAPITestCase(UserGroupTestCase):
         marketing_group = NamedUserGroup.objects.get(name="marketing", realm=hamlet.realm)
         self.assertEqual(marketing_group.can_mention_group, nobody_group.usergroup_ptr)
 
+        othello = self.example_user("othello")
+        params = {
+            "name": "backend",
+            "members": orjson.dumps([hamlet.id]).decode(),
+            "description": "Backend team",
+            "can_mention_group": orjson.dumps(
+                {
+                    "direct_members": [othello.id],
+                    "direct_subgroups": [leadership_group.id, moderators_group.id],
+                }
+            ).decode(),
+        }
+        result = self.client_post("/json/user_groups/create", info=params)
+        self.assert_json_success(result)
+        backend_group = NamedUserGroup.objects.get(name="backend", realm=hamlet.realm)
+        self.assertCountEqual(
+            list(backend_group.can_mention_group.direct_members.all()),
+            [othello],
+        )
+        self.assertCountEqual(
+            list(backend_group.can_mention_group.direct_subgroups.all()),
+            [leadership_group, moderators_group],
+        )
+
+        params = {
+            "name": "help",
+            "members": orjson.dumps([hamlet.id]).decode(),
+            "description": "Troubleshooting team",
+            "can_mention_group": orjson.dumps(
+                {
+                    "direct_members": [],
+                    "direct_subgroups": [moderators_group.id],
+                }
+            ).decode(),
+        }
+        result = self.client_post("/json/user_groups/create", info=params)
+        self.assert_json_success(result)
+        help_group = NamedUserGroup.objects.get(name="help", realm=hamlet.realm)
+        # We do not create a new UserGroup object in such case.
+        self.assertEqual(help_group.can_mention_group_id, moderators_group.id)
+
         internet_group = NamedUserGroup.objects.get(
             name="role:internet", realm=hamlet.realm, is_system_group=True
         )
@@ -471,6 +512,34 @@ class UserGroupAPITestCase(UserGroupTestCase):
         result = self.client_post("/json/user_groups/create", info=params)
         self.assert_json_error(result, "Invalid user group")
 
+        params = {
+            "name": "frontend",
+            "members": orjson.dumps([hamlet.id]).decode(),
+            "description": "Frontend team",
+            "can_mention_group": orjson.dumps(
+                {
+                    "direct_members": [1111],
+                    "direct_subgroups": [leadership_group.id, moderators_group.id],
+                }
+            ).decode(),
+        }
+        result = self.client_post("/json/user_groups/create", info=params)
+        self.assert_json_error(result, "Invalid user ID: 1111")
+
+        params = {
+            "name": "frontend",
+            "members": orjson.dumps([hamlet.id]).decode(),
+            "description": "Frontend team",
+            "can_mention_group": orjson.dumps(
+                {
+                    "direct_members": [othello.id],
+                    "direct_subgroups": [1111, moderators_group.id],
+                }
+            ).decode(),
+        }
+        result = self.client_post("/json/user_groups/create", info=params)
+        self.assert_json_error(result, "Invalid user group ID: 1111")
+
     def test_user_group_get(self) -> None:
         # Test success
         user_profile = self.example_user("hamlet")

zerver/views/user_groups.py

@@ -1,4 +1,4 @@
-from typing import List, Optional, Sequence
+from typing import Dict, List, Optional, Sequence, Union
 
 from django.conf import settings
 from django.db import transaction
@@ -23,7 +23,9 @@ from zerver.lib.exceptions import JsonableError
 from zerver.lib.mention import MentionBackend, silent_mention_syntax_for_user
 from zerver.lib.request import REQ, has_request_variables
 from zerver.lib.response import json_success
+from zerver.lib.types import Validator
 from zerver.lib.user_groups import (
+    AnonymousSettingGroupDict,
     access_user_group_by_id,
     access_user_group_for_setting,
     check_user_group_name,
@@ -36,12 +38,40 @@ from zerver.lib.user_groups import (
     user_groups_in_realm_serialized,
 )
 from zerver.lib.users import access_user_by_id, user_ids_to_users
-from zerver.lib.validator import check_bool, check_int, check_list
+from zerver.lib.validator import check_bool, check_dict_only, check_int, check_list, check_union
 from zerver.models import NamedUserGroup, UserProfile
 from zerver.models.users import get_system_bot
 from zerver.views.streams import compose_views
 
 
+def parse_group_setting_value(
+    setting_value: Union[int, Dict[str, List[int]]],
+) -> Union[int, AnonymousSettingGroupDict]:
+    if isinstance(setting_value, int):
+        return setting_value
+
+    if len(setting_value["direct_members"]) == 0 and len(setting_value["direct_subgroups"]) == 1:
+        return setting_value["direct_subgroups"][0]
+
+    return AnonymousSettingGroupDict(
+        direct_members=setting_value["direct_members"],
+        direct_subgroups=setting_value["direct_subgroups"],
+    )
+
+
+check_group_setting: Validator[Union[int, Dict[str, List[int]]]] = check_union(
+    [
+        check_int,
+        check_dict_only(
+            [
+                ("direct_members", check_list(check_int)),
+                ("direct_subgroups", check_list(check_int)),
+            ]
+        ),
+    ]
+)
+
+
 @require_user_group_edit_permission
 @has_request_variables
 def add_user_group(
@@ -50,8 +80,8 @@ def add_user_group(
     name: str = REQ(),
     members: Sequence[int] = REQ(json_validator=check_list(check_int), default=[]),
     description: str = REQ(),
-    can_mention_group_id: Optional[int] = REQ(
+    can_mention_group: Optional[Union[Dict[str, List[int]], int]] = REQ(
-        "can_mention_group", json_validator=check_int, default=None
+        json_validator=check_group_setting, default=None
     ),
 ) -> HttpResponse:
     user_profiles = user_ids_to_users(members, user_profile.realm)
@@ -60,15 +90,13 @@ def add_user_group(
     group_settings_map = {}
     request_settings_dict = locals()
     for setting_name, permission_config in NamedUserGroup.GROUP_PERMISSION_SETTINGS.items():
-        setting_group_id_name = permission_config.id_field_name
+        if setting_name not in request_settings_dict:  # nocoverage
-
-        if setting_group_id_name not in request_settings_dict:  # nocoverage
             continue
 
-        if request_settings_dict[setting_group_id_name] is not None:
+        if request_settings_dict[setting_name] is not None:
-            setting_value_group_id = request_settings_dict[setting_group_id_name]
+            setting_value = parse_group_setting_value(request_settings_dict[setting_name])
             setting_value_group = access_user_group_for_setting(
-                setting_value_group_id,
+                setting_value,
                 user_profile,
                 setting_name=setting_name,
                 permission_configuration=permission_config,