diff --git a/zerver/actions/user_settings.py b/zerver/actions/user_settings.py index b948c844b3..eee0e0f04c 100644 --- a/zerver/actions/user_settings.py +++ b/zerver/actions/user_settings.py @@ -272,6 +272,7 @@ def check_change_bot_full_name( check_bot_name_available( realm_id=user_profile.realm_id, full_name=new_full_name, + is_activation=False, ) do_change_full_name(user_profile, new_full_name, acting_user) diff --git a/zerver/lib/users.py b/zerver/lib/users.py index 3d16dfec74..78b4f684ac 100644 --- a/zerver/lib/users.py +++ b/zerver/lib/users.py @@ -94,7 +94,7 @@ def check_full_name( # name (e.g. you can get there by reactivating a deactivated bot after # making a new bot with the same name). This is just a check designed # to make it unlikely to happen by accident. -def check_bot_name_available(realm_id: int, full_name: str) -> None: +def check_bot_name_available(realm_id: int, full_name: str, *, is_activation: bool) -> None: dup_exists = UserProfile.objects.filter( realm_id=realm_id, full_name=full_name.strip(), @@ -102,7 +102,12 @@ def check_bot_name_available(realm_id: int, full_name: str) -> None: ).exists() if dup_exists: - raise JsonableError(_("Name is already in use!")) + if is_activation: + raise JsonableError( + f'There is already an active bot named "{full_name}" in this organization. To reactivate this bot, you must rename or deactivate the other one first.' + ) + else: + raise JsonableError(_("Name is already in use!")) def check_short_name(short_name_raw: str) -> str: diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index eab39c9095..fb89ac5a84 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -669,6 +669,36 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): self.assert_json_error(result, "No such user") self.assert_num_bots_equal(1) + def test_activate_bot_with_duplicate_name(self) -> None: + self.login("iago") + # Create a bot and then deactivate it + original_name = "Hamlet" + bot_info = { + "full_name": original_name, + "short_name": "hambot", + } + result = self.client_post("/json/bots", bot_info) + self.assert_json_success(result) + bot_email = "hambot-bot@zulip.testserver" + bot = self.get_bot_user(bot_email) + do_deactivate_user(bot, False, acting_user=None) + self.assertFalse( + UserProfile.objects.filter(is_bot=True, id=bot.id, is_active=True).exists() + ) + + # Create another bot with the same name + bot_info2 = { + "full_name": original_name, + "short_name": "hambot2", + } + result = self.client_post("/json/bots", bot_info2) + self.assert_json_success(result) + result = self.client_post(f"/json/users/{bot.id}/reactivate") + self.assert_json_error( + result, + 'There is already an active bot named "Hamlet" in this organization. To reactivate this bot, you must rename or deactivate the other one first.', + ) + def test_bot_permissions(self) -> None: self.login("hamlet") self.assert_num_bots_equal(0) diff --git a/zerver/views/users.py b/zerver/views/users.py index 319461f203..37780238b5 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -184,6 +184,7 @@ def reactivate_user_backend( if target.is_bot: assert target.bot_type is not None check_bot_creation_policy(user_profile, target.bot_type) + check_bot_name_available(user_profile.realm_id, target.full_name, is_activation=True) do_reactivate_user(target, acting_user=user_profile) return json_success(request) @@ -526,6 +527,7 @@ def add_bot_backend( check_bot_name_available( realm_id=user_profile.realm_id, full_name=full_name, + is_activation=False, ) check_bot_creation_policy(user_profile, bot_type)