From af56df77233e75afb5ca97a6d0617c4f88686c66 Mon Sep 17 00:00:00 2001 From: Eeshan Garg Date: Tue, 13 Mar 2018 20:06:11 -0230 Subject: [PATCH] webhooks: Enable custom topics and default PM notifications. This commit adds a generic function called check_send_webhook_message that does the following: * If a stream is specified in the webhook URL, it sends a stream message, otherwise sends a PM to the owner of the bot. * In the case of a stream message, if a custom topic is specified in the webhook URL, it uses that topic as the subject of the stream message. Also, note that we need not test this anywhere except for the helloworld webhook. Since helloworld is our default example for webhooks, it is here to stay and it made sense that tests for a generic function such as check_send_webhook_message be tested with an actual generic webhook! Fixes #8607. --- templates/zerver/api/webhook-walkthrough.md | 43 ++++++++++++--------- zerver/lib/actions.py | 1 + zerver/lib/webhooks/common.py | 24 ++++++++++++ zerver/webhooks/helloworld/tests.py | 21 +++++++++- zerver/webhooks/helloworld/view.py | 14 ++++--- zilencer/management/commands/populate_db.py | 8 +++- 6 files changed, 84 insertions(+), 27 deletions(-) create mode 100644 zerver/lib/webhooks/common.py diff --git a/templates/zerver/api/webhook-walkthrough.md b/templates/zerver/api/webhook-walkthrough.md index dbcbcb8b17..7f80ac1459 100644 --- a/templates/zerver/api/webhook-walkthrough.md +++ b/templates/zerver/api/webhook-walkthrough.md @@ -58,23 +58,25 @@ python file, `zerver/webhooks/mywebhook/view.py`. The Hello World integration is in `zerver/webhooks/helloworld/view.py`: ``` -from django.utils.translation import ugettext as _ -from zerver.lib.actions import check_send_stream_message -from zerver.lib.response import json_success, json_error -from zerver.decorator import REQ, has_request_variables, api_key_only_webhook_view -from zerver.lib.validator import check_dict, check_string - -from zerver.models import Client, UserProfile +from typing import Any, Dict, Iterable, Optional, Text from django.http import HttpRequest, HttpResponse -from typing import Dict, Any, Iterable, Optional, Text +from django.utils.translation import ugettext as _ + +from zerver.decorator import api_key_only_webhook_view +from zerver.lib.webhooks.common import check_send_webhook_message +from zerver.lib.request import REQ, has_request_variables +from zerver.lib.response import json_error, json_success +from zerver.lib.validator import check_dict, check_string +from zerver.models import UserProfile @api_key_only_webhook_view('HelloWorld') @has_request_variables -def api_helloworld_webhook(request: HttpRequest, user_profile: UserProfile, - payload: Dict[str, Iterable[Dict[str, Any]]]=REQ(argument_type='body'), - stream: Text=REQ(default='test'), - topic: Text=REQ(default='Hello World')) -> HttpResponse: +def api_helloworld_webhook( + request: HttpRequest, user_profile: UserProfile, + payload: Dict[str, Iterable[Dict[str, Any]]]=REQ(argument_type='body') +) -> HttpResponse: + # construct the body of the message body = 'Hello! I am happy to be here! :smile:' @@ -82,11 +84,11 @@ def api_helloworld_webhook(request: HttpRequest, user_profile: UserProfile, body_template = '\nThe Wikipedia featured article for today is **[{featured_title}]({featured_url})**' body += body_template.format(**payload) - # send the message - check_send_stream_message(user_profile, request.client, - stream, topic, body) + topic = "Hello World" + + # send the message + check_send_webhook_message(request, user_profile, topic, body) - # return json result return json_success() ``` @@ -136,8 +138,13 @@ link to the Wikipedia article of the day as provided by the json payload. integration checks for. In such a case, any `KeyError` thrown is handled by the server backend and will create an appropriate response. -Then we send a public (stream) message with `check_send_stream_message` which will -validate the message and then send it. +Then we send a message with `check_send_webhook_message`, which will +validate the message and do the following: + +* Send a public (stream) message if the `stream` query parameter is + specified in the webhook URL. +* If the `stream` query parameter isn't specified, it will send a private + message to the owner of the webhook bot. Finally, we return a 200 http status with a JSON format success message via `json_success()`. diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index bb9eeeb989..f2840e1b78 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -13,6 +13,7 @@ from django.conf import settings from django.core import validators from analytics.lib.counts import COUNT_STATS, do_increment_logging_stat, \ RealmCount + from zerver.lib.bugdown import ( BugdownRenderingException, version as bugdown_version, diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py new file mode 100644 index 0000000000..aa8e41328f --- /dev/null +++ b/zerver/lib/webhooks/common.py @@ -0,0 +1,24 @@ +from django.http import HttpRequest +from typing import Optional, Text + +from zerver.lib.actions import check_send_stream_message, \ + check_send_private_message +from zerver.lib.request import REQ, has_request_variables +from zerver.models import UserProfile + +@has_request_variables +def check_send_webhook_message( + request: HttpRequest, user_profile: UserProfile, + topic: Text, body: Text, stream: Optional[Text]=REQ(default=None), + user_specified_topic: Optional[Text]=REQ("topic", default=None) +) -> None: + + if stream is None: + assert user_profile.bot_owner is not None + check_send_private_message(user_profile, request.client, + user_profile.bot_owner, body) + else: + if user_specified_topic is not None: + topic = user_specified_topic + check_send_stream_message(user_profile, request.client, + stream, topic, body) diff --git a/zerver/webhooks/helloworld/tests.py b/zerver/webhooks/helloworld/tests.py index f9e0a8cf79..4971658065 100644 --- a/zerver/webhooks/helloworld/tests.py +++ b/zerver/webhooks/helloworld/tests.py @@ -5,7 +5,8 @@ from zerver.lib.test_classes import WebhookTestCase class HelloWorldHookTests(WebhookTestCase): STREAM_NAME = 'test' - URL_TEMPLATE = "/api/v1/external/helloworld?&api_key={api_key}" + URL_TEMPLATE = "/api/v1/external/helloworld?&api_key={api_key}&stream={stream}" + PM_URL_TEMPLATE = "/api/v1/external/helloworld?&api_key={api_key}" FIXTURE_DIR_NAME = 'hello' # Note: Include a test function per each distinct message condition your integration supports @@ -25,5 +26,23 @@ class HelloWorldHookTests(WebhookTestCase): self.send_and_test_stream_message('goodbye', expected_subject, expected_message, content_type="application/x-www-form-urlencoded") + def test_pm_to_bot_owner(self) -> None: + # Note that this is really just a test for check_send_webhook_message + self.URL_TEMPLATE = self.PM_URL_TEMPLATE + self.url = self.build_webhook_url() + expected_message = u"Hello! I am happy to be here! :smile:\nThe Wikipedia featured article for today is **[Goodbye](https://en.wikipedia.org/wiki/Goodbye)**" + + self.send_and_test_private_message('goodbye', expected_message=expected_message, + content_type="application/x-www-form-urlencoded") + + def test_custom_topic(self) -> None: + # Note that this is really just a test for check_send_webhook_message + expected_subject = u"Custom Topic" + self.url = self.build_webhook_url(topic=expected_subject) + expected_message = u"Hello! I am happy to be here! :smile:\nThe Wikipedia featured article for today is **[Goodbye](https://en.wikipedia.org/wiki/Goodbye)**" + + self.send_and_test_stream_message('goodbye', expected_subject, expected_message, + content_type="application/x-www-form-urlencoded") + def get_body(self, fixture_name: Text) -> Text: return self.fixture_data("helloworld", fixture_name, file_type="json") diff --git a/zerver/webhooks/helloworld/view.py b/zerver/webhooks/helloworld/view.py index 3127f89c55..4e65ae39f3 100644 --- a/zerver/webhooks/helloworld/view.py +++ b/zerver/webhooks/helloworld/view.py @@ -5,7 +5,7 @@ from django.http import HttpRequest, HttpResponse from django.utils.translation import ugettext as _ from zerver.decorator import api_key_only_webhook_view -from zerver.lib.actions import check_send_stream_message +from zerver.lib.webhooks.common import check_send_webhook_message from zerver.lib.request import REQ, has_request_variables from zerver.lib.response import json_error, json_success from zerver.lib.validator import check_dict, check_string @@ -13,10 +13,10 @@ from zerver.models import UserProfile @api_key_only_webhook_view('HelloWorld') @has_request_variables -def api_helloworld_webhook(request: HttpRequest, user_profile: UserProfile, - payload: Dict[str, Iterable[Dict[str, Any]]]=REQ(argument_type='body'), - stream: Text=REQ(default='test'), - topic: Text=REQ(default='Hello World')) -> HttpResponse: +def api_helloworld_webhook( + request: HttpRequest, user_profile: UserProfile, + payload: Dict[str, Iterable[Dict[str, Any]]]=REQ(argument_type='body') +) -> HttpResponse: # construct the body of the message body = 'Hello! I am happy to be here! :smile:' @@ -25,7 +25,9 @@ def api_helloworld_webhook(request: HttpRequest, user_profile: UserProfile, body_template = '\nThe Wikipedia featured article for today is **[{featured_title}]({featured_url})**' body += body_template.format(**payload) + topic = "Hello World" + # send the message - check_send_stream_message(user_profile, request.client, stream, topic, body) + check_send_webhook_message(request, user_profile, topic, body) return json_success() diff --git a/zilencer/management/commands/populate_db.py b/zilencer/management/commands/populate_db.py index 87ecdc4728..9570d4c8a3 100644 --- a/zilencer/management/commands/populate_db.py +++ b/zilencer/management/commands/populate_db.py @@ -181,15 +181,19 @@ class Command(BaseCommand): zulip_realm_bots.extend(all_realm_bots) create_users(zulip_realm, zulip_realm_bots, bot_type=UserProfile.DEFAULT_BOT) + zoe = get_user("zoe@zulip.com", zulip_realm) zulip_webhook_bots = [ ("Zulip Webhook Bot", "webhook-bot@zulip.com"), ] + # If a stream is not supplied in the webhook URL, the webhook + # will (in some cases) send the notification as a PM to the + # owner of the webhook bot, so bot_owner can't be None create_users(zulip_realm, zulip_webhook_bots, - bot_type=UserProfile.INCOMING_WEBHOOK_BOT) + bot_type=UserProfile.INCOMING_WEBHOOK_BOT, bot_owner=zoe) + aaron = get_user("AARON@zulip.com", zulip_realm) zulip_outgoing_bots = [ ("Outgoing Webhook", "outgoing-webhook@zulip.com") ] - aaron = get_user("AARON@zulip.com", zulip_realm) create_users(zulip_realm, zulip_outgoing_bots, bot_type=UserProfile.OUTGOING_WEBHOOK_BOT, bot_owner=aaron) # TODO: Clean up this initial bot creation code