subs: Notify organization admins when private streams are renamed.

This commit sends the event for renaming of a private stream to
organization admins of the realm, in addition to the obvious list of
subscribers of the private stream.

Normally, admins can manage a private stream (e.g. unsubscribing a
user).  But when the admin tried to unsubscribes a user from a
previously renamed stream, we previously were throwing a JS error, as
the webapp hadn't been notified about the new stream name.

Fixes #9034.
This commit is contained in:
Roman Godov
2018-04-13 03:42:30 +03:00
committed by Tim Abbott
parent e168f9938c
commit d99758129e
2 changed files with 24 additions and 2 deletions

View File

@@ -173,11 +173,13 @@ def log_event(event: MutableMapping[str, Any]) -> None:
def can_access_stream_user_ids(stream: Stream) -> Set[int]:
# return user ids of users who can access the attributes of
# a stream, such as its name/description
# a stream, such as its name/description.
if stream.is_public():
# For a public stream, this is everyone in the realm
return set(active_user_ids(stream.realm_id))
else:
return private_stream_user_ids(stream.id)
# for a private stream, it's subscribers plus realm admins.
return private_stream_user_ids(stream.id) | {user.id for user in stream.realm.get_admin_users()}
def private_stream_user_ids(stream_id: int) -> Set[int]:
# TODO: Find similar queries elsewhere and de-duplicate this code.

View File

@@ -60,6 +60,7 @@ from zerver.lib.actions import (
do_change_default_stream_group_description,
do_change_default_stream_group_name,
lookup_default_stream_groups,
can_access_stream_user_ids,
)
from zerver.views.streams import (
@@ -414,6 +415,25 @@ class StreamAdminTest(ZulipTestCase):
stream_name_mixed_exists = get_stream(u'français name', realm)
self.assertTrue(stream_name_mixed_exists)
# Test case for notified users in private streams.
stream_private = self.make_stream('stream_private_name1', realm=user_profile.realm, invite_only=True)
self.subscribe(self.example_user('cordelia'), 'stream_private_name1')
del events[:]
with tornado_redirected_to_list(events):
stream_id = get_stream('stream_private_name1', realm).id
result = self.client_patch('/json/streams/%d' % (stream_id,),
{'new_name': ujson.dumps('stream_private_name2')})
self.assert_json_success(result)
notified_user_ids = set(events[1]['users'])
self.assertEqual(notified_user_ids, can_access_stream_user_ids(stream_private))
self.assertIn(self.example_user('cordelia').id, notified_user_ids)
# An important corner case is that all organization admins are notified.
self.assertIn(self.example_user('iago').id, notified_user_ids)
# The current user, Hamlet was made an admin and thus should be notified too.
self.assertIn(user_profile.id, notified_user_ids)
self.assertNotIn(self.example_user('prospero').id,
notified_user_ids)
def test_rename_stream_requires_realm_admin(self) -> None:
user_profile = self.example_user('hamlet')
email = user_profile.email