handle_missedmessage_emails: Update codepath to queue event on commit.

Earlier, in 'handle_missedmessage_emails' codepath we were using
'queue_json_publish' which can lead to a situation where we enqueue
events but the transaction fails at a later stage.

Events should not be published until we know we're not rolling back.

zerver/lib/email_notifications.py

@@ -26,7 +26,7 @@ from zerver.lib.display_recipient import get_display_recipient
 from zerver.lib.markdown.fenced_code import FENCE_RE
 from zerver.lib.message import bulk_access_messages
 from zerver.lib.notification_data import get_mentioned_user_group
-from zerver.lib.queue import queue_json_publish
+from zerver.lib.queue import queue_event_on_commit
 from zerver.lib.send_email import FromAddress, send_future_email
 from zerver.lib.soft_deactivation import soft_reactivate_if_personal_notification
 from zerver.lib.tex import change_katex_to_raw_latex
@@ -593,7 +593,7 @@ def do_send_missedmessage_events_reply_in_zulip(
         "reply_to_email": str(Address(display_name=reply_to_name, addr_spec=reply_to_address)),
         "context": context,
     }
-    queue_json_publish("email_senders", email_dict)
+    queue_event_on_commit("email_senders", email_dict)
 
     user_profile.last_reminder = timezone_now()
     user_profile.save(update_fields=["last_reminder"])

zerver/lib/soft_deactivation.py

@@ -12,7 +12,7 @@ from django.utils.timezone import now as timezone_now
 from sentry_sdk import capture_exception
 
 from zerver.lib.logging_util import log_to_file
-from zerver.lib.queue import queue_json_publish
+from zerver.lib.queue import queue_event_on_commit
 from zerver.lib.user_message import bulk_insert_all_ums
 from zerver.lib.utils import assert_is_not_none
 from zerver.models import (
@@ -396,7 +396,7 @@ def queue_soft_reactivation(user_profile_id: int) -> None:
         "type": "soft_reactivate",
         "user_profile_id": user_profile_id,
     }
