From 4dcccf32f842eddf0238f5a252ca23c682f69324 Mon Sep 17 00:00:00 2001 From: sahil839 Date: Sat, 5 Jan 2019 03:47:38 -0800 Subject: [PATCH] zerver: Add feature for notification at rename of a stream. Feature of sending notification to the stream using notification bot is added. user_profile is also passed to do_rename_stream for using the name of user who renamed the stream in notification. Notification is sent to the stream using internal_send_stream_message in do_rename_stream. Fixes #11034. --- zerver/lib/actions.py | 14 ++++++-- zerver/management/commands/rename_stream.py | 2 +- zerver/tests/test_events.py | 40 ++++++++++++++++++--- zerver/tests/test_subs.py | 37 ++++++++++++++++--- zerver/views/streams.py | 2 +- 5 files changed, 83 insertions(+), 12 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 5ad688d628..c318e4b923 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -3256,7 +3256,10 @@ def do_change_stream_announcement_only(stream: Stream, is_announcement_only: boo stream.is_announcement_only = is_announcement_only stream.save(update_fields=['is_announcement_only']) -def do_rename_stream(stream: Stream, new_name: str, log: bool=True) -> Dict[str, str]: +def do_rename_stream(stream: Stream, + new_name: str, + user_profile: UserProfile, + log: bool=True) -> Dict[str, str]: old_name = stream.name stream.name = new_name stream.save(update_fields=["name"]) @@ -3307,7 +3310,14 @@ def do_rename_stream(stream: Stream, new_name: str, log: bool=True) -> Dict[str, name=old_name, ) send_event(stream.realm, event, can_access_stream_user_ids(stream)) - + sender = get_system_bot(settings.NOTIFICATION_BOT) + internal_send_stream_message( + stream.realm, + sender, + new_name, + "welcome", + "@**%s** renamed stream **%s** to **%s**" % (user_profile.full_name, old_name, new_name) + ) # Even though the token doesn't change, the web client needs to update the # email forwarding address to display the correctly-escaped new name. return {"email_address": new_email} diff --git a/zerver/management/commands/rename_stream.py b/zerver/management/commands/rename_stream.py index 37d0199875..66c1a6e92b 100644 --- a/zerver/management/commands/rename_stream.py +++ b/zerver/management/commands/rename_stream.py @@ -24,4 +24,4 @@ class Command(ZulipBaseCommand): new_name = options['new_name'] stream = get_stream(old_name, realm) - do_rename_stream(stream, new_name) + do_rename_stream(stream, new_name, self.user_profile) diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index b60ea91779..ee2ff2ac54 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -2094,10 +2094,9 @@ class EventsRegisterTest(ZulipTestCase): stream = self.make_stream('old_name') new_name = u'stream with a brand new name' self.subscribe(self.user_profile, stream.name) - - action = lambda: do_rename_stream(stream, new_name) - events = self.do_test(action, num_events=2) - + notification = '

@King Hamlet renamed stream old_name to stream with a brand new name

