diff --git a/docs/email.md b/docs/email.md index 18aa17d8b1..eff9fd1a2f 100644 --- a/docs/email.md +++ b/docs/email.md @@ -22,7 +22,7 @@ with only a few things you need to know to get started. * All email is eventually sent by `zerver.lib.send_email.send_email`. There are several other functions in `zerver.lib.send_email`, but all of them eventually call the `send_email` function. The most interesting one is - `send_future_email`. The `ScheduledJob` entries are eventually processed + `send_future_email`. The `ScheduledEmail` entries are eventually processed by a cron job that runs `zerver/management/commands/deliver_email.py`. * A good way to find a bunch of example email pathways is to `git grep` for `zerver/emails` in the `zerver/` directory. diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 1f46f15642..537e4457e1 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -43,7 +43,7 @@ from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity, get_user_profile_by_email, get_user, get_stream_cache_key, \ UserActivityInterval, get_active_user_dicts_in_realm, get_active_streams, \ realm_filters_for_realm, RealmFilter, receives_offline_notifications, \ - ScheduledJob, get_owned_bot_dicts, \ + get_owned_bot_dicts, \ get_old_unclaimed_attachments, get_cross_realm_emails, \ Reaction, EmailChangeStatus, CustomProfileField, \ custom_profile_fields_for_realm, \ diff --git a/zerver/lib/digest.py b/zerver/lib/digest.py index 4adbeeac23..55f077d48d 100644 --- a/zerver/lib/digest.py +++ b/zerver/lib/digest.py @@ -209,7 +209,7 @@ def handle_digest_email(user_profile_id, cutoff): if enough_traffic(context["unread_pms"], context["hot_conversations"], new_streams_count, new_users_count): logger.info("Sending digest email for %s" % (user_profile.email,)) - # Send now, as a ScheduledJob + # Send now, as a ScheduledEmail send_future_email('zerver/emails/digest', to_user_id=user_profile.id, from_name="Zulip Digest", from_address=FromAddress.NOREPLY, context=context) diff --git a/zerver/lib/export.py b/zerver/lib/export.py index d86fcd51a4..007f10b094 100644 --- a/zerver/lib/export.py +++ b/zerver/lib/export.py @@ -77,7 +77,7 @@ ALL_ZERVER_TABLES = [ 'zerver_realmemoji', 'zerver_realmfilter', 'zerver_recipient', - 'zerver_scheduledjob', + 'zerver_scheduledemail', 'zerver_stream', 'zerver_subscription', 'zerver_useractivity', @@ -96,7 +96,7 @@ NON_EXPORTED_TABLES = [ 'zerver_preregistrationuser', 'zerver_preregistrationuser_streams', 'zerver_pushdevicetoken', - 'zerver_scheduledjob', + 'zerver_scheduledemail', 'zerver_userprofile_groups', 'zerver_userprofile_user_permissions', ] diff --git a/zerver/lib/notifications.py b/zerver/lib/notifications.py index f898460a31..f1dbaf3ad4 100644 --- a/zerver/lib/notifications.py +++ b/zerver/lib/notifications.py @@ -12,7 +12,7 @@ from zerver.lib.send_email import send_future_email, \ from zerver.lib.queue import queue_json_publish from zerver.models import ( Recipient, - ScheduledJob, + ScheduledEmail, UserMessage, Stream, get_display_recipient, @@ -398,7 +398,7 @@ def clear_followup_emails_queue(email): """ Clear out queued emails that would otherwise be sent to a specific email address. """ - items = ScheduledJob.objects.filter(type=ScheduledJob.EMAIL, filter_string__iexact = email) + items = ScheduledEmail.objects.filter(address__iexact=email) items.delete() def log_digest_event(msg): diff --git a/zerver/lib/send_email.py b/zerver/lib/send_email.py index c67ce86c73..5c30d6c676 100644 --- a/zerver/lib/send_email.py +++ b/zerver/lib/send_email.py @@ -2,7 +2,8 @@ from django.conf import settings from django.core.mail import EmailMultiAlternatives from django.template import loader from django.utils.timezone import now as timezone_now -from zerver.models import UserProfile, ScheduledJob, get_user_profile_by_id +from zerver.models import UserProfile, ScheduledEmail, get_user_profile_by_id, \ + EMAIL_TYPES import datetime from email.utils import parseaddr, formataddr @@ -59,6 +60,8 @@ def build_email(template_prefix, to_user_id=None, to_email=None, from_name=None, class EmailNotDeliveredException(Exception): pass +# 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, to_user_id=None, to_email=None, from_name=None, from_address=None, reply_to_email=None, context={}): # type: (str, Optional[int], Optional[Text], Optional[Text], Optional[Text], Optional[Text], Dict[str, Any]) -> None @@ -82,6 +85,9 @@ def send_future_email(template_prefix, to_user_id=None, to_email=None, from_name to_user_id = None email_fields = {'template_prefix': template_prefix, 'to_user_id': to_user_id, 'to_email': to_email, 'from_name': from_name, 'from_address': from_address, 'context': context} - ScheduledJob.objects.create(type=ScheduledJob.EMAIL, filter_string=parseaddr(to_email)[1], - data=ujson.dumps(email_fields), - scheduled_timestamp=timezone_now() + delay) + template_name = template_prefix.split('/')[-1] + ScheduledEmail.objects.create( + address=parseaddr(to_email)[1], + type=EMAIL_TYPES[template_name], + scheduled_timestamp=timezone_now() + delay, + data=ujson.dumps(email_fields)) diff --git a/zerver/management/commands/deliver_email.py b/zerver/management/commands/deliver_email.py index de6ddc554c..a26b40e05b 100755 --- a/zerver/management/commands/deliver_email.py +++ b/zerver/management/commands/deliver_email.py @@ -15,7 +15,7 @@ from django.core.management.base import BaseCommand from django.utils.timezone import now as timezone_now from django.utils.html import format_html -from zerver.models import ScheduledJob +from zerver.models import ScheduledEmail from zerver.lib.context_managers import lockfile from zerver.lib.send_email import send_email, EmailNotDeliveredException @@ -58,8 +58,7 @@ Usage: ./manage.py deliver_email with lockfile("/tmp/zulip_email_deliver.lockfile"): while True: - email_jobs_to_deliver = ScheduledJob.objects.filter(type=ScheduledJob.EMAIL, - scheduled_timestamp__lte=timezone_now()) + email_jobs_to_deliver = ScheduledEmail.objects.filter(scheduled_timestamp__lte=timezone_now()) if email_jobs_to_deliver: for job in email_jobs_to_deliver: try: diff --git a/zerver/management/commands/export.py b/zerver/management/commands/export.py index e4cbc3226c..cf183c3662 100644 --- a/zerver/management/commands/export.py +++ b/zerver/management/commands/export.py @@ -35,7 +35,7 @@ class Command(BaseCommand): * Sessions (everyone will need to login again post-export) * Users' passwords and API keys (users will need to use SSO or reset password) * Mobile tokens for APNS/GCM (users will need to reconnect their mobile devices) - * ScheduledJob (Not relevant on a new server) + * ScheduledEmail (Not relevant on a new server) * Deployment (Unused) * third_party_api_results cache (this means rerending all old messages could be expensive) diff --git a/zerver/management/commands/print_email_delivery_backlog.py b/zerver/management/commands/print_email_delivery_backlog.py index 6321978a8d..3de7c3105a 100755 --- a/zerver/management/commands/print_email_delivery_backlog.py +++ b/zerver/management/commands/print_email_delivery_backlog.py @@ -1,7 +1,7 @@ #!/usr/bin/env python """ -Shows backlog count of ScheduledJobs of type Email +Shows backlog count of ScheduledEmail """ from __future__ import absolute_import @@ -12,12 +12,12 @@ from django.conf import settings from django.core.management.base import BaseCommand from django.utils.timezone import now as timezone_now -from zerver.models import ScheduledJob +from zerver.models import ScheduledEmail from datetime import datetime, timedelta class Command(BaseCommand): - help = """Shows backlog count of ScheduledJobs of type Email + help = """Shows backlog count of ScheduledEmail (The number of currently overdue (by at least a minute) email jobs) This is run as part of the nagios health check for the deliver_email command. @@ -27,5 +27,5 @@ Usage: ./manage.py print_email_delivery_backlog def handle(self, *args, **options): # type: (*Any, **Any) -> None - print(len(ScheduledJob.objects.filter(type=ScheduledJob.EMAIL, - scheduled_timestamp__lte=timezone_now()-timedelta(minutes=1)))) + print(ScheduledEmail.objects.filter( + scheduled_timestamp__lte=timezone_now()-timedelta(minutes=1)).count()) diff --git a/zerver/migrations/0092_create_scheduledemail.py b/zerver/migrations/0092_create_scheduledemail.py new file mode 100644 index 0000000000..d424d51154 --- /dev/null +++ b/zerver/migrations/0092_create_scheduledemail.py @@ -0,0 +1,34 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.2 on 2017-07-11 23:41 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('zerver', '0091_realm_allow_edit_history'), + ] + + operations = [ + migrations.CreateModel( + name='ScheduledEmail', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('scheduled_timestamp', models.DateTimeField(db_index=True)), + ('data', models.TextField()), + ('address', models.EmailField(db_index=True, max_length=254, null=True)), + ('type', models.PositiveSmallIntegerField()), + ('user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'abstract': False, + }, + ), + migrations.DeleteModel( + name='ScheduledJob', + ), + ] diff --git a/zerver/models.py b/zerver/models.py index c7d7b1f9e1..31110065bc 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1687,18 +1687,33 @@ class DefaultStream(models.Model): class Meta(object): unique_together = ("realm", "stream") -class ScheduledJob(models.Model): - scheduled_timestamp = models.DateTimeField(auto_now_add=False, null=False) # type: datetime.datetime - type = models.PositiveSmallIntegerField() # type: int - # Valid types are {email} - # for EMAIL, filter_string is recipient_email - EMAIL = 1 - - # JSON representation of the job's data. Be careful, as we are not relying on Django to do validation +class AbstractScheduledJob(models.Model): + scheduled_timestamp = models.DateTimeField(db_index=True) # type: datetime.datetime + # JSON representation of arguments to consumer data = models.TextField() # type: Text - # Kind if like a ForeignKey, but table is determined by type. - filter_id = models.IntegerField(null=True) # type: Optional[int] - filter_string = models.CharField(max_length=100) # type: Text + + class Meta(object): + abstract = True + +class ScheduledEmail(AbstractScheduledJob): + # Exactly one of user or address should be set. These are used to + # filter the set of ScheduledEmails. + user = models.ForeignKey(UserProfile, null=True, on_delete=CASCADE) # type: UserProfile + # Just the address part of a full "name
" email address + address = models.EmailField(null=True, db_index=True) # type: Text + + # Valid types are below + WELCOME = 1 + DIGEST = 2 + INVITATION_REMINDER = 3 + type = models.PositiveSmallIntegerField() # type: int + +EMAIL_TYPES = { + 'followup_day1': ScheduledEmail.WELCOME, + 'followup_day2': ScheduledEmail.WELCOME, + 'digest': ScheduledEmail.DIGEST, + 'invitation_reminder': ScheduledEmail.INVITATION_REMINDER, +} class RealmAuditLog(models.Model): realm = models.ForeignKey(Realm, on_delete=CASCADE) # type: Realm diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 9e46f15280..1fd5d2c636 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -17,7 +17,7 @@ from zerver.lib.actions import ( from zerver.lib.send_email import send_future_email from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import tornado_redirected_to_list -from zerver.models import get_realm, Realm, UserProfile, ScheduledJob +from zerver.models import get_realm, Realm, UserProfile, ScheduledEmail class RealmTest(ZulipTestCase): def assert_user_profile_cache_gets_new_name(self, user_profile, new_realm_name): @@ -144,10 +144,10 @@ class RealmTest(ZulipTestCase): def test_do_deactivate_realm_clears_scheduled_jobs(self): # type: () -> None user = self.example_user('hamlet') - send_future_email('template_prefix', to_user_id=user.id, delay=datetime.timedelta(hours=1)) - self.assertEqual(ScheduledJob.objects.count(), 1) + send_future_email('zerver/emails/followup_day1', to_user_id=user.id, delay=datetime.timedelta(hours=1)) + self.assertEqual(ScheduledEmail.objects.count(), 1) do_deactivate_realm(user.realm) - self.assertEqual(ScheduledJob.objects.count(), 0) + self.assertEqual(ScheduledEmail.objects.count(), 0) def test_do_deactivate_realm_on_deactived_realm(self): # type: () -> None diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 57ffad6ac1..44586f585b 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -27,8 +27,8 @@ from zerver.models import ( get_unique_open_realm, get_unique_non_system_realm, completely_open, get_recipient, PreregistrationUser, Realm, RealmDomain, Recipient, Message, - ScheduledJob, UserProfile, UserMessage, - Stream, Subscription, ScheduledJob, flush_per_request_caches + ScheduledEmail, UserProfile, UserMessage, + Stream, Subscription, flush_per_request_caches ) from zerver.lib.actions import ( set_default_streams, @@ -708,8 +708,7 @@ so we didn't send them an invitation. We did send invitations to everyone else!" send_future_email( "zerver/emails/invitation_reminder", to_email=data["email"], from_address=FromAddress.NOREPLY, context=context) - email_jobs_to_deliver = ScheduledJob.objects.filter( - type=ScheduledJob.EMAIL, + email_jobs_to_deliver = ScheduledEmail.objects.filter( scheduled_timestamp__lte=timezone_now()) self.assertEqual(len(email_jobs_to_deliver), 1) email_count = len(outbox) @@ -794,8 +793,7 @@ class EmailUnsubscribeTests(ZulipTestCase): user_profile = self.example_user('hamlet') # Simulate a new user signing up, which enqueues 2 welcome e-mails. enqueue_welcome_emails(user_profile.id) - self.assertEqual(2, len(ScheduledJob.objects.filter( - type=ScheduledJob.EMAIL, filter_string__iexact=email))) + self.assertEqual(2, ScheduledEmail.objects.filter(address__iexact=email).count()) # Simulate unsubscribing from the welcome e-mails. unsubscribe_link = one_click_unsubscribe_link(user_profile, "welcome") @@ -803,8 +801,7 @@ class EmailUnsubscribeTests(ZulipTestCase): # The welcome email jobs are no longer scheduled. self.assertEqual(result.status_code, 200) - self.assertEqual(0, len(ScheduledJob.objects.filter( - type=ScheduledJob.EMAIL, filter_string__iexact=email))) + self.assertEqual(0, ScheduledEmail.objects.filter(address__iexact=email).count()) def test_digest_unsubscribe(self): # type: () -> None @@ -824,8 +821,7 @@ class EmailUnsubscribeTests(ZulipTestCase): 'new_users': [], 'new_streams': {'plain': []}, 'unsubscribe_link': ''} send_future_email('zerver/emails/digest', to_user_id=user_profile.id, context=context) - self.assertEqual(1, len(ScheduledJob.objects.filter( - type=ScheduledJob.EMAIL, filter_string__iexact=email))) + self.assertEqual(1, ScheduledEmail.objects.filter(address__iexact=email).count()) # Simulate unsubscribing from digest e-mails. unsubscribe_link = one_click_unsubscribe_link(user_profile, "digest") @@ -836,8 +832,7 @@ class EmailUnsubscribeTests(ZulipTestCase): # Circumvent user_profile caching. user_profile = UserProfile.objects.get(email=self.example_email("hamlet")) self.assertFalse(user_profile.enable_digest_emails) - self.assertEqual(0, len(ScheduledJob.objects.filter( - type=ScheduledJob.EMAIL, filter_string__iexact=email))) + self.assertEqual(0, ScheduledEmail.objects.filter(address__iexact=email).count()) class RealmCreationTest(ZulipTestCase): diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 97f5a46a39..691ecb7076 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -22,7 +22,7 @@ from zerver.lib.test_runner import slow from zerver.models import UserProfile, Recipient, \ Realm, RealmDomain, UserActivity, \ get_user, get_realm, get_client, get_stream, \ - Message, get_context_for_message, ScheduledJob + Message, get_context_for_message, ScheduledEmail from zerver.lib.avatar import avatar_url from zerver.lib.email_mirror import create_missed_message_address @@ -345,10 +345,10 @@ class ActivateTest(ZulipTestCase): def test_clear_scheduled_jobs(self): # type: () -> None user = self.example_user('hamlet') - send_future_email('template_prefix', to_user_id=user.id, delay=datetime.timedelta(hours=1)) - self.assertEqual(ScheduledJob.objects.count(), 1) + send_future_email('zerver/emails/followup_day1', to_user_id=user.id, delay=datetime.timedelta(hours=1)) + self.assertEqual(ScheduledEmail.objects.count(), 1) do_deactivate_user(user) - self.assertEqual(ScheduledJob.objects.count(), 0) + self.assertEqual(ScheduledEmail.objects.count(), 0) class GetProfileTest(ZulipTestCase):