welcome-emails: Separate followup_day1 email from other welcome emails.

The initial followup_day1 email confirms that the new user account
has been successfully created and should be sent to the user
independently of an organization's setting for send_welcome_emails.

Here we separate out the followup_day1 email into a separate function
from enqueue_welcome_emails and create a helper function for setting
the shared welcome email sender information.

The followup_day1 email is still a scheduled email so that the initial
account creation and log-in process for the user remains unchanged.

Fixes #25268.
This commit is contained in:
Lauryn Menard 2023-06-30 13:27:25 +02:00 committed by Tim Abbott
parent 0e1acd595b
commit 3dfdbbc775
5 changed files with 63 additions and 36 deletions

View File

@ -19,7 +19,7 @@ from zerver.actions.users import change_user_is_active, get_service_dicts_for_bo
from zerver.lib.avatar import avatar_url from zerver.lib.avatar import avatar_url
from zerver.lib.create_user import create_user from zerver.lib.create_user import create_user
from zerver.lib.default_streams import get_slim_realm_default_streams from zerver.lib.default_streams import get_slim_realm_default_streams
from zerver.lib.email_notifications import enqueue_welcome_emails from zerver.lib.email_notifications import enqueue_welcome_emails, send_account_registered_email
from zerver.lib.mention import silent_mention_syntax_for_user from zerver.lib.mention import silent_mention_syntax_for_user
from zerver.lib.send_email import clear_scheduled_invitation_emails from zerver.lib.send_email import clear_scheduled_invitation_emails
from zerver.lib.stream_subscription import bulk_get_subscriber_peer_info from zerver.lib.stream_subscription import bulk_get_subscriber_peer_info
@ -277,7 +277,11 @@ def process_new_human_user(
# from being sent after the user is created. # from being sent after the user is created.
clear_scheduled_invitation_emails(user_profile.delivery_email) clear_scheduled_invitation_emails(user_profile.delivery_email)
if realm.send_welcome_emails: if realm.send_welcome_emails:
enqueue_welcome_emails(user_profile, realm_creation) enqueue_welcome_emails(user_profile)
# Schedule an initial email with the user's
# new account details and log-in information.
send_account_registered_email(user_profile, realm_creation)
# We have an import loop here; it's intentional, because we want # We have an import loop here; it's intentional, because we want
# to keep all the onboarding code in zerver/lib/onboarding.py. # to keep all the onboarding code in zerver/lib/onboarding.py.

View File

@ -756,38 +756,37 @@ def get_org_type_zulip_guide(realm: Realm) -> Tuple[Any, str]:
return (None, "") return (None, "")
def enqueue_welcome_emails(user: UserProfile, realm_creation: bool = False) -> None: def welcome_sender_information() -> Tuple[Optional[str], str]:
from zerver.context_processors import common_context
if settings.WELCOME_EMAIL_SENDER is not None: if settings.WELCOME_EMAIL_SENDER is not None:
# line break to avoid triggering lint rule
from_name = settings.WELCOME_EMAIL_SENDER["name"] from_name = settings.WELCOME_EMAIL_SENDER["name"]
from_address = settings.WELCOME_EMAIL_SENDER["email"] from_address = settings.WELCOME_EMAIL_SENDER["email"]
else: else:
from_name = None from_name = None
from_address = FromAddress.support_placeholder from_address = FromAddress.support_placeholder
other_account_count = ( return (from_name, from_address)
UserProfile.objects.filter(delivery_email__iexact=user.delivery_email)
.exclude(id=user.id)
.count() def send_account_registered_email(user: UserProfile, realm_creation: bool = False) -> None:
) # Imported here to avoid import cycles.
unsubscribe_link = one_click_unsubscribe_link(user, "welcome") from zerver.context_processors import common_context
from_name, from_address = welcome_sender_information()
realm_url = user.realm.uri realm_url = user.realm.uri
followup_day1_context = common_context(user) account_registered_context = common_context(user)
followup_day1_context.update( account_registered_context.update(
realm_creation=realm_creation, realm_creation=realm_creation,
email=user.delivery_email, email=user.delivery_email,
is_realm_admin=user.is_realm_admin, is_realm_admin=user.is_realm_admin,
is_demo_org=user.realm.demo_organization_scheduled_deletion_date is not None, is_demo_org=user.realm.demo_organization_scheduled_deletion_date is not None,
) )
followup_day1_context["getting_organization_started_link"] = ( account_registered_context["getting_organization_started_link"] = (
realm_url + "/help/getting-your-organization-started-with-zulip" realm_url + "/help/getting-your-organization-started-with-zulip"
) )
followup_day1_context["getting_user_started_link"] = ( account_registered_context["getting_user_started_link"] = (
realm_url + "/help/getting-started-with-zulip" realm_url + "/help/getting-started-with-zulip"
) )
@ -795,13 +794,13 @@ def enqueue_welcome_emails(user: UserProfile, realm_creation: bool = False) -> N
from zproject.backends import ZulipLDAPAuthBackend, email_belongs_to_ldap from zproject.backends import ZulipLDAPAuthBackend, email_belongs_to_ldap
if email_belongs_to_ldap(user.realm, user.delivery_email): if email_belongs_to_ldap(user.realm, user.delivery_email):
followup_day1_context["ldap"] = True account_registered_context["ldap"] = True
for backend in get_backends(): for backend in get_backends():
# If the user is doing authentication via LDAP, Note that # If the user is doing authentication via LDAP, Note that
# we exclude ZulipLDAPUserPopulator here, since that # we exclude ZulipLDAPUserPopulator here, since that
# isn't used for authentication. # isn't used for authentication.
if isinstance(backend, ZulipLDAPAuthBackend): if isinstance(backend, ZulipLDAPAuthBackend):
followup_day1_context["ldap_username"] = backend.django_to_ldap_username( account_registered_context["ldap_username"] = backend.django_to_ldap_username(
user.delivery_email user.delivery_email
) )
break break
@ -812,9 +811,23 @@ def enqueue_welcome_emails(user: UserProfile, realm_creation: bool = False) -> N
to_user_ids=[user.id], to_user_ids=[user.id],
from_name=from_name, from_name=from_name,
from_address=from_address, from_address=from_address,
context=followup_day1_context, context=account_registered_context,
) )
def enqueue_welcome_emails(user: UserProfile) -> None:
# Imported here to avoid import cycles.
from zerver.context_processors import common_context
from_name, from_address = welcome_sender_information()
other_account_count = (
UserProfile.objects.filter(delivery_email__iexact=user.delivery_email)
.exclude(id=user.id)
.count()
)
unsubscribe_link = one_click_unsubscribe_link(user, "welcome")
realm_url = user.realm.uri
# Any emails scheduled below should be added to the logic in get_onboarding_email_schedule # Any emails scheduled below should be added to the logic in get_onboarding_email_schedule
# to determine how long to delay sending the email based on when the user signed up. # to determine how long to delay sending the email based on when the user signed up.
onboarding_email_schedule = get_onboarding_email_schedule(user) onboarding_email_schedule = get_onboarding_email_schedule(user)

