message_edit: Queue event on commit in do_update_message codepath.

'do_update_message' is within a db transaction, this commit
updates the 'do_clear_mobile_push_notifications_for_ids' function
used in 'do_update_message' to queue event on commit.

Events should not be sent until we know we're not rolling back,
otherwise it can lead to a situation where we enqueue events but
the function fails at a later stage.

zerver/actions/message_flags.py

@@ -15,7 +15,7 @@ from zerver.lib.message import (
     format_unread_message_details,
     get_raw_unread_data,
 )
-from zerver.lib.queue import queue_json_publish
+from zerver.lib.queue import queue_event_on_commit
 from zerver.lib.stream_subscription import get_subscribed_stream_recipient_ids_for_user
 from zerver.lib.topic import filter_by_topic_name_via_message
 from zerver.lib.user_message import DEFAULT_HISTORICAL_FLAGS, create_historical_user_messages
@@ -252,9 +252,9 @@ def do_clear_mobile_push_notifications_for_ids(
         }
         if settings.MOBILE_NOTIFICATIONS_SHARDS > 1:  # nocoverage
             shard_id = user_profile_id % settings.MOBILE_NOTIFICATIONS_SHARDS + 1
-            queue_json_publish(f"missedmessage_mobile_notifications_shard{shard_id}", notice)
+            queue_event_on_commit(f"missedmessage_mobile_notifications_shard{shard_id}", notice)
         else:
-            queue_json_publish("missedmessage_mobile_notifications", notice)
+            queue_event_on_commit("missedmessage_mobile_notifications", notice)
 
 
 def do_update_message_flags(

zerver/tests/test_message_flags.py

@@ -200,10 +200,15 @@ class UnreadCountTests(ZulipTestCase):
     def test_update_flags(self) -> None:
         self.login("hamlet")
 
-        result = self.client_post(
-            "/json/messages/flags",
-            {"messages": orjson.dumps(self.unread_msg_ids).decode(), "op": "add", "flag": "read"},
-        )
+        with self.captureOnCommitCallbacks(execute=True):
+            result = self.client_post(
+                "/json/messages/flags",
+                {
+                    "messages": orjson.dumps(self.unread_msg_ids).decode(),
+                    "op": "add",
+                    "flag": "read",
+                },
+            )
         self.assert_json_success(result)
 
         # Ensure we properly set the flags
@@ -843,13 +848,14 @@ class PushNotificationMarkReadFlowsTest(ZulipTestCase):
             [message_id, second_message_id, third_message_id],
         )
 
-        result = self.client_post(
-            "/json/mark_topic_as_read",
-            {
-                "stream_id": str(stream.id),
-                "topic_name": "test_topic",
-            },
-        )
+        with self.captureOnCommitCallbacks(execute=True):
+            result = self.client_post(
+                "/json/mark_topic_as_read",
+                {
+                    "stream_id": str(stream.id),
+                    "topic_name": "test_topic",
+                },
+            )
 
         self.assert_json_success(result)
         self.assertEqual(
@@ -857,12 +863,13 @@ class PushNotificationMarkReadFlowsTest(ZulipTestCase):
             [second_message_id, third_message_id],
         )
 
-        result = self.client_post(
-            "/json/mark_stream_as_read",
-            {
-                "stream_id": str(stream.id),
-            },
-        )
+        with self.captureOnCommitCallbacks(execute=True):
+            result = self.client_post(
+                "/json/mark_stream_as_read",
+                {
+                    "stream_id": str(stream.id),
+                },
+            )
         self.assertEqual(self.get_mobile_push_notification_ids(user_profile), [third_message_id])
 
         fourth_message_id = self.send_stream_message(
@@ -873,7 +880,8 @@ class PushNotificationMarkReadFlowsTest(ZulipTestCase):
             [third_message_id, fourth_message_id],
         )
 
-        result = self.client_post("/json/mark_all_as_read", {})
+        with self.captureOnCommitCallbacks(execute=True):
+            result = self.client_post("/json/mark_all_as_read", {})
         self.assertEqual(self.get_mobile_push_notification_ids(user_profile), [])
         mock_push_notifications.assert_called()
 

zerver/tests/test_push_notifications.py

@@ -4087,7 +4087,8 @@ class TestAPNs(PushNotificationTest):
         # Mark the messages as read and test whether
         # the count decreases correctly.
         for i, message_id in enumerate(message_ids):
-            do_update_message_flags(user_profile, "add", "read", [message_id])
+            with self.captureOnCommitCallbacks(execute=True):
+                do_update_message_flags(user_profile, "add", "read", [message_id])
             self.assertEqual(get_apns_badge_count(user_profile), 0)
             self.assertEqual(get_apns_badge_count_future(user_profile), num_messages - i - 1)
 
@@ -5073,10 +5074,10 @@ class TestClearOnRead(ZulipTestCase):
             message_id__in=message_ids,
         ).update(flags=F("flags").bitor(UserMessage.flags.active_mobile_push_notification))
 
-        with mock_queue_publish("zerver.actions.message_flags.queue_json_publish") as mock_publish:
+        with mock_queue_publish("zerver.actions.message_flags.queue_event_on_commit") as m:
             assert stream.recipient_id is not None
             do_mark_stream_messages_as_read(hamlet, stream.recipient_id)
-            queue_items = [c[0][1] for c in mock_publish.call_args_list]
+            queue_items = [c[0][1] for c in m.call_args_list]
             groups = [item["message_ids"] for item in queue_items]
 
         self.assert_length(groups, 1)