-    queue_json_publish("deferred_work", event)
+    queue_event_on_commit("deferred_work", event)
 
 
 def soft_reactivate_if_personal_notification(

zerver/lib/test_classes.py

@@ -49,6 +49,7 @@ from zerver.actions.realm_settings import (
 from zerver.actions.streams import bulk_add_subscriptions, bulk_remove_subscriptions
 from zerver.decorator import do_two_factor_login
 from zerver.lib.cache import bounce_key_prefix_for_testing
+from zerver.lib.email_notifications import MissedMessageData, handle_missedmessage_emails
 from zerver.lib.initial_password import initial_password
 from zerver.lib.mdiff import diff_strings
 from zerver.lib.message import access_message
@@ -2241,6 +2242,12 @@ class ZulipTestCase(ZulipTestCaseMixin, TestCase):
             # immediately run the worker for it, producing the thumbnails.
             return self.upload_image(image_name)
 
+    def handle_missedmessage_emails(
+        self, user_profile_id: int, message_ids: dict[int, MissedMessageData]
+    ) -> None:
+        with self.captureOnCommitCallbacks(execute=True):
+            handle_missedmessage_emails(user_profile_id, message_ids)
+
 
 def get_row_ids_in_all_tables() -> Iterator[tuple[str, set[int]]]:
     all_models = apps.get_models(include_auto_created=True)

zerver/tests/test_message_notification_emails.py

@@ -110,7 +110,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         hamlet = self.example_user("hamlet")
         tokens = self._get_tokens()
         with patch("zerver.lib.email_mirror.generate_missed_message_token", side_effect=tokens):
-            handle_missedmessage_emails(
+            self.handle_missedmessage_emails(
                 hamlet.id,
                 {
                     msg_id: MissedMessageData(
@@ -690,7 +690,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             othello, "Denmark", "@*hamlet_and_cordelia*"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 hamlet_only_message_id: MissedMessageData(
@@ -727,7 +727,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             othello, "Denmark", "@**King Hamlet**"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 user_group_mentioned_message_id: MissedMessageData(
@@ -764,7 +764,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             othello, "Denmark", "@*hamlet_and_cordelia*"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 topic_wildcard_mentioned_in_followed_topic_message_id: MissedMessageData(
@@ -798,7 +798,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             othello, "Denmark", "@**topic**"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 stream_wildcard_mentioned_in_followed_topic_message_id: MissedMessageData(
@@ -829,7 +829,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             othello, "Denmark", "@**all**"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 topic_wildcard_mentioned_message_id: MissedMessageData(
@@ -860,7 +860,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             othello, "Denmark", "@**topic**"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 stream_wildcard_mentioned_message_id: MissedMessageData(
@@ -889,7 +889,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             othello, "Denmark", "@**all**"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 followed_topic_mentioned_message_id: MissedMessageData(
@@ -916,7 +916,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", "0")
         followed_topic_mentioned_message_id = self.send_stream_message(othello, "Denmark", "1")
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 stream_mentioned_message_id: MissedMessageData(
@@ -1255,7 +1255,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         msg_id_2 = self.send_stream_message(self.example_user("iago"), "Verona", "* 1\n *2")
         msg_id_3 = self.send_personal_message(self.example_user("iago"), hamlet, "Hello")
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 msg_id_1: MissedMessageData(trigger=NotificationTriggers.MENTION),
@@ -1300,7 +1300,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             self.example_user("iago"), hamlet, "Personal Message 2"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 msg_id_1: MissedMessageData(trigger=NotificationTriggers.DIRECT_MESSAGE),
@@ -1326,7 +1326,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             msg_id = self.send_stream_message(iago, "Denmark", content=str(i))
             message_ids[msg_id] = MissedMessageData(trigger=NotificationTriggers.STREAM_EMAIL)
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             message_ids,
         )
@@ -1350,7 +1350,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             self.example_user("othello"), "Denmark", "@**King Hamlet**"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 msg_id_1: MissedMessageData(trigger=NotificationTriggers.STREAM_EMAIL),
@@ -1378,7 +1378,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
 
         mention_msg_id = self.send_stream_message(user, stream_name, "@**King Hamlet**")
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             late_subscribed_user.id,
             {mention_msg_id: MissedMessageData(trigger=NotificationTriggers.MENTION)},
         )
@@ -1405,7 +1405,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         )
         msg_id_3 = self.send_stream_message(cordelia, "Denmark", "Regular message")
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 msg_id_1: MissedMessageData(trigger=NotificationTriggers.MENTION),
@@ -1425,7 +1425,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
             self.example_user("iago"), "Denmark", "Message2", topic_name="test2"
         )
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {
                 msg_id_1: MissedMessageData(trigger=NotificationTriggers.STREAM_EMAIL),
@@ -1652,7 +1652,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         def send_personal_mention() -> None:
             mention = f"@**{hamlet.full_name}**"
             stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
-            handle_missedmessage_emails(
+            self.handle_missedmessage_emails(
                 hamlet.id,
                 {
                     stream_mentioned_message_id: MissedMessageData(
@@ -1668,7 +1668,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         def send_direct_message() -> None:
             # Soft reactivate the user by sending a personal message
             personal_message_id = self.send_personal_message(othello, hamlet, "Message")
-            handle_missedmessage_emails(
+            self.handle_missedmessage_emails(
                 hamlet.id,
                 {
                     personal_message_id: MissedMessageData(
@@ -1698,7 +1698,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         def send_topic_wildcard_mention() -> None:
             mention = "@**topic**"
             stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
-            handle_missedmessage_emails(
+            self.handle_missedmessage_emails(
                 hamlet.id,
                 {
                     stream_mentioned_message_id: MissedMessageData(
@@ -1714,7 +1714,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         def send_stream_wildcard_mention() -> None:
             mention = "@**all**"
             stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
-            handle_missedmessage_emails(
+            self.handle_missedmessage_emails(
                 hamlet.id,
                 {
                     stream_mentioned_message_id: MissedMessageData(
@@ -1747,7 +1747,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         def send_small_group_mention() -> None:
             mention = "@*small_user_group*"
             stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
-            handle_missedmessage_emails(
+            self.handle_missedmessage_emails(
                 hamlet.id,
                 {
                     stream_mentioned_message_id: MissedMessageData(
@@ -1764,7 +1764,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         def send_large_group_mention() -> None:
             mention = "@*large_user_group*"
             stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
-            handle_missedmessage_emails(
+            self.handle_missedmessage_emails(
                 hamlet.id,
                 {
                     stream_mentioned_message_id: MissedMessageData(
@@ -1782,7 +1782,7 @@ class TestMessageNotificationEmails(ZulipTestCase):
         othello = self.example_user("othello")
         msg_id = self.send_stream_message(othello, "Denmark")
 
-        handle_missedmessage_emails(
+        self.handle_missedmessage_emails(
             hamlet.id,
             {msg_id: MissedMessageData(trigger=NotificationTriggers.FOLLOWED_TOPIC_EMAIL)},
         )

zerver/tests/test_signup.py

@@ -872,6 +872,7 @@ class PasswordResetTest(ZulipTestCase):
 
         def reset_password() -> None:
             # start the password reset process by supplying an email address
+            with self.captureOnCommitCallbacks(execute=True):
                 result = self.client_post("/accounts/password/reset/", {"email": email})
 
             # check the redirect link telling you to check mail for password reset link