View File

@ -30,6 +30,7 @@ from zerver.lib.email_notifications import (
handle_missedmessage_emails, handle_missedmessage_emails,
include_realm_name_in_missedmessage_emails_subject, include_realm_name_in_missedmessage_emails_subject,
relative_to_full_url, relative_to_full_url,
send_account_registered_email,
) )
from zerver.lib.send_email import FromAddress, deliver_scheduled_emails, send_custom_email from zerver.lib.send_email import FromAddress, deliver_scheduled_emails, send_custom_email
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
@ -224,7 +225,7 @@ class TestCustomEmails(ZulipTestCase):
class TestFollowupEmails(ZulipTestCase): class TestFollowupEmails(ZulipTestCase):
def test_day1_email_context(self) -> None: def test_day1_email_context(self) -> None:
hamlet = self.example_user("hamlet") hamlet = self.example_user("hamlet")
enqueue_welcome_emails(hamlet) send_account_registered_email(hamlet)
scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by( scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by(
"scheduled_timestamp" "scheduled_timestamp"
) )
@ -240,7 +241,7 @@ class TestFollowupEmails(ZulipTestCase):
ScheduledEmail.objects.all().delete() ScheduledEmail.objects.all().delete()
iago = self.example_user("iago") iago = self.example_user("iago")
enqueue_welcome_emails(iago) send_account_registered_email(iago)
scheduled_emails = ScheduledEmail.objects.filter(users=iago).order_by("scheduled_timestamp") scheduled_emails = ScheduledEmail.objects.filter(users=iago).order_by("scheduled_timestamp")
email_data = orjson.loads(scheduled_emails[0].data) email_data = orjson.loads(scheduled_emails[0].data)
self.assertEqual(email_data["context"]["email"], self.example_email("iago")) self.assertEqual(email_data["context"]["email"], self.example_email("iago"))
@ -344,6 +345,7 @@ class TestFollowupEmails(ZulipTestCase):
realm = get_realm("zulip") realm = get_realm("zulip")
# Hamlet has account only in Zulip realm so day1, day2 and zulip_guide emails should be sent # Hamlet has account only in Zulip realm so day1, day2 and zulip_guide emails should be sent
send_account_registered_email(self.example_user("hamlet"))
enqueue_welcome_emails(self.example_user("hamlet")) enqueue_welcome_emails(self.example_user("hamlet"))
scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by( scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by(
"scheduled_timestamp" "scheduled_timestamp"
@ -368,6 +370,7 @@ class TestFollowupEmails(ZulipTestCase):
realm.save() realm.save()
# Hamlet is not an admin so the `/for/communities/` zulip_guide should not be sent # Hamlet is not an admin so the `/for/communities/` zulip_guide should not be sent
send_account_registered_email(self.example_user("hamlet"))
enqueue_welcome_emails(self.example_user("hamlet")) enqueue_welcome_emails(self.example_user("hamlet"))
scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by( scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by(
"scheduled_timestamp" "scheduled_timestamp"
@ -383,6 +386,7 @@ class TestFollowupEmails(ZulipTestCase):
ScheduledEmail.objects.all().delete() ScheduledEmail.objects.all().delete()
# Iago is an admin so the `/for/communities/` zulip_guide should be sent # Iago is an admin so the `/for/communities/` zulip_guide should be sent
send_account_registered_email(self.example_user("iago"))
enqueue_welcome_emails(self.example_user("iago")) enqueue_welcome_emails(self.example_user("iago"))
scheduled_emails = ScheduledEmail.objects.filter(users=iago).order_by("scheduled_timestamp") scheduled_emails = ScheduledEmail.objects.filter(users=iago).order_by("scheduled_timestamp")
self.assert_length(scheduled_emails, 3) self.assert_length(scheduled_emails, 3)
@ -404,6 +408,7 @@ class TestFollowupEmails(ZulipTestCase):
realm.save() realm.save()
# Cordelia has account in more than 1 realm so day2 email should not be sent # Cordelia has account in more than 1 realm so day2 email should not be sent
send_account_registered_email(self.example_user("cordelia"))
enqueue_welcome_emails(self.example_user("cordelia")) enqueue_welcome_emails(self.example_user("cordelia"))
scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by( scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by(
"scheduled_timestamp" "scheduled_timestamp"
@ -428,6 +433,7 @@ class TestFollowupEmails(ZulipTestCase):
realm.save() realm.save()
# In this case, Cordelia should only be sent the day1 email # In this case, Cordelia should only be sent the day1 email
send_account_registered_email(self.example_user("cordelia"))
enqueue_welcome_emails(self.example_user("cordelia")) enqueue_welcome_emails(self.example_user("cordelia"))
scheduled_emails = ScheduledEmail.objects.filter(users=cordelia) scheduled_emails = ScheduledEmail.objects.filter(users=cordelia)
self.assert_length(scheduled_emails, 1) self.assert_length(scheduled_emails, 1)
@ -437,7 +443,8 @@ class TestFollowupEmails(ZulipTestCase):
def test_followup_emails_for_regular_realms(self) -> None: def test_followup_emails_for_regular_realms(self) -> None:
cordelia = self.example_user("cordelia") cordelia = self.example_user("cordelia")
enqueue_welcome_emails(self.example_user("cordelia"), realm_creation=True) send_account_registered_email(self.example_user("cordelia"), realm_creation=True)
enqueue_welcome_emails(self.example_user("cordelia"))
scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by( scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by(
"scheduled_timestamp" "scheduled_timestamp"
) )
@ -466,7 +473,8 @@ class TestFollowupEmails(ZulipTestCase):
days=30 days=30
) )
cordelia.realm.save() cordelia.realm.save()
enqueue_welcome_emails(self.example_user("cordelia"), realm_creation=True) send_account_registered_email(self.example_user("cordelia"), realm_creation=True)
enqueue_welcome_emails(self.example_user("cordelia"))
scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by( scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by(
"scheduled_timestamp" "scheduled_timestamp"
) )
@ -500,10 +508,7 @@ class TestFollowupEmails(ZulipTestCase):
enqueue_welcome_emails(self.example_user("cordelia")) enqueue_welcome_emails(self.example_user("cordelia"))
scheduled_emails = ScheduledEmail.objects.filter(users=cordelia) scheduled_emails = ScheduledEmail.objects.filter(users=cordelia)
self.assert_length(scheduled_emails, 1) self.assert_length(scheduled_emails, 0)
self.assertEqual(
orjson.loads(scheduled_emails[0].data)["template_prefix"], "zerver/emails/followup_day1"
)
self.assertEqual( self.assertEqual(
m.output, m.output,
[f"ERROR:root:Unknown organization type '{invalid_org_type_id}'"], [f"ERROR:root:Unknown organization type '{invalid_org_type_id}'"],
@ -2185,8 +2190,8 @@ class TestFollowupEmailDelay(ZulipTestCase):
) )
class TestCustomEmailSender(ZulipTestCase): class TestCustomWelcomeEmailSender(ZulipTestCase):
def test_custom_email_sender(self) -> None: def test_custom_welcome_email_sender(self) -> None:
name = "Nonreg Email" name = "Nonreg Email"
email = self.nonreg_email("test") email = self.nonreg_email("test")
with override_settings( with override_settings(
@ -2201,6 +2206,5 @@ class TestCustomEmailSender(ZulipTestCase):
"scheduled_timestamp" "scheduled_timestamp"
) )
email_data = orjson.loads(scheduled_emails[0].data) email_data = orjson.loads(scheduled_emails[0].data)
self.assertEqual(email_data["context"]["email"], self.example_email("hamlet"))
self.assertEqual(email_data["from_name"], name) self.assertEqual(email_data["from_name"], name)
self.assertEqual(email_data["from_address"], email) self.assertEqual(email_data["from_address"], email)

View File

@ -1134,9 +1134,9 @@ class EmailUnsubscribeTests(ZulipTestCase):
click even when logged out to stop receiving them. click even when logged out to stop receiving them.
""" """
user_profile = self.example_user("hamlet") user_profile = self.example_user("hamlet")
# Simulate a new user signing up, which enqueues 3 welcome e-mails. # Simulate scheduling welcome e-mails for a new user.
enqueue_welcome_emails(user_profile) enqueue_welcome_emails(user_profile)
self.assertEqual(3, ScheduledEmail.objects.filter(users=user_profile).count()) self.assertEqual(2, ScheduledEmail.objects.filter(users=user_profile).count())
# Simulate unsubscribing from the welcome e-mails. # Simulate unsubscribing from the welcome e-mails.
unsubscribe_link = one_click_unsubscribe_link(user_profile, "welcome") unsubscribe_link = one_click_unsubscribe_link(user_profile, "welcome")

View File

@ -13,7 +13,7 @@ from confirmation.models import Confirmation, confirmation_url
from zerver.actions.realm_settings import do_send_realm_reactivation_email from zerver.actions.realm_settings import do_send_realm_reactivation_email
from zerver.actions.user_settings import do_change_user_delivery_email from zerver.actions.user_settings import do_change_user_delivery_email
from zerver.actions.users import change_user_is_active from zerver.actions.users import change_user_is_active
from zerver.lib.email_notifications import enqueue_welcome_emails from zerver.lib.email_notifications import enqueue_welcome_emails, send_account_registered_email
from zerver.lib.request import REQ, has_request_variables from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_success from zerver.lib.response import json_success
from zerver.models import Realm, get_realm, get_realm_stream, get_user_by_delivery_email from zerver.models import Realm, get_realm, get_realm_stream, get_user_by_delivery_email
@ -137,11 +137,17 @@ def generate_all_emails(request: HttpRequest) -> HttpResponse:
# Reset the email value so we can run this again # Reset the email value so we can run this again
do_change_user_delivery_email(user_profile, registered_email) do_change_user_delivery_email(user_profile, registered_email)
# Follow up day1 day2 emails for normal user # Initial email with new account information for normal user.
send_account_registered_email(user_profile)
# Follow up day2 and onboarding zulip guide emails for normal user
enqueue_welcome_emails(user_profile) enqueue_welcome_emails(user_profile)
# Follow up day1 day2 emails for admin user # Initial email with new account information for admin user
enqueue_welcome_emails(get_user_by_delivery_email("iago@zulip.com", realm), realm_creation=True) send_account_registered_email(get_user_by_delivery_email("iago@zulip.com", realm))
# Follow up day2 and onboarding zulip guide emails for admin user
enqueue_welcome_emails(get_user_by_delivery_email("iago@zulip.com", realm))
# Realm reactivation email # Realm reactivation email
do_send_realm_reactivation_email(realm, acting_user=None) do_send_realm_reactivation_email(realm, acting_user=None)