From ba009079466d898a8d219a3e80c5fcc7bdf15a39 Mon Sep 17 00:00:00 2001
From: Sahil Batra <sahil@zulip.com>
Date: Thu, 21 Apr 2022 19:34:52 +0530
Subject: [PATCH] bots: Re-parent bot while reactivating if original owner is
 deactivated.

Since the original owner is deactivated, we change the owner to the
admin who is reactivating the bot.
---
 tools/linter_lib/custom_check.py |  4 ++++
 zerver/actions/create_user.py    | 26 ++++++++++++++++++++++++++
 zerver/tests/test_bots.py        | 23 +++++++++++++++++++++++
 zerver/tests/test_events.py      | 13 +++++++++++++
 4 files changed, 66 insertions(+)

diff --git a/tools/linter_lib/custom_check.py b/tools/linter_lib/custom_check.py
index 30af9fb927..7f8e5e2ad4 100644
--- a/tools/linter_lib/custom_check.py
+++ b/tools/linter_lib/custom_check.py
@@ -255,6 +255,10 @@ python_rules = RuleList(
                     "zerver/actions/bots.py",
                     "user_profile.save()  # Can't use update_fields because of how the foreign key works.",
                 ),
+                (
+                    "zerver/actions/create_user.py",
+                    "user_profile.save()  # Can't use update_fields because of how the foreign key works.",
+                ),
             },
             "exclude": {"zerver/tests", "zerver/lib/create_user.py"},
             "good_lines": ['user_profile.save(update_fields=["pointer"])'],
diff --git a/zerver/actions/create_user.py b/zerver/actions/create_user.py
index 099fe1c3b0..90491b5259 100644
--- a/zerver/actions/create_user.py
+++ b/zerver/actions/create_user.py
@@ -606,6 +606,26 @@ def do_reactivate_user(user_profile: UserProfile, *, acting_user: Optional[UserP
             }
         ).decode(),
     )
+
+    bot_owner_changed = False
+    if (
+        user_profile.is_bot
+        and user_profile.bot_owner is not None
+        and not user_profile.bot_owner.is_active
+        and acting_user is not None
+    ):
+        previous_owner = user_profile.bot_owner
+        user_profile.bot_owner = acting_user
+        user_profile.save()  # Can't use update_fields because of how the foreign key works.
+        RealmAuditLog.objects.create(
+            realm=user_profile.realm,
+            acting_user=acting_user,
+            modified_user=user_profile,
+            event_type=RealmAuditLog.USER_BOT_OWNER_CHANGED,
+            event_time=event_time,
+        )
+        bot_owner_changed = True
+
     do_increment_logging_stat(
         user_profile.realm,
         COUNT_STATS["active_users_log:is_bot:day"],
@@ -620,6 +640,12 @@ def do_reactivate_user(user_profile: UserProfile, *, acting_user: Optional[UserP
     if user_profile.is_bot:
         notify_created_bot(user_profile)
 
+        if bot_owner_changed:
+            from zerver.actions.bots import send_bot_owner_update_events
+
+            assert acting_user is not None
+            send_bot_owner_update_events(user_profile, acting_user, previous_owner)
+
     subscribed_recipient_ids = Subscription.objects.filter(
         user_profile_id=user_profile.id, active=True, recipient__type=Recipient.STREAM
     ).values_list("recipient__type_id", flat=True)
diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py
index babffed2fa..11e074e8ca 100644
--- a/zerver/tests/test_bots.py
+++ b/zerver/tests/test_bots.py
@@ -849,6 +849,29 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
         profile = get_user(bot_email, bot_realm)
         self.assertEqual(profile.bot_type, UserProfile.DEFAULT_BOT)
 
+    def test_reactivating_bot_with_deactivated_owner(self) -> None:
+        self.login("hamlet")
+        bot_info = {
+            "full_name": "Test bot",
+            "short_name": "testbot",
+            "bot_type": "1",
+        }
+        result = self.client_post("/json/bots", bot_info)
+        bot_id = result.json()["user_id"]
+
+        test_bot = UserProfile.objects.get(id=bot_id, is_bot=True)
+        do_deactivate_user(test_bot, acting_user=None)
+
+        # Deactivate the bot owner
+        do_deactivate_user(self.example_user("hamlet"), acting_user=None)
+
+        self.login("iago")
+        result = self.client_post(f"/json/users/{bot_id}/reactivate")
+        self.assert_json_success(result)
+        test_bot = UserProfile.objects.get(id=bot_id, is_bot=True)
+        assert test_bot.bot_owner is not None
+        self.assertEqual(test_bot.bot_owner.id, self.example_user("iago").id)
+
     def test_patch_bot_full_name(self) -> None:
         self.login("hamlet")
         bot_info = {
diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py
index 0b9d9e2a11..5be5d19828 100644
--- a/zerver/tests/test_events.py
+++ b/zerver/tests/test_events.py
@@ -2445,6 +2445,19 @@ class NormalActionsTest(BaseAction):
         check_subscription_peer_add("events[2]", events[2])
         check_subscription_peer_add("events[3]", events[3])
 
+        do_deactivate_user(bot, acting_user=None)
+        do_deactivate_user(self.example_user("hamlet"), acting_user=None)
+
+        reset_email_visibility_to_everyone_in_zulip_realm()
+        bot.refresh_from_db()
+
+        self.user_profile = self.example_user("iago")
+        action = lambda: do_reactivate_user(bot, acting_user=self.example_user("iago"))
+        events = self.verify_action(action, num_events=6)
+        check_realm_bot_add("events[1]", events[1])
+        check_realm_bot_update("events[2]", events[2], "owner_id")
+        check_realm_user_update("events[3]", events[3], "bot_owner_id")
+
     def test_do_deactivate_realm(self) -> None:
         realm = self.user_profile.realm
         action = lambda: do_deactivate_realm(realm, acting_user=None)