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

streams: Add notifications for permission policy changes. 

The change to curl_param_value_generators.py warrants a brief
explanation. Stream permission changes now generate a notification
message. Our curl example test for removing a reaction comes after
the two tests for updating the stream permission changes, thus the
hardcoded message ID in that test needs to be incremented by 2 to
account for the two notification messages that now come before it.

This is a part of #20289.
This commit is contained in:
Eeshan Garg 2021-12-10 18:41:25 -05:00 committed by Tim Abbott
parent fab1b7f5d5
commit 0d99809fd3
11 changed files with 244 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
zerver/lib/actions.py
View File

@ -150,6 +150,7 @@ from zerver.lib.streams import (
check_stream_access_based_on_stream_post_policy,
create_stream_if_needed,
get_default_value_for_history_public_to_subscribers,
get_stream_permission_policy_name,
get_web_public_streams_queryset,
render_stream_description,
send_stream_creation_event,
@ -5007,13 +5008,45 @@ def do_change_can_create_users(user_profile: UserProfile, value: bool) -> None:
user_profile.save(update_fields=["can_create_users"])
def send_change_stream_permission_notification(
stream: Stream,
*,
old_policy_name: str,
new_policy_name: str,
acting_user: UserProfile,
) -> None:
sender = get_system_bot(settings.NOTIFICATION_BOT, acting_user.realm_id)
user_mention = silent_mention_syntax_for_user(acting_user)
with override_language(stream.realm.default_language):
notification_string = _(
"{user} changed the [access permissions](/help/stream-permissions) "
"for this stream from **{old_policy}** to **{new_policy}**."
)
notification_string = notification_string.format(
user=user_mention,
old_policy=old_policy_name,
new_policy=new_policy_name,
)
internal_send_stream_message(
sender, stream, Realm.STREAM_EVENTS_NOTIFICATION_TOPIC, notification_string
)
def do_change_stream_permission(
stream: Stream,
*,
invite_only: Optional[bool] = None,
history_public_to_subscribers: Optional[bool] = None,
is_web_public: Optional[bool] = None,
acting_user: UserProfile,
) -> None:
old_policy_name = get_stream_permission_policy_name(
invite_only=stream.invite_only,
history_public_to_subscribers=stream.history_public_to_subscribers,
is_web_public=stream.is_web_public,
)
# A note on these assertions: It's possible we'd be better off
# making all callers of this function pass the full set of
# parameters, rather than having default values. Doing so would
@ -5042,6 +5075,12 @@ def do_change_stream_permission(
stream.save(update_fields=["invite_only", "history_public_to_subscribers", "is_web_public"])
new_policy_name = get_stream_permission_policy_name(
invite_only=stream.invite_only,
history_public_to_subscribers=stream.history_public_to_subscribers,
is_web_public=stream.is_web_public,
)
event = dict(
op="update",
type="stream",
@ -5053,6 +5092,12 @@ def do_change_stream_permission(
name=stream.name,
)
send_event(stream.realm, event, can_access_stream_user_ids(stream))
send_change_stream_permission_notification(
stream,
old_policy_name=old_policy_name,
new_policy_name=new_policy_name,
acting_user=acting_user,
)
def send_change_stream_post_policy_notification(

20
zerver/lib/streams.py
View File

@ -57,6 +57,26 @@ class StreamDict(TypedDict, total=False):
message_retention_days: Optional[int]
def get_stream_permission_policy_name(
*,
invite_only: Optional[bool] = None,
history_public_to_subscribers: Optional[bool] = None,
is_web_public: Optional[bool] = None,
) -> str:
policy_name = None
for permission, permission_dict in Stream.PERMISSION_POLICIES.items():
if (
permission_dict["invite_only"] == invite_only
and permission_dict["history_public_to_subscribers"] == history_public_to_subscribers
and permission_dict["is_web_public"] == is_web_public
):
policy_name = permission_dict["policy_name"]
break
assert policy_name is not None
return policy_name
def get_default_value_for_history_public_to_subscribers(
realm: Realm,
invite_only: bool,

35
zerver/models.py
View File

@ -2239,6 +2239,41 @@ class Stream(models.Model):
# Foreign key to the Recipient object for STREAM type messages to this stream.
recipient = models.ForeignKey(Recipient, null=True, on_delete=models.SET_NULL)
# Various permission policy configurations
PERMISSION_POLICIES: Dict[str, Dict[str, Any]] = {
"web_public": {
"invite_only": False,
"history_public_to_subscribers": True,
"is_web_public": True,
"policy_name": gettext_lazy("Web public"),
},
"public": {
"invite_only": False,
"history_public_to_subscribers": True,
"is_web_public": False,
"policy_name": gettext_lazy("Public"),
},
"private_shared_history": {
"invite_only": True,
"history_public_to_subscribers": True,
"is_web_public": False,
"policy_name": gettext_lazy("Private, shared history"),
},
"private_protected_history": {
"invite_only": True,
"history_public_to_subscribers": False,
"is_web_public": False,
"policy_name": gettext_lazy("Private, protected history"),
},
# Public streams with protected history are currently only
# available in Zephyr realms
"public_protected_history": {
"invite_only": False,
"history_public_to_subscribers": False,
"is_web_public": False,
"policy_name": gettext_lazy("Public, protected history"),
},
}
invite_only: Optional[bool] = models.BooleanField(null=True, default=False)
history_public_to_subscribers: bool = models.BooleanField(default=False)

2
zerver/openapi/curl_param_value_generators.py
View File

@ -128,7 +128,7 @@ def add_emoji_to_message() -> Dict[str, object]:
# The message ID here is hardcoded based on the corresponding value
# for the example message IDs we use in zulip.yaml.
message_id = 44
message_id = 46
emoji_name = "octopus"
emoji_code = "1f419"
reaction_type = "unicode_emoji"

24
zerver/tests/test_bots.py
View File

@ -420,7 +420,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
user_profile = self.example_user("hamlet")
stream = get_stream("Denmark", user_profile.realm)
self.subscribe(user_profile, stream.name)
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(stream, invite_only=True, acting_user=user_profile)
self.assert_num_bots_equal(0)
events: List[Mapping[str, Any]] = []
@ -464,7 +464,9 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
realm = self.example_user("hamlet").realm
stream = get_stream("Denmark", realm)
self.unsubscribe(self.example_user("hamlet"), "Denmark")
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(
stream, invite_only=True, acting_user=self.example_user("hamlet")
)
bot_info = {
"full_name": "The Bot of Hamlet",
@ -492,7 +494,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.login("hamlet")
user_profile = self.example_user("hamlet")
stream = self.subscribe(user_profile, "Denmark")
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(stream, invite_only=True, acting_user=user_profile)
self.assert_num_bots_equal(0)
events: List[Mapping[str, Any]] = []
@ -536,7 +538,9 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
realm = self.example_user("hamlet").realm
stream = get_stream("Denmark", realm)
self.unsubscribe(self.example_user("hamlet"), "Denmark")
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(
stream, invite_only=True, acting_user=self.example_user("hamlet")
)
self.assert_num_bots_equal(0)
bot_info = {
@ -1132,7 +1136,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.login("hamlet")
user_profile = self.example_user("hamlet")
stream = self.subscribe(user_profile, "Denmark")
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(stream, invite_only=True, acting_user=user_profile)
bot_info = {
"full_name": "The Bot of Hamlet",
@ -1158,7 +1162,9 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
realm = self.example_user("hamlet").realm
stream = get_stream("Denmark", realm)
self.unsubscribe(self.example_user("hamlet"), "Denmark")
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(
stream, invite_only=True, acting_user=self.example_user("hamlet")
)
bot_info = {
"full_name": "The Bot of Hamlet",
@ -1239,7 +1245,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
self.login("hamlet")
user_profile = self.example_user("hamlet")
stream = self.subscribe(user_profile, "Denmark")
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(stream, invite_only=True, acting_user=user_profile)
bot_info = {
"full_name": "The Bot of Hamlet",
@ -1264,7 +1270,9 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
realm = self.example_user("hamlet").realm
stream = get_stream("Denmark", realm)
self.unsubscribe(self.example_user("hamlet"), "Denmark")
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(
stream, invite_only=True, acting_user=self.example_user("hamlet")
)
bot_info = {
"full_name": "The Bot of Hamlet",

18
zerver/tests/test_events.py
View File

@ -2578,17 +2578,27 @@ class SubscribeActionTest(BaseAction):
# Update stream privacy - make stream web public
action = lambda: do_change_stream_permission(
stream, invite_only=False, history_public_to_subscribers=True, is_web_public=True
stream,
invite_only=False,
history_public_to_subscribers=True,
is_web_public=True,
acting_user=self.example_user("hamlet"),
)
events = self.verify_action(action, include_subscribers=include_subscribers)
events = self.verify_action(action, include_subscribers=include_subscribers, num_events=2)
check_stream_update("events[0]", events[0])
check_message("events[1]", events[1])
# Update stream privacy - make stream private
action = lambda: do_change_stream_permission(
stream, invite_only=True, history_public_to_subscribers=True, is_web_public=False
stream,
invite_only=True,
history_public_to_subscribers=True,
is_web_public=False,
acting_user=self.example_user("hamlet"),
)
events = self.verify_action(action, include_subscribers=include_subscribers)
events = self.verify_action(action, include_subscribers=include_subscribers, num_events=2)
check_stream_update("events[0]", events[0])
check_message("events[1]", events[1])
# Update stream stream_post_policy property
action = lambda: do_change_stream_post_policy(

15
zerver/tests/test_message_flags.py
View File

@ -1264,12 +1264,16 @@ class MessageAccessTests(ZulipTestCase):
# Guest user can access messages of subscribed private streams if
# history is public to subscribers
do_change_stream_permission(stream, invite_only=True, history_public_to_subscribers=True)
do_change_stream_permission(
stream, invite_only=True, history_public_to_subscribers=True, acting_user=guest_user
)
result = self.change_star(message_id)
self.assert_json_success(result)
# With history not public to subscribers, they can still see new messages
do_change_stream_permission(stream, invite_only=True, history_public_to_subscribers=False)
do_change_stream_permission(
stream, invite_only=True, history_public_to_subscribers=False, acting_user=guest_user
)
self.login_user(normal_user)
message_id = [
self.send_stream_message(normal_user, stream_name, "test 2"),
@ -1312,7 +1316,12 @@ class MessageAccessTests(ZulipTestCase):
self.assert_length(filtered_messages, 1)
self.assertEqual(filtered_messages[0].id, message_two_id)
do_change_stream_permission(stream, invite_only=True, history_public_to_subscribers=True)
do_change_stream_permission(
stream,
invite_only=True,
history_public_to_subscribers=True,
acting_user=self.example_user("cordelia"),
)
with queries_captured() as queries:
filtered_messages = bulk_access_messages(later_subscribed_user, messages, stream=stream)

24
zerver/tests/test_message_topics.py
View File

@ -132,7 +132,9 @@ class TopicHistoryTest(ZulipTestCase):
)
# Now make stream private, but subscribe cordelia
do_change_stream_permission(stream, invite_only=True)
do_change_stream_permission(
stream, invite_only=True, acting_user=self.example_user("cordelia")
)
self.subscribe(self.example_user("cordelia"), stream.name)
result = self.client_get(endpoint, {})
@ -231,10 +233,12 @@ class TopicDeleteTest(ZulipTestCase):
},
)
self.assert_json_error(result, "Must be an organization administrator")
self.assertEqual(self.get_last_message().id, last_msg_id)
self.assertTrue(Message.objects.filter(id=last_msg_id).exists())
# Make stream private with limited history
do_change_stream_permission(stream, invite_only=True, history_public_to_subscribers=False)
do_change_stream_permission(
stream, invite_only=True, history_public_to_subscribers=False, acting_user=user_profile
)
# ADMIN USER subscribed now
user_profile = self.example_user("iago")
@ -252,7 +256,7 @@ class TopicDeleteTest(ZulipTestCase):
},
)
self.assert_json_success(result)
self.assertEqual(self.get_last_message().id, last_msg_id)
self.assertTrue(Message.objects.filter(id=last_msg_id).exists())
# Try to delete all messages in the topic again. There are no messages accessible
# to the administrator, so this should do nothing.
@ -263,10 +267,12 @@ class TopicDeleteTest(ZulipTestCase):
},
)
self.assert_json_success(result)
self.assertEqual(self.get_last_message().id, last_msg_id)
self.assertTrue(Message.objects.filter(id=last_msg_id).exists())
# Make the stream's history public to subscribers
do_change_stream_permission(stream, invite_only=True, history_public_to_subscribers=True)
do_change_stream_permission(
stream, invite_only=True, history_public_to_subscribers=True, acting_user=user_profile
)
# Delete the topic should now remove all messages
result = self.client_post(
endpoint,
@ -275,7 +281,8 @@ class TopicDeleteTest(ZulipTestCase):
},
)
self.assert_json_success(result)
self.assertEqual(self.get_last_message().id, initial_last_msg_id)
self.assertFalse(Message.objects.filter(id=last_msg_id).exists())
self.assertTrue(Message.objects.filter(id=initial_last_msg_id).exists())
# Delete again, to test the edge case of deleting an empty topic.
result = self.client_post(
@ -285,4 +292,5 @@ class TopicDeleteTest(ZulipTestCase):
},
)
self.assert_json_success(result)
self.assertEqual(self.get_last_message().id, initial_last_msg_id)
self.assertFalse(Message.objects.filter(id=last_msg_id).exists())
self.assertTrue(Message.objects.filter(id=initial_last_msg_id).exists())

6
zerver/tests/test_reactions.py
View File

@ -543,7 +543,9 @@ class ReactionEventTest(ZulipTestCase):
self.assert_json_success(remove)
# Make stream history public to subscribers
do_change_stream_permission(stream, invite_only=False, history_public_to_subscribers=True)
do_change_stream_permission(
stream, invite_only=False, history_public_to_subscribers=True, acting_user=iago
)
# Since stream history is public to subscribers, reacting to
# message_before_id should notify all subscribers:
# Iago and Hamlet.
@ -562,7 +564,7 @@ class ReactionEventTest(ZulipTestCase):
self.assert_json_success(remove)
# Make stream web_public as well.
do_change_stream_permission(stream, is_web_public=True)
do_change_stream_permission(stream, is_web_public=True, acting_user=iago)
# For is_web_public streams, events even on old messages
# should go to all subscribers, including guests like polonius.
with self.tornado_redirected_to_list(events, expected_num_events=1):

84
zerver/tests/test_subs.py
View File

@ -467,6 +467,14 @@ class StreamAdminTest(ZulipTestCase):
self.assertFalse(stream.invite_only)
self.assertTrue(stream.history_public_to_subscribers)
messages = get_topic_messages(user_profile, stream, "stream events")
self.assert_length(messages, 1)
expected_notification = (
f"@_**King Hamlet|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
"for this stream from **Private, protected history** to **Public**."
)
self.assertEqual(messages[0].content, expected_notification)
do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=None)
params = {
"stream_name": orjson.dumps("private_stream_2").decode(),
@ -493,6 +501,14 @@ class StreamAdminTest(ZulipTestCase):
self.assertFalse(stream.invite_only)
self.assertTrue(stream.history_public_to_subscribers)
messages = get_topic_messages(user_profile, stream, "stream events")
self.assert_length(messages, 1)
expected_notification = (
f"@_**King Hamlet|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
"for this stream from **Private, protected history** to **Public**."
)
self.assertEqual(messages[0].content, expected_notification)
def test_make_stream_private(self) -> None:
user_profile = self.example_user("hamlet")
self.login_user(user_profile)
@ -505,13 +521,21 @@ class StreamAdminTest(ZulipTestCase):
"stream_name": orjson.dumps("public_stream_1").decode(),
"is_private": orjson.dumps(True).decode(),
}
stream_id = get_stream("public_stream_1", realm).id
stream_id = self.subscribe(user_profile, "public_stream_1").id
result = self.client_patch(f"/json/streams/{stream_id}", params)
self.assert_json_success(result)
stream = get_stream("public_stream_1", realm)
self.assertTrue(stream.invite_only)
self.assertFalse(stream.history_public_to_subscribers)
messages = get_topic_messages(user_profile, stream, "stream events")
self.assert_length(messages, 1)
expected_notification = (
f"@_**King Hamlet|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
"for this stream from **Public** to **Private, protected history**."
)
self.assertEqual(messages[0].content, expected_notification)
default_stream = self.make_stream("default_stream", realm=realm)
do_add_default_stream(default_stream)
params = {
@ -548,6 +572,14 @@ class StreamAdminTest(ZulipTestCase):
self.assertTrue(stream.invite_only)
self.assertFalse(stream.history_public_to_subscribers)
messages = get_topic_messages(user_profile, stream, "stream events")
self.assert_length(messages, 1)
expected_notification = (
f"@_**King Hamlet|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
"for this stream from **Public** to **Private, protected history**."
)
self.assertEqual(messages[0].content, expected_notification)
def test_create_web_public_stream(self) -> None:
user_profile = self.example_user("hamlet")
owner = self.example_user("desdemona")
@ -614,6 +646,14 @@ class StreamAdminTest(ZulipTestCase):
self.assertFalse(stream.invite_only)
self.assertFalse(stream.history_public_to_subscribers)
messages = get_topic_messages(user_profile, stream, "stream events")
self.assert_length(messages, 1)
expected_notification = (
f"@_**{user_profile.full_name}|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
"for this stream from **Private, protected history** to **Public, protected history**."
)
self.assertEqual(messages[0].content, expected_notification)
def test_make_stream_private_with_public_history(self) -> None:
user_profile = self.example_user("hamlet")
self.login_user(user_profile)
@ -626,19 +666,27 @@ class StreamAdminTest(ZulipTestCase):
"is_private": orjson.dumps(True).decode(),
"history_public_to_subscribers": orjson.dumps(True).decode(),
}
stream_id = get_stream("public_history_stream", realm).id
stream_id = self.subscribe(user_profile, "public_history_stream").id
result = self.client_patch(f"/json/streams/{stream_id}", params)
self.assert_json_success(result)
stream = get_stream("public_history_stream", realm)
self.assertTrue(stream.invite_only)
self.assertTrue(stream.history_public_to_subscribers)
messages = get_topic_messages(user_profile, stream, "stream events")
self.assert_length(messages, 1)
expected_notification = (
f"@_**King Hamlet|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
"for this stream from **Public** to **Private, shared history**."
)
self.assertEqual(messages[0].content, expected_notification)
def test_make_stream_web_public(self) -> None:
user_profile = self.example_user("hamlet")
self.login_user(user_profile)
realm = user_profile.realm
self.make_stream("test_stream", realm=realm)
stream_id = get_stream("test_stream", realm).id
stream_id = self.subscribe(user_profile, "test_stream").id
params = {
"stream_name": orjson.dumps("test_stream").decode(),
@ -699,6 +747,14 @@ class StreamAdminTest(ZulipTestCase):
self.assertFalse(stream.invite_only)
self.assertTrue(stream.history_public_to_subscribers)
messages = get_topic_messages(user_profile, stream, "stream events")
self.assert_length(messages, 1)
expected_notification = (
f"@_**King Hamlet|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
"for this stream from **Public** to **Web public**."
)
self.assertEqual(messages[0].content, expected_notification)
def test_try_make_stream_public_with_private_history(self) -> None:
user_profile = self.example_user("hamlet")
self.login_user(user_profile)
@ -711,13 +767,33 @@ class StreamAdminTest(ZulipTestCase):
"is_private": orjson.dumps(False).decode(),
"history_public_to_subscribers": orjson.dumps(False).decode(),
}
stream_id = get_stream("public_stream", realm).id
stream_id = self.subscribe(user_profile, "public_stream").id
result = self.client_patch(f"/json/streams/{stream_id}", params)
self.assert_json_success(result)
stream = get_stream("public_stream", realm)
self.assertFalse(stream.invite_only)
self.assertTrue(stream.history_public_to_subscribers)
messages = get_topic_messages(user_profile, stream, "stream events")
self.assert_length(messages, 1)
# This test verifies the (weird) outcome for a transition that
# is not currently possible. For background, we only support
# public streams with private history if
# is_zephyr_mirror_realm, and don't allow changing stream
# permissions in such realms. So changing the
# history_public_to_subscribers property of a public stream is
# not possible in Zulip today; this test covers that situation
# and will produce the odd/wrong output of "Public to Public".
#
# This test should be corrected if we add support for such a
# stream configuration transition.
expected_notification = (
f"@_**King Hamlet|{user_profile.id}** changed the [access permissions](/help/stream-permissions) "
"for this stream from **Public** to **Public**."
)
self.assertEqual(messages[0].content, expected_notification)
def test_subscriber_ids_with_stream_history_access(self) -> None:
hamlet = self.example_user("hamlet")
polonius = self.example_user("polonius")

1
zerver/views/streams.py
View File

@ -329,6 +329,7 @@ def update_stream_backend(
invite_only=is_private,
history_public_to_subscribers=history_public_to_subscribers,
is_web_public=is_web_public,
acting_user=user_profile,
)
return json_success()