From 6b2ca03174653edfc0c7e25caa47fec7642add43 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Thu, 15 Jun 2023 08:54:23 +0530 Subject: [PATCH] user_groups: Add support to update can_mention_group setting. This commit adds API support to update can_mention_group setting of a user group. Fixes a part of #25927. --- api_docs/changelog.md | 3 ++ version.py | 2 +- zerver/actions/user_groups.py | 23 ++++++++++- zerver/lib/event_schema.py | 1 + zerver/openapi/zulip.yaml | 23 +++++++++++ zerver/tests/test_events.py | 12 ++++++ zerver/tests/test_user_groups.py | 67 ++++++++++++++++++++++++++++++++ zerver/views/user_groups.py | 26 ++++++++++++- 8 files changed, 153 insertions(+), 4 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index d58b18b8f3..888a207416 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -28,6 +28,9 @@ format used by the Zulip server that they are interacting with. * [`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. +* [`PATCH /user_groups/{user_group_id}`](/api/update-user-group): Added + `can_mention_group_id` parameter to support changing the user group whose + members can mention the specified user group. **Feature level 190** diff --git a/version.py b/version.py index 2c26e9f8c5..087260ac6e 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 190 +API_FEATURE_LEVEL = 191 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/actions/user_groups.py b/zerver/actions/user_groups.py index 4338a7274e..d5bb8f86c8 100644 --- a/zerver/actions/user_groups.py +++ b/zerver/actions/user_groups.py @@ -1,5 +1,5 @@ import datetime -from typing import Dict, List, Mapping, Optional, Sequence, TypedDict +from typing import Dict, List, Mapping, Optional, Sequence, TypedDict, Union import django.db.utils from django.db import transaction @@ -175,7 +175,9 @@ def check_add_user_group( raise JsonableError(_("User group '{}' already exists.").format(name)) -def do_send_user_group_update_event(user_group: UserGroup, data: Dict[str, str]) -> None: +def do_send_user_group_update_event( + user_group: UserGroup, data: Dict[str, Union[str, int]] +) -> None: event = dict(type="user_group", op="update", group_id=user_group.id, data=data) send_event(user_group.realm, event, active_user_ids(user_group.realm_id)) @@ -273,3 +275,20 @@ def check_delete_user_group( user_group = access_user_group_by_id(user_group_id, user_profile) user_group.delete() do_send_delete_user_group_event(user_profile.realm, user_group_id, user_profile.realm.id) + + +def do_change_user_group_permission_setting( + user_group: UserGroup, + setting_name: str, + setting_value_group: UserGroup, + *, + acting_user: Optional[UserProfile], +) -> None: + setattr(user_group, setting_name, setting_value_group) + user_group.save() + + # RealmAuditLog changes are being done in a separate PR and will be + # added here once that is merged. + setting_id_name = setting_name + "_id" + event_data_dict: Dict[str, Union[str, int]] = {setting_id_name: setting_value_group.id} + do_send_user_group_update_event(user_group, event_data_dict) diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 232c3fae9c..c68877f5c0 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -1741,6 +1741,7 @@ user_group_data_type = DictType( # force vertical ("name", str), ("description", str), + ("can_mention_group_id", int), ], ) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 6d53ebfcee..7a89b80340 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -2946,6 +2946,14 @@ paths: description: | The new description of the group. Only present if the description changed. + can_mention_group_id: + type: integer + description: | + ID of the new user group whose members are allowed to mention the + group. + + **Changes**: New in Zulip 8.0 (feature level 191). Previously, groups + could be mentioned if and only if they were not system groups. example: { "type": "user_group", @@ -16301,6 +16309,21 @@ paths: schema: type: string example: The marketing team. + - name: can_mention_group_id + in: query + description: | + ID of the new user group whose members are allowed to mention the + 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: 12 + required: false responses: "200": $ref: "#/components/responses/SimpleSuccess" diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index a853fd0f73..de367a16ad 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -101,6 +101,7 @@ from zerver.actions.user_groups import ( bulk_add_members_to_user_group, 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, @@ -1397,6 +1398,17 @@ class NormalActionsTest(BaseAction): ) check_user_group_update("events[0]", events[0], "description") + # Test can_mention_group setting update + moderators_group = UserGroup.objects.get( + name="@role:moderators", realm=self.user_profile.realm, is_system_group=True + ) + events = self.verify_action( + lambda: do_change_user_group_permission_setting( + backend, "can_mention_group", moderators_group, acting_user=None + ) + ) + check_user_group_update("events[0]", events[0], "can_mention_group_id") + # Test add members hamlet = self.example_user("hamlet") events = self.verify_action( diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index 5c82e5207b..505518d359 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -411,6 +411,73 @@ class UserGroupAPITestCase(UserGroupTestCase): result = self.client_patch(f"/json/user_groups/{lear_test_group.id}", info=params) self.assert_json_error(result, "Invalid user group") + def test_update_can_mention_group_setting(self) -> None: + hamlet = self.example_user("hamlet") + support_group = check_add_user_group(hamlet.realm, "support", [hamlet], acting_user=None) + marketing_group = check_add_user_group( + hamlet.realm, "marketing", [hamlet], acting_user=None + ) + + moderators_group = UserGroup.objects.get( + name="@role:moderators", realm=hamlet.realm, is_system_group=True + ) + + self.login("hamlet") + params = { + "can_mention_group_id": orjson.dumps(moderators_group.id).decode(), + } + result = self.client_patch(f"/json/user_groups/{support_group.id}", 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 = { + "can_mention_group_id": orjson.dumps(marketing_group.id).decode(), + } + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_success(result) + support_group = UserGroup.objects.get(name="support", realm=hamlet.realm) + self.assertEqual(support_group.can_mention_group, marketing_group) + + nobody_group = UserGroup.objects.get( + name="@role:nobody", realm=hamlet.realm, is_system_group=True + ) + params = { + "can_mention_group_id": orjson.dumps(nobody_group.id).decode(), + } + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_success(result) + support_group = UserGroup.objects.get(name="support", realm=hamlet.realm) + self.assertEqual(support_group.can_mention_group, nobody_group) + + owners_group = UserGroup.objects.get( + name="@role:owners", realm=hamlet.realm, is_system_group=True + ) + params = { + "can_mention_group_id": orjson.dumps(owners_group.id).decode(), + } + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_error( + result, "'can_mention_group' setting cannot be set to '@role:owners' group." + ) + + internet_group = UserGroup.objects.get( + name="@role:internet", realm=hamlet.realm, is_system_group=True + ) + params = { + "can_mention_group_id": orjson.dumps(internet_group.id).decode(), + } + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_error( + result, "'can_mention_group' setting cannot be set to '@role:internet' group." + ) + + params = { + "can_mention_group_id": orjson.dumps(1111).decode(), + } + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_error(result, "Invalid user group") + def test_user_group_update_to_already_existing_name(self) -> None: hamlet = self.example_user("hamlet") self.login_user(hamlet) diff --git a/zerver/views/user_groups.py b/zerver/views/user_groups.py index e0f06ecd59..a7cf0bfb88 100644 --- a/zerver/views/user_groups.py +++ b/zerver/views/user_groups.py @@ -11,6 +11,7 @@ from zerver.actions.user_groups import ( bulk_add_members_to_user_group, 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, @@ -98,8 +99,9 @@ def edit_user_group( user_group_id: int = REQ(json_validator=check_int, path_only=True), name: Optional[str] = REQ(default=None), description: Optional[str] = REQ(default=None), + can_mention_group_id: Optional[int] = REQ(json_validator=check_int, default=None), ) -> HttpResponse: - if name is None and description is None: + if name is None and description is None and can_mention_group_id is None: raise JsonableError(_("No new data supplied")) user_group = access_user_group_by_id(user_group_id, user_profile) @@ -110,6 +112,28 @@ def edit_user_group( if description is not None and description != user_group.description: do_update_user_group_description(user_group, description, acting_user=user_profile) + 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, + ) + do_change_user_group_permission_setting( + user_group, setting_name, setting_value_group, acting_user=user_profile + ) + return json_success(request)