users: Fix ordering issue with deactivating bots.

The new comment explains the issue in some detail, but basically if we
deactivate the bots first, then an error partway through is corrected
by a retry; if we deactivate the user first, then we may leak
undeactivated bots if a failure occurs.
This commit is contained in:
Tim Abbott 2021-03-30 09:21:29 -07:00
parent f329878376
commit 53ed759fc1
1 changed files with 10 additions and 7 deletions

View File

@ -1146,6 +1146,16 @@ def do_deactivate_user(
if not user_profile.is_active: if not user_profile.is_active:
return return
if _cascade:
# We need to deactivate bots before the target user, to ensure
# that a failure partway through this function cannot result
# in only the user being deactivated.
bot_profiles = UserProfile.objects.filter(
is_bot=True, is_active=True, bot_owner=user_profile
)
for profile in bot_profiles:
do_deactivate_user(profile, _cascade=False, acting_user=acting_user)
with transaction.atomic(): with transaction.atomic():
if user_profile.realm.is_zephyr_mirror_realm: # nocoverage if user_profile.realm.is_zephyr_mirror_realm: # nocoverage
# For zephyr mirror users, we need to make them a mirror dummy # For zephyr mirror users, we need to make them a mirror dummy
@ -1200,13 +1210,6 @@ def do_deactivate_user(
) )
send_event(user_profile.realm, event, bot_owner_user_ids(user_profile)) send_event(user_profile.realm, event, bot_owner_user_ids(user_profile))
if _cascade:
bot_profiles = UserProfile.objects.filter(
is_bot=True, is_active=True, bot_owner=user_profile
)
for profile in bot_profiles:
do_deactivate_user(profile, _cascade=False, acting_user=acting_user)
def do_deactivate_stream( def do_deactivate_stream(
stream: Stream, log: bool = True, acting_user: Optional[UserProfile] = None stream: Stream, log: bool = True, acting_user: Optional[UserProfile] = None