user_status: Update do_update_user_status to send event on commit.

Earlier, we were using 'send_event' in 'do_update_user_status' which
can lead to a situation, if any db operation is added after the
'send_event' in future, where we enqueue events but the action
function fails at a later stage.

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

Fixes part of #30489.
This commit is contained in:
Prakhar Pratyush 2024-07-31 19:06:00 +05:30 committed by Tim Abbott
parent dc27711399
commit 7302d533fe
3 changed files with 20 additions and 14 deletions

View File

@ -1,10 +1,13 @@
from django.db import transaction
from zerver.actions.user_settings import do_change_user_setting
from zerver.lib.user_status import update_user_status
from zerver.lib.users import get_user_ids_who_can_access_user
from zerver.models import UserProfile
from zerver.tornado.django_api import send_event
from zerver.tornado.django_api import send_event_on_commit
@transaction.atomic(savepoint=False)
def do_update_user_status(
user_profile: UserProfile,
away: bool | None,
@ -49,4 +52,4 @@ def do_update_user_status(
event["emoji_name"] = emoji_name
event["emoji_code"] = emoji_code
event["reaction_type"] = reaction_type
send_event(realm, event, get_user_ids_who_can_access_user(user_profile))
send_event_on_commit(realm, event, get_user_ids_who_can_access_user(user_profile))

View File

@ -1676,13 +1676,13 @@ class NormalActionsTest(BaseAction):
client_id=client.id,
)
check_user_settings_update("events[0]", events[0])
check_update_global_notifications("events[1]", events[1], not away_val)
check_user_status(
"events[0]",
events[0],
"events[2]",
events[2],
{"away", "status_text", "emoji_name", "emoji_code", "reaction_type"},
)
check_user_settings_update("events[1]", events[1])
check_update_global_notifications("events[2]", events[2], not away_val)
check_presence(
"events[3]",
events[3],
@ -1704,13 +1704,13 @@ class NormalActionsTest(BaseAction):
client_id=client.id,
)
check_user_settings_update("events[0]", events[0])
check_update_global_notifications("events[1]", events[1], not away_val)
check_user_status(
"events[0]",
events[0],
"events[2]",
events[2],
{"away", "status_text", "emoji_name", "emoji_code", "reaction_type"},
)
check_user_settings_update("events[1]", events[1])
check_update_global_notifications("events[2]", events[2], not away_val)
check_presence(
"events[3]",
events[3],
@ -1732,9 +1732,9 @@ class NormalActionsTest(BaseAction):
client_id=client.id,
)
check_user_status("events[0]", events[0], {"away"})
check_user_settings_update("events[1]", events[1])
check_update_global_notifications("events[2]", events[2], not away_val)
check_user_settings_update("events[0]", events[0])
check_update_global_notifications("events[1]", events[1], not away_val)
check_user_status("events[2]", events[2], {"away"})
check_presence(
"events[3]",
events[3],

View File

@ -183,7 +183,10 @@ class UserStatusTest(ZulipTestCase):
with self.capture_send_event_calls(expected_num_events=num_events) as events:
result = self.client_post("/json/users/me/status", payload)
self.assert_json_success(result)
if num_events == 1:
self.assertEqual(events[0]["event"], expected_event)
else:
self.assertEqual(events[2]["event"], expected_event)
def test_endpoints(self) -> None:
hamlet = self.example_user("hamlet")