models: Replace ScheduledJob with ScheduledEmail.

ScheduledJob was written for much more generality than it ended up being
used for. Currently it is used by send_future_email, and nothing
else. Tailoring the model to emails in particular will make it easier to do
things like selectively clear emails when people unsubscribe from particular
email types, or seamlessly handle using the same email on multiple realms.

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.

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, \

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)

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',
 ]

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):

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))

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:

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)

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())

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',
+        ),
+    ]

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 <address>" 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

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

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):
 

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):