From e7f2a0958a028922fc5002dd83e6be6bd5909735 Mon Sep 17 00:00:00 2001
From: Prakhar Pratyush <prakhar@zulip.com>
Date: Mon, 12 Aug 2024 09:46:43 +0530
Subject: [PATCH] custom_profile_fields: Send event on commit in
 notify_realm...fields.

Earlier, we were using 'send_event' in
'notify_realm_custom_profile_fields' which can lead to a situation,
if any db operation is added after the 'send_event' in the action
functions using it, where we enqueue event 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/custom_profile_fields.py | 9 +++++++--
 zerver/tests/test_event_system.py       | 8 +++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/zerver/actions/custom_profile_fields.py b/zerver/actions/custom_profile_fields.py
index 6c7e338a9c..cdefb577f7 100644
--- a/zerver/actions/custom_profile_fields.py
+++ b/zerver/actions/custom_profile_fields.py
@@ -12,15 +12,16 @@ from zerver.lib.users import get_user_ids_who_can_access_user
 from zerver.models import CustomProfileField, CustomProfileFieldValue, Realm, UserProfile
 from zerver.models.custom_profile_fields import custom_profile_fields_for_realm
 from zerver.models.users import active_user_ids
-from zerver.tornado.django_api import send_event
+from zerver.tornado.django_api import send_event, send_event_on_commit
 
 
 def notify_realm_custom_profile_fields(realm: Realm) -> None:
     fields = custom_profile_fields_for_realm(realm.id)
     event = dict(type="custom_profile_fields", fields=[f.as_dict() for f in fields])
-    send_event(realm, event, active_user_ids(realm.id))
+    send_event_on_commit(realm, event, active_user_ids(realm.id))
 
 
+@transaction.atomic(durable=True)
 def try_add_realm_default_custom_profile_field(
     realm: Realm,
     field_subtype: str,
@@ -44,6 +45,7 @@ def try_add_realm_default_custom_profile_field(
     return custom_profile_field
 
 
+@transaction.atomic(durable=True)
 def try_add_realm_custom_profile_field(
     realm: Realm,
     name: str,
@@ -74,6 +76,7 @@ def try_add_realm_custom_profile_field(
     return custom_profile_field
 
 
+@transaction.atomic(durable=True)
 def do_remove_realm_custom_profile_field(realm: Realm, field: CustomProfileField) -> None:
     """
     Deleting a field will also delete the user profile data
@@ -98,6 +101,7 @@ def remove_custom_profile_field_value_if_required(
         CustomProfileFieldValue.objects.filter(field=field, value__in=removed_values).delete()
 
 
+@transaction.atomic(durable=True)
 def try_update_realm_custom_profile_field(
     realm: Realm,
     field: CustomProfileField,
@@ -133,6 +137,7 @@ def try_update_realm_custom_profile_field(
     notify_realm_custom_profile_fields(realm)
 
 
+@transaction.atomic(durable=True)
 def try_reorder_realm_custom_profile_fields(realm: Realm, order: Iterable[int]) -> None:
     order_mapping = {_[1]: _[0] for _ in enumerate(order)}
     custom_profile_fields = CustomProfileField.objects.filter(realm=realm)
diff --git a/zerver/tests/test_event_system.py b/zerver/tests/test_event_system.py
index 90e797daa3..582f404b85 100644
--- a/zerver/tests/test_event_system.py
+++ b/zerver/tests/test_event_system.py
@@ -580,9 +580,11 @@ class GetEventsTest(ZulipTestCase):
 
             client = allocate_client_descriptor(queue_data)
 
-            try_update_realm_custom_profile_field(
-                realm=user_profile.realm, field=profile_field, name=new_name
-            )
+            with self.captureOnCommitCallbacks(execute=True):
+                try_update_realm_custom_profile_field(
+                    realm=user_profile.realm, field=profile_field, name=new_name
+                )
+
             result = self.tornado_call(
                 get_events,
                 user_profile,