Groups: Can perform any join, leave, add, remove for deactivated group.

Fixes #33804.

We still do not allow permission settings to be set to deactivated
groups.

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 391**
+
+* [`POST /user_groups/{user_group_id}/members`](/api/update-user-group-members),
+  [`POST /user_groups/{user_group_id}/subgroups`](/api/update-user-group-subgroups):
+  Adding/removing members and subgroups to a deactivated group is now allowed.
+
 **Feature level 390**
 
 * [`GET /events`](/api/get-events): Events with `type: "navigation_view"` are

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 = 390
+API_FEATURE_LEVEL = 391
 
 # 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

web/src/settings_data.ts

@@ -176,10 +176,6 @@ export function can_manage_user_group(group_id: number): boolean {
 
 export function can_add_members_to_user_group(group_id: number): boolean {
     const group = user_groups.get_user_group_from_id(group_id);
-    // We cannot add members if the group is deactivated.
-    if (group.deactivated) {
-        return false;
-    }
     if (
         user_has_permission_for_group_setting(
             group.can_add_members_group,
@@ -195,10 +191,6 @@ export function can_add_members_to_user_group(group_id: number): boolean {
 
 export function can_remove_members_from_user_group(group_id: number): boolean {
     const group = user_groups.get_user_group_from_id(group_id);
-    // We cannot remove members if the group is deactivated.
-    if (group.deactivated) {
-        return false;
-    }
     if (
         user_has_permission_for_group_setting(
             group.can_remove_members_group,
@@ -214,10 +206,6 @@ export function can_remove_members_from_user_group(group_id: number): boolean {
 
 export function can_join_user_group(group_id: number): boolean {
     const group = user_groups.get_user_group_from_id(group_id);
-    // One cannot join a deactivated group.
-    if (group.deactivated) {
-        return false;
-    }
     if (user_has_permission_for_group_setting(group.can_join_group, "can_join_group", "group")) {
         return true;
     }
@@ -226,11 +214,7 @@ export function can_join_user_group(group_id: number): boolean {
 }
 
 export function can_leave_user_group(group_id: number): boolean {
-    // One cannot leave a deactivated group.
     const group = user_groups.get_user_group_from_id(group_id);
-    if (group.deactivated) {
-        return false;
-    }
     if (user_has_permission_for_group_setting(group.can_leave_group, "can_leave_group", "group")) {
         return true;
     }

web/src/user_group_edit.ts

@@ -164,9 +164,9 @@ function update_add_members_elements(group: UserGroup): void {
         $button_element.prop("disabled", true);
         $add_members_container.addClass("add_members_disabled");
 
-        const disable_hint = group.deactivated
-            ? $t({defaultMessage: "Can't add members to a deactivated group"})
-            : $t({defaultMessage: "You are not allowed to add members to this group"});
+        const disable_hint = $t({
+            defaultMessage: "You are not allowed to add members to this group",
+        });
         settings_components.initialize_disable_button_hint_popover(
             $add_members_container,
             disable_hint,
@@ -309,13 +309,8 @@ function initialize_tooltip_for_membership_button(group_id: number): void {
         ".join_leave_button_wrapper",
     );
     const is_member = user_groups.is_user_in_group(group_id, people.my_current_user_id());
-    const is_deactivated = user_groups.get_user_group_from_id(group_id).deactivated;
     let tooltip_message;
-    if (is_deactivated && is_member) {
-        tooltip_message = $t({defaultMessage: "You cannot leave a deactivated user group."});
-    } else if (is_deactivated) {
-        tooltip_message = $t({defaultMessage: "You cannot join a deactivated user group."});
-    } else if (is_member) {
+    if (is_member) {
         tooltip_message = $t({defaultMessage: "You do not have permission to leave this group."});
     } else {
         tooltip_message = $t({defaultMessage: "You do not have permission to join this group."});

web/tests/settings_data.test.cjs

@@ -110,6 +110,8 @@ const deactivated_group = {
     members: new Set([1, 2, 3]),
     is_system_group: false,
     direct_subgroup_ids: new Set([4, 5, 6]),
+    can_add_members_group: 4,
+    can_remove_members_group: 4,
     can_join_group: 1,
     can_leave_group: 1,
     can_manage_group: 1,
@@ -436,8 +438,8 @@ function test_user_group_permission_setting(override, setting_name, permission_f
     override(current_user, "user_id", 2);
     assert.ok(permission_func(students.id));
 
-    // Cannot perform any join, leave, add, remove if group is deactivated
-    assert.ok(!permission_func(deactivated_group.id));
+    // Can perform any join, leave, add, remove even if the group is deactivated
+    assert.ok(permission_func(deactivated_group.id));
 }
 
 run_test("can_join_user_group", ({override}) => {

zerver/lib/user_groups.py

@@ -326,7 +326,10 @@ def lock_subgroups_with_respect_to_supergroup(
         else:
             assert permission_setting is not None
             potential_supergroup = access_user_group_for_update(
-                potential_supergroup_id, acting_user, permission_setting=permission_setting
+                potential_supergroup_id,
+                acting_user,
+                permission_setting=permission_setting,
+                allow_deactivated=True,
             )
         # We avoid making a separate query for user_group_ids because the
         # recursive query already returns those user groups.

zerver/openapi/zulip.yaml

@@ -22518,6 +22518,9 @@ paths:
         Update the members of a [user group](/help/user-groups). The
         user IDs must correspond to non-deactivated users.
 
+        **Changes**: Prior to Zulip 11.0 (feature level 391), members
+        could not be added or removed from a deactivated group.
+
         **Changes**: Prior to Zulip 10.0 (feature level 303), group memberships of
         deactivated users were visible to the API and could be edited via this endpoint.
       x-curl-examples-parameters:
@@ -23111,6 +23114,9 @@ paths:
       description: |
         Update the subgroups of a [user group](/help/user-groups).
 
+        **Changes**: Prior to Zulip 11.0 (feature level 391), subgroups
+        could not be added or removed from a deactivated group.
+
         **Changes**: New in Zulip 6.0 (feature level 127).
       x-curl-examples-parameters:
         oneOf:

zerver/tests/test_user_groups.py

@@ -2122,12 +2122,12 @@ class UserGroupAPITestCase(UserGroupTestCase):
 
         params = {"delete": orjson.dumps([hamlet.id]).decode()}
         result = self.client_post(f"/json/user_groups/{user_group.id}/members", info=params)
-        self.assert_json_error(result, "User group is deactivated.")
-        self.assert_user_membership(user_group, [hamlet])
+        self.assert_json_success(result)
+        self.assert_member_not_in_group(user_group, hamlet)
 
-        params = {"add": orjson.dumps([iago.id]).decode()}
+        params = {"add": orjson.dumps([hamlet.id]).decode()}
         result = self.client_post(f"/json/user_groups/{user_group.id}/members", info=params)
-        self.assert_json_error(result, "User group is deactivated.")
+        self.assert_json_success(result)
         self.assert_user_membership(user_group, [hamlet])
 
     def test_mentions(self) -> None:
@@ -3317,12 +3317,12 @@ class UserGroupAPITestCase(UserGroupTestCase):
 
         params = {"delete": orjson.dumps([leadership_group.id]).decode()}
         result = self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info=params)
-        self.assert_json_error(result, "User group is deactivated.")
-        self.assert_subgroup_membership(support_group, [leadership_group])
+        self.assert_json_success(result)
+        self.assert_subgroup_membership(support_group, [])
 
-        params = {"add": orjson.dumps([test_group.id]).decode()}
+        params = {"add": orjson.dumps([leadership_group.id]).decode()}
         result = self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info=params)
-        self.assert_json_error(result, "User group is deactivated.")
+        self.assert_json_success(result)
         self.assert_subgroup_membership(support_group, [leadership_group])
 
         # Test that a deactivated group cannot be used as a subgroup.

zerver/views/user_groups.py

@@ -334,17 +334,26 @@ def add_members_to_group_backend(
     if len(members) == 1 and user_profile.id == members[0]:
         try:
             user_group = access_user_group_for_update(
-                user_group_id, user_profile, permission_setting="can_join_group"
+                user_group_id,
+                user_profile,
+                permission_setting="can_join_group",
+                allow_deactivated=True,
             )
         except JsonableError:
             # User can still join the group if user has permission to add
             # anyone in the group.
             user_group = access_user_group_for_update(
-                user_group_id, user_profile, permission_setting="can_add_members_group"
+                user_group_id,
+                user_profile,
+                permission_setting="can_add_members_group",
+                allow_deactivated=True,
             )
     else:
         user_group = access_user_group_for_update(
-            user_group_id, user_profile, permission_setting="can_add_members_group"
+            user_group_id,
+            user_profile,
+            permission_setting="can_add_members_group",
+            allow_deactivated=True,
         )
 
     member_users = user_ids_to_users(members, user_profile.realm, allow_deactivated=False)
@@ -381,17 +390,26 @@ def remove_members_from_group_backend(
     if len(members) == 1 and user_profile.id == members[0]:
         try:
             user_group = access_user_group_for_update(
-                user_group_id, user_profile, permission_setting="can_leave_group"
+                user_group_id,
+                user_profile,
+                permission_setting="can_leave_group",
+                allow_deactivated=True,
             )
         except JsonableError:
             # User can still leave the group if user has permission to remove
             # anyone from the group.
             user_group = access_user_group_for_update(
-                user_group_id, user_profile, permission_setting="can_remove_members_group"
+                user_group_id,
+                user_profile,
+                permission_setting="can_remove_members_group",
+                allow_deactivated=True,
             )
     else:
         user_group = access_user_group_for_update(
-            user_group_id, user_profile, permission_setting="can_remove_members_group"
+            user_group_id,
+            user_profile,
+            permission_setting="can_remove_members_group",
+            allow_deactivated=True,
         )
 
     group_member_ids = get_user_group_direct_member_ids(user_group)