From ef772ee12f4cf9af01790c849e1893ffb6119175 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Thu, 19 Mar 2020 16:13:09 +0000 Subject: [PATCH] bot events: Prevent duplicate add-bot notifications. We don't need `do_create_user` to send a partial event here for bots. The only caller to `do_create_user` that actually creates bots (apart from some tests that just need data setup) is `add_bot_backend`, which sends the more complete event including bot "extras" like service info. The modified event tests show the simplification here (2 events instead of 3). Also, the bot tests now use tuple unpacking, which will force a ValueError if we duplicate events again. --- zerver/lib/actions.py | 6 +++--- zerver/tests/test_bots.py | 21 +++++++++++++++++---- zerver/tests/test_events.py | 10 +++++----- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index e4e5a26b9a..d2da808f67 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -523,10 +523,10 @@ def do_create_user(email: str, password: Optional[str], realm: Realm, full_name: if settings.BILLING_ENABLED: update_license_ledger_if_needed(user_profile.realm, event_time) + # Note that for bots, the caller will send an additional event + # with bot-specific info like services. notify_created_user(user_profile) - if bot_type: - notify_created_bot(user_profile) - else: + if bot_type is None: process_new_human_user(user_profile, prereg_user=prereg_user, newsletter_data=newsletter_data, default_stream_groups=default_stream_groups, diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index 1fd3f3bf58..76552df9bb 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -160,7 +160,11 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): email = 'hambot-bot@zulip.testserver' bot = self.get_bot_user(email) - event = [e for e in events if e['event']['type'] == 'realm_bot'][0] + (event,) = [ + e for e in events + if e['event']['type'] == 'realm_bot' + ] + self.assertEqual( dict( type='realm_bot', @@ -291,7 +295,10 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): email = 'hambot-bot@zulip.testserver' bot = self.get_bot_user(email) - event = [e for e in events if e['event']['type'] == 'realm_bot'][0] + (event,) = [ + e for e in events + if e['event']['type'] == 'realm_bot' + ] self.assertEqual( dict( type='realm_bot', @@ -390,7 +397,10 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): assert(profile.default_sending_stream is not None) self.assertEqual(profile.default_sending_stream.name, 'Denmark') - event = [e for e in events if e['event']['type'] == 'realm_bot'][0] + (event,) = [ + e for e in events + if e['event']['type'] == 'realm_bot' + ] self.assertEqual( dict( type='realm_bot', @@ -462,7 +472,10 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): assert(bot_profile.default_events_register_stream is not None) self.assertEqual(bot_profile.default_events_register_stream.name, 'Denmark') - event = [e for e in events if e['event']['type'] == 'realm_bot'][0] + (event,) = [ + e for e in events + if e['event']['type'] == 'realm_bot' + ] self.assertEqual( dict( type='realm_bot', diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 8f377c8fd7..fc14dd0ca5 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -2108,7 +2108,7 @@ class EventsRegisterTest(ZulipTestCase): ])), ]) action = lambda: self.create_bot('test') - events = self.do_test(action, num_events=3) + events = self.do_test(action, num_events=2) error = get_bot_created_checker(bot_type="GENERIC_BOT")('events[1]', events[1]) self.assert_on_error(error) @@ -2117,10 +2117,10 @@ class EventsRegisterTest(ZulipTestCase): payload_url=ujson.dumps('https://foo.bar.com'), interface_type=Service.GENERIC, bot_type=UserProfile.OUTGOING_WEBHOOK_BOT) - events = self.do_test(action, num_events=3) + events = self.do_test(action, num_events=2) # The third event is the second call of notify_created_bot, which contains additional # data for services (in contrast to the first call). - error = get_bot_created_checker(bot_type="OUTGOING_WEBHOOK_BOT")('events[2]', events[2]) + error = get_bot_created_checker(bot_type="OUTGOING_WEBHOOK_BOT")('events[1]', events[1]) self.assert_on_error(error) action = lambda: self.create_bot('test_embedded', @@ -2128,8 +2128,8 @@ class EventsRegisterTest(ZulipTestCase): service_name='helloworld', config_data=ujson.dumps({'foo': 'bar'}), bot_type=UserProfile.EMBEDDED_BOT) - events = self.do_test(action, num_events=3) - error = get_bot_created_checker(bot_type="EMBEDDED_BOT")('events[2]', events[2]) + events = self.do_test(action, num_events=2) + error = get_bot_created_checker(bot_type="EMBEDDED_BOT")('events[1]', events[1]) self.assert_on_error(error) def test_change_bot_full_name(self) -> None: