From 3ccd53ce20c799debec45e1c806704c4e2589c10 Mon Sep 17 00:00:00 2001
From: Mateusz Mandera <mateusz.mandera@zulip.com>
Date: Wed, 9 Oct 2024 20:47:36 +0200
Subject: [PATCH] custom_profile_fields: Make transaction no longer durable.

Fixes ##31935.

do_update_user_custom_profile_data_if_change can't be durable as it's
invoked within `sync_ldap_user_data`, which is already in
transaction.atomic.

This change requires a few additional tweaks to untangle other related
transactions. The top level view functions up the codepath now use
durable=True. check_remove_custom_profile_field_value is called inside
do_update_user, so it no longer can be durable and should be switched to
savepoint=False. In turn, its remaining caller - the view
remove_user_custom_profile_data - gets switched to durable=True.
---
 zerver/actions/custom_profile_fields.py |  4 ++--
 zerver/views/custom_profile_fields.py   | 12 ++++++++----
 zerver/views/users.py                   |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/zerver/actions/custom_profile_fields.py b/zerver/actions/custom_profile_fields.py
index 34db9d869a..d26a5426d1 100644
--- a/zerver/actions/custom_profile_fields.py
+++ b/zerver/actions/custom_profile_fields.py
@@ -169,7 +169,7 @@ def notify_user_update_custom_profile_data(
     send_event_on_commit(user_profile.realm, event, get_user_ids_who_can_access_user(user_profile))
 
 
-@transaction.atomic(durable=True)
+@transaction.atomic(savepoint=False)
 def do_update_user_custom_profile_data_if_changed(
     user_profile: UserProfile,
     data: list[ProfileDataElementUpdateDict],
@@ -210,7 +210,7 @@ def do_update_user_custom_profile_data_if_changed(
         )
 
 
-@transaction.atomic(durable=True)
+@transaction.atomic(savepoint=False)
 def check_remove_custom_profile_field_value(
     user_profile: UserProfile, field_id: int, acting_user: UserProfile
 ) -> None:
diff --git a/zerver/views/custom_profile_fields.py b/zerver/views/custom_profile_fields.py
index 68d3c55290..f8b3d62b52 100644
--- a/zerver/views/custom_profile_fields.py
+++ b/zerver/views/custom_profile_fields.py
@@ -2,7 +2,7 @@ from typing import Annotated
 
 import orjson
 from django.core.exceptions import ValidationError
-from django.db import IntegrityError
+from django.db import IntegrityError, transaction
 from django.http import HttpRequest, HttpResponse
 from django.utils.translation import gettext as _
 from pydantic import Json, StringConstraints
@@ -307,8 +307,11 @@ def remove_user_custom_profile_data(
     *,
     data: Json[list[int]],
 ) -> HttpResponse:
-    for field_id in data:
-        check_remove_custom_profile_field_value(user_profile, field_id, acting_user=user_profile)
+    with transaction.atomic(durable=True):
+        for field_id in data:
+            check_remove_custom_profile_field_value(
+                user_profile, field_id, acting_user=user_profile
+            )
     return json_success(request)
 
 
@@ -321,6 +324,7 @@ def update_user_custom_profile_data(
     data: Json[list[ProfileDataElementUpdateDict]],
 ) -> HttpResponse:
     validate_user_custom_profile_data(user_profile.realm.id, data, acting_user=user_profile)
-    do_update_user_custom_profile_data_if_changed(user_profile, data)
+    with transaction.atomic(durable=True):
+        do_update_user_custom_profile_data_if_changed(user_profile, data)
     # We need to call this explicitly otherwise constraints are not check
     return json_success(request)
diff --git a/zerver/views/users.py b/zerver/views/users.py
index 79696aa933..ce69de8feb 100644
--- a/zerver/views/users.py
+++ b/zerver/views/users.py
@@ -5,6 +5,7 @@ from typing import Annotated, Any, TypeAlias
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
 from django.core.files.uploadedfile import UploadedFile
+from django.db import transaction
 from django.http import HttpRequest, HttpResponse
 from django.shortcuts import redirect
 from django.utils.translation import gettext as _
@@ -197,6 +198,7 @@ class ProfileDataElement(BaseModel):
 
 
 @typed_endpoint
+@transaction.atomic(durable=True)
 def update_user_backend(
     request: HttpRequest,
     user_profile: UserProfile,