From 656f882a44b2a29414aafe22d724a09c493f1468 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Thu, 29 Mar 2018 11:32:25 -0700 Subject: [PATCH] bots: Eliminate NEW_USER_BOT. This bot was basically a duplicate of NOTIFICATION_BOT for some specific corner cases, and didn't add much value. It's better to just eliminate it, which also removes some ugly corner cases around what happens if the user account doesn't exist. --- zerver/lib/actions.py | 12 ++++++------ zerver/tests/test_home.py | 5 ++--- zerver/tests/test_realm.py | 3 --- zerver/views/home.py | 5 +++-- zerver/views/realm.py | 2 -- zproject/dev_settings.py | 1 - zproject/settings.py | 3 --- 7 files changed, 11 insertions(+), 20 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index a108ff81b4..ce58446d83 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -302,8 +302,8 @@ def send_signup_message(sender: UserProfile, admin_realm_signup_notifications_st ) def notify_new_user(user_profile: UserProfile, internal: bool=False) -> None: - if settings.NEW_USER_BOT is not None: - send_signup_message(settings.NEW_USER_BOT, "signups", user_profile, internal) + if settings.NOTIFICATION_BOT is not None: + send_signup_message(settings.NOTIFICATION_BOT, "signups", user_profile, internal) statsd.gauge("users.signups.%s" % (user_profile.realm.string_id), 1, delta=True) # We also clear any scheduled invitation emails to prevent them @@ -1890,7 +1890,7 @@ def check_message(sender: UserProfile, client: Client, addressee: Addressee, elif sender.email == settings.WELCOME_BOT: # The welcome bot welcomes folks to the stream. pass - elif sender.email == settings.NEW_USER_BOT: + elif sender.email == settings.NOTIFICATION_BOT: pass else: # All other cases are an error. @@ -3005,10 +3005,10 @@ def do_create_realm(string_id: Text, name: Text, restricted_to_domain: Optional[ "org_type": org_type}) # Send a notification to the admin realm (if configured) - if settings.NEW_USER_BOT is not None: + if settings.NOTIFICATION_BOT is not None: signup_message = "Signups enabled" - admin_realm = get_system_bot(settings.NEW_USER_BOT).realm - internal_send_message(admin_realm, settings.NEW_USER_BOT, "stream", + admin_realm = get_system_bot(settings.NOTIFICATION_BOT).realm + internal_send_message(admin_realm, settings.NOTIFICATION_BOT, "stream", "signups", realm.display_subdomain, signup_message) return realm diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index fdafd60394..db8d5ca180 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -95,7 +95,6 @@ class HomeTest(ZulipTestCase): "narrow_stream", "needs_tutorial", "never_subscribed", - "new_user_bot_configured", "night_mode", "password_min_guesses", "password_min_length", @@ -190,7 +189,7 @@ class HomeTest(ZulipTestCase): with patch('zerver.lib.cache.cache_set') as cache_mock: result = self._get_home_page(stream='Denmark') - self.assert_length(queries, 44) + self.assert_length(queries, 43) self.assert_length(cache_mock.call_args_list, 8) html = result.content.decode('utf-8') @@ -234,7 +233,7 @@ class HomeTest(ZulipTestCase): result = self._get_home_page() self.assertEqual(result.status_code, 200) self.assert_length(cache_mock.call_args_list, 17) - self.assert_length(queries, 60) + self.assert_length(queries, 59) @slow("Creates and subscribes 10 users in a loop. Should use bulk queries.") def test_num_queries_with_streams(self) -> None: diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 070dba0548..11d8e50188 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -222,9 +222,6 @@ class RealmTest(ZulipTestCase): new_signup_notifications_stream_id = 4 req = dict(signup_notifications_stream_id = ujson.dumps(new_signup_notifications_stream_id)) - with self.settings(NEW_USER_BOT=None): - result = self.client_patch("/json/realm", req) - self.assert_json_error(result, 'NEW_USER_BOT must configured first.') result = self.client_patch('/json/realm', req) self.assert_json_success(result) diff --git a/zerver/views/home.py b/zerver/views/home.py index 64be0d7c17..e5d03d5939 100644 --- a/zerver/views/home.py +++ b/zerver/views/home.py @@ -126,7 +126,9 @@ def home_real(request: HttpRequest) -> HttpResponse: # Reset our don't-spam-users-with-email counter since the # user has since logged in - if user_profile.last_reminder is not None: + if user_profile.last_reminder is not None: # nocoverage + # TODO: Look into the history of last_reminder; we may have + # eliminated that as a useful concept for non-bot users. user_profile.last_reminder = None user_profile.save(update_fields=["last_reminder"]) @@ -172,7 +174,6 @@ def home_real(request: HttpRequest) -> HttpResponse: # These end up in a global JavaScript Object named 'page_params'. page_params = dict( # Server settings. - new_user_bot_configured = settings.NEW_USER_BOT is not None, development_environment = settings.DEVELOPMENT, debug_mode = settings.DEBUG, test_suite = settings.TEST_SUITE, diff --git a/zerver/views/realm.py b/zerver/views/realm.py index 2f01bdaa37..8c399ca751 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -66,8 +66,6 @@ def update_realm( return json_error(_("Organization name is too long.")) if authentication_methods is not None and True not in list(authentication_methods.values()): return json_error(_("At least one authentication method must be enabled.")) - if signup_notifications_stream_id is not None and settings.NEW_USER_BOT is None: - return json_error(_("NEW_USER_BOT must configured first.")) # Additional validation of permissions values to add new bot if bot_creation_policy is not None and bot_creation_policy not in Realm.BOT_CREATION_POLICY_TYPES: diff --git a/zproject/dev_settings.py b/zproject/dev_settings.py index 1753cc82b0..26f2859774 100644 --- a/zproject/dev_settings.py +++ b/zproject/dev_settings.py @@ -46,7 +46,6 @@ EXTERNAL_URI_SCHEME = "http://" EMAIL_GATEWAY_PATTERN = "%s@" + EXTERNAL_HOST NOTIFICATION_BOT = "notification-bot@zulip.com" ERROR_BOT = "error-bot@zulip.com" -NEW_USER_BOT = "new-user-bot@zulip.com" EMAIL_GATEWAY_BOT = "emailgateway@zulip.com" PHYSICAL_ADDRESS = "Zulip Headquarters, 123 Octo Stream, South Pacific Ocean" EXTRA_INSTALLED_APPS = ["zilencer", "analytics"] diff --git a/zproject/settings.py b/zproject/settings.py index d1449aed38..5613e75c0e 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -226,9 +226,6 @@ DEFAULT_SETTINGS.update({ # ERROR_BOT sends Django exceptions to an "errors" stream in the # system realm. 'ERROR_BOT': None, - # NEW_USER_BOT sends notifications about new user signups to a - # "signups" stream in the system realm. - 'NEW_USER_BOT': None, # These are extra bot users for our end-to-end Nagios message # sending tests. 'NAGIOS_STAGING_SEND_BOT': None,