From 6f0a7656ac03cec6349f2b8be5f22ae9e81bb7c1 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Wed, 2 Mar 2022 16:28:37 +0530 Subject: [PATCH] user_groups: Add API endpoint for updating subgroups of a user group. --- templates/zerver/api/changelog.md | 2 + .../zerver/help/include/rest-endpoints.md | 1 + zerver/lib/user_groups.py | 15 +++- zerver/openapi/zulip.yaml | 46 +++++++++++ zerver/tests/test_user_groups.py | 81 +++++++++++++++++++ zerver/views/user_groups.py | 70 ++++++++++++++++ zproject/urls.py | 2 + 7 files changed, 216 insertions(+), 1 deletion(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index be8aa20a95..d254108769 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -29,6 +29,8 @@ format used by the Zulip server that they are interacting with. * [`GET /events`](/api/get-events): Added new `user_group` events operations for live updates to subgroups (`add_subgroups` and `remove_subgroups`). +* [`PATCH /user_groups/{user_group_id}/subgroups`](/api/update-user-group-subgroups): + Added new endpoint for updating subgroups of a user group. **Feature level 126** diff --git a/templates/zerver/help/include/rest-endpoints.md b/templates/zerver/help/include/rest-endpoints.md index e93f5a1800..ee9f265405 100644 --- a/templates/zerver/help/include/rest-endpoints.md +++ b/templates/zerver/help/include/rest-endpoints.md @@ -61,6 +61,7 @@ * [Update a user group](/api/update-user-group) * [Delete a user group](/api/remove-user-group) * [Update user group members](/api/update-user-group-members) +* [Update user group subgroups](/api/update-user-group-subgroups) * [Mute a user](/api/mute-user) * [Unmute a user](/api/unmute-user) diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py index ec44b38c52..7e2d0341db 100644 --- a/zerver/lib/user_groups.py +++ b/zerver/lib/user_groups.py @@ -1,4 +1,4 @@ -from typing import Any, Dict, List +from typing import Any, Dict, List, Sequence from django.db import transaction from django.db.models import QuerySet @@ -26,6 +26,19 @@ def access_user_group_by_id(user_group_id: int, user_profile: UserProfile) -> Us return user_group +def access_user_groups_as_potential_subgroups( + user_group_ids: Sequence[int], acting_user: UserProfile +) -> List[UserGroup]: + user_groups = UserGroup.objects.filter(id__in=user_group_ids, realm=acting_user.realm) + + valid_group_ids = [group.id for group in user_groups] + invalid_group_ids = [group_id for group_id in user_group_ids if group_id not in valid_group_ids] + if invalid_group_ids: + raise JsonableError(_("Invalid user group ID: {}").format(invalid_group_ids[0])) + + return list(user_groups) + + def user_groups_in_realm_serialized(realm: Realm) -> List[Dict[str, Any]]: """This function is used in do_events_register code path so this code should be performant. We need to do 2 database queries because diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 7998f53081..a746ed652f 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -13929,6 +13929,52 @@ paths: }, ], } + /user_groups/{user_group_id}/subgroups: + post: + operationId: update-user-group-subgroups + summary: Update subgroups of a user group + tags: ["users"] + description: | + Update the subgroups of a [user group](/help/user-groups). + + `POST {{ api_url }}/v1/user_groups/{user_group_id}/subgroups` + + **Changes**: New in Zulip 6.0 (feature level 127). + x-curl-examples-parameters: + oneOf: + - type: exclude + parameters: + enum: + - delete + parameters: + - $ref: "#/components/parameters/UserGroupId" + - name: delete + in: query + description: | + The list of user group ids to be removed from the user group. + content: + application/json: + schema: + type: array + items: + type: integer + example: [10] + required: false + - name: add + in: query + description: | + The list of user group ids to be added to the user group. + content: + application/json: + schema: + type: array + items: + type: integer + example: [1, 2] + required: false + responses: + "200": + $ref: "#/components/responses/SimpleSuccess" /real-time: # This entry is a hack; it exists to give us a place to put the text # documenting the parameters for call_on_each_event and friends. diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index c5c55b170a..567c626b7b 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -779,3 +779,84 @@ class UserGroupAPITestCase(UserGroupTestCase): user_profile=hamlet, user_group=full_members_group ).exists() ) + + def test_updating_subgroups_of_user_group(self) -> None: + realm = get_realm("zulip") + desdemona = self.example_user("desdemona") + iago = self.example_user("iago") + hamlet = self.example_user("hamlet") + othello = self.example_user("othello") + + leadership_group = create_user_group("leadership", [desdemona, iago, hamlet], realm) + support_group = create_user_group("support", [hamlet, othello], realm) + + self.login("cordelia") + # Non-admin and non-moderators who are not a member of group cannot add or remove subgroups. + 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, "Insufficient permission") + + self.login("iago") + result = self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info=params) + self.assert_json_success(result) + + 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_success(result) + + self.login("shiva") + 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_success(result) + + 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_success(result) + + self.login("hamlet") + # Non-admin and non-moderators who are a member of the user group can add or remove subgroups. + 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_success(result) + + 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_success(result) + + # Users need not be part of the subgroup to add or remove it from a user group. + self.login("othello") + 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_success(result) + + 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_success(result) + + result = self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info=params) + self.assert_json_error( + result, + ("User group {group_id} is not a subgroup of this group.").format( + group_id=leadership_group.id + ), + ) + + params = {"add": orjson.dumps([leadership_group.id]).decode()} + self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info=params) + + result = self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info=params) + self.assert_json_error( + result, + ("User group {group_id} is already a subgroup of this group.").format( + group_id=leadership_group.id + ), + ) + + # Invalid subgroup id will raise an error. + params = {"add": orjson.dumps([leadership_group.id, 101]).decode()} + result = self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info=params) + self.assert_json_error(result, "Invalid user group ID: 101") + + # Test when nothing is provided + result = self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info={}) + self.assert_json_error(result, 'Nothing to do. Specify at least one of "add" or "delete".') diff --git a/zerver/views/user_groups.py b/zerver/views/user_groups.py index 87a5746f5b..4d4ec88475 100644 --- a/zerver/views/user_groups.py +++ b/zerver/views/user_groups.py @@ -4,12 +4,14 @@ from django.http import HttpRequest, HttpResponse from django.utils.translation import gettext as _ from zerver.actions.user_groups import ( + add_subgroups_to_user_group, bulk_add_members_to_user_group, check_add_user_group, check_delete_user_group, do_update_user_group_description, do_update_user_group_name, remove_members_from_user_group, + remove_subgroups_from_user_group, ) from zerver.decorator import require_member_or_admin, require_user_group_edit_permission from zerver.lib.exceptions import JsonableError @@ -17,6 +19,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_groups_as_potential_subgroups, get_direct_memberships_of_users, get_user_group_direct_members, user_groups_in_realm_serialized, @@ -147,3 +150,70 @@ def remove_members_from_group_backend( user_profile_ids = [user.id for user in user_profiles] remove_members_from_user_group(user_group, user_profile_ids) return json_success(request) + + +def add_subgroups_to_group_backend( + request: HttpRequest, user_profile: UserProfile, user_group_id: int, subgroup_ids: Sequence[int] +) -> HttpResponse: + if not subgroup_ids: + return json_success(request) + + subgroups = access_user_groups_as_potential_subgroups(subgroup_ids, user_profile) + user_group = access_user_group_by_id(user_group_id, user_profile) + existing_subgroups = user_group.direct_subgroups.all().values_list("id", flat=True) + for group in subgroups: + if group.id in existing_subgroups: + raise JsonableError( + _("User group {group_id} is already a subgroup of this group.").format( + group_id=group.id + ) + ) + + add_subgroups_to_user_group(user_group, subgroups) + return json_success(request) + + +def remove_subgroups_from_group_backend( + request: HttpRequest, user_profile: UserProfile, user_group_id: int, subgroup_ids: Sequence[int] +) -> HttpResponse: + if not subgroup_ids: + return json_success(request) + + subgroups = access_user_groups_as_potential_subgroups(subgroup_ids, user_profile) + user_group = access_user_group_by_id(user_group_id, user_profile) + existing_subgroups = user_group.direct_subgroups.all().values_list("id", flat=True) + for group in subgroups: + if group.id not in existing_subgroups: + raise JsonableError( + _("User group {group_id} is not a subgroup of this group.").format( + group_id=group.id + ) + ) + + remove_subgroups_from_user_group(user_group, subgroups) + return json_success(request) + + +@require_user_group_edit_permission +@has_request_variables +def update_subgroups_of_user_group( + request: HttpRequest, + user_profile: UserProfile, + user_group_id: int = REQ(json_validator=check_int, path_only=True), + delete: Sequence[int] = REQ(json_validator=check_list(check_int), default=[]), + add: Sequence[int] = REQ(json_validator=check_list(check_int), default=[]), +) -> HttpResponse: + if not add and not delete: + raise JsonableError(_('Nothing to do. Specify at least one of "add" or "delete".')) + + thunks = [ + lambda: add_subgroups_to_group_backend( + request, user_profile, user_group_id=user_group_id, subgroup_ids=add + ), + lambda: remove_subgroups_from_group_backend( + request, user_profile, user_group_id=user_group_id, subgroup_ids=delete + ), + ] + data = compose_views(thunks) + + return json_success(request, data) diff --git a/zproject/urls.py b/zproject/urls.py index 311a0d3652..2f97ca0361 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -177,6 +177,7 @@ from zerver.views.user_groups import ( delete_user_group, edit_user_group, get_user_group, + update_subgroups_of_user_group, update_user_group_backend, ) from zerver.views.user_settings import ( @@ -372,6 +373,7 @@ v1_api_and_json_patterns = [ rest_path("user_groups/create", POST=add_user_group), rest_path("user_groups/", PATCH=edit_user_group, DELETE=delete_user_group), rest_path("user_groups//members", POST=update_user_group_backend), + rest_path("user_groups//subgroups", POST=update_subgroups_of_user_group), # users/me -> zerver.views.user_settings rest_path("users/me/avatar", POST=set_avatar_backend, DELETE=delete_avatar_backend), # users/me/hotspots -> zerver.views.hotspots