' + action = lambda: do_rename_stream(stream, new_name, self.user_profile) + events = self.do_test(action, num_events=3) schema_checker = self.check_events_dict([ ('type', equals('stream')), ('op', equals('update')), @@ -2118,6 +2117,39 @@ class EventsRegisterTest(ZulipTestCase): ]) error = schema_checker('events[1]', events[1]) self.assert_on_error(error) + schema_checker = check_dict([ + ('stream_email_notify', equals(False)), + ('flags', check_list(check_string)), + ('email_notified', equals(True)), + ('type', equals('message')), + ('message', check_dict([ + ('timestamp', check_int), + ('content', equals(notification)), + ('content_type', equals('text/html')), + ('sender_email', equals('notification-bot@zulip.com')), + ('sender_id', check_int), + ('sender_short_name', equals('notification-bot')), + ('display_recipient', equals(new_name)), + ('id', check_int), + ('stream_id', check_int), + ('sender_realm_str', check_string), + ('sender_full_name', equals('Notification Bot')), + ('is_me_message', equals(False)), + ('type', equals('stream')), + ('submessages', check_list(check_string)), + (TOPIC_LINKS, check_list(check_url)), + ('avatar_url', check_url), + ('reactions', check_list(None)), + ('client', equals('Internal')), + (TOPIC_NAME, equals('welcome')), + ('recipient_id', check_int) + ])), + ('id', check_int), + ('push_notified', equals(True)), + ('stream_push_notify', equals(False)), + ]) + error = schema_checker('events[2]', events[2]) + self.assert_on_error(error) def test_deactivate_stream_neversubscribed(self) -> None: stream = self.make_stream('old_name') diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 5d2e4125b6..4ab298d1a0 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -64,6 +64,7 @@ from zerver.lib.actions import ( do_remove_default_stream_group, do_change_default_stream_group_description, do_change_default_stream_group_name, + do_rename_stream, lookup_default_stream_groups, can_access_stream_user_ids, validate_user_access_to_subscribers_helper, @@ -464,14 +465,22 @@ class StreamAdminTest(ZulipTestCase): result = self.client_patch('/json/streams/%d' % (stream_id,), {'new_name': ujson.dumps('whatever')}) self.assert_json_success(result) - # Should be a name event and an email address event - self.assert_length(events, 2) + # Should be a name event, an email address event and a notification event + self.assert_length(events, 3) - notified_user_ids = set(events[-1]['users']) + notified_user_ids = set(events[0]['users']) self.assertIn(user_profile.id, notified_user_ids) self.assertIn(cordelia.id, notified_user_ids) self.assertNotIn(prospero.id, notified_user_ids) + notified_with_bot_users = events[-1]['users'] + notified_with_bot_user_ids = [] + notified_with_bot_user_ids.append(notified_with_bot_users[0]['id']) + notified_with_bot_user_ids.append(notified_with_bot_users[1]['id']) + self.assertIn(user_profile.id, notified_with_bot_user_ids) + self.assertIn(cordelia.id, notified_with_bot_user_ids) + self.assertNotIn(prospero.id, notified_with_bot_user_ids) + def test_rename_stream(self) -> None: user_profile = self.example_user('hamlet') email = user_profile.email @@ -501,7 +510,6 @@ class StreamAdminTest(ZulipTestCase): result = self.client_patch('/json/streams/%d' % (stream_id,), {'new_name': ujson.dumps('stream_name2')}) self.assert_json_success(result) - event = events[1]['event'] self.assertEqual(event, dict( op='update', @@ -599,6 +607,27 @@ class StreamAdminTest(ZulipTestCase): {'new_name': ujson.dumps('stream_name2')}) self.assert_json_error(result, 'Must be an organization administrator') + def test_notify_on_stream_rename(self) -> None: + user_profile = self.example_user('hamlet') + self.login(user_profile.email) + self.make_stream('stream_name1') + + stream = self.subscribe(user_profile, 'stream_name1') + do_change_is_admin(user_profile, True) + result = self.client_patch('/json/streams/%d' % (stream.id,), + {'new_name': ujson.dumps('stream_name2')}) + self.assert_json_success(result) + + # Inspect the notification message sent + message = self.get_last_message() + actual_stream = Stream.objects.get(id=message.recipient.type_id) + message_content = '@**King Hamlet** renamed stream **stream_name1** to **stream_name2**' + self.assertEqual(message.sender.realm, user_profile.realm) + self.assertEqual(actual_stream.name, 'stream_name2') + self.assertEqual(message.recipient.type, Recipient.STREAM) + self.assertEqual(message.content, message_content) + self.assertEqual(message.sender.email, 'notification-bot@zulip.com') + def test_realm_admin_can_update_unsub_private_stream(self) -> None: iago = self.example_user('iago') self.login(iago.email) diff --git a/zerver/views/streams.py b/zerver/views/streams.py index e08511b1a0..2872302338 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -167,7 +167,7 @@ def update_stream_backend( # Check that the stream name is available (unless we are # are only changing the casing of the stream name). check_stream_name_available(user_profile.realm, new_name) - do_rename_stream(stream, new_name) + do_rename_stream(stream, new_name, user_profile) if is_announcement_only is not None: do_change_stream_announcement_only(stream, is_announcement_only)