From 32e38d36aa23b62790831d75ed8d7f8a7fecaabf Mon Sep 17 00:00:00 2001 From: Shubham Padia Date: Wed, 7 Mar 2018 03:02:03 +0530 Subject: [PATCH] bots: Send add/delete event on bot ownership change. Adds realm_bot delete event. On bot ownership change, add event is sent to the bot_owner(if not admin) and delete event to the previous bot owner(if not admin). For admin, update event is sent. --- zerver/lib/actions.py | 38 +++++++++++++++++++++++++++++--- zerver/lib/events.py | 4 ++++ zerver/tests/test_events.py | 43 +++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index a490324f5a..b85de6abeb 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -408,7 +408,7 @@ def notify_created_user(user_profile: UserProfile) -> None: is_bot=user_profile.is_bot)) send_event(event, active_user_ids(user_profile.realm_id)) -def notify_created_bot(user_profile: UserProfile) -> None: +def created_bot_event(user_profile: UserProfile) -> Dict[str, Any]: def stream_name(stream: Optional[Stream]) -> Optional[Text]: if not stream: return None @@ -436,7 +436,10 @@ def notify_created_bot(user_profile: UserProfile) -> None: if user_profile.bot_owner is not None: bot['owner'] = user_profile.bot_owner.email - event = dict(type="realm_bot", op="add", bot=bot) + return dict(type="realm_bot", op="add", bot=bot) + +def notify_created_bot(user_profile: UserProfile) -> None: + event = created_bot_event(user_profile) send_event(event, bot_owner_user_ids(user_profile)) def create_users(realm: Realm, name_list: Iterable[Tuple[Text, Text]], bot_type: int=None) -> None: @@ -2630,19 +2633,48 @@ def check_change_full_name(user_profile: UserProfile, full_name_raw: Text, def do_change_bot_owner(user_profile: UserProfile, bot_owner: UserProfile, acting_user: UserProfile) -> None: + previous_owner = user_profile.bot_owner + if previous_owner == bot_owner: + return + user_profile.bot_owner = bot_owner user_profile.save() # Can't use update_fields because of how the foreign key works. event_time = timezone_now() RealmAuditLog.objects.create(realm=user_profile.realm, acting_user=acting_user, modified_user=user_profile, event_type='bot_owner_changed', event_time=event_time) + + update_users = bot_owner_user_ids(user_profile) + + # For admins, update event is sent instead of delete/add + # event. bot_data of admin contains all the + # bots and none of them should be removed/(added again). + + # Delete the bot from previous owner's bot data. + if previous_owner and not previous_owner.is_realm_admin: + send_event(dict(type='realm_bot', + op="delete", + bot=dict(email=user_profile.email, + user_id=user_profile.id, + )), + {previous_owner.id, }) + # Do not send update event for previous bot owner. + update_users = update_users - {previous_owner.id, } + + # Notify the new owner that the bot has been added. + if not bot_owner.is_realm_admin: + add_event = created_bot_event(user_profile) + send_event(add_event, {bot_owner.id, }) + # Do not send update event for bot_owner. + update_users = update_users - {bot_owner.id, } + send_event(dict(type='realm_bot', op='update', bot=dict(email=user_profile.email, user_id=user_profile.id, owner_id=user_profile.bot_owner.id, )), - bot_owner_user_ids(user_profile)) + update_users) def do_change_tos_version(user_profile: UserProfile, tos_version: Text) -> None: user_profile.tos_version = tos_version diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 1a1329634f..f5eb9876d0 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -376,6 +376,10 @@ def apply_event(state: Dict[str, Any], if bot['email'] == email: bot['is_active'] = False + if event['op'] == 'delete': + state['realm_bots'] = [item for item + in state['realm_bots'] if item['email'] != event['bot']['email']] + if event['op'] == 'update': for bot in state['realm_bots']: if bot['email'] == event['bot']['email']: diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 92cc90dec7..24c4cdc09a 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1693,6 +1693,49 @@ class EventsRegisterTest(ZulipTestCase): error = change_bot_owner_checker('events[0]', events[0]) self.assert_on_error(error) + change_bot_owner_checker = self.check_events_dict([ + ('type', equals('realm_bot')), + ('op', equals('delete')), + ('bot', check_dict_only([ + ('email', check_string), + ('user_id', check_int), + ])), + ]) + self.user_profile = self.example_user('aaron') + owner = self.example_user('hamlet') + bot = self.create_bot('test1') + action = lambda: do_change_bot_owner(bot, owner, self.user_profile) + events = self.do_test(action) + error = change_bot_owner_checker('events[0]', events[0]) + self.assert_on_error(error) + + check_services = check_list(sub_validator=None, length=0) + change_bot_owner_checker = self.check_events_dict([ + ('type', equals('realm_bot')), + ('op', equals('add')), + ('bot', check_dict_only([ + ('email', check_string), + ('user_id', check_int), + ('bot_type', check_int), + ('full_name', check_string), + ('is_active', check_bool), + ('api_key', check_string), + ('default_sending_stream', check_none_or(check_string)), + ('default_events_register_stream', check_none_or(check_string)), + ('default_all_public_streams', check_bool), + ('avatar_url', check_string), + ('owner', check_string), + ('services', check_services), + ])), + ]) + previous_owner = self.example_user('aaron') + self.user_profile = self.example_user('hamlet') + bot = self.create_test_bot('test2', previous_owner) + action = lambda: do_change_bot_owner(bot, self.user_profile, previous_owner) + events = self.do_test(action) + error = change_bot_owner_checker('events[0]', events[0]) + self.assert_on_error(error) + def test_do_update_outgoing_webhook_service(self): # type: () -> None update_outgoing_webhook_service_checker = self.check_events_dict([