mirror of https://github.com/zulip/zulip.git
groups: Update group objects to pass anonymous groups data for settings.
This commit updates code to handle the can_mention_group field correctly if the setting is set to an anonymous user group.
This commit is contained in:
parent
6b68d7e80d
commit
206014f263
|
@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with.
|
||||||
|
|
||||||
## Changes in Zulip 9.0
|
## Changes in Zulip 9.0
|
||||||
|
|
||||||
|
**Feature level 258**:
|
||||||
|
|
||||||
|
* [`GET /user_groups`](/api/get-user-groups), [`POST
|
||||||
|
/register`](/api/register-queue): `can_mention_group` field can now
|
||||||
|
either be an ID of a named user group with the permission, or an
|
||||||
|
object describing the set of users and groups with the permission.
|
||||||
|
|
||||||
**Feature level 257**:
|
**Feature level 257**:
|
||||||
|
|
||||||
* [`POST /register`](/api/register-queue),
|
* [`POST /register`](/api/register-queue),
|
||||||
|
|
|
@ -3,7 +3,7 @@ from dataclasses import dataclass
|
||||||
from typing import Collection, Dict, Iterable, Iterator, List, Mapping, Optional, TypedDict, Union
|
from typing import Collection, Dict, Iterable, Iterator, List, Mapping, Optional, TypedDict, Union
|
||||||
|
|
||||||
from django.db import connection, transaction
|
from django.db import connection, transaction
|
||||||
from django.db.models import F, QuerySet
|
from django.db.models import F, Prefetch, QuerySet
|
||||||
from django.utils.timezone import now as timezone_now
|
from django.utils.timezone import now as timezone_now
|
||||||
from django.utils.translation import gettext as _
|
from django.utils.translation import gettext as _
|
||||||
from django_cte import With
|
from django_cte import With
|
||||||
|
@ -38,7 +38,7 @@ class UserGroupDict(TypedDict):
|
||||||
members: List[int]
|
members: List[int]
|
||||||
direct_subgroup_ids: List[int]
|
direct_subgroup_ids: List[int]
|
||||||
is_system_group: bool
|
is_system_group: bool
|
||||||
can_mention_group: int
|
can_mention_group: Union[int, AnonymousSettingGroupDict]
|
||||||
|
|
||||||
|
|
||||||
@dataclass
|
@dataclass
|
||||||
|
@ -319,13 +319,44 @@ def check_user_group_name(group_name: str) -> str:
|
||||||
return group_name
|
return group_name
|
||||||
|
|
||||||
|
|
||||||
|
def get_group_setting_value_for_api(
|
||||||
|
setting_value_group: UserGroup,
|
||||||
|
) -> Union[int, AnonymousSettingGroupDict]:
|
||||||
|
if hasattr(setting_value_group, "named_user_group"):
|
||||||
|
return setting_value_group.id
|
||||||
|
|
||||||
|
return AnonymousSettingGroupDict(
|
||||||
|
direct_members=[member.id for member in setting_value_group.direct_members.all()],
|
||||||
|
direct_subgroups=[subgroup.id for subgroup in setting_value_group.direct_subgroups.all()],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def user_groups_in_realm_serialized(realm: Realm) -> List[UserGroupDict]:
|
def user_groups_in_realm_serialized(realm: Realm) -> List[UserGroupDict]:
|
||||||
"""This function is used in do_events_register code path so this code
|
"""This function is used in do_events_register code path so this code
|
||||||
should be performant. We need to do 2 database queries because
|
should be performant. We need to do 2 database queries because
|
||||||
Django's ORM doesn't properly support the left join between
|
Django's ORM doesn't properly support the left join between
|
||||||
UserGroup and UserGroupMembership that we need.
|
UserGroup and UserGroupMembership that we need.
|
||||||
"""
|
"""
|
||||||
realm_groups = NamedUserGroup.objects.filter(realm=realm)
|
realm_groups = (
|
||||||
|
NamedUserGroup.objects.select_related(
|
||||||
|
"can_mention_group", "can_mention_group__named_user_group"
|
||||||
|
)
|
||||||
|
# Using prefetch_related results in one query for each field. This is fine
|
||||||
|
# for now but would be problematic when more settings would be added.
|
||||||
|
#
|
||||||
|
# TODO: We should refactor it such that we only make two queries - one
|
||||||
|
# to fetch all the realm groups and the second to fetch all the groups
|
||||||
|
# that they point to and then set the setting fields for realm groups
|
||||||
|
# accordingly in Python.
|
||||||
|
.prefetch_related(
|
||||||
|
Prefetch("can_mention_group__direct_members", queryset=UserProfile.objects.only("id")),
|
||||||
|
Prefetch(
|
||||||
|
"can_mention_group__direct_subgroups", queryset=NamedUserGroup.objects.only("id")
|
||||||
|
),
|
||||||
|
)
|
||||||
|
.filter(realm=realm)
|
||||||
|
)
|
||||||
|
|
||||||
group_dicts: Dict[int, UserGroupDict] = {}
|
group_dicts: Dict[int, UserGroupDict] = {}
|
||||||
for user_group in realm_groups:
|
for user_group in realm_groups:
|
||||||
group_dicts[user_group.id] = dict(
|
group_dicts[user_group.id] = dict(
|
||||||
|
@ -335,7 +366,7 @@ def user_groups_in_realm_serialized(realm: Realm) -> List[UserGroupDict]:
|
||||||
members=[],
|
members=[],
|
||||||
direct_subgroup_ids=[],
|
direct_subgroup_ids=[],
|
||||||
is_system_group=user_group.is_system_group,
|
is_system_group=user_group.is_system_group,
|
||||||
can_mention_group=user_group.can_mention_group_id,
|
can_mention_group=get_group_setting_value_for_api(user_group.can_mention_group),
|
||||||
)
|
)
|
||||||
|
|
||||||
membership = (
|
membership = (
|
||||||
|
|
|
@ -18672,15 +18672,21 @@ paths:
|
||||||
|
|
||||||
**Changes**: New in Zulip 5.0 (feature level 93).
|
**Changes**: New in Zulip 5.0 (feature level 93).
|
||||||
can_mention_group:
|
can_mention_group:
|
||||||
type: integer
|
allOf:
|
||||||
description: |
|
- $ref: "#/components/schemas/CanMentionGroup"
|
||||||
ID of the user group whose members are allowed to mention the group.
|
- description: |
|
||||||
|
Either the ID of a named user group that has permission to
|
||||||
|
mention the group, or an object describing the set of users
|
||||||
|
and groups who have permission to mention the group.
|
||||||
|
|
||||||
**Changes**: Before Zulip 8.0 (feature level 198),
|
**Changes**: Before Zulip 9.0 (feature level 258), the
|
||||||
the `can_mention_group` setting was named `can_mention_group_id`.
|
`can_mention_group` field was always an integer.
|
||||||
|
|
||||||
New in Zulip 8.0 (feature level 191). Previously, groups
|
**Changes**: Before Zulip 8.0 (feature level 198),
|
||||||
could be mentioned if and only if they were not system groups.
|
the `can_mention_group` setting was named `can_mention_group_id`.
|
||||||
|
|
||||||
|
New in Zulip 8.0 (feature level 191). Previously, groups
|
||||||
|
could be mentioned if and only if they were not system groups.
|
||||||
description: |
|
description: |
|
||||||
A list of `user_group` objects.
|
A list of `user_group` objects.
|
||||||
example:
|
example:
|
||||||
|
@ -19878,15 +19884,41 @@ components:
|
||||||
|
|
||||||
**Changes**: New in Zulip 5.0 (feature level 93).
|
**Changes**: New in Zulip 5.0 (feature level 93).
|
||||||
can_mention_group:
|
can_mention_group:
|
||||||
type: integer
|
allOf:
|
||||||
description: |
|
- $ref: "#/components/schemas/CanMentionGroup"
|
||||||
ID of the user group whose members are allowed to mention the group.
|
- description: |
|
||||||
|
Either the ID of a named user group that has permission to
|
||||||
|
mention the group, or an object describing the set of users
|
||||||
|
and groups who have permission mention the group.
|
||||||
|
|
||||||
**Changes**: Before Zulip 8.0 (feature level 198),
|
**Changes**: Before Zulip 9.0 (feature level 258), the
|
||||||
the `can_mention_group` setting was named `can_mention_group_id`.
|
`can_mention_group` field was always an integer.
|
||||||
|
|
||||||
New in Zulip 8.0 (feature level 191). Previously, groups
|
**Changes**: Before Zulip 8.0 (feature level 198),
|
||||||
could be mentioned if and only if they were not system groups.
|
the `can_mention_group` setting was named `can_mention_group_id`.
|
||||||
|
|
||||||
|
New in Zulip 8.0 (feature level 191). Previously, groups
|
||||||
|
could be mentioned if and only if they were not system groups.
|
||||||
|
CanMentionGroup:
|
||||||
|
oneOf:
|
||||||
|
- type: object
|
||||||
|
additionalProperties: false
|
||||||
|
properties:
|
||||||
|
direct_members:
|
||||||
|
description: |
|
||||||
|
The list of user IDs that have permission to
|
||||||
|
mention the group.
|
||||||
|
type: array
|
||||||
|
items:
|
||||||
|
type: integer
|
||||||
|
direct_subgroups:
|
||||||
|
description: |
|
||||||
|
The list of user group IDs that have permission
|
||||||
|
to mention the group.
|
||||||
|
type: array
|
||||||
|
items:
|
||||||
|
type: integer
|
||||||
|
- type: integer
|
||||||
Invite:
|
Invite:
|
||||||
type: object
|
type: object
|
||||||
description: |
|
description: |
|
||||||
|
|
|
@ -1155,7 +1155,7 @@ class FetchQueriesTest(ZulipTestCase):
|
||||||
|
|
||||||
self.login_user(user)
|
self.login_user(user)
|
||||||
|
|
||||||
with self.assert_database_query_count(41):
|
with self.assert_database_query_count(43):
|
||||||
with mock.patch("zerver.lib.events.always_want") as want_mock:
|
with mock.patch("zerver.lib.events.always_want") as want_mock:
|
||||||
fetch_initial_state_data(user)
|
fetch_initial_state_data(user)
|
||||||
|
|
||||||
|
@ -1180,7 +1180,7 @@ class FetchQueriesTest(ZulipTestCase):
|
||||||
realm_linkifiers=0,
|
realm_linkifiers=0,
|
||||||
realm_playgrounds=1,
|
realm_playgrounds=1,
|
||||||
realm_user=3,
|
realm_user=3,
|
||||||
realm_user_groups=3,
|
realm_user_groups=5,
|
||||||
realm_user_settings_defaults=1,
|
realm_user_settings_defaults=1,
|
||||||
recent_private_conversations=1,
|
recent_private_conversations=1,
|
||||||
scheduled_messages=1,
|
scheduled_messages=1,
|
||||||
|
|
|
@ -257,7 +257,7 @@ class HomeTest(ZulipTestCase):
|
||||||
self.client_post("/json/bots", bot_info)
|
self.client_post("/json/bots", bot_info)
|
||||||
|
|
||||||
# Verify succeeds once logged-in
|
# Verify succeeds once logged-in
|
||||||
with self.assert_database_query_count(51):
|
with self.assert_database_query_count(53):
|
||||||
with patch("zerver.lib.cache.cache_set") as cache_mock:
|
with patch("zerver.lib.cache.cache_set") as cache_mock:
|
||||||
result = self._get_home_page(stream="Denmark")
|
result = self._get_home_page(stream="Denmark")
|
||||||
self.check_rendered_logged_in_app(result)
|
self.check_rendered_logged_in_app(result)
|
||||||
|
@ -562,7 +562,7 @@ class HomeTest(ZulipTestCase):
|
||||||
def test_num_queries_for_realm_admin(self) -> None:
|
def test_num_queries_for_realm_admin(self) -> None:
|
||||||
# Verify number of queries for Realm admin isn't much higher than for normal users.
|
# Verify number of queries for Realm admin isn't much higher than for normal users.
|
||||||
self.login("iago")
|
self.login("iago")
|
||||||
with self.assert_database_query_count(51):
|
with self.assert_database_query_count(53):
|
||||||
with patch("zerver.lib.cache.cache_set") as cache_mock:
|
with patch("zerver.lib.cache.cache_set") as cache_mock:
|
||||||
result = self._get_home_page()
|
result = self._get_home_page()
|
||||||
self.check_rendered_logged_in_app(result)
|
self.check_rendered_logged_in_app(result)
|
||||||
|
@ -593,7 +593,7 @@ class HomeTest(ZulipTestCase):
|
||||||
self._get_home_page()
|
self._get_home_page()
|
||||||
|
|
||||||
# Then for the second page load, measure the number of queries.
|
# Then for the second page load, measure the number of queries.
|
||||||
with self.assert_database_query_count(46):
|
with self.assert_database_query_count(48):
|
||||||
result = self._get_home_page()
|
result = self._get_home_page()
|
||||||
|
|
||||||
# Do a sanity check that our new streams were in the payload.
|
# Do a sanity check that our new streams were in the payload.
|
||||||
|
|
|
@ -22,6 +22,7 @@ from zerver.lib.streams import ensure_stream
|
||||||
from zerver.lib.test_classes import ZulipTestCase
|
from zerver.lib.test_classes import ZulipTestCase
|
||||||
from zerver.lib.test_helpers import most_recent_usermessage
|
from zerver.lib.test_helpers import most_recent_usermessage
|
||||||
from zerver.lib.user_groups import (
|
from zerver.lib.user_groups import (
|
||||||
|
AnonymousSettingGroupDict,
|
||||||
get_direct_user_groups,
|
get_direct_user_groups,
|
||||||
get_recursive_group_members,
|
get_recursive_group_members,
|
||||||
get_recursive_membership_groups,
|
get_recursive_membership_groups,
|
||||||
|
@ -75,6 +76,7 @@ class UserGroupTestCase(ZulipTestCase):
|
||||||
self.assertEqual(user_groups[0]["description"], "Nobody")
|
self.assertEqual(user_groups[0]["description"], "Nobody")
|
||||||
self.assertEqual(user_groups[0]["members"], [])
|
self.assertEqual(user_groups[0]["members"], [])
|
||||||
self.assertEqual(user_groups[0]["direct_subgroup_ids"], [])
|
self.assertEqual(user_groups[0]["direct_subgroup_ids"], [])
|
||||||
|
self.assertEqual(user_groups[0]["can_mention_group"], user_group.id)
|
||||||
|
|
||||||
owners_system_group = NamedUserGroup.objects.get(name=SystemGroups.OWNERS, realm=realm)
|
owners_system_group = NamedUserGroup.objects.get(name=SystemGroups.OWNERS, realm=realm)
|
||||||
membership = UserGroupMembership.objects.filter(user_group=owners_system_group).values_list(
|
membership = UserGroupMembership.objects.filter(user_group=owners_system_group).values_list(
|
||||||
|
@ -85,6 +87,7 @@ class UserGroupTestCase(ZulipTestCase):
|
||||||
self.assertEqual(user_groups[1]["description"], "Owners of this organization")
|
self.assertEqual(user_groups[1]["description"], "Owners of this organization")
|
||||||
self.assertEqual(set(user_groups[1]["members"]), set(membership))
|
self.assertEqual(set(user_groups[1]["members"]), set(membership))
|
||||||
self.assertEqual(user_groups[1]["direct_subgroup_ids"], [])
|
self.assertEqual(user_groups[1]["direct_subgroup_ids"], [])
|
||||||
|
self.assertEqual(user_groups[1]["can_mention_group"], user_group.id)
|
||||||
|
|
||||||
admins_system_group = NamedUserGroup.objects.get(
|
admins_system_group = NamedUserGroup.objects.get(
|
||||||
name=SystemGroups.ADMINISTRATORS, realm=realm
|
name=SystemGroups.ADMINISTRATORS, realm=realm
|
||||||
|
@ -93,10 +96,39 @@ class UserGroupTestCase(ZulipTestCase):
|
||||||
# Check that owners system group is present in "direct_subgroup_ids"
|
# Check that owners system group is present in "direct_subgroup_ids"
|
||||||
self.assertEqual(user_groups[2]["direct_subgroup_ids"], [owners_system_group.id])
|
self.assertEqual(user_groups[2]["direct_subgroup_ids"], [owners_system_group.id])
|
||||||
|
|
||||||
|
everyone_group = NamedUserGroup.objects.get(
|
||||||
|
name=SystemGroups.EVERYONE, realm=realm, is_system_group=True
|
||||||
|
)
|
||||||
self.assertEqual(user_groups[9]["id"], empty_user_group.id)
|
self.assertEqual(user_groups[9]["id"], empty_user_group.id)
|
||||||
self.assertEqual(user_groups[9]["name"], "newgroup")
|
self.assertEqual(user_groups[9]["name"], "newgroup")
|
||||||
self.assertEqual(user_groups[9]["description"], "")
|
self.assertEqual(user_groups[9]["description"], "")
|
||||||
self.assertEqual(user_groups[9]["members"], [])
|
self.assertEqual(user_groups[9]["members"], [])
|
||||||
|
self.assertEqual(user_groups[9]["can_mention_group"], everyone_group.id)
|
||||||
|
|
||||||
|
othello = self.example_user("othello")
|
||||||
|
setting_group = UserGroup.objects.create(realm=realm)
|
||||||
|
setting_group.direct_members.set([othello])
|
||||||
|
setting_group.direct_subgroups.set([admins_system_group])
|
||||||
|
|
||||||
|
new_user_group = check_add_user_group(
|
||||||
|
realm,
|
||||||
|
"newgroup2",
|
||||||
|
[othello],
|
||||||
|
group_settings_map={"can_mention_group": setting_group},
|
||||||
|
acting_user=None,
|
||||||
|
)
|
||||||
|
user_groups = user_groups_in_realm_serialized(realm)
|
||||||
|
self.assertEqual(user_groups[10]["id"], new_user_group.id)
|
||||||
|
self.assertEqual(user_groups[10]["name"], "newgroup2")
|
||||||
|
self.assertEqual(user_groups[10]["description"], "")
|
||||||
|
self.assertEqual(user_groups[10]["members"], [othello.id])
|
||||||
|
self.assertEqual(
|
||||||
|
user_groups[10]["can_mention_group"],
|
||||||
|
AnonymousSettingGroupDict(
|
||||||
|
direct_members=[othello.id],
|
||||||
|
direct_subgroups=[admins_system_group.id],
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
def test_get_direct_user_groups(self) -> None:
|
def test_get_direct_user_groups(self) -> None:
|
||||||
othello = self.example_user("othello")
|
othello = self.example_user("othello")
|
||||||
|
|
Loading…
Reference in New Issue