From 4bea6ffaa83b7ccbc8445873299393d7ee12fb88 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Wed, 14 Jun 2023 20:18:58 +0530 Subject: [PATCH] user_groups: Add support to set can_mention_group during creation. This commit adds API support to set can_mention_group while creating a user group. Fixes a part of #25927. --- api_docs/changelog.md | 3 ++ zerver/actions/user_groups.py | 20 ++++++-- zerver/lib/user_groups.py | 10 +++- zerver/openapi/zulip.yaml | 15 ++++++ zerver/tests/test_user_groups.py | 82 ++++++++++++++++++++++++++++++++ zerver/views/user_groups.py | 31 +++++++++++- 6 files changed, 154 insertions(+), 7 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index f6d9cc02e8..d58b18b8f3 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -25,6 +25,9 @@ format used by the Zulip server that they are interacting with. * [`GET /events`](/api/get-events), [`POST /register`](/api/register-queue), [`GET /user_groups`](/api/get-user-groups): Add `can_mention_group_id` to user group objects. +* [`POST /user_groups/create`](/api/create-user-group): Added `can_mention_group_id` + parameter to support setting the user group whose members can mention the new user + group. **Feature level 190** diff --git a/zerver/actions/user_groups.py b/zerver/actions/user_groups.py index 7d8b378143..4338a7274e 100644 --- a/zerver/actions/user_groups.py +++ b/zerver/actions/user_groups.py @@ -1,5 +1,5 @@ import datetime -from typing import Dict, List, Optional, Sequence, TypedDict +from typing import Dict, List, Mapping, Optional, Sequence, TypedDict import django.db.utils from django.db import transaction @@ -37,14 +37,20 @@ def create_user_group_in_database( *, acting_user: Optional[UserProfile], description: str = "", + group_settings_map: Mapping[str, UserGroup] = {}, is_system_group: bool = False, ) -> UserGroup: user_group = UserGroup( name=name, realm=realm, description=description, is_system_group=is_system_group ) - system_groups_name_dict = get_role_based_system_groups_dict(realm) - user_group = set_defaults_for_group_settings(user_group, system_groups_name_dict) + for setting_name, setting_value in group_settings_map.items(): + setattr(user_group, setting_name, setting_value) + + system_groups_name_dict = get_role_based_system_groups_dict(realm) + user_group = set_defaults_for_group_settings( + user_group, group_settings_map, system_groups_name_dict + ) user_group.save() UserGroupMembership.objects.bulk_create( @@ -150,12 +156,18 @@ def check_add_user_group( name: str, initial_members: List[UserProfile], description: str = "", + group_settings_map: Mapping[str, UserGroup] = {}, *, acting_user: Optional[UserProfile], ) -> UserGroup: try: user_group = create_user_group_in_database( - name, initial_members, realm, description=description, acting_user=acting_user + name, + initial_members, + realm, + description=description, + group_settings_map=group_settings_map, + acting_user=acting_user, ) do_send_create_user_group_event(user_group, initial_members) return user_group diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py index cdeb0b4c97..386e234d5f 100644 --- a/zerver/lib/user_groups.py +++ b/zerver/lib/user_groups.py @@ -1,4 +1,4 @@ -from typing import Dict, Iterable, List, Sequence, TypedDict +from typing import Dict, Iterable, List, Mapping, Sequence, TypedDict from django.db import transaction from django.db.models import F, QuerySet @@ -239,9 +239,15 @@ def get_role_based_system_groups_dict(realm: Realm) -> Dict[str, UserGroup]: def set_defaults_for_group_settings( user_group: UserGroup, + group_settings_map: Mapping[str, UserGroup], system_groups_name_dict: Dict[str, UserGroup], ) -> UserGroup: for setting_name, permission_config in UserGroup.GROUP_PERMISSION_SETTINGS.items(): + if setting_name in group_settings_map: + # We skip the settings for which a value is passed + # in user group creation API request. + continue + if user_group.is_system_group and permission_config.default_for_system_groups is not None: default_group_name = permission_config.default_for_system_groups else: @@ -315,7 +321,7 @@ def create_system_user_groups_for_realm(realm: Realm) -> Dict[int, UserGroup]: groups_with_updated_settings = [] system_groups_name_dict = get_role_based_system_groups_dict(realm) for group in system_user_groups_list: - user_group = set_defaults_for_group_settings(group, system_groups_name_dict) + user_group = set_defaults_for_group_settings(group, {}, system_groups_name_dict) groups_with_updated_settings.append(group) UserGroup.objects.bulk_update(groups_with_updated_settings, ["can_mention_group"]) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 5cea26d44c..6d53ebfcee 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -16164,6 +16164,21 @@ paths: type: integer example: [1, 2, 3, 4] required: true + - name: can_mention_group_id + in: query + description: | + ID of the user group whose members are allowed to mention the new + user group. + + This setting cannot be set to `"@role:internet"` and `"@role:owners"` + system groups. + + **Changes**: New in Zulip 8.0 (feature level 191). Previously, groups + could be mentioned if and only if they were not system groups. + schema: + type: integer + example: 11 + required: false responses: "200": $ref: "#/components/responses/SimpleSuccess" diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index 0a6d13ea88..5c82e5207b 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -267,6 +267,88 @@ class UserGroupAPITestCase(UserGroupTestCase): self.assert_json_error(result, "User group 'support' already exists.") self.assert_length(UserGroup.objects.filter(realm=hamlet.realm), 10) + def test_can_mention_group_setting_during_user_group_creation(self) -> None: + self.login("hamlet") + hamlet = self.example_user("hamlet") + leadership_group = check_add_user_group( + hamlet.realm, "leadership", [hamlet], acting_user=None + ) + moderators_group = UserGroup.objects.get( + name="@role:moderators", realm=hamlet.realm, is_system_group=True + ) + params = { + "name": "support", + "members": orjson.dumps([hamlet.id]).decode(), + "description": "Support team", + "can_mention_group_id": orjson.dumps(moderators_group.id).decode(), + } + result = self.client_post("/json/user_groups/create", info=params) + self.assert_json_success(result) + support_group = UserGroup.objects.get(name="support", realm=hamlet.realm) + self.assertEqual(support_group.can_mention_group, moderators_group) + + params = { + "name": "test", + "members": orjson.dumps([hamlet.id]).decode(), + "description": "Test group", + "can_mention_group_id": orjson.dumps(leadership_group.id).decode(), + } + result = self.client_post("/json/user_groups/create", info=params) + self.assert_json_success(result) + test_group = UserGroup.objects.get(name="test", realm=hamlet.realm) + self.assertEqual(test_group.can_mention_group, leadership_group) + + nobody_group = UserGroup.objects.get( + name="@role:nobody", realm=hamlet.realm, is_system_group=True + ) + params = { + "name": "marketing", + "members": orjson.dumps([hamlet.id]).decode(), + "description": "Marketing team", + "can_mention_group_id": orjson.dumps(nobody_group.id).decode(), + } + result = self.client_post("/json/user_groups/create", info=params) + self.assert_json_success(result) + marketing_group = UserGroup.objects.get(name="marketing", realm=hamlet.realm) + self.assertEqual(marketing_group.can_mention_group, nobody_group) + + internet_group = UserGroup.objects.get( + name="@role:internet", realm=hamlet.realm, is_system_group=True + ) + params = { + "name": "frontend", + "members": orjson.dumps([hamlet.id]).decode(), + "description": "Frontend team", + "can_mention_group_id": orjson.dumps(internet_group.id).decode(), + } + result = self.client_post("/json/user_groups/create", info=params) + self.assert_json_error( + result, "'can_mention_group' setting cannot be set to '@role:internet' group." + ) + + owners_group = UserGroup.objects.get( + name="@role:owners", realm=hamlet.realm, is_system_group=True + ) + params = { + "name": "frontend", + "members": orjson.dumps([hamlet.id]).decode(), + "description": "Frontend team", + "can_mention_group_id": orjson.dumps(owners_group.id).decode(), + } + result = self.client_post("/json/user_groups/create", info=params) + self.assert_json_error( + result, "'can_mention_group' setting cannot be set to '@role:owners' group." + ) + + params = { + "name": "frontend", + "members": orjson.dumps([hamlet.id]).decode(), + "description": "Frontend team", + "can_mention_group_id": orjson.dumps(1111).decode(), + } + result = self.client_post("/json/user_groups/create", info=params) + self.assert_json_error(result, "Invalid user group") + def test_user_group_get(self) -> None: # Test success user_profile = self.example_user("hamlet") diff --git a/zerver/views/user_groups.py b/zerver/views/user_groups.py index 75539284b0..e0f06ecd59 100644 --- a/zerver/views/user_groups.py +++ b/zerver/views/user_groups.py @@ -23,6 +23,7 @@ from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_success from zerver.lib.user_groups import ( access_user_group_by_id, + access_user_group_for_setting, access_user_groups_as_potential_subgroups, get_direct_memberships_of_users, get_recursive_subgroups_for_groups, @@ -46,10 +47,38 @@ 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(json_validator=check_int, default=None), ) -> HttpResponse: user_profiles = user_ids_to_users(members, user_profile.realm) + + group_settings_map = {} + request_settings_dict = locals() + for setting_name, permission_config in UserGroup.GROUP_PERMISSION_SETTINGS.items(): + setting_group_id_name = setting_name + "_id" + + if setting_group_id_name not in request_settings_dict: # nocoverage + continue + + if request_settings_dict[setting_group_id_name] is not None: + setting_value_group_id = request_settings_dict[setting_group_id_name] + setting_value_group = access_user_group_for_setting( + setting_value_group_id, + user_profile, + setting_name=setting_name, + require_system_group=permission_config.require_system_group, + allow_internet_group=permission_config.allow_internet_group, + allow_owners_group=permission_config.allow_owners_group, + allow_nobody_group=permission_config.allow_nobody_group, + ) + group_settings_map[setting_name] = setting_value_group + check_add_user_group( - user_profile.realm, name, user_profiles, description, acting_user=user_profile + user_profile.realm, + name, + user_profiles, + description, + group_settings_map=group_settings_map, + acting_user=user_profile, ) return json_success(request)