bots: Do not reply in group PMs unless explicitly mentioned.

Previously, if a bot was accidentally added to a group PM, we would
have no option but to leave that conversation because the bot would
reply to all the messages sent in that conversation. This also has
potential to cause infinite loops in case two bots are added to a
group PM since they could keep on replying to each other's messages.

Fixes #551.
This commit is contained in:
Rohitt Vashishtha 2020-03-19 02:39:30 +05:30 committed by showell
parent fe78a363b0
commit 5c32054415
2 changed files with 30 additions and 6 deletions

View file

@ -268,7 +268,7 @@ def extract_query_without_mention(message: Dict[str, Any], client: ExternalBotHa
return None return None
def is_private_message_from_another_user(message_dict: Dict[str, Any], current_user_id: int) -> bool: def is_private_message_but_not_group_pm(message_dict: Dict[str, Any], current_user: ExternalBotHandler) -> bool:
""" """
Checks whether a message dict represents a PM from another user. Checks whether a message dict represents a PM from another user.
@ -276,9 +276,11 @@ def is_private_message_from_another_user(message_dict: Dict[str, Any], current_u
zulip/zulip project, so refactor with care. See the comments in zulip/zulip project, so refactor with care. See the comments in
extract_query_without_mention. extract_query_without_mention.
""" """
if message_dict['type'] == 'private': if not message_dict['type'] == 'private':
return current_user_id != message_dict['sender_id']
return False return False
is_message_from_self = current_user.user_id == message_dict['sender_id']
recipients = [x['email'] for x in message_dict['display_recipient'] if current_user.email != x['email']]
return len(recipients) == 1 and not is_message_from_self
def display_config_file_errors(error_msg: str, config_file: str) -> None: def display_config_file_errors(error_msg: str, config_file: str) -> None:
@ -346,7 +348,7 @@ def run_message_handler_for_bot(
# `mentioned` will be in `flags` if the bot is mentioned at ANY position # `mentioned` will be in `flags` if the bot is mentioned at ANY position
# (not necessarily the first @mention in the message). # (not necessarily the first @mention in the message).
is_mentioned = 'mentioned' in flags is_mentioned = 'mentioned' in flags
is_private_message = is_private_message_from_another_user(message, restricted_client.user_id) is_private_message = is_private_message_but_not_group_pm(message, restricted_client)
# Provide bots with a way to access the full, unstripped message # Provide bots with a way to access the full, unstripped message
message['full_content'] = message['content'] message['full_content'] = message['content']

View file

@ -4,7 +4,8 @@ from zulip_bots.lib import (
ExternalBotHandler, ExternalBotHandler,
StateHandler, StateHandler,
run_message_handler_for_bot, run_message_handler_for_bot,
extract_query_without_mention extract_query_without_mention,
is_private_message_but_not_group_pm
) )
import io import io
@ -209,6 +210,27 @@ class LibTest(TestCase):
message = {'content': "Not at start @**Alice|alice** Hello World"} message = {'content': "Not at start @**Alice|alice** Hello World"}
self.assertEqual(extract_query_without_mention(message, handler), None) self.assertEqual(extract_query_without_mention(message, handler), None)
def test_is_private_message_but_not_group_pm(self):
client = FakeClient()
handler = ExternalBotHandler(
client=client,
root_dir=None,
bot_details=None,
bot_config_file=None
)
message = {}
message['display_recipient'] = 'some stream'
message['type'] = 'stream'
self.assertFalse(is_private_message_but_not_group_pm(message, handler))
message['type'] = 'private'
message['display_recipient'] = [{'email': 'a1@b.com'}]
message['sender_id'] = handler.user_id
self.assertFalse(is_private_message_but_not_group_pm(message, handler))
message['sender_id'] = 0 # someone else
self.assertTrue(is_private_message_but_not_group_pm(message, handler))
message['display_recipient'] = [{'email': 'a1@b.com'}, {'email': 'a2@b.com'}]
self.assertFalse(is_private_message_but_not_group_pm(message, handler))
def _create_client_and_handler_for_file_upload(self): def _create_client_and_handler_for_file_upload(self):
client = FakeClient() client = FakeClient()
client.upload_file = MagicMock() client.upload_file = MagicMock()