Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

streams: Allow setting can_remove_subscribers_group_id while creating streams. 

This commit adds API support to set can_remove_subscribers_group setting
when creating streams.
This commit is contained in:
Sahil Batra 2022-09-17 12:05:29 +05:30 committed by Tim Abbott
parent c3759814be
commit 73f11853ec
6 changed files with 113 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api_docs/changelog.md
View File

@ -25,6 +25,10 @@ format used by the Zulip server that they are interacting with.
* [`PATCH /streams/{stream_id}`](/api/update-stream): Added
`can_remove_subscribers_group_id` parameter to support
changing `can_remove_subscribers_group` setting.
* [`POST /users/me/subscriptions`](/api/subscribe): Added
`can_remove_subscribers_group_id` parameter to set
`can_remove_subscribers_group` setting while creating
streams.
**Feature level 160**

2
version.py
View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the
# new level means in api_docs/changelog.md, as well as "**Changes**"
# entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 159
API_FEATURE_LEVEL = 161
# 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

14
zerver/lib/streams.py
View File

@ -62,6 +62,7 @@ class StreamDict(TypedDict, total=False):
stream_post_policy: int
history_public_to_subscribers: Optional[bool]
message_retention_days: Optional[int]
can_remove_subscribers_group: Optional[UserGroup]
def get_stream_permission_policy_name(
@ -125,15 +126,19 @@ def create_stream_if_needed(
history_public_to_subscribers: Optional[bool] = None,
stream_description: str = "",
message_retention_days: Optional[int] = None,
can_remove_subscribers_group: Optional[UserGroup] = None,
acting_user: Optional[UserProfile] = None,
) -> Tuple[Stream, bool]:
history_public_to_subscribers = get_default_value_for_history_public_to_subscribers(
realm, invite_only, history_public_to_subscribers
)
administrators_user_group = UserGroup.objects.get(
name=UserGroup.ADMINISTRATORS_GROUP_NAME, is_system_group=True, realm=realm
)
if can_remove_subscribers_group is None:
can_remove_subscribers_group = UserGroup.objects.get(
name=UserGroup.ADMINISTRATORS_GROUP_NAME, is_system_group=True, realm=realm
)
assert can_remove_subscribers_group is not None
with transaction.atomic():
(stream, created) = Stream.objects.get_or_create(
realm=realm,
@ -147,7 +152,7 @@ def create_stream_if_needed(
history_public_to_subscribers=history_public_to_subscribers,
is_in_zephyr_realm=realm.is_zephyr_mirror_realm,
message_retention_days=message_retention_days,
can_remove_subscribers_group=administrators_user_group,
can_remove_subscribers_group=can_remove_subscribers_group,
),
)
@ -196,6 +201,7 @@ def create_streams_if_needed(
history_public_to_subscribers=stream_dict.get("history_public_to_subscribers"),
stream_description=stream_dict.get("description", ""),
message_retention_days=stream_dict.get("message_retention_days", None),
can_remove_subscribers_group=stream_dict.get("can_remove_subscribers_group", None),
acting_user=acting_user,
)

1
zerver/openapi/zulip.yaml
View File

@ -7855,6 +7855,7 @@ paths:
- $ref: "#/components/parameters/HistoryPublicToSubscribers"
- $ref: "#/components/parameters/StreamPostPolicy"
- $ref: "#/components/parameters/MessageRetentionDays"
- $ref: "#/components/parameters/CanRemoveSubscribersGroupId"
responses:
"200":
description: Success.

85
zerver/tests/test_subs.py
View File

@ -269,6 +269,9 @@ class TestCreateStreams(ZulipTestCase):
self.assertTrue(self.example_user("desdemona").id in events[0]["users"])
self.assertEqual(events[0]["event"]["streams"][0]["name"], "Private stream")
moderators_system_group = UserGroup.objects.get(
name="@role:moderators", realm=realm, is_system_group=True
)
new_streams, existing_streams = create_streams_if_needed(
realm,
[
@ -278,6 +281,7 @@ class TestCreateStreams(ZulipTestCase):
"invite_only": True,
"stream_post_policy": Stream.STREAM_POST_POLICY_ADMINS,
"message_retention_days": -1,
"can_remove_subscribers_group": moderators_system_group,
}
for (stream_name, stream_description) in zip(stream_names, stream_descriptions)
],
@ -294,6 +298,7 @@ class TestCreateStreams(ZulipTestCase):
self.assertTrue(stream.invite_only)
self.assertTrue(stream.stream_post_policy == Stream.STREAM_POST_POLICY_ADMINS)
self.assertTrue(stream.message_retention_days == -1)
self.assertEqual(stream.can_remove_subscribers_group.id, moderators_system_group.id)
new_streams, existing_streams = create_streams_if_needed(
realm,
@ -466,6 +471,78 @@ class TestCreateStreams(ZulipTestCase):
# But it should be marked as read for Iago, the stream creator.
self.assert_length(iago_unread_messages, 0)
def test_can_remove_subscribers_group_on_stream_creation(self) -> None:
user = self.example_user("hamlet")
realm = user.realm
self.login_user(user)
moderators_system_group = UserGroup.objects.get(
name="@role:moderators", realm=realm, is_system_group=True
)
admins_system_group = UserGroup.objects.get(
name="@role:administrators", realm=realm, is_system_group=True
)
post_data = {
"subscriptions": orjson.dumps(
[{"name": "new_stream1", "description": "First new stream"}]
).decode(),
"can_remove_subscribers_group_id": orjson.dumps(moderators_system_group.id).decode(),
}
result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
self.assert_json_success(result)
stream = get_stream("new_stream1", realm)
self.assertEqual(stream.can_remove_subscribers_group.id, moderators_system_group.id)
post_data = {
"subscriptions": orjson.dumps(
[{"name": "new_stream2", "description": "Second new stream"}]
).decode(),
}
result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
self.assert_json_success(result)
stream = get_stream("new_stream2", realm)
self.assertEqual(stream.can_remove_subscribers_group.id, admins_system_group.id)
hamletcharacters_group = UserGroup.objects.get(name="hamletcharacters", realm=realm)
post_data = {
"subscriptions": orjson.dumps(
[{"name": "new_stream3", "description": "Third new stream"}]
).decode(),
"can_remove_subscribers_group_id": orjson.dumps(hamletcharacters_group.id).decode(),
}
result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
self.assert_json_error(
result, "'can_remove_subscribers_group' must be a system user group."
)
internet_group = UserGroup.objects.get(
name="@role:internet", is_system_group=True, realm=realm
)
post_data = {
"subscriptions": orjson.dumps(
[{"name": "new_stream3", "description": "Third new stream"}]
).decode(),
"can_remove_subscribers_group_id": orjson.dumps(internet_group.id).decode(),
}
result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
self.assert_json_error(
result,
"'can_remove_subscribers_group' setting cannot be set to '@role:internet' group.",
)
owners_group = UserGroup.objects.get(name="@role:owners", is_system_group=True, realm=realm)
post_data = {
"subscriptions": orjson.dumps(
[{"name": "new_stream3", "description": "Third new stream"}]
).decode(),
"can_remove_subscribers_group_id": orjson.dumps(owners_group.id).decode(),
}
result = self.api_post(user, "/api/v1/users/me/subscriptions", post_data, subdomain="zulip")
self.assert_json_error(
result,
"'can_remove_subscribers_group' setting cannot be set to '@role:owners' group.",
)
class RecipientTest(ZulipTestCase):
def test_recipient(self) -> None:
@ -4325,7 +4402,7 @@ class SubscriptionAPITest(ZulipTestCase):
# Now add ourselves
with self.tornado_redirected_to_list(events, expected_num_events=2):
with self.assert_database_query_count(12):
with self.assert_database_query_count(13):
self.common_subscribe_to_streams(
self.test_user,
streams_to_sub,
@ -4650,7 +4727,7 @@ class SubscriptionAPITest(ZulipTestCase):
# any tornado subscription events
events: List[Mapping[str, Any]] = []
with self.tornado_redirected_to_list(events, expected_num_events=0):
with self.assert_database_query_count(4):
with self.assert_database_query_count(5):
self.common_subscribe_to_streams(
mit_user,
stream_names,
@ -4698,7 +4775,7 @@ class SubscriptionAPITest(ZulipTestCase):
test_user_ids = [user.id for user in test_users]
with self.assert_database_query_count(19):
with self.assert_database_query_count(20):
with cache_tries_captured() as cache_tries:
with mock.patch("zerver.views.streams.send_messages_for_new_subscribers"):
self.common_subscribe_to_streams(
@ -5611,7 +5688,7 @@ class GetSubscribersTest(ZulipTestCase):
polonius.id,
]
with self.assert_database_query_count(46):
with self.assert_database_query_count(47):
self.common_subscribe_to_streams(
self.user_profile,
streams,

16
zerver/views/streams.py
View File

@ -99,6 +99,7 @@ from zerver.lib.validator import (
from zerver.models import (
Realm,
Stream,
UserGroup,
UserMessage,
UserProfile,
get_active_user,
@ -555,6 +556,7 @@ def add_subscriptions_backend(
message_retention_days: Union[str, int] = REQ(
json_validator=check_string_or_int, default=RETENTION_DEFAULT
),
can_remove_subscribers_group_id: Optional[int] = REQ(json_validator=check_int, default=None),
announce: bool = REQ(json_validator=check_bool, default=False),
principals: Union[Sequence[str], Sequence[int]] = REQ(
json_validator=check_principals,
@ -565,6 +567,19 @@ def add_subscriptions_backend(
realm = user_profile.realm
stream_dicts = []
color_map = {}
if can_remove_subscribers_group_id is not None:
can_remove_subscribers_group = access_user_group_for_setting(
can_remove_subscribers_group_id,
user_profile,
setting_name="can_remove_subscribers_group",
require_system_group=True,
)
else:
can_remove_subscribers_group = UserGroup.objects.get(
name="@role:administrators", realm=user_profile.realm, is_system_group=True
)
for stream_dict in streams_raw:
# 'color' field is optional
# check for its presence in the streams_raw first
@ -585,6 +600,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_dicts.append(stream_dict_copy)