From e6ef34a96498ad53327590e392e371e1992bd7ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20H=C3=B6nig?= Date: Mon, 28 May 2018 17:06:57 +0200 Subject: [PATCH] botserver: Strip messages like we do in zulip-run-bot. Previously, messages weren't stripped at all. This caused most bots to break and send replies similar to "I didn't understand your command". Nobody noticed, because the tests were only validating that replies were sent, but not the content in them. Thus, this commit also adds tests to avoid further regressions. --- zulip_botserver/tests/server_test_lib.py | 1 + zulip_botserver/tests/test_server.py | 15 +++++++++------ zulip_botserver/zulip_botserver/server.py | 22 +++++++++++++++++----- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/zulip_botserver/tests/server_test_lib.py b/zulip_botserver/tests/server_test_lib.py index 8823772..97edca6 100644 --- a/zulip_botserver/tests/server_test_lib.py +++ b/zulip_botserver/tests/server_test_lib.py @@ -35,6 +35,7 @@ class BotServerTestCase(TestCase): server.app.config["BOT_HANDLERS"] = bot_handlers server.app.config["MESSAGE_HANDLERS"] = message_handlers + mock_ExternalBotHandler.return_value.full_name = "test" response = self.app.post(data=json.dumps(event)) # NOTE: Currently, assert_bot_server_response can only check the expected_response diff --git a/zulip_botserver/tests/test_server.py b/zulip_botserver/tests/test_server.py index 558f201..29c117e 100644 --- a/zulip_botserver/tests/test_server.py +++ b/zulip_botserver/tests/test_server.py @@ -30,8 +30,9 @@ class BotServerTests(BotServerTestCase): } self.assert_bot_server_response(available_bots=available_bots, bots_config=bots_config, - event=dict(message={'content': "test message"}, - bot_email='helloworld-bot@zulip.com'), + event=dict(message={'content': "@**test** test message"}, + bot_email='helloworld-bot@zulip.com', + trigger='mention'), expected_response="beep boop", check_success=True) @@ -50,8 +51,9 @@ class BotServerTests(BotServerTestCase): } } self.assert_bot_server_response(available_bots=available_bots, - event=dict(message={'content': "test message"}, - bot_email='helloworld-bot@zulip.com'), + event=dict(message={'content': "@**test** test message"}, + bot_email='helloworld-bot@zulip.com', + trigger='mention'), expected_response="beep boop", bots_config=bots_config, check_success=True) @@ -89,8 +91,9 @@ class BotServerTests(BotServerTestCase): "make sure you have set up the flaskbotrc file correctly.", lambda: self.assert_bot_server_response( available_bots=available_bots, - event=dict(message={'content': "test message"}, - bot_email='helloworld-bot@zulip.com'), + event=dict(message={'content': "@**test** test message"}, + bot_email='helloworld-bot@zulip.com', + trigger='mention'), bots_config=bots_config)) @mock.patch('sys.argv', ['zulip-bot-server', '--config-file', '/foo/bar/baz.conf']) diff --git a/zulip_botserver/zulip_botserver/server.py b/zulip_botserver/zulip_botserver/server.py index 0c3634c..680de18 100644 --- a/zulip_botserver/zulip_botserver/server.py +++ b/zulip_botserver/zulip_botserver/server.py @@ -113,12 +113,24 @@ def handle_bot() -> Union[str, BadRequest]: "configuration file. Do the emails in your flaskbotrc " "match the bot emails on the server?".format(event['bot_email'])) else: - lib_module = app.config.get("BOTS_LIB_MODULES", {}).get(bot) - bot_handler = app.config.get("BOT_HANDLERS", {}).get(bot) - message_handler = app.config.get("MESSAGE_HANDLERS", {}).get(bot) + lib_module = app.config.get("BOTS_LIB_MODULES", {})[bot] + bot_handler = app.config.get("BOT_HANDLERS", {})[bot] + message_handler = app.config.get("MESSAGE_HANDLERS", {})[bot] + is_mentioned = event['trigger'] == "mention" + is_private_message = event['trigger'] == "private_message" + message = event["message"] + message['full_content'] = message['content'] + # Strip at-mention botname from the message + if is_mentioned: + # message['content'] will be None when the bot's @-mention is not at the beginning. + # In that case, the message shall not be handled. + message['content'] = lib.extract_query_without_mention(message=message, client=bot_handler) + if message['content'] is None: + return json.dumps("") - message_handler.handle_message(message=event["message"], bot_handler=bot_handler) - return json.dumps("") + if is_private_message or is_mentioned: + message_handler.handle_message(message=message, bot_handler=bot_handler) + return json.dumps("") def main() -> None: