Change Zulip onboarding, ask for the zuliprc file

Fixes: #202

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

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"

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": "[email protected]",
-            "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": "[email protected]",
-            "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"')

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": "[email protected]",
+            "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": "[email protected]",
+            "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": "[email protected]",
+            "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"])

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,

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)
 

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": "[email protected]",
+        "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": "[email protected]",
-            "api_key": "fake-key",
-            "mtype": "stream",
-            "to": "general",
-        }
-
         self.client.login(username="[email protected]", 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="[email protected]", 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": "[email protected]",
-            "api_key": "",
-            "mtype": "stream",
-            "to": "general",
-        }
+        payload = _get_payload(api_key="")
+        self.client.login(username="[email protected]", 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="[email protected]", 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": "[email protected]",
-            "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="[email protected]", 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="[email protected]", 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": "[email protected]",
-            "api_key": "fake-key",
-            "mtype": "stream",
-            "to": "",
-        }
-
+        payload = _get_payload(to="")
         self.client.login(username="[email protected]", 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):

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);
+        });
+    })
+
 });

templates/integrations/add_zulip.html

@@ -67,7 +67,8 @@
         <div class="col-sm-6">
             <span class="step-no"></span>
             <p>
-                Copy the displayed bot's credentials into the form below.
+                Download the bot's <code>zuliprc</code> file by clicking on the
+                <strong>cyan download icon</strong>, and upload it in the form below.
                 Also specify the stream or the private user you want {{ site_name }}
                 to post notifications to.
             </p>
@@ -87,44 +88,23 @@
 
     <form method="post" class="form-horizontal">
         {% csrf_token %}
+        <input type="hidden" name="api_key" id="zulip-api-key">
+        <input type="hidden" name="bot_email" id="zulip-bot-email">
+        <input type="hidden" name="site" id="zulip-site">
 
-        <div class="form-group {{ form.bot_email.css_classes }}">
-            <label for="bot-email" class="col-sm-2 control-label">Bot Email</label>
+        <div class="form-group">
+            <label for="zuliprc" class="col-sm-2 control-label">The zuliprc File</label>
             <div class="col-sm-4">
-                <input
-                    id="bot-email"
-                    type="text"
-                    class="form-control"
-                    name="bot_email"
-                    value="{{ form.bot_email.value|default:"" }}">
-
-                <div class="help-block">
-                {% if form.bot_email.errors %}
-                    {{ form.bot_email.errors|join:"" }}
-                {% else %}
-                    Example: [email protected]
+                <input id="zuliprc" type="file">
+                <div id="zuliprc-help" class="help-block">
+                {% if form.api_key.errors or form.bot_email.errors or form.site.errors %}
+                    Invalid file format.
                 {% endif %}
                 </div>
             </div>
         </div>
 
-        <div class="form-group {{ form.api_key.css_classes }}">
-            <label for="api-key" class="col-sm-2 control-label">API Key</label>
-            <div class="col-sm-4">
-                <input
-                    id="api-key"
-                    type="text"
-                    class="form-control"
-                    name="api_key"
-                    value="{{ form.api_key.value|default:"" }}">
 
-                {% if form.api_key.errors %}
-                <div class="help-block">
-                    {{ form.api_key.errors|join:"" }}
-                </div>
-                {% endif %}
-            </div>
-        </div>
 
         <div id="z-mtype-group" class="form-group {{ form.mtype.css_classes }}">
             <label class="col-sm-2 control-label">Post To</label>
@@ -178,7 +158,11 @@
 
         <div class="form-group">
             <div class="col-sm-offset-2 col-sm-10">
-                <button type="submit" class="btn btn-primary">Save Integration</button>
+                <button
+                    id="save-integration"
+                    type="submit"
+                    disabled
+                    class="btn btn-primary">Save Integration</button>
             </div>
         </div>
     </form>