send_email: Skip the ScheduledEmail table for 0-delay emails.

zerver/lib/digest.py

@@ -404,7 +404,7 @@ def bulk_handle_digest_email(user_ids: list[int], cutoff: float) -> None:
             continue
 
         digest_users.append(user)
-        logger.info("Sending digest email for user %s", user.id)
+        logger.info("Enqueuing digest email for user %s", user.id)
 
         # Send now, as a ScheduledEmail
         send_future_email(

zerver/lib/send_email.py

@@ -381,6 +381,14 @@ def send_future_email(
         )
         # For logging the email
 
+    if delay == timedelta(0):
+        # Immediately queue, rather than go through the ScheduledEmail table
+        queue_event_on_commit(
+            "deferred_email_senders",
+            {**email_fields, "to_user_ids": to_user_ids, "to_emails": to_emails},
+        )
+        return
+
     assert (to_user_ids is None) ^ (to_emails is None)
     with transaction.atomic(savepoint=False):
         email = ScheduledEmail.objects.create(

zerver/tests/test_email_log.py

@@ -32,7 +32,7 @@ class EmailLogTest(ZulipTestCase):
             output_log = (
                 "INFO:root:Emails sent in development are available at http://testserver/emails"
             )
-            self.assertEqual(m.output, [output_log for i in range(18)])
+            self.assertEqual(m.output, [output_log for i in range(20)])
 
     def test_forward_address_details(self) -> None:
         try:

zerver/tests/test_email_notifications.py

@@ -16,12 +16,9 @@ from zerver.lib.email_notifications import (
     get_onboarding_email_schedule,
     send_account_registered_email,
 )
-from zerver.lib.send_email import (
-    queue_scheduled_emails,
-    send_custom_email,
-    send_custom_server_email,
-)
+from zerver.lib.send_email import send_custom_email, send_custom_server_email
 from zerver.lib.test_classes import ZulipTestCase
+from zerver.lib.test_helpers import mock_queue_publish
 from zerver.models import Realm, ScheduledEmail, UserProfile
 from zerver.models.realms import get_realm
 from zilencer.models import RemoteZulipServer
@@ -244,11 +241,10 @@ class TestCustomEmails(ZulipTestCase):
 class TestFollowupEmails(ZulipTestCase):
     def test_account_registered_email_context(self) -> None:
         hamlet = self.example_user("hamlet")
+        with mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m:
             send_account_registered_email(hamlet)
-        scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by(
-            "scheduled_timestamp"
-        )
-        email_data = orjson.loads(scheduled_emails[0].data)
+        m.assert_called_once()
+        email_data = m.call_args[0][1]
         self.assertEqual(email_data["context"]["email"], self.example_email("hamlet"))
         self.assertEqual(email_data["context"]["is_realm_admin"], False)
         self.assertEqual(
@@ -260,9 +256,10 @@ class TestFollowupEmails(ZulipTestCase):
         ScheduledEmail.objects.all().delete()
 
         iago = self.example_user("iago")
+        with mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m:
             send_account_registered_email(iago)
-        scheduled_emails = ScheduledEmail.objects.filter(users=iago).order_by("scheduled_timestamp")
-        email_data = orjson.loads(scheduled_emails[0].data)
+        m.assert_called_once()
+        email_data = m.call_args[0][1]
         self.assertEqual(email_data["context"]["email"], self.example_email("iago"))
         self.assertEqual(email_data["context"]["is_realm_admin"], True)
         self.assertEqual(
@@ -291,18 +288,18 @@ class TestFollowupEmails(ZulipTestCase):
         self.init_default_ldap_database()
         ldap_user_attr_map = {"full_name": "cn"}
 
-        with self.settings(AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map):
+        with (
+            self.settings(AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map),
+            mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m,
+        ):
             self.login_with_return(
                 "newuser_email_as_uid@zulip.com",
                 self.ldap_password("newuser_email_as_uid@zulip.com"),
             )
             user = UserProfile.objects.get(delivery_email="newuser_email_as_uid@zulip.com")
-            scheduled_emails = ScheduledEmail.objects.filter(users=user).order_by(
-                "scheduled_timestamp"
-            )
-
-            self.assert_length(scheduled_emails, 3)
-            email_data = orjson.loads(scheduled_emails[0].data)
+            self.assert_length(ScheduledEmail.objects.filter(users=user), 2)
+            m.assert_called_once()
+            email_data = m.call_args[0][1]
             self.assertEqual(email_data["context"]["ldap"], True)
             self.assertEqual(
                 email_data["context"]["ldap_username"], "newuser_email_as_uid@zulip.com"
@@ -318,18 +315,18 @@ class TestFollowupEmails(ZulipTestCase):
         self.init_default_ldap_database()
         ldap_user_attr_map = {"full_name": "cn"}
 
-        with self.settings(
-            LDAP_APPEND_DOMAIN="zulip.com",
-            AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map,
+        with (
+            self.settings(
+                LDAP_APPEND_DOMAIN="zulip.com", AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map
+            ),
+            mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m,
         ):
             self.login_with_return("newuser@zulip.com", self.ldap_password("newuser"))
 
             user = UserProfile.objects.get(delivery_email="newuser@zulip.com")
-            scheduled_emails = ScheduledEmail.objects.filter(users=user).order_by(
-                "scheduled_timestamp"
-            )
-            self.assert_length(scheduled_emails, 3)
-            email_data = orjson.loads(scheduled_emails[0].data)
+            self.assert_length(ScheduledEmail.objects.filter(users=user), 2)
+            m.assert_called_once()
+            email_data = m.call_args[0][1]
             self.assertEqual(email_data["context"]["ldap"], True)
             self.assertEqual(email_data["context"]["ldap_username"], "newuser")
 
@@ -343,17 +340,15 @@ class TestFollowupEmails(ZulipTestCase):
         self.init_default_ldap_database()
         ldap_user_attr_map = {"full_name": "cn"}
 
-        with self.settings(
-            LDAP_EMAIL_ATTR="mail",
-            AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map,
+        with (
+            self.settings(LDAP_EMAIL_ATTR="mail", AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map),
+            mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m,
         ):
             self.login_with_return("newuser_with_email", self.ldap_password("newuser_with_email"))
             user = UserProfile.objects.get(delivery_email="newuser_email@zulip.com")
-            scheduled_emails = ScheduledEmail.objects.filter(users=user).order_by(
-                "scheduled_timestamp"
-            )
-            self.assert_length(scheduled_emails, 3)
-            email_data = orjson.loads(scheduled_emails[0].data)
+            self.assert_length(ScheduledEmail.objects.filter(users=user), 2)
+            m.assert_called_once()
+            email_data = m.call_args[0][1]
             self.assertEqual(email_data["context"]["ldap"], True)
             self.assertEqual(email_data["context"]["ldap_username"], "newuser_with_email")
 
@@ -364,22 +359,22 @@ class TestFollowupEmails(ZulipTestCase):
         realm = get_realm("zulip")
 
         # Hamlet has account only in Zulip realm so all onboarding emails should be sent
+        with mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m:
             send_account_registered_email(self.example_user("hamlet"))
             enqueue_welcome_emails(self.example_user("hamlet"))
+        m.assert_called_once()
+        self.assertEqual(m.call_args[0][1]["template_prefix"], "zerver/emails/account_registered")
+
         scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by(
             "scheduled_timestamp"
         )
-        self.assert_length(scheduled_emails, 3)
+        self.assert_length(scheduled_emails, 2)
         self.assertEqual(
             orjson.loads(scheduled_emails[0].data)["template_prefix"],
-            "zerver/emails/account_registered",
-        )
-        self.assertEqual(
-            orjson.loads(scheduled_emails[1].data)["template_prefix"],
             "zerver/emails/onboarding_zulip_topics",
         )
         self.assertEqual(
-            orjson.loads(scheduled_emails[2].data)["template_prefix"],
+            orjson.loads(scheduled_emails[1].data)["template_prefix"],
             "zerver/emails/onboarding_zulip_guide",
         )
 
@@ -391,38 +386,38 @@ class TestFollowupEmails(ZulipTestCase):
         realm.save()
 
         # Hamlet is not an admin so the `/for/communities/` zulip_guide should not be sent
+        with mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m:
             send_account_registered_email(self.example_user("hamlet"))
             enqueue_welcome_emails(self.example_user("hamlet"))
+        m.assert_called_once()
+        self.assertEqual(m.call_args[0][1]["template_prefix"], "zerver/emails/account_registered")
+
         scheduled_emails = ScheduledEmail.objects.filter(users=hamlet).order_by(
             "scheduled_timestamp"
         )
-        self.assert_length(scheduled_emails, 2)
+        self.assert_length(scheduled_emails, 1)
         self.assertEqual(
             orjson.loads(scheduled_emails[0].data)["template_prefix"],
-            "zerver/emails/account_registered",
-        )
-        self.assertEqual(
-            orjson.loads(scheduled_emails[1].data)["template_prefix"],
             "zerver/emails/onboarding_zulip_topics",
         )
 
         ScheduledEmail.objects.all().delete()
 
         # Iago is an admin so the `/for/communities/` zulip_guide should be sent
+        with mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m:
             send_account_registered_email(self.example_user("iago"))
             enqueue_welcome_emails(self.example_user("iago"))
+        m.assert_called_once()
+        self.assertEqual(m.call_args[0][1]["template_prefix"], "zerver/emails/account_registered")
+
         scheduled_emails = ScheduledEmail.objects.filter(users=iago).order_by("scheduled_timestamp")
-        self.assert_length(scheduled_emails, 3)
+        self.assert_length(scheduled_emails, 2)
         self.assertEqual(
             orjson.loads(scheduled_emails[0].data)["template_prefix"],
-            "zerver/emails/account_registered",
-        )
-        self.assertEqual(
-            orjson.loads(scheduled_emails[1].data)["template_prefix"],
             "zerver/emails/onboarding_zulip_topics",
         )
         self.assertEqual(
-            orjson.loads(scheduled_emails[2].data)["template_prefix"],
+            orjson.loads(scheduled_emails[1].data)["template_prefix"],
             "zerver/emails/onboarding_zulip_guide",
         )
 
@@ -433,24 +428,20 @@ class TestFollowupEmails(ZulipTestCase):
         realm.save()
 
         # Cordelia has account in more than 1 realm so onboarding_zulip_topics email should not be sent
+        with mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m:
             send_account_registered_email(self.example_user("cordelia"))
             enqueue_welcome_emails(self.example_user("cordelia"))
+        m.assert_called_once()
+        self.assertEqual(m.call_args[0][1]["template_prefix"], "zerver/emails/account_registered")
+
         scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by(
             "scheduled_timestamp"
         )
-        self.assert_length(scheduled_emails, 2)
+        self.assert_length(scheduled_emails, 1)
         self.assertEqual(
             orjson.loads(scheduled_emails[0].data)["template_prefix"],
-            "zerver/emails/account_registered",
-        )
-        self.assertEqual(
-            orjson.loads(scheduled_emails[1].data)["template_prefix"],
             "zerver/emails/onboarding_zulip_guide",
         )
-        self.assertEqual(
-            orjson.loads(scheduled_emails[1].data)["context"]["organization_type"],
-            "education",
-        )
 
         ScheduledEmail.objects.all().delete()
 
@@ -459,39 +450,37 @@ class TestFollowupEmails(ZulipTestCase):
         realm.save()
 
         # In this case, Cordelia should only be sent the account_registered email
+        with mock_queue_publish("zerver.lib.send_email.queue_event_on_commit") as m:
             send_account_registered_email(self.example_user("cordelia"))
             enqueue_welcome_emails(self.example_user("cordelia"))
+        m.assert_called_once()
+        self.assertEqual(m.call_args[0][1]["template_prefix"], "zerver/emails/account_registered")
         scheduled_emails = ScheduledEmail.objects.filter(users=cordelia)
-        self.assert_length(scheduled_emails, 1)
-        self.assertEqual(
-            orjson.loads(scheduled_emails[0].data)["template_prefix"],
-            "zerver/emails/account_registered",
-        )
+        self.assert_length(scheduled_emails, 0)
 
     def test_followup_emails_for_regular_realms(self) -> None:
         cordelia = self.example_user("cordelia")
+        with self.captureOnCommitCallbacks(execute=True) as callbacks:
             send_account_registered_email(self.example_user("cordelia"), realm_creation=True)
             enqueue_welcome_emails(self.example_user("cordelia"), realm_creation=True)
+
             scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by(
                 "scheduled_timestamp"
             )
-        assert scheduled_emails is not None
-        self.assert_length(scheduled_emails, 3)
+            self.assert_length(scheduled_emails, 2)
             self.assertEqual(
                 orjson.loads(scheduled_emails[0].data)["template_prefix"],
-            "zerver/emails/account_registered",
-        )
-        self.assertEqual(
-            orjson.loads(scheduled_emails[1].data)["template_prefix"],
                 "zerver/emails/onboarding_zulip_guide",
             )
             self.assertEqual(
-            orjson.loads(scheduled_emails[2].data)["template_prefix"],
+                orjson.loads(scheduled_emails[1].data)["template_prefix"],
                 "zerver/emails/onboarding_team_to_zulip",
             )
 
-        with self.captureOnCommitCallbacks(execute=True):
-            queue_scheduled_emails(scheduled_emails[0])
+        # The insert into the deferred_email_senders queue
+        self.assert_length(callbacks, 1)
+
+        # Exiting the block does the email-sending
         from django.core.mail import outbox
 
         self.assert_length(outbox, 1)
@@ -506,28 +495,27 @@ class TestFollowupEmails(ZulipTestCase):
             days=30
         )
         cordelia.realm.save()
+        with self.captureOnCommitCallbacks(execute=True) as callbacks:
             send_account_registered_email(self.example_user("cordelia"), realm_creation=True)
             enqueue_welcome_emails(self.example_user("cordelia"), realm_creation=True)
+
             scheduled_emails = ScheduledEmail.objects.filter(users=cordelia).order_by(
                 "scheduled_timestamp"
             )
-        assert scheduled_emails is not None
-        self.assert_length(scheduled_emails, 3)
+            self.assert_length(scheduled_emails, 2)
             self.assertEqual(
                 orjson.loads(scheduled_emails[0].data)["template_prefix"],
-            "zerver/emails/account_registered",
-        )
-        self.assertEqual(
-            orjson.loads(scheduled_emails[1].data)["template_prefix"],
                 "zerver/emails/onboarding_zulip_guide",
             )
             self.assertEqual(
-            orjson.loads(scheduled_emails[2].data)["template_prefix"],
+                orjson.loads(scheduled_emails[1].data)["template_prefix"],
                 "zerver/emails/onboarding_team_to_zulip",
             )
 
-        with self.captureOnCommitCallbacks(execute=True):
-            queue_scheduled_emails(scheduled_emails[0])
+        # The insert into the deferred_email_senders queue
+        self.assert_length(callbacks, 1)
+
+        # Exiting the block does the email-sending
         from django.core.mail import outbox
 
         self.assert_length(outbox, 1)

zerver/tests/test_example.py

@@ -419,7 +419,7 @@ class TestDevelopmentEmailsLog(ZulipTestCase):
             )
 
             # info_log.output is a list of all the log messages captured.
-            self.assertEqual(info_log.output, [expected_log_line] * 18)
+            self.assertEqual(info_log.output, [expected_log_line] * 20)
 
             # Now, lets actually go the URL the above call redirects to, i.e., /emails/
             result = self.client_get(result["Location"])

zerver/tests/test_invite.py

@@ -47,10 +47,9 @@ from zerver.actions.streams import do_change_stream_group_based_setting
 from zerver.actions.user_groups import check_add_user_group, do_change_user_group_permission_setting
 from zerver.actions.user_settings import do_change_full_name
 from zerver.actions.users import change_user_is_active
-from zerver.context_processors import common_context
 from zerver.lib.create_user import create_user
 from zerver.lib.default_streams import get_slim_realm_default_streams
-from zerver.lib.send_email import FromAddress, queue_scheduled_emails, send_future_email
+from zerver.lib.send_email import queue_scheduled_emails
 from zerver.lib.streams import ensure_stream
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import find_key_by_email
@@ -1809,60 +1808,31 @@ so we didn't send them an invitation. We did send invitations to everyone else!"
         self.assertTrue(find_key_by_email(invitee_email))
         self.check_sent_emails([invitee_email])
 
-        data = {"email": invitee_email, "referrer_email": current_user.email}
-        invitee = PreregistrationUser.objects.get(email=data["email"])
-        referrer = self.example_user(referrer_name)
-        validity_in_minutes = 2 * 24 * 60
-        link = create_confirmation_link(
-            invitee, Confirmation.INVITATION, validity_in_minutes=validity_in_minutes
-        )
-        context = common_context(referrer)
-        context.update(
-            activate_url=link,
-            referrer_name=referrer.full_name,
-            referrer_email=referrer.email,
-            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_emails=[email],
-                from_address=FromAddress.no_reply_placeholder,
-                context=context,
-            )
-        email_jobs_to_deliver = ScheduledEmail.objects.filter(
-            scheduled_timestamp__lte=timezone_now()
-        )
+        email_jobs_to_deliver = ScheduledEmail.objects.all()
         self.assert_length(email_jobs_to_deliver, 1)
-        email_count = len(mail.outbox)
+
+        mail.outbox = []
         for job in email_jobs_to_deliver:
             with self.captureOnCommitCallbacks(execute=True):
                 queue_scheduled_emails(job)
-        self.assert_length(mail.outbox, email_count + 1)
-        self.assertEqual(self.email_envelope_from(mail.outbox[-1]), settings.NOREPLY_EMAIL_ADDRESS)
-        self.assertIn(FromAddress.NOREPLY, self.email_display_from(mail.outbox[-1]))
+        self.assert_length(mail.outbox, 1)
+        self.assertEqual(self.email_envelope_from(mail.outbox[0]), settings.NOREPLY_EMAIL_ADDRESS)
 
         # Now verify that signing up clears invite_reminder emails
-        with self.settings(EMAIL_BACKEND="django.core.mail.backends.console.EmailBackend"):
-            email = data["email"]
-            send_future_email(
-                "zerver/emails/invitation_reminder",
-                referrer.realm,
-                to_emails=[email],
-                from_address=FromAddress.no_reply_placeholder,
-                context=context,
-            )
+        mail.outbox = []
+        invitee_email = self.nonreg_email("bob")
+        self.assert_json_success(self.invite(invitee_email, ["Denmark"]))
+        self.assertTrue(find_key_by_email(invitee_email))
+        self.check_sent_emails([invitee_email])
 
         email_jobs_to_deliver = ScheduledEmail.objects.filter(
-            scheduled_timestamp__lte=timezone_now(), type=ScheduledEmail.INVITATION_REMINDER
+            type=ScheduledEmail.INVITATION_REMINDER
         )
         self.assert_length(email_jobs_to_deliver, 1)
 
         self.register(invitee_email, "test")
         email_jobs_to_deliver = ScheduledEmail.objects.filter(
-            scheduled_timestamp__lte=timezone_now(), type=ScheduledEmail.INVITATION_REMINDER
+            type=ScheduledEmail.INVITATION_REMINDER
         )
         self.assert_length(email_jobs_to_deliver, 0)
 

zerver/tests/test_signup.py

@@ -1040,8 +1040,8 @@ class LoginTest(ZulipTestCase):
         # to sending messages, such as getting the welcome bot, looking up
         # the alert words for a realm, etc.
         with (
-            self.assert_database_query_count(95),
-            self.assert_memcached_count(14),
+            self.assert_database_query_count(94),
+            self.assert_memcached_count(15),
             self.captureOnCommitCallbacks(execute=True),
         ):
             self.register(self.nonreg_email("test"), "test")
@@ -1239,7 +1239,7 @@ class EmailUnsubscribeTests(ZulipTestCase):
         unsubscribe_link = one_click_unsubscribe_link(user_profile, "welcome")
         result = self.client_get(urlsplit(unsubscribe_link).path)
 
-        # The welcome email jobs are no longer scheduled.
+        # The welcome email job are no longer scheduled.
         self.assertEqual(result.status_code, 200)
         self.assertEqual(0, ScheduledEmail.objects.filter(users=user_profile).count())
 
@@ -1248,8 +1248,8 @@ class EmailUnsubscribeTests(ZulipTestCase):
         We provide one-click unsubscribe links in digest e-mails that you can
         click even when logged out to stop receiving them.
 
-        Unsubscribing from these emails also dequeues any digest email jobs that
-        have been queued.
+        Since pending digests are in a RabbitMQ queue, we cannot unqueue them
+        once they're enqueued.
         """
         user_profile = self.example_user("hamlet")
         self.assertTrue(user_profile.enable_digest_emails)
@@ -1270,8 +1270,7 @@ class EmailUnsubscribeTests(ZulipTestCase):
             to_user_ids=[user_profile.id],
             context=context,
         )
-
-        self.assertEqual(1, ScheduledEmail.objects.filter(users=user_profile).count())
+        self.assert_length(ScheduledEmail.objects.filter(users=user_profile), 0)
 
         # Simulate unsubscribing from digest e-mails.
         unsubscribe_link = one_click_unsubscribe_link(user_profile, "digest")
@@ -1283,7 +1282,6 @@ class EmailUnsubscribeTests(ZulipTestCase):
 
         user_profile.refresh_from_db()
         self.assertFalse(user_profile.enable_digest_emails)
-        self.assertEqual(0, ScheduledEmail.objects.filter(users=user_profile).count())
 
     def test_login_unsubscribe(self) -> None:
         """

zerver/tests/test_users.py

@@ -1017,8 +1017,8 @@ class QueryCountTest(ZulipTestCase):
         prereg_user = PreregistrationUser.objects.get(email="fred@zulip.com")
 
         with (
-            self.assert_database_query_count(87),
-            self.assert_memcached_count(19),
+            self.assert_database_query_count(86),
+            self.assert_memcached_count(20),
             self.capture_send_event_calls(expected_num_events=10) as events,
         ):
             fred = do_create_user(