From 1e818c470828e573c43f6e1f67daa9da8b79b723 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Mon, 14 Oct 2024 16:09:22 +0530 Subject: [PATCH] user_groups: Allow updating subgroups and members using same endpoint. `POST /user_groups/{user_group_id}/members` now allows updating subgroups as well. --- api_docs/changelog.md | 6 ++++++ version.py | 2 +- zerver/openapi/zulip.yaml | 24 +++++++++++++++++++++++- zerver/tests/test_user_groups.py | 20 +++++++++++++++++++- zerver/views/user_groups.py | 27 +++++++++++++++++++++++---- 5 files changed, 72 insertions(+), 7 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index bdc2064de8..8c4805a3b6 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 10.0 +**Feature level 311** + +* [`POST /user_groups/{user_group_id}/members`](/api/update-user-group-members): + Added `add_subgroups` and `delete_subgroups` parameters to support updating + subgroups of a user group using this endpoint. + **Feature level 310** * `PATCH /realm`, [`GET /events`](/api/get-events), diff --git a/version.py b/version.py index a64ac69648..63b61606be 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 = 310 # Last bumped for adding `can_move_messages_between_channels_group`. +API_FEATURE_LEVEL = 311 # Last bumped for updating subgroups. # 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 1bf05eecff..c3868426c9 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -20430,11 +20430,33 @@ paths: items: type: integer example: [12, 13] + delete_subgroups: + description: | + The list of user group IDs to be removed from the user group. + + **Changes**: New in Zulip 10.0 (feature level 311). + type: array + items: + type: integer + example: [9] + add_subgroups: + description: | + The list of user group IDs to be added to the user group. + + **Changes**: New in Zulip 10.0 (feature level 311). + type: array + items: + type: integer + example: [9] encoding: delete: contentType: application/json add: contentType: application/json + delete_subgroups: + contentType: application/json + add_subgroups: + contentType: application/json responses: "200": $ref: "#/components/responses/SimpleSuccess" @@ -21004,7 +21026,7 @@ paths: type: array items: type: integer - example: [9, 10] + example: [10] encoding: delete: contentType: application/json diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index 739d98a654..af6ff72c53 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -1672,9 +1672,27 @@ class UserGroupAPITestCase(UserGroupTestCase): # No notification message is sent for removing from user group. self.assertEqual(self.get_last_message(), initial_last_message) + # Test adding and removing subgroups. + admins_group = NamedUserGroup.objects.get( + name=SystemGroups.ADMINISTRATORS, realm=hamlet.realm, is_system_group=True + ) + cordelia = self.example_user("cordelia") + subgroup = check_add_user_group( + hamlet.realm, "leadership", [cordelia], acting_user=cordelia + ) + params = {"add_subgroups": orjson.dumps([subgroup.id, admins_group.id]).decode()} + result = self.client_post(f"/json/user_groups/{user_group.id}/members", info=params) + self.assert_json_success(result) + self.assert_subgroup_membership(user_group, [subgroup, admins_group]) + + params = {"delete_subgroups": orjson.dumps([admins_group.id]).decode()} + result = self.client_post(f"/json/user_groups/{user_group.id}/members", info=params) + self.assert_json_success(result) + self.assert_subgroup_membership(user_group, [subgroup]) + # Test when nothing is provided result = self.client_post(f"/json/user_groups/{user_group.id}/members", info={}) - msg = 'Nothing to do. Specify at least one of "add" or "delete".' + msg = 'Nothing to do. Specify at least one of "add", "delete", "add_subgroups" or "delete_subgroups".' self.assert_json_error(result, msg) self.assert_user_membership(user_group, [hamlet]) diff --git a/zerver/views/user_groups.py b/zerver/views/user_groups.py index 860d1ea73b..341aeeffab 100644 --- a/zerver/views/user_groups.py +++ b/zerver/views/user_groups.py @@ -212,6 +212,7 @@ def deactivate_user_group( @require_member_or_admin @typed_endpoint +@transaction.atomic(durable=True) def update_user_group_backend( request: HttpRequest, user_profile: UserProfile, @@ -219,9 +220,15 @@ def update_user_group_backend( user_group_id: PathOnly[Json[int]], delete: Json[list[int]] | None = None, add: Json[list[int]] | None = None, + delete_subgroups: Json[list[int]] | None = None, + add_subgroups: Json[list[int]] | None = None, ) -> HttpResponse: - if not add and not delete: - raise JsonableError(_('Nothing to do. Specify at least one of "add" or "delete".')) + if not add and not delete and not add_subgroups and not delete_subgroups: + raise JsonableError( + _( + 'Nothing to do. Specify at least one of "add", "delete", "add_subgroups" or "delete_subgroups".' + ) + ) thunks = [] if add: @@ -237,6 +244,20 @@ def update_user_group_backend( ) ) + if add_subgroups: + thunks.append( + lambda: add_subgroups_to_group_backend( + request, user_profile, user_group_id=user_group_id, subgroup_ids=add_subgroups + ) + ) + + if delete_subgroups: + thunks.append( + lambda: remove_subgroups_from_group_backend( + request, user_profile, user_group_id=user_group_id, subgroup_ids=delete_subgroups + ) + ) + data = compose_views(thunks) return json_success(request, data) @@ -290,7 +311,6 @@ def notify_for_user_group_subscription_changes( do_send_messages(notifications) -@transaction.atomic def add_members_to_group_backend( request: HttpRequest, user_profile: UserProfile, @@ -337,7 +357,6 @@ def add_members_to_group_backend( return json_success(request) -@transaction.atomic def remove_members_from_group_backend( request: HttpRequest, user_profile: UserProfile,