paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-31 12:03:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

streams: Backend changes to support anonymous groups.

Browse Source 
can_remove_subscribers_group setting can now be set to
anonymous user groups.

Co-authored-by: Sahil Batra <sahil@zulip.com>
This commit is contained in:
Shubham Padia
2024-10-24 19:50:49 +00:00
committed by Tim Abbott
parent 005a27a9cf
commit b6ebf143cc
14 changed files with 458 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
api_docs/changelog.md
View File

@@ -20,6 +20,18 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 10.0
**Feature level 320**
* [`GET /users/me/subscriptions`](/api/get-subscriptions),
[`GET /streams`](/api/get-streams), [`GET /events`](/api/get-events),
[`POST /register`](/api/register-queue): `can_remove_subscribers_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.
* [`POST /users/me/subscriptions`](/api/subscribe),
[`PATCH /streams/{stream_id}`](/api/update-stream): The
`can_remove_subscribers_group` parameter can now either be an ID of a
named user group or an object describing a set of users and groups.
**Feature level 319**
* [Markdown message

4
version.py
View File

@@ -34,7 +34,9 @@ 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 = 319 # Last bumped for message-link class
API_FEATURE_LEVEL = (
320 # Last bumped for supporting anonymous groups for can_remove_subscribers_group
)
# 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

59
zerver/actions/streams.py
View File

@@ -40,13 +40,18 @@ from zerver.lib.streams import (
can_access_stream_user_ids,
check_basic_stream_access,
get_occupied_streams,
get_setting_values_for_group_settings,
get_stream_permission_policy_name,
render_stream_description,
send_stream_creation_event,
stream_to_dict,
)
from zerver.lib.subscription_info import get_subscribers_query
from zerver.lib.types import APISubscriptionDict
from zerver.lib.types import AnonymousSettingGroupDict, APISubscriptionDict
from zerver.lib.user_groups import (
get_group_setting_value_for_api,
get_group_setting_value_for_audit_log_data,
)
from zerver.lib.users import (
get_subscribers_of_target_user_subscriptions,
get_users_involved_in_dms_with_target_users,
@@ -349,13 +354,21 @@ def send_subscription_add_events(
subscribers = list(subscriber_dict[stream.id])
stream_subscribers_dict[stream.id] = subscribers
setting_group_ids = set()
for sub_info in sub_info_list:
stream = sub_info.stream
for setting_name in Stream.stream_permission_group_settings:
setting_group_ids.add(getattr(stream, setting_name + "_id"))
setting_groups_dict = get_setting_values_for_group_settings(list(setting_group_ids))
for user_id, sub_infos in info_by_user.items():
sub_dicts: list[APISubscriptionDict] = []
for sub_info in sub_infos:
stream = sub_info.stream
stream_subscribers = stream_subscribers_dict[stream.id]
subscription = sub_info.sub
stream_dict = stream_to_dict(stream, recent_traffic)
stream_dict = stream_to_dict(stream, recent_traffic, setting_groups_dict)
# This is verbose as we cannot unpack existing TypedDict
# to initialize another TypedDict while making mypy happy.
# https://github.com/python/mypy/issues/5382
@@ -447,6 +460,14 @@ def send_stream_creation_events_for_previously_inaccessible_streams(
stream_ids = set(altered_user_dict.keys())
recent_traffic = get_streams_traffic(stream_ids, realm)
setting_group_ids = set()
for stream_id in stream_ids:
stream = stream_dict[stream_id]
for setting_name in Stream.stream_permission_group_settings:
setting_group_ids.add(getattr(stream, setting_name + "_id"))
setting_groups_dict = get_setting_values_for_group_settings(list(setting_group_ids))
for stream_id, stream_users_ids in altered_user_dict.items():
stream = stream_dict[stream_id]
@@ -467,7 +488,9 @@ def send_stream_creation_events_for_previously_inaccessible_streams(
notify_user_ids = list(stream_users_ids & altered_guests)
if notify_user_ids:
send_stream_creation_event(realm, stream, notify_user_ids, recent_traffic)
send_stream_creation_event(
realm, stream, notify_user_ids, recent_traffic, setting_groups_dict
)
def send_peer_subscriber_events(
@@ -1561,16 +1584,30 @@ def do_change_stream_group_based_setting(
setting_name: str,
user_group: UserGroup,
*,
old_setting_api_value: int | AnonymousSettingGroupDict | None = None,
acting_user: UserProfile | None = None,
) -> None:
old_user_group = getattr(stream, setting_name)
old_user_group_id = None
if old_user_group is not None:
old_user_group_id = old_user_group.id
setattr(stream, setting_name, user_group)
stream.save()
if old_setting_api_value is None:
# Most production callers will have computed this as part of
# verifying whether there's an actual change to make, but it
# feels quite clumsy to have to pass it from unit tests, so we
# compute it here if not provided by the caller.
old_setting_api_value = get_group_setting_value_for_api(old_user_group)
new_setting_api_value = get_group_setting_value_for_api(user_group)
if not hasattr(old_user_group, "named_user_group") and hasattr(user_group, "named_user_group"):
# We delete the UserGroup which the setting was set to
# previously if it does not have any linked NamedUserGroup
# object, as it is not used anywhere else. A new UserGroup
# object would be created if the setting is later set to
# a combination of users and groups.
old_user_group.delete()
RealmAuditLog.objects.create(
realm=stream.realm,
acting_user=acting_user,
@@ -1578,8 +1615,12 @@ def do_change_stream_group_based_setting(
event_type=AuditLogEventType.CHANNEL_GROUP_BASED_SETTING_CHANGED,
event_time=timezone_now(),
extra_data={
RealmAuditLog.OLD_VALUE: old_user_group_id,
RealmAuditLog.NEW_VALUE: user_group.id,
RealmAuditLog.OLD_VALUE: get_group_setting_value_for_audit_log_data(
old_setting_api_value
),
RealmAuditLog.NEW_VALUE: get_group_setting_value_for_audit_log_data(
new_setting_api_value
),
"property": setting_name,
},
)
@@ -1587,7 +1628,7 @@ def do_change_stream_group_based_setting(
op="update",
type="stream",
property=setting_name,
value=user_group.id,
value=new_setting_api_value,
stream_id=stream.id,
name=stream.name,
)

35
zerver/lib/event_schema.py
View File

@@ -45,13 +45,26 @@ from zerver.lib.data_types import (
make_checker,
)
from zerver.lib.topic import ORIG_TOPIC, TOPIC_LINKS, TOPIC_NAME
from zerver.lib.types import AnonymousSettingGroupDict
from zerver.models import Realm, RealmUserDefault, Stream, UserProfile
group_setting_type = UnionType(
[
int,
DictType(
required_keys=[
("direct_members", ListType(int)),
("direct_subgroups", ListType(int)),
]
),
]
)
# These fields are used for "stream" events, and are included in the
# larger "subscription" events that also contain personal settings.
default_stream_fields = [
("is_archived", bool),
("can_remove_subscribers_group", int),
("can_remove_subscribers_group", group_setting_type),
("creator_id", OptionalType(int)),
("date_created", int),
("description", str),
@@ -103,6 +116,12 @@ optional_value_type = UnionType(
bool,
int,
str,
DictType(
required_keys=[
("direct_members", ListType(int)),
("direct_subgroups", ListType(int)),
]
),
Equals(None),
]
)
@@ -1046,18 +1065,6 @@ night_logo_data = DictType(
]
)
group_setting_type = UnionType(
[
int,
DictType(
required_keys=[
("direct_members", ListType(int)),
("direct_subgroups", ListType(int)),
]
),
]
)
group_setting_update_data_type = DictType(
required_keys=[],
optional_keys=[
@@ -1442,7 +1449,7 @@ def check_stream_update(
assert value in Stream.STREAM_POST_POLICY_TYPES
elif prop == "can_remove_subscribers_group":
assert extra_keys == set()
assert isinstance(value, int)
assert isinstance(value, int | AnonymousSettingGroupDict)
elif prop == "first_message_id":
assert extra_keys == set()
assert isinstance(value, int)

77
zerver/lib/streams.py
View File

@@ -21,10 +21,14 @@ from zerver.lib.stream_subscription import (
from zerver.lib.stream_traffic import get_average_weekly_stream_traffic, get_streams_traffic
from zerver.lib.string_validation import check_stream_name
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.types import APIStreamDict
from zerver.lib.user_groups import user_has_permission_for_group_setting
from zerver.lib.types import AnonymousSettingGroupDict, APIStreamDict
from zerver.lib.user_groups import (
get_group_setting_value_for_api,
user_has_permission_for_group_setting,
)
from zerver.models import (
DefaultStreamGroup,
GroupGroupMembership,
NamedUserGroup,
Realm,
RealmAuditLog,
@@ -32,6 +36,7 @@ from zerver.models import (
Stream,
Subscription,
UserGroup,
UserGroupMembership,
UserProfile,
)
from zerver.models.groups import SystemGroups
@@ -123,8 +128,13 @@ def send_stream_creation_event(
stream: Stream,
user_ids: list[int],
recent_traffic: dict[int, int] | None = None,
setting_groups_dict: dict[int, int | AnonymousSettingGroupDict] | None = None,
) -> None:
event = dict(type="stream", op="create", streams=[stream_to_dict(stream, recent_traffic)])
event = dict(
type="stream",
op="create",
streams=[stream_to_dict(stream, recent_traffic, setting_groups_dict)],
)
send_event_on_commit(realm, event, user_ids)
@@ -870,7 +880,11 @@ def get_occupied_streams(realm: Realm) -> QuerySet[Stream]:
return occupied_streams
def stream_to_dict(stream: Stream, recent_traffic: dict[int, int] | None = None) -> APIStreamDict:
def stream_to_dict(
stream: Stream,
recent_traffic: dict[int, int] | None = None,
setting_groups_dict: dict[int, int | AnonymousSettingGroupDict] | None = None,
) -> APIStreamDict:
if recent_traffic is not None:
stream_weekly_traffic = get_average_weekly_stream_traffic(
stream.id, stream.date_created, recent_traffic
@@ -884,9 +898,16 @@ def stream_to_dict(stream: Stream, recent_traffic: dict[int, int] | None = None)
# passing stream data to spectators.
stream_weekly_traffic = None
if setting_groups_dict is not None:
can_remove_subscribers_group = setting_groups_dict[stream.can_remove_subscribers_group_id]
else:
can_remove_subscribers_group = get_group_setting_value_for_api(
stream.can_remove_subscribers_group
)
return APIStreamDict(
is_archived=stream.deactivated,
can_remove_subscribers_group=stream.can_remove_subscribers_group_id,
can_remove_subscribers_group=can_remove_subscribers_group,
creator_id=stream.creator_id,
date_created=datetime_to_timestamp(stream.date_created),
description=stream.description,
@@ -978,6 +999,38 @@ def get_streams_for_user(
return list(streams)
def get_setting_values_for_group_settings(
group_ids: list[int],
) -> dict[int, int | AnonymousSettingGroupDict]:
setting_value_dict = {}
setting_groups = UserGroup.objects.filter(id__in=group_ids).select_related("named_user_group")
anonymous_groups = []
for group in setting_groups:
if hasattr(group, "named_user_group"):
setting_value_dict[group.id] = group.id
else:
setting_value_dict[group.id] = AnonymousSettingGroupDict(
direct_members=[],
direct_subgroups=[],
)
anonymous_groups.append(group.id)
group_members = (
UserGroupMembership.objects.filter(user_group_id__in=anonymous_groups)
.exclude(user_profile__is_active=False)
.values_list("user_group_id", "user_profile_id")
)
group_subgroups = GroupGroupMembership.objects.filter(
supergroup_id__in=anonymous_groups
).values_list("supergroup_id", "subgroup_id")
for user_group_id, user_profile_id in group_members:
setting_value_dict[user_group_id].direct_members.append(user_profile_id)
for supergroup_id, subgroup_id in group_subgroups:
setting_value_dict[supergroup_id].direct_subgroups.append(subgroup_id)
return setting_value_dict
def do_get_streams(
user_profile: UserProfile,
include_public: bool = True,
@@ -1003,14 +1056,22 @@ def do_get_streams(
stream_ids = {stream.id for stream in streams}
recent_traffic = get_streams_traffic(stream_ids, user_profile.realm)
setting_group_ids = set()
for stream in streams:
for setting_name in Stream.stream_permission_group_settings:
setting_group_ids.add(getattr(stream, setting_name + "_id"))
setting_groups_dict = get_setting_values_for_group_settings(list(setting_group_ids))
stream_dicts = sorted(
(stream_to_dict(stream, recent_traffic) for stream in streams), key=lambda elt: elt["name"]
(stream_to_dict(stream, recent_traffic, setting_groups_dict) for stream in streams),
key=lambda elt: elt["name"],
)
if include_default:
default_stream_ids = get_default_stream_ids_for_realm(user_profile.realm_id)
for stream in stream_dicts:
stream["is_default"] = stream["stream_id"] in default_stream_ids
for stream_dict in stream_dicts:
stream_dict["is_default"] = stream_dict["stream_id"] in default_stream_ids
return stream_dicts

35
zerver/lib/subscription_info.py
View File

@@ -16,9 +16,14 @@ from zerver.lib.stream_subscription import (
get_stream_subscriptions_for_user,
)
from zerver.lib.stream_traffic import get_average_weekly_stream_traffic, get_streams_traffic
from zerver.lib.streams import get_web_public_streams_queryset, subscribed_to_stream
from zerver.lib.streams import (
get_setting_values_for_group_settings,
get_web_public_streams_queryset,
subscribed_to_stream,
)
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.types import (
AnonymousSettingGroupDict,
APIStreamDict,
NeverSubscribedStreamDict,
RawStreamDict,
@@ -26,6 +31,7 @@ from zerver.lib.types import (
SubscriptionInfo,
SubscriptionStreamDict,
)
from zerver.lib.user_groups import get_group_setting_value_for_api
from zerver.models import Realm, Stream, Subscription, UserProfile
from zerver.models.streams import get_all_streams
@@ -43,7 +49,9 @@ def get_web_public_subs(realm: Realm) -> SubscriptionInfo:
for stream in get_web_public_streams_queryset(realm):
# Add Stream fields.
is_archived = stream.deactivated
can_remove_subscribers_group_id = stream.can_remove_subscribers_group_id
can_remove_subscribers_group = get_group_setting_value_for_api(
stream.can_remove_subscribers_group
)
creator_id = stream.creator_id
date_created = datetime_to_timestamp(stream.date_created)
description = stream.description
@@ -76,7 +84,7 @@ def get_web_public_subs(realm: Realm) -> SubscriptionInfo:
sub = SubscriptionStreamDict(
is_archived=is_archived,
audible_notifications=audible_notifications,
can_remove_subscribers_group=can_remove_subscribers_group_id,
can_remove_subscribers_group=can_remove_subscribers_group,
color=color,
creator_id=creator_id,
date_created=date_created,
@@ -118,7 +126,9 @@ def build_unsubscribed_sub_from_stream_dict(
def build_stream_api_dict(
raw_stream_dict: RawStreamDict, recent_traffic: dict[int, int] | None
raw_stream_dict: RawStreamDict,
recent_traffic: dict[int, int] | None,
setting_groups_dict: dict[int, int | AnonymousSettingGroupDict],
) -> APIStreamDict:
# Add a few computed fields not directly from the data models.
if recent_traffic is not None:
@@ -133,9 +143,13 @@ def build_stream_api_dict(
# migration.
is_announcement_only = raw_stream_dict["stream_post_policy"] == Stream.STREAM_POST_POLICY_ADMINS
can_remove_subscribers_group = setting_groups_dict[
raw_stream_dict["can_remove_subscribers_group_id"]
]
return APIStreamDict(
is_archived=raw_stream_dict["deactivated"],
can_remove_subscribers_group=raw_stream_dict["can_remove_subscribers_group_id"],
can_remove_subscribers_group=can_remove_subscribers_group,
creator_id=raw_stream_dict["creator_id"],
date_created=datetime_to_timestamp(raw_stream_dict["date_created"]),
description=raw_stream_dict["description"],
@@ -471,6 +485,13 @@ def gather_subscriptions_helper(
}
all_streams_map: dict[int, RawStreamDict] = {stream["id"]: stream for stream in all_streams}
setting_group_ids = set()
for stream in all_streams:
for setting_name in Stream.stream_permission_group_settings:
setting_group_ids.add(stream[setting_name + "_id"])
setting_groups_dict = get_setting_values_for_group_settings(list(setting_group_ids))
sub_dicts_query: Iterable[RawSubscriptionDict] = (
get_stream_subscriptions_for_user(user_profile)
.values(
@@ -505,7 +526,9 @@ def gather_subscriptions_helper(
stream_id = get_stream_id(sub_dict)
sub_unsub_stream_ids.add(stream_id)
raw_stream_dict = all_streams_map[stream_id]
stream_api_dict = build_stream_api_dict(raw_stream_dict, recent_traffic)
stream_api_dict = build_stream_api_dict(
raw_stream_dict, recent_traffic, setting_groups_dict
)
stream_dict = build_stream_dict_for_sub(
user=user_profile,
sub_dict=sub_dict,

4
zerver/lib/types.py
View File

@@ -191,7 +191,7 @@ class SubscriptionStreamDict(TypedDict):
"""
audible_notifications: bool | None
can_remove_subscribers_group: int
can_remove_subscribers_group: int | AnonymousSettingGroupDict
color: str
creator_id: int | None
date_created: int
@@ -245,7 +245,7 @@ class DefaultStreamDict(TypedDict):
"""
is_archived: bool
can_remove_subscribers_group: int
can_remove_subscribers_group: int | AnonymousSettingGroupDict
creator_id: int | None
date_created: int
description: str

2
zerver/models/streams.py
View File

@@ -143,7 +143,7 @@ class Stream(models.Model):
stream_permission_group_settings = {
"can_remove_subscribers_group": GroupPermissionSetting(
require_system_group=True,
require_system_group=False,
allow_internet_group=False,
allow_owners_group=False,
allow_nobody_group=False,

121
zerver/openapi/zulip.yaml
View File

@@ -1459,7 +1459,33 @@ paths:
value:
description: |
The new value of the changed property.
**Changes**: Starting with Zulip 10.0 (feature level 320), this
field can be an object for `can_remove_subscribers_group` property,
which is a [group-setting value][setting-values], when the setting
is set to a combination of users and groups.
[setting-values]: /api/group-setting-values
oneOf:
- type: object
additionalProperties: false
properties:
direct_members:
description: |
The list of IDs of individual users in the collection of users with this permission.
type: array
items:
type: integer
direct_subgroups:
description: |
The list of IDs of the groups in the collection of users with this permission.
type: array
items:
type: integer
description: |
If an object, it will be a [group-setting value][setting-values] with these fields:
[setting-values]: /api/group-setting-values
- type: integer
- type: boolean
- type: string
@@ -10364,7 +10390,7 @@ paths:
message_retention_days:
$ref: "#/components/schemas/MessageRetentionDays"
can_remove_subscribers_group:
$ref: "#/components/schemas/CanRemoveSubscribersGroupId"
$ref: "#/components/schemas/CanRemoveSubscribersGroup"
required:
- subscriptions
encoding:
@@ -19958,7 +19984,43 @@ paths:
message_retention_days:
$ref: "#/components/schemas/MessageRetentionDays"
can_remove_subscribers_group:
$ref: "#/components/schemas/CanRemoveSubscribersGroupId"
description: |
The set of users who have permission to unsubscribe others from this
channel expressed as an [update to a group-setting value][update-group-setting].
Note that a user who is a member of the specified user group must
also [have access](/help/channel-permissions) to the channel in
order to unsubscribe others.
**Changes**: Prior to Zulip 10.0 (feature level 320), this value was
always the integer ID of a system group.
Before Zulip 8.0 (feature level 197), the `can_remove_subscribers_group`
setting was named `can_remove_subscribers_group_id`.
New in Zulip 7.0 (feature level 161).
type: object
additionalProperties: false
properties:
new:
allOf:
- description: |
The new [group-setting value](/api/group-setting-values) for who would
have the permission to remove subscribers from the channel.
- $ref: "#/components/schemas/GroupSettingValue"
old:
allOf:
- description: |
The expected current [group-setting value](/api/group-setting-values)
for who has the permission to remove subscribers from the channel.
- $ref: "#/components/schemas/GroupSettingValue"
required:
- new
example:
{
"new": {"direct_members": [10], "direct_subgroups": [11]},
"old": 15,
}
encoding:
is_private:
contentType: application/json
@@ -21819,16 +21881,7 @@ components:
**Changes**: Deprecated in Zulip 3.0 (feature level 1). Clients
should use `stream_post_policy` instead.
can_remove_subscribers_group:
type: integer
description: |
ID of the user group whose members are allowed to unsubscribe others
from the channel.
**Changes**: Before Zulip 8.0 (feature level 197),
the `can_remove_subscribers_group` setting
was named `can_remove_subscribers_group_id`.
New in Zulip 6.0 (feature level 142).
$ref: "#/components/schemas/CanRemoveSubscribersGroup"
BasicBot:
allOf:
- $ref: "#/components/schemas/BasicBotBase"
@@ -22835,16 +22888,7 @@ components:
If `null`, the channel was recently created and there is
insufficient data to estimate the average traffic.
can_remove_subscribers_group:
type: integer
description: |
ID of the user group whose members are allowed to unsubscribe others
from the channel.
**Changes**: Before Zulip 8.0 (feature level 197),
the `can_remove_subscribers_group` setting
was named `can_remove_subscribers_group_id`.
New in Zulip 6.0 (feature level 142).
$ref: "#/components/schemas/CanRemoveSubscribersGroup"
is_archived:
type: boolean
description: |
@@ -24515,25 +24559,26 @@ components:
- type: string
- type: integer
example: "20"
CanRemoveSubscribersGroupId:
description: |
ID of the [user group](/api/get-user-groups) whose members are
allowed to unsubscribe others from the channel. Note that a user
who is a member of the specified user group must also [have
access](/help/channel-permissions) to the channel in order to
unsubscribe others.
CanRemoveSubscribersGroup:
allOf:
- $ref: "#/components/schemas/GroupSettingValue"
- description: |
A [group-setting value][setting-values] defining the set of users
who have permission to remove subscribers from this channel.
This setting can currently only be set to user groups that are
system groups, except for the system groups named
`"role:internet"` and `"role:owners"`.
Note that a user who is a member of the specified user group must
also [have access](/help/channel-permissions) to the channel in
order to unsubscribe others.
**Changes**: Before Zulip 8.0 (feature level 197),
the `can_remove_subscribers_group` setting
was named `can_remove_subscribers_group_id`.
**Changes**: Prior to Zulip 10.0 (feature level 320), this value was
always the integer ID of a system group.
New in Zulip 7.0 (feature level 161).
type: integer
example: 20
Before Zulip 8.0 (feature level 197), the `can_remove_subscribers_group`
setting was named `can_remove_subscribers_group_id`.
New in Zulip 6.0 (feature level 142).
[setting-values]: /api/group-setting-values
LinkifierPattern:
description: |
The [Python regular

6
zerver/tests/test_event_system.py
View File

@@ -1192,7 +1192,7 @@ class FetchQueriesTest(ZulipTestCase):
realm = get_realm_with_settings(realm_id=user.realm_id)
with (
self.assert_database_query_count(40),
self.assert_database_query_count(42),
mock.patch("zerver.lib.events.always_want") as want_mock,
):
fetch_initial_state_data(user, realm=realm)
@@ -1224,9 +1224,9 @@ class FetchQueriesTest(ZulipTestCase):
saved_snippets=1,
scheduled_messages=1,
starred_messages=1,
stream=3,
stream=4,
stop_words=0,
subscription=4,
subscription=5,
update_display_settings=0,
update_global_notifications=0,
update_message_flags=5,

20
zerver/tests/test_events.py
View File

@@ -4538,6 +4538,26 @@ class SubscribeActionTest(BaseAction):
acting_user=self.example_user("hamlet"),
)
check_stream_update("events[0]", events[0])
self.assertEqual(events[0]["value"], moderators_group.id)
setting_group = self.create_or_update_anonymous_group_for_setting(
[self.user_profile],
[moderators_group],
)
with self.verify_action(include_subscribers=include_subscribers, num_events=1) as events:
do_change_stream_group_based_setting(
stream,
"can_remove_subscribers_group",
setting_group,
acting_user=self.example_user("hamlet"),
)
check_stream_update("events[0]", events[0])
self.assertEqual(
events[0]["value"],
AnonymousSettingGroupDict(
direct_members=[self.user_profile.id], direct_subgroups=[moderators_group.id]
),
)
# Subscribe to a totally new invite-only stream, so it's just Hamlet on it
stream = self.make_stream("private", self.user_profile.realm, invite_only=True)

6
zerver/tests/test_home.py
View File

@@ -271,7 +271,7 @@ class HomeTest(ZulipTestCase):
# Verify succeeds once logged-in
with (
self.assert_database_query_count(51),
self.assert_database_query_count(52),
patch("zerver.lib.cache.cache_set") as cache_mock,
):
result = self._get_home_page(stream="Denmark")
@@ -576,7 +576,7 @@ class HomeTest(ZulipTestCase):
# Verify number of queries for Realm admin isn't much higher than for normal users.
self.login("iago")
with (
self.assert_database_query_count(51),
self.assert_database_query_count(52),
patch("zerver.lib.cache.cache_set") as cache_mock,
):
result = self._get_home_page()
@@ -608,7 +608,7 @@ class HomeTest(ZulipTestCase):
self._get_home_page()
# Then for the second page load, measure the number of queries.
with self.assert_database_query_count(46):
with self.assert_database_query_count(47):
result = self._get_home_page()
# Do a sanity check that our new streams were in the payload.

146
zerver/tests/test_subs.py
View File

@@ -570,10 +570,31 @@ class TestCreateStreams(ZulipTestCase):
allow_fail=True,
subdomain="zulip",
)
self.assert_json_error(
result, "'can_remove_subscribers_group' must be a system user group."
self.assert_json_success(result)
stream = get_stream("new_stream3", realm)
self.assertEqual(stream.can_remove_subscribers_group.id, hamletcharacters_group.id)
subscriptions = [{"name": "new_stream4", "description": "Fourth new stream"}]
result = self.common_subscribe_to_streams(
user,
subscriptions,
{
"can_remove_subscribers_group": orjson.dumps(
{"direct_members": [user.id], "direct_subgroups": [moderators_system_group.id]}
).decode()
},
allow_fail=True,
subdomain="zulip",
)
self.assert_json_success(result)
stream = get_stream("new_stream4", realm)
self.assertEqual(list(stream.can_remove_subscribers_group.direct_members.all()), [user])
self.assertEqual(
list(stream.can_remove_subscribers_group.direct_subgroups.all()),
[moderators_system_group],
)
subscriptions = [{"name": "new_stream5", "description": "Fifth new stream"}]
internet_group = NamedUserGroup.objects.get(
name="role:internet", is_system_group=True, realm=realm
)
@@ -2273,14 +2294,22 @@ class StreamAdminTest(ZulipTestCase):
self.login("shiva")
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group": orjson.dumps(moderators_system_group.id).decode()},
{
"can_remove_subscribers_group": orjson.dumps(
{"new": moderators_system_group.id}
).decode()
},
)
self.assert_json_error(result, "Must be an organization administrator")
self.login("iago")
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group": orjson.dumps(moderators_system_group.id).decode()},
{
"can_remove_subscribers_group": orjson.dumps(
{"new": moderators_system_group.id}
).decode()
},
)
self.assert_json_success(result)
stream = get_stream("stream_name1", realm)
@@ -2290,10 +2319,56 @@ class StreamAdminTest(ZulipTestCase):
hamletcharacters_group = NamedUserGroup.objects.get(name="hamletcharacters", realm=realm)
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group": orjson.dumps(hamletcharacters_group.id).decode()},
{
"can_remove_subscribers_group": orjson.dumps(
{"new": hamletcharacters_group.id}
).decode()
},
)
self.assert_json_error(
result, "'can_remove_subscribers_group' must be a system user group."
self.assert_json_success(result)
stream = get_stream("stream_name1", realm)
self.assertEqual(stream.can_remove_subscribers_group.id, hamletcharacters_group.id)
# Test changing it to anonymous group.
hamlet = self.example_user("hamlet")
# Test passing incorrect old value.
result = self.client_patch(
f"/json/streams/{stream.id}",
{
"can_remove_subscribers_group": orjson.dumps(
{
"new": {
"direct_members": [hamlet.id],
"direct_subgroups": [moderators_system_group.id],
},
"old": moderators_system_group.id,
}
).decode()
},
)
self.assert_json_error(result, "'old' value does not match the expected value.")
result = self.client_patch(
f"/json/streams/{stream.id}",
{
"can_remove_subscribers_group": orjson.dumps(
{
"new": {
"direct_members": [hamlet.id],
"direct_subgroups": [moderators_system_group.id],
},
"old": hamletcharacters_group.id,
}
).decode()
},
)
self.assert_json_success(result)
stream = get_stream("stream_name1", realm)
self.assertEqual(list(stream.can_remove_subscribers_group.direct_members.all()), [hamlet])
self.assertEqual(
list(stream.can_remove_subscribers_group.direct_subgroups.all()),
[moderators_system_group],
)
internet_group = NamedUserGroup.objects.get(
@@ -2301,7 +2376,7 @@ class StreamAdminTest(ZulipTestCase):
)
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group": orjson.dumps(internet_group.id).decode()},
{"can_remove_subscribers_group": orjson.dumps({"new": internet_group.id}).decode()},
)
self.assert_json_error(
result,
@@ -2313,7 +2388,7 @@ class StreamAdminTest(ZulipTestCase):
)
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group": orjson.dumps(owners_group.id).decode()},
{"can_remove_subscribers_group": orjson.dumps({"new": owners_group.id}).decode()},
)
self.assert_json_error(
result,
@@ -2325,7 +2400,7 @@ class StreamAdminTest(ZulipTestCase):
)
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group": orjson.dumps(nobody_group.id).decode()},
{"can_remove_subscribers_group": orjson.dumps({"new": nobody_group.id}).decode()},
)
self.assert_json_error(
result,
@@ -2337,14 +2412,22 @@ class StreamAdminTest(ZulipTestCase):
stream = self.make_stream("stream_name2", invite_only=True)
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group": orjson.dumps(moderators_system_group.id).decode()},
{
"can_remove_subscribers_group": orjson.dumps(
{"new": moderators_system_group.id}
).decode()
},
)
self.assert_json_error(result, "Invalid channel ID")
self.subscribe(user_profile, "stream_name2")
result = self.client_patch(
f"/json/streams/{stream.id}",
{"can_remove_subscribers_group": orjson.dumps(moderators_system_group.id).decode()},
{
"can_remove_subscribers_group": orjson.dumps(
{"new": moderators_system_group.id}
).decode()
},
)
self.assert_json_success(result)
stream = get_stream("stream_name2", realm)
@@ -2676,7 +2759,7 @@ class StreamAdminTest(ZulipTestCase):
are on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=17,
query_count=18,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=True,
@@ -2693,7 +2776,7 @@ class StreamAdminTest(ZulipTestCase):
streams you aren't on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=17,
query_count=18,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=False,
@@ -2863,6 +2946,17 @@ class StreamAdminTest(ZulipTestCase):
self.subscribe(self.example_user("shiva"), stream.name)
check_unsubscribing_user(self.example_user("shiva"), leadership_group)
# Test changing setting to anonymous group.
setting_group = self.create_or_update_anonymous_group_for_setting(
[hamlet],
[leadership_group],
)
check_unsubscribing_user(self.example_user("othello"), setting_group, expect_fail=True)
check_unsubscribing_user(self.example_user("desdemona"), setting_group, expect_fail=True)
check_unsubscribing_user(self.example_user("hamlet"), setting_group)
check_unsubscribing_user(self.example_user("iago"), setting_group)
check_unsubscribing_user(self.example_user("shiva"), setting_group)
def test_remove_invalid_user(self) -> None:
"""
Trying to unsubscribe an invalid user from a stream fails gracefully.
@@ -4707,7 +4801,7 @@ class SubscriptionAPITest(ZulipTestCase):
streams_to_sub = ["multi_user_stream"]
with (
self.capture_send_event_calls(expected_num_events=5) as events,
self.assert_database_query_count(37),
self.assert_database_query_count(40),
):
self.common_subscribe_to_streams(
self.test_user,
@@ -4733,7 +4827,7 @@ class SubscriptionAPITest(ZulipTestCase):
# Now add ourselves
with (
self.capture_send_event_calls(expected_num_events=2) as events,
self.assert_database_query_count(14),
self.assert_database_query_count(16),
):
self.common_subscribe_to_streams(
self.test_user,
@@ -5027,7 +5121,7 @@ class SubscriptionAPITest(ZulipTestCase):
# Sends 3 peer-remove events, 2 unsubscribe events
# and 2 stream delete events for private streams.
with (
self.assert_database_query_count(16),
self.assert_database_query_count(17),
self.assert_memcached_count(3),
self.capture_send_event_calls(expected_num_events=7) as events,
):
@@ -5098,7 +5192,7 @@ class SubscriptionAPITest(ZulipTestCase):
# Verify that peer_event events are never sent in Zephyr
# realm. This does generate stream creation events from
# send_stream_creation_events_for_previously_inaccessible_streams.
with self.assert_database_query_count(num_streams + 11):
with self.assert_database_query_count(num_streams + 13):
with self.capture_send_event_calls(expected_num_events=num_streams + 1) as events:
self.common_subscribe_to_streams(
mit_user,
@@ -5179,7 +5273,7 @@ class SubscriptionAPITest(ZulipTestCase):
test_user_ids = [user.id for user in test_users]
with (
self.assert_database_query_count(16),
self.assert_database_query_count(18),
self.assert_memcached_count(3),
mock.patch("zerver.views.streams.send_messages_for_new_subscribers"),
):
@@ -5547,7 +5641,7 @@ class SubscriptionAPITest(ZulipTestCase):
]
# Test creating a public stream when realm does not have a notification stream.
with self.assert_database_query_count(37):
with self.assert_database_query_count(40):
self.common_subscribe_to_streams(
self.test_user,
[new_streams[0]],
@@ -5555,7 +5649,7 @@ class SubscriptionAPITest(ZulipTestCase):
)
# Test creating private stream.
with self.assert_database_query_count(39):
with self.assert_database_query_count(42):
self.common_subscribe_to_streams(
self.test_user,
[new_streams[1]],
@@ -5567,7 +5661,7 @@ class SubscriptionAPITest(ZulipTestCase):
new_stream_announcements_stream = get_stream(self.streams[0], self.test_realm)
self.test_realm.new_stream_announcements_stream_id = new_stream_announcements_stream.id
self.test_realm.save()
with self.assert_database_query_count(48):
with self.assert_database_query_count(51):
self.common_subscribe_to_streams(
self.test_user,
[new_streams[2]],
@@ -6047,7 +6141,7 @@ class GetSubscribersTest(ZulipTestCase):
polonius.id,
]
with self.assert_database_query_count(43):
with self.assert_database_query_count(45):
self.common_subscribe_to_streams(
self.user_profile,
streams,
@@ -6095,7 +6189,7 @@ class GetSubscribersTest(ZulipTestCase):
for user in [cordelia, othello, polonius]:
self.assert_user_got_subscription_notification(user, msg)
with self.assert_database_query_count(4):
with self.assert_database_query_count(5):
subscribed_streams, _ = gather_subscriptions(
self.user_profile, include_subscribers=True
)
@@ -6170,7 +6264,7 @@ class GetSubscribersTest(ZulipTestCase):
create_private_streams()
def get_never_subscribed() -> list[NeverSubscribedStreamDict]:
with self.assert_database_query_count(4):
with self.assert_database_query_count(5):
sub_data = gather_subscriptions_helper(self.user_profile)
self.verify_sub_fields(sub_data)
never_subscribed = sub_data.never_subscribed
@@ -6367,7 +6461,7 @@ class GetSubscribersTest(ZulipTestCase):
subdomain="zephyr",
)
with self.assert_database_query_count(3):
with self.assert_database_query_count(4):
subscribed_streams, _ = gather_subscriptions(mit_user_profile, include_subscribers=True)
self.assertGreaterEqual(len(subscribed_streams), 2)

64
zerver/views/streams.py
View File

@@ -74,7 +74,14 @@ from zerver.lib.topic import (
)
from zerver.lib.typed_endpoint import ApiParamConfig, PathOnly, typed_endpoint
from zerver.lib.typed_endpoint_validators import check_color, check_int_in_validator
from zerver.lib.user_groups import access_user_group_for_setting
from zerver.lib.types import AnonymousSettingGroupDict
from zerver.lib.user_groups import (
GroupSettingChangeRequest,
access_user_group_for_setting,
get_group_setting_value_for_api,
parse_group_setting_value,
validate_group_setting_value_change,
)
from zerver.lib.users import bulk_access_users_by_email, bulk_access_users_by_id
from zerver.lib.utils import assert_is_not_none
from zerver.models import NamedUserGroup, Realm, Stream, UserProfile
@@ -249,9 +256,7 @@ def update_stream_backend(
is_web_public: Json[bool] | None = None,
new_name: str | None = None,
message_retention_days: Json[str] | Json[int] | None = None,
can_remove_subscribers_group_id: Annotated[
Json[int | None], ApiParamConfig("can_remove_subscribers_group")
] = None,
can_remove_subscribers_group: Json[GroupSettingChangeRequest] | None = None,
) -> HttpResponse:
# We allow realm administrators to update the stream name and
# description even for private streams.
@@ -379,28 +384,42 @@ def update_stream_backend(
for setting_name, permission_configuration in Stream.stream_permission_group_settings.items():
request_settings_dict = locals()
setting_group_id_name = permission_configuration.id_field_name
if setting_group_id_name not in request_settings_dict: # nocoverage
assert setting_name in request_settings_dict
if request_settings_dict[setting_name] is None:
continue
if request_settings_dict[setting_group_id_name] is not None and request_settings_dict[
setting_group_id_name
] != getattr(stream, setting_group_id_name):
setting_value = request_settings_dict[setting_name]
new_setting_value = parse_group_setting_value(setting_value.new, setting_name)
expected_current_setting_value = None
if setting_value.old is not None:
expected_current_setting_value = parse_group_setting_value(
setting_value.old, setting_name
)
current_value = getattr(stream, setting_name)
current_setting_api_value = get_group_setting_value_for_api(current_value)
if validate_group_setting_value_change(
current_setting_api_value, new_setting_value, expected_current_setting_value
):
if sub is None and stream.invite_only:
# Admins cannot change this setting for unsubscribed private streams.
raise JsonableError(_("Invalid channel ID"))
user_group_id = request_settings_dict[setting_group_id_name]
with transaction.atomic(durable=True):
user_group = access_user_group_for_setting(
user_group_id,
new_setting_value,
user_profile,
setting_name=setting_name,
permission_configuration=permission_configuration,
current_setting_value=current_value,
)
do_change_stream_group_based_setting(
stream, setting_name, user_group, acting_user=user_profile
stream,
setting_name,
user_group,
old_setting_api_value=current_setting_api_value,
acting_user=user_profile,
)
return json_success(request)
@@ -558,9 +577,7 @@ def add_subscriptions_backend(
] = Stream.STREAM_POST_POLICY_EVERYONE,
history_public_to_subscribers: Json[bool] | None = None,
message_retention_days: Json[str] | Json[int] = RETENTION_DEFAULT,
can_remove_subscribers_group_id: Annotated[
Json[int | None], ApiParamConfig("can_remove_subscribers_group")
] = None,
can_remove_subscribers_group: Json[int | AnonymousSettingGroupDict] | None = None,
announce: Json[bool] = False,
principals: Json[list[str] | list[int]] | None = None,
authorization_errors_fatal: Json[bool] = True,
@@ -572,12 +589,15 @@ def add_subscriptions_backend(
if principals is None:
principals = []
if can_remove_subscribers_group_id is not None:
if can_remove_subscribers_group is not None:
setting_value = parse_group_setting_value(
can_remove_subscribers_group, "can_remove_subscribers_group"
)
permission_configuration = Stream.stream_permission_group_settings[
"can_remove_subscribers_group"
]
can_remove_subscribers_group = access_user_group_for_setting(
can_remove_subscribers_group_id,
can_remove_subscribers_group_value = access_user_group_for_setting(
setting_value,
user_profile,
setting_name="can_remove_subscribers_group",
permission_configuration=permission_configuration,
@@ -586,7 +606,7 @@ def add_subscriptions_backend(
can_remove_subscribers_group_default_name = Stream.stream_permission_group_settings[
"can_remove_subscribers_group"
].default_group_name
can_remove_subscribers_group = NamedUserGroup.objects.get(
can_remove_subscribers_group_value = NamedUserGroup.objects.get(
name=can_remove_subscribers_group_default_name,
realm=user_profile.realm,
is_system_group=True,
@@ -612,7 +632,7 @@ def add_subscriptions_backend(
stream_dict_copy["message_retention_days"] = parse_message_retention_days(
message_retention_days, Stream.MESSAGE_RETENTION_SPECIAL_VALUES_MAP
)
stream_dict_copy["can_remove_subscribers_group"] = can_remove_subscribers_group
stream_dict_copy["can_remove_subscribers_group"] = can_remove_subscribers_group_value
stream_dicts.append(stream_dict_copy)