diff --git a/CHANGELOG.md b/CHANGELOG.md index c7e373fc..eeb57a6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file. - Update the email notification template to include more check and last ping details - Improve the crontab snippet in the "Check Details" page (#465) - Add Signal integration (#428) +- Change Zulip onboarding, ask for the zuliprc file (#202) ## Bug Fixes - Fix unwanted HTML escaping in SMS and WhatsApp notifications diff --git a/hc/api/models.py b/hc/api/models.py index 8d7ea80a..0e684001 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -754,6 +754,18 @@ class Channel(models.Model): doc = json.loads(self.value) return doc["bot_email"] + @property + def zulip_site(self): + assert self.kind == "zulip" + doc = json.loads(self.value) + if "site" in doc: + return doc["site"] + + # Fallback if we don't have the site value: + # derive it from bot's email + _, domain = doc["bot_email"].split("@") + return "https://" + domain + @property def zulip_api_key(self): assert self.kind == "zulip" diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index acb730fd..ec7dca5b 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -783,38 +783,3 @@ class NotifyTestCase(BaseTestCase): n = Notification.objects.get() self.assertEqual(n.error, "Shell commands are not enabled") - - @patch("hc.api.transports.requests.request") - def test_zulip(self, mock_post): - definition = { - "bot_email": "bot@example.org", - "api_key": "fake-key", - "mtype": "stream", - "to": "general", - } - self._setup_data("zulip", json.dumps(definition)) - mock_post.return_value.status_code = 200 - - self.channel.notify(self.check) - assert Notification.objects.count() == 1 - - args, kwargs = mock_post.call_args - payload = kwargs["data"] - self.assertIn("DOWN", payload["topic"]) - - @patch("hc.api.transports.requests.request") - def test_zulip_returns_error(self, mock_post): - definition = { - "bot_email": "bot@example.org", - "api_key": "fake-key", - "mtype": "stream", - "to": "general", - } - self._setup_data("zulip", json.dumps(definition)) - mock_post.return_value.status_code = 403 - mock_post.return_value.json.return_value = {"msg": "Nice try"} - - self.channel.notify(self.check) - - n = Notification.objects.first() - self.assertEqual(n.error, 'Received status code 403 with a message: "Nice try"') diff --git a/hc/api/tests/test_notify_zulip.py b/hc/api/tests/test_notify_zulip.py new file mode 100644 index 00000000..b4a44ab9 --- /dev/null +++ b/hc/api/tests/test_notify_zulip.py @@ -0,0 +1,86 @@ +# coding: utf-8 + +from datetime import timedelta as td +import json +from unittest.mock import patch + +from django.utils.timezone import now +from hc.api.models import Channel, Check, Notification +from hc.test import BaseTestCase + + +class NotifyTestCase(BaseTestCase): + def _setup_data(self, kind, value, status="down", email_verified=True): + self.check = Check(project=self.project) + self.check.status = status + self.check.last_ping = now() - td(minutes=61) + self.check.save() + + self.channel = Channel(project=self.project) + self.channel.kind = kind + self.channel.value = value + self.channel.email_verified = email_verified + self.channel.save() + self.channel.checks.add(self.check) + + @patch("hc.api.transports.requests.request") + def test_zulip(self, mock_post): + definition = { + "bot_email": "bot@example.org", + "api_key": "fake-key", + "mtype": "stream", + "to": "general", + } + self._setup_data("zulip", json.dumps(definition)) + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + assert Notification.objects.count() == 1 + + args, kwargs = mock_post.call_args + + method, url = args + self.assertEqual(url, "https://example.org/api/v1/messages") + + payload = kwargs["data"] + self.assertIn("DOWN", payload["topic"]) + + @patch("hc.api.transports.requests.request") + def test_zulip_returns_error(self, mock_post): + definition = { + "bot_email": "bot@example.org", + "api_key": "fake-key", + "mtype": "stream", + "to": "general", + } + self._setup_data("zulip", json.dumps(definition)) + mock_post.return_value.status_code = 403 + mock_post.return_value.json.return_value = {"msg": "Nice try"} + + self.channel.notify(self.check) + + n = Notification.objects.first() + self.assertEqual(n.error, 'Received status code 403 with a message: "Nice try"') + + @patch("hc.api.transports.requests.request") + def test_zulip_uses_site_parameter(self, mock_post): + definition = { + "bot_email": "bot@example.org", + "site": "https://custom.example.org", + "api_key": "fake-key", + "mtype": "stream", + "to": "general", + } + self._setup_data("zulip", json.dumps(definition)) + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + assert Notification.objects.count() == 1 + + args, kwargs = mock_post.call_args + + method, url = args + self.assertEqual(url, "https://custom.example.org/api/v1/messages") + + payload = kwargs["data"] + self.assertIn("DOWN", payload["topic"]) diff --git a/hc/api/transports.py b/hc/api/transports.py index 4b241af5..4b6c3c34 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -629,8 +629,7 @@ class Zulip(HttpTransport): pass def notify(self, check): - _, domain = self.channel.zulip_bot_email.split("@") - url = "https://%s/api/v1/messages" % domain + url = self.channel.zulip_site + "/api/v1/messages" auth = (self.channel.zulip_bot_email, self.channel.zulip_api_key) data = { "type": self.channel.zulip_type, diff --git a/hc/front/forms.py b/hc/front/forms.py index 8978eb57..8babe1d0 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -274,6 +274,7 @@ class AddZulipForm(forms.Form): error_css_class = "has-error" bot_email = forms.EmailField(max_length=100) api_key = forms.CharField(max_length=50) + site = forms.URLField(max_length=100, validators=[WebhookValidator()]) mtype = forms.ChoiceField(choices=ZULIP_TARGETS) to = forms.CharField(max_length=100) diff --git a/hc/front/tests/test_add_zulip.py b/hc/front/tests/test_add_zulip.py index 860a5082..aac10089 100644 --- a/hc/front/tests/test_add_zulip.py +++ b/hc/front/tests/test_add_zulip.py @@ -2,6 +2,19 @@ from hc.api.models import Channel from hc.test import BaseTestCase +def _get_payload(**kwargs): + payload = { + "bot_email": "foo@example.org", + "api_key": "fake-key", + "site": "https://example.org", + "mtype": "stream", + "to": "general", + } + + payload.update(kwargs) + return payload + + class AddZulipTestCase(BaseTestCase): def setUp(self): super().setUp() @@ -13,15 +26,8 @@ class AddZulipTestCase(BaseTestCase): self.assertContains(r, "open-source group chat app") def test_it_works(self): - form = { - "bot_email": "foo@example.org", - "api_key": "fake-key", - "mtype": "stream", - "to": "general", - } - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) + r = self.client.post(self.url, _get_payload()) self.assertRedirects(r, self.channels_url) c = Channel.objects.get() @@ -32,51 +38,39 @@ class AddZulipTestCase(BaseTestCase): self.assertEqual(c.zulip_to, "general") def test_it_rejects_bad_email(self): - form = { - "bot_email": "not@an@email", - "api_key": "fake-key", - "mtype": "stream", - "to": "general", - } - + payload = _get_payload(bot_email="not@an@email") self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) - self.assertContains(r, "Enter a valid email address.") + r = self.client.post(self.url, payload) + self.assertContains(r, "Invalid file format.") def test_it_rejects_missing_api_key(self): - form = { - "bot_email": "foo@example.org", - "api_key": "", - "mtype": "stream", - "to": "general", - } + payload = _get_payload(api_key="") + self.client.login(username="alice@example.org", password="password") + r = self.client.post(self.url, payload) + self.assertContains(r, "Invalid file format.") + def test_it_rejects_missing_site(self): + payload = _get_payload(site="") self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) - self.assertContains(r, "This field is required.") + r = self.client.post(self.url, payload) + self.assertContains(r, "Invalid file format.") - def test_it_rejects_bad_mtype(self): - form = { - "bot_email": "foo@example.org", - "api_key": "fake-key", - "mtype": "this-should-not-work", - "to": "general", - } + def test_it_rejects_malformed_site(self): + payload = _get_payload(site="not-an-url") + self.client.login(username="alice@example.org", password="password") + r = self.client.post(self.url, payload) + self.assertContains(r, "Invalid file format.") + def test_it_rejects_bad_mtype(self): + payload = _get_payload(mtype="this-should-not-work") self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) + r = self.client.post(self.url, payload) self.assertEqual(r.status_code, 200) def test_it_rejects_missing_stream_name(self): - form = { - "bot_email": "foo@example.org", - "api_key": "fake-key", - "mtype": "stream", - "to": "", - } - + payload = _get_payload(to="") self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, form) + r = self.client.post(self.url, payload) self.assertContains(r, "This field is required.") def test_it_requires_rw_access(self): diff --git a/static/img/integrations/setup_zulip_3.png b/static/img/integrations/setup_zulip_3.png index f5507731..c4e557a5 100644 Binary files a/static/img/integrations/setup_zulip_3.png and b/static/img/integrations/setup_zulip_3.png differ diff --git a/static/js/add_zulip.js b/static/js/add_zulip.js index 0c18e6a4..7a4dc018 100644 --- a/static/js/add_zulip.js +++ b/static/js/add_zulip.js @@ -14,4 +14,25 @@ $(function() { // Update form labels when user clicks on radio buttons $('input[type=radio][name=mtype]').change(updateForm); + $("#zuliprc").change(function() { + this.files[0].text().then(function(contents) { + var keyMatch = contents.match(/key=(.*)/); + var emailMatch = contents.match(/email=(.*@.*)/); + var siteMatch = contents.match(/site=(.*)/); + + if (!keyMatch || !emailMatch || !siteMatch) { + $("#zuliprc-help").text("Invalid file format."); + $("#save-integration").prop("disabled", true); + return + } + + $("#zulip-api-key").val(keyMatch[1]); + $("#zulip-bot-email").val(emailMatch[1]); + $("#zulip-site").val(siteMatch[1]); + $("#zuliprc-help").text(""); + + $("#save-integration").prop("disabled", false); + }); + }) + }); diff --git a/templates/integrations/add_zulip.html b/templates/integrations/add_zulip.html index 154c9472..1f4dc582 100644 --- a/templates/integrations/add_zulip.html +++ b/templates/integrations/add_zulip.html @@ -67,7 +67,8 @@

- Copy the displayed bot's credentials into the form below. + Download the bot's zuliprc file by clicking on the + cyan download icon, and upload it in the form below. Also specify the stream or the private user you want {{ site_name }} to post notifications to.

@@ -87,44 +88,23 @@
{% csrf_token %} + + + -
- +
+
- - -
- {% if form.bot_email.errors %} - {{ form.bot_email.errors|join:"" }} - {% else %} - Example: healthchecks-bot@zulip.mycompany.com + +
+ {% if form.api_key.errors or form.bot_email.errors or form.site.errors %} + Invalid file format. {% endif %}
-
- -
- - {% if form.api_key.errors %} -
- {{ form.api_key.errors|join:"" }} -
- {% endif %} -
-
@@ -178,7 +158,11 @@
- +