create-user: Remove notifications sent to admin realm.

Removes the notification message that was sent if a stream named
"signups" exists in the `settings.SYSTEM_BOT_REALM`. This was a
undocumented feature that would send a notification message when
a new user registered with a Zulip organization that was hosted
by an admin realm like Zulip Cloud.

This removes two database queries when a new user is created: one
to get the system bot realm and the other to get the notification
bot in said realm.

Note that there are still notification messages sent when a new
organization is registered with the admin realm if the "signups"
stream exists.
This commit is contained in:
Lauryn Menard
2023-03-13 12:09:08 +01:00
committed by Tim Abbott
parent 9e4529c48b
commit 213d0f4990
4 changed files with 9 additions and 54 deletions

View File

@@ -23,7 +23,6 @@ from zerver.lib.email_notifications import enqueue_welcome_emails
from zerver.lib.mention import silent_mention_syntax_for_user
from zerver.lib.send_email import clear_scheduled_invitation_emails
from zerver.lib.stream_subscription import bulk_get_subscriber_peer_info
from zerver.lib.streams import get_signups_stream
from zerver.lib.user_counts import realm_user_count, realm_user_count_by_role
from zerver.lib.user_groups import get_system_user_group_for_user
from zerver.lib.users import (
@@ -46,7 +45,6 @@ from zerver.models import (
UserMessage,
UserProfile,
bot_owner_user_ids,
get_realm,
get_system_bot,
)
from zerver.tornado.django_api import send_event
@@ -115,26 +113,6 @@ def notify_new_user(user_profile: UserProfile) -> None:
send_message_to_signup_notification_stream(sender, user_profile.realm, message)
# We also send a notification to the Zulip administrative realm
admin_realm = get_realm(settings.SYSTEM_BOT_REALM)
admin_realm_sender = get_system_bot(sender_email, admin_realm.id)
try:
# Check whether the stream exists
signups_stream = get_signups_stream(admin_realm)
# We intentionally use the same strings as above to avoid translation burden.
with override_language(admin_realm.default_language):
message = _("{user} just signed up for Zulip. (total: {user_count})").format(
user=f"{user_profile.full_name} <`{user_profile.email}`>", user_count=user_count
)
internal_send_stream_message(
admin_realm_sender, signups_stream, user_profile.realm.display_subdomain, message
)
except Stream.DoesNotExist:
# If the signups stream hasn't been created in the admin
# realm, don't auto-create it to send to it; just do nothing.
pass
def add_new_user_history(user_profile: UserProfile, streams: Iterable[Stream]) -> None:
"""Give you the last ONBOARDING_TOTAL_MESSAGES messages on your public

View File

@@ -11,7 +11,6 @@ from corporate.lib.stripe import get_latest_seat_count
from zerver.actions.create_user import notify_new_user
from zerver.actions.user_settings import do_change_user_setting
from zerver.lib.initial_password import initial_password
from zerver.lib.streams import create_stream_if_needed
from zerver.lib.test_classes import ZulipTestCase
from zerver.models import Message, Realm, Recipient, Stream, UserProfile, get_realm
from zerver.signals import JUST_CREATED_THRESHOLD, get_device_browser, get_device_os
@@ -275,13 +274,11 @@ class TestNotifyNewUser(ZulipTestCase):
def test_notify_realm_of_new_user(self) -> None:
realm = get_realm("zulip")
new_user = self.example_user("cordelia")
admin_realm = get_realm("zulipinternal")
admin_realm_signups_stream, created = create_stream_if_needed(admin_realm, "signups")
message_count = self.get_message_count()
notify_new_user(new_user)
self.assertEqual(self.get_message_count(), message_count + 2)
message = self.get_second_to_last_message()
self.assertEqual(self.get_message_count(), message_count + 1)
message = self.get_last_message()
self.assertEqual(message.recipient.type, Recipient.STREAM)
actual_stream = Stream.objects.get(id=message.recipient.type_id)
self.assertEqual(actual_stream.name, Realm.INITIAL_PRIVATE_STREAM_NAME)
@@ -289,31 +286,12 @@ class TestNotifyNewUser(ZulipTestCase):
f"@_**Cordelia, Lear's daughter|{new_user.id}** just signed up for Zulip.",
message.content,
)
message = self.get_last_message()
self.assertEqual(message.recipient.type, Recipient.STREAM)
actual_stream = Stream.objects.get(id=message.recipient.type_id)
self.assertEqual(actual_stream.name, "signups")
self.assertIn(
f"Cordelia, Lear's daughter <`{new_user.email}`> just signed up for Zulip. (total:",
message.content,
)
admin_realm_signups_stream.delete()
notify_new_user(new_user)
self.assertEqual(self.get_message_count(), message_count + 3)
message = self.get_last_message()
self.assertEqual(message.recipient.type, Recipient.STREAM)
actual_stream = Stream.objects.get(id=message.recipient.type_id)
self.assertEqual(actual_stream.name, Realm.INITIAL_PRIVATE_STREAM_NAME)
self.assertIn(
f"@_**Cordelia, Lear's daughter|{new_user.id}** just signed up for Zulip.",
message.content,
)
realm.signup_notifications_stream = None
realm.save(update_fields=["signup_notifications_stream"])
new_user.refresh_from_db()
notify_new_user(new_user)
self.assertEqual(self.get_message_count(), message_count + 3)
self.assertEqual(self.get_message_count(), message_count + 1)
def test_notify_realm_of_new_user_in_manual_license_management(self) -> None:
realm = get_realm("zulip")

View File

@@ -958,7 +958,7 @@ class LoginTest(ZulipTestCase):
ContentType.objects.clear_cache()
# Ensure the number of queries we make is not O(streams)
with self.assert_database_query_count(95), cache_tries_captured() as cache_tries:
with self.assert_database_query_count(93), cache_tries_captured() as cache_tries:
with self.captureOnCommitCallbacks(execute=True):
self.register(self.nonreg_email("test"), "test")
@@ -966,7 +966,7 @@ class LoginTest(ZulipTestCase):
# seem to be any O(N) behavior. Some of the cache hits are related
# to sending messages, such as getting the welcome bot, looking up
# the alert words for a realm, etc.
self.assert_length(cache_tries, 22)
self.assert_length(cache_tries, 20)
user_profile = self.nonreg_user("test")
self.assert_logged_in_user_id(user_profile.id)
@@ -3451,10 +3451,9 @@ class RealmCreationTest(ZulipTestCase):
# Check signup messages
recipient = signups_stream.recipient
messages = Message.objects.filter(recipient=recipient).order_by("id")
self.assert_length(messages, 2)
self.assert_length(messages, 1)
self.assertIn("Signups enabled", messages[0].content)
self.assertIn("signed up", messages[1].content)
self.assertEqual("zuliptest", messages[1].topic_name())
self.assertEqual("zuliptest", messages[0].topic_name())
realm_creation_audit_log = RealmAuditLog.objects.get(
realm=realm, event_type=RealmAuditLog.REALM_CREATED

View File

@@ -790,7 +790,7 @@ class QueryCountTest(ZulipTestCase):
events: List[Mapping[str, Any]] = []
with self.assert_database_query_count(90):
with self.assert_database_query_count(88):
with cache_tries_captured() as cache_tries:
with self.tornado_redirected_to_list(events, expected_num_events=11):
fred = do_create_user(
@@ -802,7 +802,7 @@ class QueryCountTest(ZulipTestCase):
acting_user=None,
)
self.assert_length(cache_tries, 28)
self.assert_length(cache_tries, 26)
peer_add_events = [event for event in events if event["event"].get("op") == "peer_add"]
notifications = set()