user_groups: Migrate to @typed_endpoint.

Migrate the following endpoints from @has_request_variables
to @typed_endpoint:

- get_user_group()
- delete_user_group()
- update_user_group_backend()
- update_subgroups_of_user_group()
- get_is_user_group_member()
- get_user_group_members()
- get_subgroups_of_user_group()

With tweaks from tabbott to avoid calling thunks unnecessarily.
This commit is contained in:
bedo 2024-07-26 12:15:15 +03:00 committed by Tim Abbott
parent 163863c0d1
commit 1ab4723d59
2 changed files with 73 additions and 63 deletions

View File

@ -1,7 +1,6 @@
import inspect
import os
import types
from collections import abc
from collections.abc import Callable, Mapping, Sequence
from typing import Any, Union, get_args, get_origin
from unittest.mock import MagicMock, patch
@ -341,9 +340,6 @@ so maybe we shouldn't mark it as intentionally undocumented in the URLs.
elif origin in (Union, types.UnionType):
subtypes = [self.get_standardized_argument_type(st) for st in get_args(t)]
return self.get_type_by_priority(subtypes)
elif origin in [list, abc.Sequence]:
[st] = get_args(t)
return (list, self.get_standardized_argument_type(st))
raise AssertionError(f"Unknown origin {origin}")
def render_openapi_type_exception(

View File

@ -1,5 +1,3 @@
from collections.abc import Sequence
from django.conf import settings
from django.db import transaction
from django.http import HttpRequest, HttpResponse
@ -22,9 +20,8 @@ from zerver.actions.user_groups import (
from zerver.decorator import require_member_or_admin, require_user_group_edit_permission
from zerver.lib.exceptions import JsonableError
from zerver.lib.mention import MentionBackend, silent_mention_syntax_for_user
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_success
from zerver.lib.typed_endpoint import PathOnly, typed_endpoint
from zerver.lib.typed_endpoint import PathOnly, typed_endpoint, typed_endpoint_without_parameters
from zerver.lib.user_groups import (
AnonymousSettingGroupDict,
GroupSettingChangeRequest,
@ -43,7 +40,6 @@ from zerver.lib.user_groups import (
validate_group_setting_value_change,
)
from zerver.lib.users import access_user_by_id, user_ids_to_users
from zerver.lib.validator import check_bool, check_int, check_list
from zerver.models import NamedUserGroup, UserProfile
from zerver.models.users import get_system_bot
from zerver.views.streams import compose_views
@ -56,7 +52,7 @@ def add_user_group(
user_profile: UserProfile,
*,
name: str,
members: Json[Sequence[int]],
members: Json[list[int]],
description: str,
can_mention_group: Json[int | AnonymousSettingGroupDict] | None = None,
) -> HttpResponse:
@ -93,7 +89,7 @@ def add_user_group(
@require_member_or_admin
@has_request_variables
@typed_endpoint_without_parameters
def get_user_group(request: HttpRequest, user_profile: UserProfile) -> HttpResponse:
user_groups = user_groups_in_realm_serialized(user_profile.realm)
return json_success(request, data={"user_groups": user_groups})
@ -164,11 +160,12 @@ def edit_user_group(
@require_user_group_edit_permission
@has_request_variables
@typed_endpoint
def delete_user_group(
request: HttpRequest,
user_profile: UserProfile,
user_group_id: int = REQ(json_validator=check_int, path_only=True),
*,
user_group_id: PathOnly[Json[int]],
) -> HttpResponse:
# For deletion, the user group's recursive subgroups and the user group itself are locked.
with lock_subgroups_with_respect_to_supergroup(
@ -179,25 +176,32 @@ def delete_user_group(
@require_user_group_edit_permission
@has_request_variables
@typed_endpoint
def update_user_group_backend(
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=[]),
*,
user_group_id: PathOnly[Json[int]],
delete: Json[list[int]] | None = None,
add: 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".'))
thunks = [
lambda: add_members_to_group_backend(
request, user_profile, user_group_id=user_group_id, members=add
),
lambda: remove_members_from_group_backend(
request, user_profile, user_group_id=user_group_id, members=delete
),
]
thunks = []
if add:
thunks.append(
lambda: add_members_to_group_backend(
request, user_profile, user_group_id=user_group_id, members=add
)
)
if delete:
thunks.append(
lambda: remove_members_from_group_backend(
request, user_profile, user_group_id=user_group_id, members=delete
)
)
data = compose_views(thunks)
return json_success(request, data)
@ -254,11 +258,11 @@ def notify_for_user_group_subscription_changes(
@transaction.atomic
def add_members_to_group_backend(
request: HttpRequest, user_profile: UserProfile, user_group_id: int, members: Sequence[int]
request: HttpRequest,
user_profile: UserProfile,
user_group_id: int,
members: list[int],
) -> HttpResponse:
if not members:
return json_success(request)
user_group = access_user_group_by_id(user_group_id, user_profile, for_read=False)
member_users = user_ids_to_users(members, user_profile.realm)
existing_member_ids = set(
@ -286,11 +290,11 @@ def add_members_to_group_backend(
@transaction.atomic
def remove_members_from_group_backend(
request: HttpRequest, user_profile: UserProfile, user_group_id: int, members: Sequence[int]
request: HttpRequest,
user_profile: UserProfile,
user_group_id: int,
members: list[int],
) -> HttpResponse:
if not members:
return json_success(request)
user_profiles = user_ids_to_users(members, user_profile.realm)
user_group = access_user_group_by_id(user_group_id, user_profile, for_read=False)
group_member_ids = get_user_group_direct_member_ids(user_group)
@ -312,11 +316,11 @@ def remove_members_from_group_backend(
def add_subgroups_to_group_backend(
request: HttpRequest, user_profile: UserProfile, user_group_id: int, subgroup_ids: Sequence[int]
request: HttpRequest,
user_profile: UserProfile,
user_group_id: int,
subgroup_ids: list[int],
) -> HttpResponse:
if not subgroup_ids:
return json_success(request)
with lock_subgroups_with_respect_to_supergroup(
subgroup_ids, user_group_id, user_profile
) as context:
@ -348,11 +352,11 @@ def add_subgroups_to_group_backend(
def remove_subgroups_from_group_backend(
request: HttpRequest, user_profile: UserProfile, user_group_id: int, subgroup_ids: Sequence[int]
request: HttpRequest,
user_profile: UserProfile,
user_group_id: int,
subgroup_ids: list[int],
) -> HttpResponse:
if not subgroup_ids:
return json_success(request)
with lock_subgroups_with_respect_to_supergroup(
subgroup_ids, user_group_id, user_profile
) as context:
@ -378,38 +382,46 @@ def remove_subgroups_from_group_backend(
@require_user_group_edit_permission
@has_request_variables
@typed_endpoint
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=[]),
*,
user_group_id: PathOnly[Json[int]],
delete: Json[list[int]] | None = None,
add: 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".'))
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
),
]
thunks = []
if add:
thunks.append(
lambda: add_subgroups_to_group_backend(
request, user_profile, user_group_id=user_group_id, subgroup_ids=add
)
)
if delete:
thunks.append(
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)
@require_member_or_admin
@has_request_variables
@typed_endpoint
def get_is_user_group_member(
request: HttpRequest,
user_profile: UserProfile,
user_group_id: int = REQ(json_validator=check_int, path_only=True),
user_id: int = REQ(json_validator=check_int, path_only=True),
direct_member_only: bool = REQ(json_validator=check_bool, default=False),
*,
user_group_id: PathOnly[Json[int]],
user_id: PathOnly[Json[int]],
direct_member_only: Json[bool] = False,
) -> HttpResponse:
user_group = access_user_group_by_id(user_group_id, user_profile, for_read=True)
target_user = access_user_by_id(user_profile, user_id, for_admin=False)
@ -425,12 +437,13 @@ def get_is_user_group_member(
@require_member_or_admin
@has_request_variables
@typed_endpoint
def get_user_group_members(
request: HttpRequest,
user_profile: UserProfile,
user_group_id: int = REQ(json_validator=check_int, path_only=True),
direct_member_only: bool = REQ(json_validator=check_bool, default=False),
*,
user_group_id: PathOnly[Json[int]],
direct_member_only: Json[bool] = False,
) -> HttpResponse:
user_group = access_user_group_by_id(user_group_id, user_profile, for_read=True)
@ -443,12 +456,13 @@ def get_user_group_members(
@require_member_or_admin
@has_request_variables
@typed_endpoint
def get_subgroups_of_user_group(
request: HttpRequest,
user_profile: UserProfile,
user_group_id: int = REQ(json_validator=check_int, path_only=True),
direct_subgroup_only: bool = REQ(json_validator=check_bool, default=False),
*,
user_group_id: PathOnly[Json[int]],
direct_subgroup_only: Json[bool] = False,
) -> HttpResponse:
user_group = access_user_group_by_id(user_group_id, user_profile, for_read=True)