Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

send_email: Add support for multiple recipients. 

This adds a function that sends provided email to all administrators
of a realm, but in a single email. As a result, send_email now takes
arguments to_user_ids and to_emails instead of to_user_id and
to_email.

We adjust other APIs to match, but note that send_future_email does
not yet support the multiple recipients model for good reasons.

Tweaked by tabbott to modify `manage.py deliver_email` to handle
backwards-compatibily for any ScheduledEmail objects already in the
database.

Fixes #10896.
This commit is contained in:
Raymond Akornor 2018-12-03 22:26:51 +00:00 committed by Tim Abbott
parent 0fddf9a610
commit 92dc3637df
16 changed files with 76 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
zerver/forms.py
View File

@ -250,7 +250,7 @@ class ZulipPasswordResetForm(PasswordResetForm):
if user is not None:
context['active_account_in_realm'] = True
context['reset_url'] = generate_password_reset_url(user, token_generator)
send_email('zerver/emails/password_reset', to_user_id=user.id,
send_email('zerver/emails/password_reset', to_user_ids=[user.id],
from_name="Zulip Account Security",
from_address=FromAddress.tokenized_no_reply_address(),
context=context)
@ -259,7 +259,7 @@ class ZulipPasswordResetForm(PasswordResetForm):
active_accounts_in_other_realms = UserProfile.objects.filter(email__iexact=email, is_active=True)
if active_accounts_in_other_realms:
context['active_accounts_in_other_realms'] = active_accounts_in_other_realms
send_email('zerver/emails/password_reset', to_email=email,
send_email('zerver/emails/password_reset', to_emails=[email],
from_name="Zulip Account Security",
from_address=FromAddress.tokenized_no_reply_address(),
context=context)

16
zerver/lib/actions.py
View File

@ -43,7 +43,7 @@ from zerver.lib.message import (
)
from zerver.lib.realm_icon import realm_icon_url
from zerver.lib.retention import move_messages_to_archive
from zerver.lib.send_email import send_email, FromAddress
from zerver.lib.send_email import send_email, FromAddress, send_email_to_admins
from zerver.lib.stream_subscription import (
get_active_subscriptions_for_stream_id,
get_active_subscriptions_for_stream_ids,
@ -795,7 +795,7 @@ def do_start_email_change_process(user_profile: UserProfile, new_email: str) ->
'new_email': new_email,
'activate_url': activation_url
})
send_email('zerver/emails/confirm_new_email', to_email=new_email,
send_email('zerver/emails/confirm_new_email', to_emails=[new_email],
from_name='Zulip Account Security', from_address=FromAddress.tokenized_no_reply_address(),
context=context)
@ -4429,7 +4429,7 @@ def do_send_confirmation_email(invitee: PreregistrationUser,
context = {'referrer_full_name': referrer.full_name, 'referrer_email': referrer.email,
'activate_url': activation_url, 'referrer_realm_name': referrer.realm.name}
from_name = "%s (via Zulip)" % (referrer.full_name,)
send_email('zerver/emails/invitation', to_email=invitee.email, from_name=from_name,
send_email('zerver/emails/invitation', to_emails=[invitee.email], from_name=from_name,
from_address=FromAddress.tokenized_no_reply_address(), context=context)
def email_not_system_bot(email: str) -> None:
@ -5185,12 +5185,10 @@ def missing_any_realm_internal_bots() -> bool:
def do_send_realm_reactivation_email(realm: Realm) -> None:
confirmation_url = create_confirmation_link(realm, realm.host, Confirmation.REALM_REACTIVATION)
admins = realm.get_admin_users()
context = {'confirmation_url': confirmation_url,
'realm_uri': realm.uri,
'realm_name': realm.name}
for admin in admins:
send_email(
'zerver/emails/realm_reactivation', to_email=admin.email,
from_address=FromAddress.tokenized_no_reply_address(),
from_name="Zulip Account Security", context=context)
send_email_to_admins(
'zerver/emails/realm_reactivation', realm,
from_address=FromAddress.tokenized_no_reply_address(),
from_name="Zulip Account Security", context=context)

2
zerver/lib/digest.py
View File

@ -262,6 +262,6 @@ def handle_digest_email(user_profile_id: int, cutoff: float) -> None:
new_streams_count, new_users_count):
logger.info("Sending digest email for %s" % (user_profile.email,))
# Send now, as a ScheduledEmail
send_future_email('zerver/emails/digest', user_profile.realm, to_user_id=user_profile.id,
send_future_email('zerver/emails/digest', user_profile.realm, to_user_ids=[user_profile.id],
from_name="Zulip Digest", from_address=FromAddress.NOREPLY,
context=context)

6
zerver/lib/notifications.py
View File

@ -382,7 +382,7 @@ def do_send_missedmessage_events_reply_in_zulip(user_profile: UserProfile,
email_dict = {
'template_prefix': 'zerver/emails/missed_message',
'to_user_id': user_profile.id,
'to_user_ids': [user_profile.id],
'from_name': from_name,
'from_address': from_address,
'reply_to_email': formataddr((reply_to_name, reply_to_address)),
@ -534,12 +534,12 @@ def enqueue_welcome_emails(user: UserProfile, realm_creation: bool=False) -> Non
context["ldap_username"] = user.email
send_future_email(
"zerver/emails/followup_day1", user.realm, to_user_id=user.id, from_name=from_name,
"zerver/emails/followup_day1", user.realm, to_user_ids=[user.id], from_name=from_name,
from_address=from_address, context=context)
if other_account_count == 0:
send_future_email(
"zerver/emails/followup_day2", user.realm, to_user_id=user.id, from_name=from_name,
"zerver/emails/followup_day2", user.realm, to_user_ids=[user.id], from_name=from_name,
from_address=from_address, context=context, delay=followup_day2_email_delay(user))
def convert_html_to_markdown(html: str) -> str:

60
zerver/lib/send_email.py
View File

@ -33,17 +33,17 @@ class FromAddress:
return parseaddr(settings.TOKENIZED_NOREPLY_EMAIL_ADDRESS)[1].format(token=generate_key())
return FromAddress.NOREPLY
def build_email(template_prefix: str, to_user_id: Optional[int]=None,
to_email: Optional[str]=None, from_name: Optional[str]=None,
def build_email(template_prefix: str, to_user_ids: Optional[List[int]]=None,
to_emails: Optional[List[str]]=None, from_name: Optional[str]=None,
from_address: Optional[str]=None, reply_to_email: Optional[str]=None,
context: Optional[Dict[str, Any]]=None) -> EmailMultiAlternatives:
# Callers should pass exactly one of to_user_id and to_email.
assert (to_user_id is None) ^ (to_email is None)
if to_user_id is not None:
to_user = get_user_profile_by_id(to_user_id)
assert (to_user_ids is None) ^ (to_emails is None)
if to_user_ids is not None:
to_users = [get_user_profile_by_id(to_user_id) for to_user_id in to_user_ids]
# Change to formataddr((to_user.full_name, to_user.email)) once
# https://github.com/zulip/zulip/issues/4676 is resolved
to_email = to_user.delivery_email
to_emails = [to_user.delivery_email for to_user in to_users]
if context is None:
context = {}
@ -81,7 +81,7 @@ def build_email(template_prefix: str, to_user_id: Optional[int]=None,
elif from_address == FromAddress.NOREPLY:
reply_to = [FromAddress.NOREPLY]
mail = EmailMultiAlternatives(subject, message, from_email, [to_email], reply_to=reply_to)
mail = EmailMultiAlternatives(subject, message, from_email, to_emails, reply_to=reply_to)
if html_message is not None:
mail.attach_alternative(html_message, 'text/html')
return mail
@ -91,10 +91,11 @@ class EmailNotDeliveredException(Exception):
# When changing the arguments to this function, you may need to write a
# migration to change or remove any emails in ScheduledEmail.
def send_email(template_prefix: str, to_user_id: Optional[int]=None, to_email: Optional[str]=None,
from_name: Optional[str]=None, from_address: Optional[str]=None,
reply_to_email: Optional[str]=None, context: Dict[str, Any]={}) -> None:
mail = build_email(template_prefix, to_user_id=to_user_id, to_email=to_email, from_name=from_name,
def send_email(template_prefix: str, to_user_ids: Optional[List[int]]=None,
to_emails: Optional[List[str]]=None, from_name: Optional[str]=None,
from_address: Optional[str]=None, reply_to_email: Optional[str]=None,
context: Dict[str, Any]={}) -> None:
mail = build_email(template_prefix, to_user_ids=to_user_ids, to_emails=to_emails, from_name=from_name,
from_address=from_address, reply_to_email=reply_to_email, context=context)
template = template_prefix.split("/")[-1]
logger.info("Sending %s email to %s" % (template, mail.to))
@ -106,27 +107,39 @@ def send_email(template_prefix: str, to_user_id: Optional[int]=None, to_email: O
def send_email_from_dict(email_dict: Mapping[str, Any]) -> None:
send_email(**dict(email_dict))
def send_future_email(template_prefix: str, realm: Realm, to_user_id: Optional[int]=None,
to_email: Optional[str]=None, from_name: Optional[str]=None,
def send_future_email(template_prefix: str, realm: Realm, to_user_ids: Optional[List[int]]=None,
to_emails: Optional[List[str]]=None, from_name: Optional[str]=None,
from_address: Optional[str]=None, context: Dict[str, Any]={},
delay: datetime.timedelta=datetime.timedelta(0)) -> None:
# WARNING: Be careful when using this with multiple recipients;
# because the current ScheduledEmail model (used primarily for
# cancelling planned emails) does not support multiple recipients,
# this is only valid to use for such emails where we don't expect
# the cancellation feature to be relevant.
#
# For that reason, we currently assert that the list of
# to_user_ids/to_emails is 1 below, but in theory that could be
# changed as long as the callers are in a situation where the
# above problem is not relevant.
template_name = template_prefix.split('/')[-1]
email_fields = {'template_prefix': template_prefix, 'to_user_id': to_user_id, 'to_email': to_email,
email_fields = {'template_prefix': template_prefix, 'to_user_ids': to_user_ids, 'to_emails': to_emails,
'from_name': from_name, 'from_address': from_address, 'context': context}
if settings.DEVELOPMENT and not settings.TEST_SUITE:
send_email(template_prefix, to_user_id=to_user_id, to_email=to_email, from_name=from_name,
send_email(template_prefix, to_user_ids=to_user_ids, to_emails=to_emails, from_name=from_name,
from_address=from_address, context=context)
# For logging the email
assert (to_user_id is None) ^ (to_email is None)
if to_user_id is not None:
assert (to_user_ids is None) ^ (to_emails is None)
if to_user_ids is not None:
# The realm is redundant if we have a to_user_id; this assert just
# expresses that fact
assert(UserProfile.objects.filter(id=to_user_id, realm=realm).exists())
to_field = {'user_id': to_user_id} # type: Dict[str, Any]
assert(len(to_user_ids) == 1)
assert(UserProfile.objects.filter(id__in=to_user_ids, realm=realm).exists())
to_field = {'user_id': to_user_ids[0]} # type: Dict[str, Any]
else:
to_field = {'address': parseaddr(to_email)[1]}
assert(len(to_emails) == 1)
to_field = {'address': parseaddr(to_emails[0])[1]}
ScheduledEmail.objects.create(
type=EMAIL_TYPES[template_name],
@ -134,3 +147,10 @@ def send_future_email(template_prefix: str, realm: Realm, to_user_id: Optional[i
realm=realm,
data=ujson.dumps(email_fields),
**to_field)
def send_email_to_admins(template_prefix: str, realm: Realm, from_name: Optional[str]=None,
from_address: Optional[str]=None, context: Dict[str, Any]={}) -> None:
admins = realm.get_admin_users()
admin_user_ids = [admin.id for admin in admins]
send_email(template_prefix, to_user_ids=admin_user_ids, from_name=from_name,
from_address=from_address, context=context)

8
zerver/management/commands/deliver_email.py
View File

@ -48,6 +48,14 @@ Usage: ./manage.py deliver_email
scheduled_timestamp__lte=timezone_now())
if email_jobs_to_deliver:
for job in email_jobs_to_deliver:
# Reformat any jobs that used the old to_email
# and to_user_ids argument formats.
if 'to_email' in job:
job['to_emails'] = [job['to_email']]
del job['to_email']
if 'to_user_ids' in job:
job['to_user_ids'] = [job['to_user_id']]
del job['to_user_ids']
try:
send_email(**loads(job.data))
job.delete()

2
zerver/management/commands/send_password_reset_email.py
View File

@ -47,6 +47,6 @@ class Command(ZulipBaseCommand):
'realm_uri': user_profile.realm.uri,
'active_account_in_realm': True,
}
send_email('zerver/emails/password_reset', to_user_id=user_profile.id,
send_email('zerver/emails/password_reset', to_user_ids=[user_profile.id],
from_address=FromAddress.tokenized_no_reply_address(),
from_name="Zulip Account Security", context=context)

2
zerver/signals.py
View File

@ -96,7 +96,7 @@ def email_on_new_login(sender: Any, user: UserProfile, request: Any, **kwargs: A
email_dict = {
'template_prefix': 'zerver/emails/notify_new_login',
'to_user_id': user.id,
'to_user_ids': [user.id],
'from_name': 'Zulip Account Security',
'from_address': FromAddress.NOREPLY,
'context': context}

6
zerver/tests/test_digest.py
View File

@ -40,7 +40,7 @@ class TestDigestEmailMessages(ZulipTestCase):
self.assertEqual(mock_send_future_email.call_count, 1)
kwargs = mock_send_future_email.call_args[1]
self.assertEqual(kwargs['to_user_id'], user_profile.id)
self.assertEqual(kwargs['to_user_ids'], [user_profile.id])
html = kwargs['context']['unread_pms'][0]['header']['html']
expected_url = "'http://zulip.testserver/#narrow/pm-with/{id}-hamlet'".format(id=hamlet.id)
self.assertIn(expected_url, html)
@ -74,7 +74,7 @@ class TestDigestEmailMessages(ZulipTestCase):
self.assertEqual(mock_send_future_email.call_count, 1)
kwargs = mock_send_future_email.call_args[1]
self.assertEqual(kwargs['to_user_id'], user_profile.id)
self.assertEqual(kwargs['to_user_ids'], [user_profile.id])
html = kwargs['context']['unread_pms'][0]['header']['html']
other_user_ids = sorted([
@ -125,7 +125,7 @@ class TestDigestEmailMessages(ZulipTestCase):
self.assertEqual(mock_send_future_email.call_count, 1)
kwargs = mock_send_future_email.call_args[1]
self.assertEqual(kwargs['to_user_id'], othello.id)
self.assertEqual(kwargs['to_user_ids'], [othello.id])
hot_convo = kwargs['context']['hot_conversations'][0]

2
zerver/tests/test_queue_worker.py
View File

@ -248,7 +248,7 @@ class WorkerTest(ZulipTestCase):
data = {
'template_prefix': 'zerver/emails/confirm_new_email',
'to_email': self.example_email("hamlet"),
'to_emails': [self.example_email("hamlet")],
'from_name': 'Zulip Account Security',
'from_address': FromAddress.NOREPLY,
'context': {}

2
zerver/tests/test_realm.py
View File

@ -182,7 +182,7 @@ class RealmTest(ZulipTestCase):
def test_do_deactivate_realm_clears_scheduled_jobs(self) -> None:
user = self.example_user('hamlet')
send_future_email('zerver/emails/followup_day1', user.realm,
to_user_id=user.id, delay=datetime.timedelta(hours=1))
to_user_ids=[user.id], delay=datetime.timedelta(hours=1))
self.assertEqual(ScheduledEmail.objects.count(), 1)
do_deactivate_realm(user.realm)
self.assertEqual(ScheduledEmail.objects.count(), 0)

5
zerver/tests/test_signup.py
View File

@ -1159,8 +1159,9 @@ so we didn't send them an invitation. We did send invitations to everyone else!"
'referrer_realm_name': referrer.realm.name,
})
with self.settings(EMAIL_BACKEND='django.core.mail.backends.console.EmailBackend'):
email = data["email"]
send_future_email(
"zerver/emails/invitation_reminder", referrer.realm, to_email=data["email"],
"zerver/emails/invitation_reminder", referrer.realm, to_emails=[email],
from_address=FromAddress.NOREPLY, context=context)
email_jobs_to_deliver = ScheduledEmail.objects.filter(
scheduled_timestamp__lte=timezone_now())
@ -1553,7 +1554,7 @@ class EmailUnsubscribeTests(ZulipTestCase):
context = {'name': '', 'realm_uri': '', 'unread_pms': [], 'hot_conversations': [],
'new_users': [], 'new_streams': {'plain': []}, 'unsubscribe_link': ''}
send_future_email('zerver/emails/digest', user_profile.realm,
to_user_id=user_profile.id, context=context)
to_user_ids=[user_profile.id], context=context)
self.assertEqual(1, ScheduledEmail.objects.filter(user=user_profile).count())

2
zerver/tests/test_users.py
View File

@ -810,7 +810,7 @@ class ActivateTest(ZulipTestCase):
def test_clear_scheduled_jobs(self) -> None:
user = self.example_user('hamlet')
send_future_email('zerver/emails/followup_day1', user.realm,
to_user_id=user.id, delay=datetime.timedelta(hours=1))
to_user_ids=[user.id], delay=datetime.timedelta(hours=1))
self.assertEqual(ScheduledEmail.objects.count(), 1)
do_deactivate_user(user)
self.assertEqual(ScheduledEmail.objects.count(), 0)

4
zerver/views/registration.py
View File

@ -360,7 +360,7 @@ def prepare_activation_url(email: str, request: HttpRequest,
return activation_url
def send_confirm_registration_email(email: str, activation_url: str) -> None:
send_email('zerver/emails/confirm_registration', to_email=email,
send_email('zerver/emails/confirm_registration', to_emails=[email],
from_address=FromAddress.tokenized_no_reply_address(),
context={'activate_url': activation_url})
@ -490,7 +490,7 @@ def find_account(request: HttpRequest) -> HttpResponse:
'realm_uri': user_profile.realm.uri,
'realm_name': user_profile.realm.name,
}
send_email('zerver/emails/find_team', to_user_id=user_profile.id, context=ctx)
send_email('zerver/emails/find_team', to_user_ids=[user_profile.id], context=ctx)
# Note: Show all the emails in the result otherwise this
# feature can be used to ascertain which email addresses

2
zerver/views/user_settings.py
View File

@ -42,7 +42,7 @@ def confirm_email_change(request: HttpRequest, confirmation_key: str) -> HttpRes
do_change_user_email(user_profile, new_email)
context = {'realm_name': user_profile.realm.name, 'new_email': new_email}
send_email('zerver/emails/notify_change_in_email', to_email=old_email,
send_email('zerver/emails/notify_change_in_email', to_emails=[old_email],
from_name="Zulip Account Security", from_address=FromAddress.SUPPORT,
context=context)

2
zerver/worker/queue_processors.py
View File

@ -232,7 +232,7 @@ class ConfirmationEmailWorker(QueueProcessingWorker):
send_future_email(
"zerver/emails/invitation_reminder",
referrer.realm,
to_email=invitee.email,
to_emails=[invitee.email],
from_address=FromAddress.tokenized_no_reply_address(),
context=context,
delay=datetime.timedelta(days=2))