From 54b51823e613fceb260d2c1b91113b9c26f6676e Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Mon, 7 Apr 2025 17:19:49 +0530 Subject: [PATCH] user_groups: Add API support to reactivate a user group. This commit adds support to reactivate a user group using `PATCH /user_groups/{user_group_id}` endpoint. Fixes part of #23568. --- api_docs/changelog.md | 6 ++ version.py | 2 +- zerver/openapi/zulip.yaml | 21 +++++- zerver/tests/test_user_groups.py | 112 +++++++++++++++++++++++++++++++ zerver/views/user_groups.py | 6 ++ 5 files changed, 145 insertions(+), 2 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 0bffb2dbd6..9f9a9a4705 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 11.0 +**Feature level 386** + +* [`PATCH /user_groups/{user_group_id}`](/api/update-user-group): + Added support to reactivate groups by passing `deactivated` + parameter as `False`. + **Feature level 385** * [`POST /register`](/api/register-queue), [`PATCH/settings`](/api/update-settings), diff --git a/version.py b/version.py index 88e34c2403..04c8096890 100644 --- a/version.py +++ b/version.py @@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 385 +API_FEATURE_LEVEL = 386 # 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/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 03d3573390..24065da2bf 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -22135,13 +22135,18 @@ paths: Update the name, description or any of the permission settings of a [user group](/help/user-groups). + This endpoint is also used to reactivate a user group. + Note that while permissions settings of deactivated groups can be edited by this API endpoint, and those permissions settings do affect the ability to modify the deactivated group and its membership, the deactivated group itself cannot be mentioned or used in the value of any permission without first being reactivated. - **Changes**: Prior to Zulip 10.0 (feature level 340), only the name field + **Changes**: Starting with Zulip 11.0 (feature level ZF-61b9bf), this + endpoint can be used to reactivate a user group. + + Prior to Zulip 10.0 (feature level 340), only the name field of deactivated groups could be modified. parameters: - $ref: "#/components/parameters/UserGroupId" @@ -22293,6 +22298,18 @@ paths: "old": 11, } - $ref: "#/components/schemas/GroupSettingValueUpdate" + deactivated: + description: | + A deactivated user group can be reactivated by passing this + parameter as `false`. + + Passing `true` does nothing as user group is deactivated + using [`POST /user_groups/{user_group_id}/deactivate`](deactivate-user-group) + endpoint. + + **Changes**: New in Zulip 11.0 (feature level 386). + type: boolean + example: false encoding: can_add_members_group: contentType: application/json @@ -22306,6 +22323,8 @@ paths: contentType: application/json can_remove_members_group: contentType: application/json + deactivated: + contentType: application/json responses: "200": $ref: "#/components/responses/SimpleSuccess" diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index d6df82ed4c..85334c0637 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -1824,6 +1824,118 @@ class UserGroupAPITestCase(UserGroupTestCase): leadership_group, setting_name, moderators_group, acting_user=None ) + def test_user_group_reactivation(self) -> None: + support_group = self.create_user_group_for_test( + "support", acting_user=self.example_user("desdemona") + ) + realm = get_realm("zulip") + + admins_group = NamedUserGroup.objects.get(name=SystemGroups.ADMINISTRATORS, realm=realm) + do_change_realm_permission_group_setting( + realm, + "can_manage_all_groups", + admins_group, + acting_user=None, + ) + + do_deactivate_user_group(support_group, acting_user=None) + self.login("othello") + params = {"deactivated": orjson.dumps(False).decode()} + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_error(result, "Insufficient permission") + + members_group = NamedUserGroup.objects.get(name=SystemGroups.MEMBERS, realm=realm) + do_change_realm_permission_group_setting( + realm, + "can_manage_all_groups", + members_group, + acting_user=None, + ) + + self.login("othello") + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_success(result) + support_group = NamedUserGroup.objects.get(name="support", realm=realm) + self.assertFalse(support_group.deactivated) + + do_deactivate_user_group(support_group, acting_user=None) + + # Check admins can deactivate groups even if they are not members + # of the group. + self.login("iago") + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_success(result) + support_group = NamedUserGroup.objects.get(name="support", realm=realm) + self.assertFalse(support_group.deactivated) + + do_deactivate_user_group(support_group, acting_user=None) + + # Check moderators can deactivate groups if they are allowed by + # can_manage_all_groups even when they are not members of the group. + admins_group = NamedUserGroup.objects.get( + name=SystemGroups.ADMINISTRATORS, realm=realm, is_system_group=True + ) + do_change_realm_permission_group_setting( + realm, + "can_manage_all_groups", + admins_group, + acting_user=None, + ) + self.login("shiva") + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_error(result, "Insufficient permission") + + moderators_group = NamedUserGroup.objects.get( + name=SystemGroups.MODERATORS, realm=realm, is_system_group=True + ) + do_change_realm_permission_group_setting( + realm, + "can_manage_all_groups", + moderators_group, + acting_user=None, + ) + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_success(result) + support_group = NamedUserGroup.objects.get(name="support", realm=realm) + self.assertFalse(support_group.deactivated) + + do_deactivate_user_group(support_group, acting_user=None) + + do_change_realm_permission_group_setting( + realm, + "can_manage_all_groups", + admins_group, + acting_user=None, + ) + do_change_user_group_permission_setting( + support_group, "can_manage_group", admins_group, acting_user=None + ) + + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_error(result, "Insufficient permission") + + do_change_user_group_permission_setting( + support_group, "can_manage_group", moderators_group, acting_user=None + ) + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_success(result) + support_group = NamedUserGroup.objects.get(name="support", realm=realm) + self.assertFalse(support_group.deactivated) + + do_deactivate_user_group(support_group, acting_user=None) + + setting_group = self.create_or_update_anonymous_group_for_setting( + [self.example_user("shiva")], [admins_group] + ) + do_change_user_group_permission_setting( + support_group, "can_manage_group", setting_group, acting_user=None + ) + + result = self.client_patch(f"/json/user_groups/{support_group.id}", info=params) + self.assert_json_success(result) + support_group = NamedUserGroup.objects.get(name="support", realm=realm) + self.assertFalse(support_group.deactivated) + def test_query_counts(self) -> None: hamlet = self.example_user("hamlet") cordelia = self.example_user("cordelia") diff --git a/zerver/views/user_groups.py b/zerver/views/user_groups.py index c256002323..1b22171c00 100644 --- a/zerver/views/user_groups.py +++ b/zerver/views/user_groups.py @@ -13,6 +13,7 @@ from zerver.actions.user_groups import ( check_add_user_group, do_change_user_group_permission_setting, do_deactivate_user_group, + do_reactivate_user_group, do_update_user_group_description, do_update_user_group_name, remove_subgroups_from_user_group, @@ -139,6 +140,7 @@ def edit_user_group( can_manage_group: Json[GroupSettingChangeRequest] | None = None, can_mention_group: Json[GroupSettingChangeRequest] | None = None, can_remove_members_group: Json[GroupSettingChangeRequest] | None = None, + deactivated: Json[bool] | None = None, ) -> HttpResponse: if ( name is None @@ -149,6 +151,7 @@ def edit_user_group( and can_manage_group is None and can_mention_group is None and can_remove_members_group is None + and deactivated is None ): raise JsonableError(_("No new data supplied")) @@ -163,6 +166,9 @@ 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) + if deactivated is not None and not deactivated and user_group.deactivated: + do_reactivate_user_group(user_group, acting_user=user_profile) + request_settings_dict = locals() nobody_group = get_system_user_group_by_name(SystemGroups.NOBODY, user_profile.realm_id) for setting_name, permission_config in NamedUserGroup.GROUP_PERMISSION_SETTINGS.items():