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.
This commit is contained in:
Tim Abbott
2018-03-29 11:32:25 -07:00
parent 2bc51931a8
commit 656f882a44
7 changed files with 11 additions and 20 deletions

View File

@@ -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: def notify_new_user(user_profile: UserProfile, internal: bool=False) -> None:
if settings.NEW_USER_BOT is not None: if settings.NOTIFICATION_BOT is not None:
send_signup_message(settings.NEW_USER_BOT, "signups", user_profile, internal) send_signup_message(settings.NOTIFICATION_BOT, "signups", user_profile, internal)
statsd.gauge("users.signups.%s" % (user_profile.realm.string_id), 1, delta=True) statsd.gauge("users.signups.%s" % (user_profile.realm.string_id), 1, delta=True)
# We also clear any scheduled invitation emails to prevent them # 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: elif sender.email == settings.WELCOME_BOT:
# The welcome bot welcomes folks to the stream. # The welcome bot welcomes folks to the stream.
pass pass
elif sender.email == settings.NEW_USER_BOT: elif sender.email == settings.NOTIFICATION_BOT:
pass pass
else: else:
# All other cases are an error. # 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}) "org_type": org_type})
# Send a notification to the admin realm (if configured) # 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" signup_message = "Signups enabled"
admin_realm = get_system_bot(settings.NEW_USER_BOT).realm admin_realm = get_system_bot(settings.NOTIFICATION_BOT).realm
internal_send_message(admin_realm, settings.NEW_USER_BOT, "stream", internal_send_message(admin_realm, settings.NOTIFICATION_BOT, "stream",
"signups", realm.display_subdomain, signup_message) "signups", realm.display_subdomain, signup_message)
return realm return realm

View File

@@ -95,7 +95,6 @@ class HomeTest(ZulipTestCase):
"narrow_stream", "narrow_stream",
"needs_tutorial", "needs_tutorial",
"never_subscribed", "never_subscribed",
"new_user_bot_configured",
"night_mode", "night_mode",
"password_min_guesses", "password_min_guesses",
"password_min_length", "password_min_length",
@@ -190,7 +189,7 @@ class HomeTest(ZulipTestCase):
with patch('zerver.lib.cache.cache_set') as cache_mock: with patch('zerver.lib.cache.cache_set') as cache_mock:
result = self._get_home_page(stream='Denmark') 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) self.assert_length(cache_mock.call_args_list, 8)
html = result.content.decode('utf-8') html = result.content.decode('utf-8')
@@ -234,7 +233,7 @@ class HomeTest(ZulipTestCase):
result = self._get_home_page() result = self._get_home_page()
self.assertEqual(result.status_code, 200) self.assertEqual(result.status_code, 200)
self.assert_length(cache_mock.call_args_list, 17) 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.") @slow("Creates and subscribes 10 users in a loop. Should use bulk queries.")
def test_num_queries_with_streams(self) -> None: def test_num_queries_with_streams(self) -> None:

View File

@@ -222,9 +222,6 @@ class RealmTest(ZulipTestCase):
new_signup_notifications_stream_id = 4 new_signup_notifications_stream_id = 4
req = dict(signup_notifications_stream_id = ujson.dumps(new_signup_notifications_stream_id)) 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) result = self.client_patch('/json/realm', req)
self.assert_json_success(result) self.assert_json_success(result)

View File

@@ -126,7 +126,9 @@ def home_real(request: HttpRequest) -> HttpResponse:
# Reset our don't-spam-users-with-email counter since the # Reset our don't-spam-users-with-email counter since the
# user has since logged in # 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.last_reminder = None
user_profile.save(update_fields=["last_reminder"]) 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'. # These end up in a global JavaScript Object named 'page_params'.
page_params = dict( page_params = dict(
# Server settings. # Server settings.
new_user_bot_configured = settings.NEW_USER_BOT is not None,
development_environment = settings.DEVELOPMENT, development_environment = settings.DEVELOPMENT,
debug_mode = settings.DEBUG, debug_mode = settings.DEBUG,
test_suite = settings.TEST_SUITE, test_suite = settings.TEST_SUITE,

View File

@@ -66,8 +66,6 @@ def update_realm(
return json_error(_("Organization name is too long.")) return json_error(_("Organization name is too long."))
if authentication_methods is not None and True not in list(authentication_methods.values()): 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.")) 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 # 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: if bot_creation_policy is not None and bot_creation_policy not in Realm.BOT_CREATION_POLICY_TYPES:

View File

@@ -46,7 +46,6 @@ EXTERNAL_URI_SCHEME = "http://"
EMAIL_GATEWAY_PATTERN = "%s@" + EXTERNAL_HOST EMAIL_GATEWAY_PATTERN = "%s@" + EXTERNAL_HOST
NOTIFICATION_BOT = "notification-bot@zulip.com" NOTIFICATION_BOT = "notification-bot@zulip.com"
ERROR_BOT = "error-bot@zulip.com" ERROR_BOT = "error-bot@zulip.com"
NEW_USER_BOT = "new-user-bot@zulip.com"
EMAIL_GATEWAY_BOT = "emailgateway@zulip.com" EMAIL_GATEWAY_BOT = "emailgateway@zulip.com"
PHYSICAL_ADDRESS = "Zulip Headquarters, 123 Octo Stream, South Pacific Ocean" PHYSICAL_ADDRESS = "Zulip Headquarters, 123 Octo Stream, South Pacific Ocean"
EXTRA_INSTALLED_APPS = ["zilencer", "analytics"] EXTRA_INSTALLED_APPS = ["zilencer", "analytics"]

View File

@@ -226,9 +226,6 @@ DEFAULT_SETTINGS.update({
# ERROR_BOT sends Django exceptions to an "errors" stream in the # ERROR_BOT sends Django exceptions to an "errors" stream in the
# system realm. # system realm.
'ERROR_BOT': None, '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 # These are extra bot users for our end-to-end Nagios message
# sending tests. # sending tests.
'NAGIOS_STAGING_SEND_BOT': None, 'NAGIOS_STAGING_SEND_BOT': None,