From 7302d533fe3fc97c22b05e2db5db1135e2f50b4d Mon Sep 17 00:00:00 2001
From: Prakhar Pratyush <prakhar841301@gmail.com>
Date: Wed, 31 Jul 2024 19:06:00 +0530
Subject: [PATCH] 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.
---
 zerver/actions/user_status.py    |  7 +++++--
 zerver/tests/test_events.py      | 22 +++++++++++-----------
 zerver/tests/test_user_status.py |  5 ++++-
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/zerver/actions/user_status.py b/zerver/actions/user_status.py
index 4ec0a0e55c..cf6f2708a1 100644
--- a/zerver/actions/user_status.py
+++ b/zerver/actions/user_status.py
@@ -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))
diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py
index 5fe2b19d18..5fc0661fc9 100644
--- a/zerver/tests/test_events.py
+++ b/zerver/tests/test_events.py
@@ -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],
diff --git a/zerver/tests/test_user_status.py b/zerver/tests/test_user_status.py
index fb62181b4a..55939ef25e 100644
--- a/zerver/tests/test_user_status.py
+++ b/zerver/tests/test_user_status.py
@@ -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)
-        self.assertEqual(events[0]["event"], expected_event)
+        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")