message_edit: Update do_update_message codepath to send event on commit.

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

Events should not be sent until we know we're not rolling back.
This commit is contained in:
Prakhar Pratyush 2024-05-24 17:38:41 +05:30 committed by Tim Abbott
parent bca8338ee1
commit 0ce79c8c50
5 changed files with 19 additions and 15 deletions

View File

@ -81,7 +81,7 @@ from zerver.models import (
)
from zerver.models.streams import get_stream_by_id_in_realm
from zerver.models.users import get_system_bot
from zerver.tornado.django_api import send_event, send_event_on_commit
from zerver.tornado.django_api import send_event_on_commit
def subscriber_info(user_id: int) -> Dict[str, Any]:
@ -730,7 +730,9 @@ def do_update_message(
"stream_id": stream_being_edited.id,
"topic": orig_topic_name,
}
send_event(user_profile.realm, delete_event, [user.id for user in users_losing_access])
send_event_on_commit(
user_profile.realm, delete_event, [user.id for user in users_losing_access]
)
# Reset the Attachment.is_*_public caches for all messages
# moved to another stream with different access permissions.
@ -1003,7 +1005,7 @@ def do_update_message(
visibility_policy=new_visibility_policy,
)
send_event(user_profile.realm, event, users_to_be_notified)
send_event_on_commit(user_profile.realm, event, users_to_be_notified)
resolved_topic_message_id = None
if topic_name is not None and content is None:

View File

@ -1127,7 +1127,7 @@ class EditMessageTest(ZulipTestCase):
do_edit_message_assert_success(id_, "G", "cordelia")
do_edit_message_assert_success(id_, "H", "hamlet")
@mock.patch("zerver.actions.message_edit.send_event")
@mock.patch("zerver.actions.message_edit.send_event_on_commit")
def test_topic_wildcard_mention_in_followed_topic(
self, mock_send_event: mock.MagicMock
) -> None:
@ -1185,7 +1185,7 @@ class EditMessageTest(ZulipTestCase):
called = True
self.assertTrue(called)
@mock.patch("zerver.actions.message_edit.send_event")
@mock.patch("zerver.actions.message_edit.send_event_on_commit")
def test_stream_wildcard_mention_in_followed_topic(
self, mock_send_event: mock.MagicMock
) -> None:
@ -1243,7 +1243,7 @@ class EditMessageTest(ZulipTestCase):
called = True
self.assertTrue(called)
@mock.patch("zerver.actions.message_edit.send_event")
@mock.patch("zerver.actions.message_edit.send_event_on_commit")
def test_topic_wildcard_mention(self, mock_send_event: mock.MagicMock) -> None:
stream_name = "Macbeth"
hamlet = self.example_user("hamlet")
@ -1349,7 +1349,7 @@ class EditMessageTest(ZulipTestCase):
)
self.assert_json_success(result)
@mock.patch("zerver.actions.message_edit.send_event")
@mock.patch("zerver.actions.message_edit.send_event_on_commit")
def test_stream_wildcard_mention(self, mock_send_event: mock.MagicMock) -> None:
stream_name = "Macbeth"
hamlet = self.example_user("hamlet")

View File

@ -100,6 +100,7 @@ class EditMessageSideEffectsTest(ZulipTestCase):
)
with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as m:
with self.captureOnCommitCallbacks(execute=True):
result = self.client_patch(url, request)
cordelia = self.example_user("cordelia")

View File

@ -111,7 +111,7 @@ class MessageMoveTopicTest(ZulipTestCase):
)
self.assert_json_error(result, "Invalid character in topic, at position 8!")
@mock.patch("zerver.actions.message_edit.send_event")
@mock.patch("zerver.actions.message_edit.send_event_on_commit")
def test_edit_topic_public_history_stream(self, mock_send_event: mock.MagicMock) -> None:
stream_name = "Macbeth"
hamlet = self.example_user("hamlet")

View File

@ -333,6 +333,7 @@ class MutedUsersTests(ZulipTestCase):
self.login("cordelia")
with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as m:
with self.captureOnCommitCallbacks(execute=True):
result = self.client_patch(
"/json/messages/" + str(message_id),
dict(