From 9a15c4e3ffbd6ed0ef5d296598c4e4e52fb577bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20H=C3=B6nig?= Date: Tue, 16 Jan 2018 20:34:12 +0100 Subject: [PATCH] Add bot services to page_params. This is the first step for allowing users to edit a bot's service entries, name the outgoing webhook configuration entries. The chosen data structures allow for a future with multiple services per bot; right now, only one service per bot is supported. --- zerver/lib/actions.py | 22 ++++++++++++++++++- zerver/models.py | 9 ++++++++ zerver/tests/test_bots.py | 44 +++++++++++++++++++++++++++++++++++++ zerver/tests/test_events.py | 36 +++++++++++++++++++++++++++++- zerver/tests/test_home.py | 3 ++- zerver/views/users.py | 10 ++++++++- 6 files changed, 120 insertions(+), 4 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 5e7203791a..4fd49d6550 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -71,7 +71,8 @@ from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity, custom_profile_fields_for_realm, get_huddle_user_ids, \ CustomProfileFieldValue, validate_attachment_request, get_system_bot, \ get_display_recipient_by_id, query_for_ids, get_huddle_recipient, \ - UserGroup, UserGroupMembership, get_default_stream_groups + UserGroup, UserGroupMembership, get_default_stream_groups, \ + get_service_dicts_for_bots, get_bot_services from zerver.lib.alert_words import alert_words_in_realm from zerver.lib.avatar import avatar_url @@ -416,6 +417,7 @@ def notify_created_bot(user_profile: UserProfile) -> None: default_events_register_stream=default_events_register_stream_name, default_all_public_streams=user_profile.default_all_public_streams, avatar_url=avatar_url(user_profile), + services = get_service_dicts_for_bots(user_profile.id), ) # Set the owner key only when the bot has an owner. @@ -4393,6 +4395,24 @@ def do_update_user_group_description(user_group: UserGroup, description: Text) - user_group.save(update_fields=['description']) do_send_user_group_update_event(user_group, dict(description=description)) +def do_update_outgoing_webhook_service(bot_profile, service_interface, service_payload_url): + # type: (UserProfile, int, Text) -> None + # TODO: First service is chosen because currently one bot can only have one service. + # Update this once multiple services are supported. + service = get_bot_services(bot_profile.id)[0] + service.base_url = service_payload_url + service.interface = service_interface + service.save() + send_event(dict(type='realm_bot', + op='update', + bot=dict(email=bot_profile.email, + user_id=bot_profile.id, + services = [dict(base_url=service.base_url, + interface=service.interface)], + ), + ), + bot_owner_user_ids(bot_profile)) + def do_send_user_group_members_update_event(event_name: Text, user_group: UserGroup, user_ids: List[int]) -> None: diff --git a/zerver/models.py b/zerver/models.py index c0a21103f3..5ff2d68a41 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1461,6 +1461,7 @@ def get_owned_bot_dicts(user_profile: UserProfile, 'default_all_public_streams': botdict['default_all_public_streams'], 'owner': botdict['bot_owner__email'], 'avatar_url': avatar_url_from_dict(botdict), + 'services': get_service_dicts_for_bots(botdict['id']), } for botdict in result] @@ -1921,6 +1922,14 @@ def get_realm_outgoing_webhook_services_name(realm: Realm) -> List[Any]: def get_bot_services(user_profile_id: str) -> List[Service]: return list(Service.objects.filter(user_profile__id=user_profile_id)) +def get_service_dicts_for_bots(user_profile_id: str) -> List[Dict[str, Any]]: + services = get_bot_services(user_profile_id) + service_dicts = [{'base_url': service.base_url, + 'interface': service.interface, + } + for service in services] + return service_dicts + def get_service_profile(user_profile_id: str, service_name: str) -> Service: return Service.objects.get(user_profile__id=user_profile_id, name=service_name) diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index 87f0b8082f..214691483c 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -141,6 +141,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): default_sending_stream=None, default_events_register_stream=None, default_all_public_streams=False, + services=[], owner=self.example_email('hamlet')) ), event['event'] @@ -302,6 +303,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): default_sending_stream='Denmark', default_events_register_stream=None, default_all_public_streams=False, + services=[], owner=self.example_email('hamlet')) ), event['event'] @@ -369,6 +371,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): default_sending_stream=None, default_events_register_stream='Denmark', default_all_public_streams=False, + services=[], owner=self.example_email('hamlet')) ), event['event'] @@ -932,6 +935,47 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): self.assert_json_error(result, 'No such user') self.assert_num_bots_equal(1) + def test_patch_outgoing_webhook_bot(self) -> None: + self.login(self.example_email('hamlet')) + bot_info = { + 'full_name': u'The Bot of Hamlet', + 'short_name': u'hambot', + 'bot_type': UserProfile.OUTGOING_WEBHOOK_BOT, + 'payload_url': ujson.dumps("http://foo.bar.com"), + 'service_interface': Service.GENERIC, + } + result = self.client_post("/json/bots", bot_info) + self.assert_json_success(result) + bot_info = { + 'service_payload_url': ujson.dumps("http://foo.bar2.com"), + 'service_interface': Service.SLACK, + } + result = self.client_patch("/json/bots/hambot-bot@zulip.testserver", bot_info) + self.assert_json_success(result) + + service_interface = ujson.loads(result.content)['service_interface'] + self.assertEqual(service_interface, Service.SLACK) + + service_payload_url = ujson.loads(result.content)['service_payload_url'] + self.assertEqual(service_payload_url, "http://foo.bar2.com") + + def test_outgoing_webhook_invalid_interface(self): + # type: () -> None + self.login(self.example_email('hamlet')) + bot_info = { + 'full_name': 'Outgoing Webhook test bot', + 'short_name': 'outgoingservicebot', + 'bot_type': UserProfile.OUTGOING_WEBHOOK_BOT, + 'payload_url': ujson.dumps('http://127.0.0.1:5002/bots/followup'), + 'interface_type': -1, + } + result = self.client_post("/json/bots", bot_info) + self.assert_json_error(result, 'Invalid interface type') + + bot_info['interface_type'] = Service.GENERIC + result = self.client_post("/json/bots", bot_info) + self.assert_json_success(result) + def test_create_outgoing_webhook_bot(self, **extras: Any) -> None: self.login(self.example_email('hamlet')) bot_info = { diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index a47dff0e43..54b81f4c74 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -73,6 +73,7 @@ from zerver.lib.actions import ( do_update_embedded_data, do_update_message, do_update_message_flags, + do_update_outgoing_webhook_service, do_update_pointer, do_update_user_presence, log_event, @@ -106,10 +107,11 @@ from zerver.lib.topic_mutes import ( ) from zerver.lib.validator import ( check_bool, check_dict, check_dict_only, check_float, check_int, check_list, check_string, - equals, check_none_or, Validator + equals, check_none_or, Validator, check_url ) from zerver.views.events_register import _default_all_public_streams, _default_narrow +from zerver.views.users import add_service from zerver.tornado.event_queue import ( allocate_client_descriptor, @@ -1534,6 +1536,10 @@ class EventsRegisterTest(ZulipTestCase): ('default_all_public_streams', check_bool), ('avatar_url', check_string), ('owner', check_string), + ('services', check_list(check_dict_only([ # type: ignore # check_url doesn't completely fit the default validator spec, but is de facto working here. + ('base_url', check_url), + ('interface', check_int), + ]))), ])), ]) action = lambda: self.create_bot('test-bot@zulip.com') @@ -1632,6 +1638,30 @@ class EventsRegisterTest(ZulipTestCase): error = change_bot_owner_checker('events[0]', events[0]) self.assert_on_error(error) + def test_do_update_outgoing_webhook_service(self): + # type: () -> None + update_outgoing_webhook_service_checker = self.check_events_dict([ + ('type', equals('realm_bot')), + ('op', equals('update')), + ('bot', check_dict_only([ + ('email', check_string), + ('user_id', check_int), + ('services', check_list(check_dict_only([ # type: ignore # check_url doesn't completely fit the default validator spec, but is de facto working here. + ('base_url', check_url), + ('interface', check_int), + ]))), + ])), + ]) + self.user_profile = self.example_user('iago') + bot = do_create_user('test-bot@zulip.com', '123', get_realm('zulip'), 'Test Bot', 'test', + bot_type=UserProfile.OUTGOING_WEBHOOK_BOT, bot_owner=self.user_profile) + add_service(user_profile=bot, name="test", base_url="http://hostname.domain1.com", + interface=1, token="abced") + action = lambda: do_update_outgoing_webhook_service(bot, 2, 'http://hostname.domain2.com') + events = self.do_test(action) + error = update_outgoing_webhook_service_checker('events[0]', events[0]) + self.assert_on_error(error) + def test_do_deactivate_user(self) -> None: bot_deactivate_checker = self.check_events_dict([ ('type', equals('realm_bot')), @@ -1664,6 +1694,10 @@ class EventsRegisterTest(ZulipTestCase): ('default_all_public_streams', check_bool), ('avatar_url', check_string), ('owner', check_none_or(check_string)), + ('services', check_list(check_dict_only([ # type: ignore # check_url doesn't completely fit the default validator spec, but is de facto working here. + ('base_url', check_url), + ('interface', check_int), + ]))), ])), ]) bot = self.create_bot('foo-bot@zulip.com') diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 373db68efd..99e74b91ee 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -183,7 +183,7 @@ class HomeTest(ZulipTestCase): with patch('zerver.lib.cache.cache_set') as cache_mock: result = self._get_home_page(stream='Denmark') - self.assert_length(queries, 41) + self.assert_length(queries, 42) self.assert_length(cache_mock.call_args_list, 7) html = result.content.decode('utf-8') @@ -211,6 +211,7 @@ class HomeTest(ZulipTestCase): 'full_name', 'is_active', 'owner', + 'services', 'user_id', ] diff --git a/zerver/views/users.py b/zerver/views/users.py index e8172f8d86..31712a87a1 100644 --- a/zerver/views/users.py +++ b/zerver/views/users.py @@ -15,7 +15,7 @@ from zerver.lib.actions import do_change_avatar_fields, do_change_bot_owner, \ do_change_is_admin, do_change_default_all_public_streams, \ do_change_default_events_register_stream, do_change_default_sending_stream, \ do_create_user, do_deactivate_user, do_reactivate_user, do_regenerate_api_key, \ - check_change_full_name, notify_created_bot + check_change_full_name, notify_created_bot, do_update_outgoing_webhook_service from zerver.lib.avatar import avatar_url, get_gravatar_url, get_avatar_field from zerver.lib.bot_config import set_bot_config from zerver.lib.exceptions import JsonableError @@ -156,6 +156,8 @@ def patch_bot_backend( request: HttpRequest, user_profile: UserProfile, email: Text, full_name: Optional[Text]=REQ(default=None), bot_owner: Optional[Text]=REQ(default=None), + service_payload_url: Optional[Text]=REQ(validator=check_url, default=None), + service_interface: Optional[int]=REQ(validator=check_int, default=1), default_sending_stream: Optional[Text]=REQ(default=None), default_events_register_stream: Optional[Text]=REQ(default=None), default_all_public_streams: Optional[bool]=REQ(default=None, validator=check_bool) @@ -190,6 +192,10 @@ def patch_bot_backend( if default_all_public_streams is not None: do_change_default_all_public_streams(bot, default_all_public_streams) + if service_payload_url is not None: + check_valid_interface_type(service_interface) + do_update_outgoing_webhook_service(bot, service_interface, service_payload_url) + if len(request.FILES) == 0: pass elif len(request.FILES) == 1: @@ -203,6 +209,8 @@ def patch_bot_backend( json_result = dict( full_name=bot.full_name, avatar_url=avatar_url(bot), + service_interface = service_interface, + service_payload_url = service_payload_url, default_sending_stream=get_stream_name(bot.default_sending_stream), default_events_register_stream=get_stream_name(bot.default_events_register_stream), default_all_public_streams=bot.default_all_public_streams,