diff --git a/CHANGELOG.md b/CHANGELOG.md index 746e5119..8352086c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes to this project will be documented in this file. - Upgrade to cronsim 2.3 - Add support for the $BODY placeholder in webhook payloads (#708) - Implement the "Clear Events" function +- Add support for custom topics in Zulip notifications (#583) ### Bug Fixes - Fix the handling of TooManyRedirects exceptions diff --git a/hc/api/models.py b/hc/api/models.py index 92e8adc4..bb7ff82e 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -952,6 +952,11 @@ class Channel(models.Model): assert self.kind == "zulip" return self.json["to"] + @property + def zulip_topic(self): + assert self.kind == "zulip" + return self.json.get("topic", "") + @property def linenotify_token(self): assert self.kind == "linenotify" diff --git a/hc/api/tests/test_notify_zulip.py b/hc/api/tests/test_notify_zulip.py index 3a057c27..ce47e052 100644 --- a/hc/api/tests/test_notify_zulip.py +++ b/hc/api/tests/test_notify_zulip.py @@ -20,18 +20,21 @@ class NotifyZulipTestCase(BaseTestCase): self.check.last_ping = now() - td(minutes=61) self.check.save() - definition = { + self.channel = Channel(project=self.project) + self.channel.kind = "zulip" + self.channel.value = json.dumps(self.definition()) + self.channel.save() + self.channel.checks.add(self.check) + + def definition(self, **kwargs): + d = { "bot_email": "bot@example.org", "api_key": "fake-key", "mtype": "stream", "to": "general", } - - self.channel = Channel(project=self.project) - self.channel.kind = "zulip" - self.channel.value = json.dumps(definition) - self.channel.save() - self.channel.checks.add(self.check) + d.update(kwargs) + return d @patch("hc.api.transports.curl.request") def test_it_works(self, mock_post): @@ -51,6 +54,18 @@ class NotifyZulipTestCase(BaseTestCase): serialized = json.dumps(payload) self.assertNotIn(str(self.check.code), serialized) + @patch("hc.api.transports.curl.request") + def test_it_uses_custom_topic(self, mock_post): + self.channel.value = json.dumps(self.definition(topic="foo")) + self.channel.save() + + mock_post.return_value.status_code = 200 + self.channel.notify(self.check) + + args, kwargs = mock_post.call_args + payload = kwargs["data"] + self.assertEqual(payload["topic"], "foo") + @patch("hc.api.transports.curl.request") def test_it_returns_error(self, mock_post): mock_post.return_value.status_code = 403 diff --git a/hc/api/transports.py b/hc/api/transports.py index ffd0d373..48a4ad42 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -262,7 +262,9 @@ class HttpTransport(Transport): class Webhook(HttpTransport): - def prepare(self, template: str, check, urlencode=False, latin1=False, allow_ping_body=False) -> str: + def prepare( + self, template: str, check, urlencode=False, latin1=False, allow_ping_body=False + ) -> str: """Replace variables with actual values.""" def safe(s: str) -> str: @@ -821,12 +823,16 @@ class Zulip(HttpTransport): if not settings.ZULIP_ENABLED: raise TransportError("Zulip notifications are not enabled.") + topic = self.channel.zulip_topic + if not topic: + topic = tmpl("zulip_topic.html", check=check) + 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, "to": self.channel.zulip_to, - "topic": tmpl("zulip_topic.html", check=check), + "topic": topic, "content": tmpl("zulip_content.html", check=check), } diff --git a/hc/front/forms.py b/hc/front/forms.py index 44d5088d..5682bff1 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -311,6 +311,7 @@ class AddZulipForm(forms.Form): site = forms.URLField(max_length=100, validators=[WebhookValidator()]) mtype = forms.ChoiceField(choices=ZULIP_TARGETS) to = forms.CharField(max_length=100) + topic = forms.CharField(max_length=100, required=False) def get_value(self): return json.dumps(dict(self.cleaned_data), sort_keys=True) @@ -349,3 +350,7 @@ class SeekForm(forms.Form): def clean_end(self): return datetime.fromtimestamp(self.cleaned_data["end"], tz=timezone.utc) + + +class TransferForm(forms.Form): + project = forms.UUIDField() diff --git a/hc/front/tests/test_add_zulip.py b/hc/front/tests/test_add_zulip.py index 3f6ecc1c..7800078d 100644 --- a/hc/front/tests/test_add_zulip.py +++ b/hc/front/tests/test_add_zulip.py @@ -10,6 +10,7 @@ def _get_payload(**kwargs): "site": "https://example.org", "mtype": "stream", "to": "general", + "topic": "foo", } payload.update(kwargs) @@ -37,6 +38,7 @@ class AddZulipTestCase(BaseTestCase): self.assertEqual(c.zulip_api_key, "fake-key") self.assertEqual(c.zulip_type, "stream") self.assertEqual(c.zulip_to, "general") + self.assertEqual(c.zulip_topic, "foo") def test_it_rejects_bad_email(self): payload = _get_payload(bot_email="not@an@email") diff --git a/hc/front/tests/test_transfer.py b/hc/front/tests/test_transfer.py index 29a2f901..2e35dceb 100644 --- a/hc/front/tests/test_transfer.py +++ b/hc/front/tests/test_transfer.py @@ -73,3 +73,9 @@ class TransferTestCase(BaseTestCase): self.client.login(username="bob@example.org", password="password") r = self.client.post(self.url, payload) self.assertEqual(r.status_code, 403) + + def test_it_handles_bad_project_uuid(self): + self.client.login(username="bob@example.org", password="password") + payload = {"project": "not-uuid"} + r = self.client.post(self.url, payload) + self.assertEqual(r.status_code, 400) diff --git a/hc/front/views.py b/hc/front/views.py index 03678518..ca2efef0 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -139,10 +139,10 @@ def _get_rw_channel_for_user(request, code): return channel -def _get_project_for_user(request, project_code): +def _get_project_for_user(request: HttpRequest, code: UUID) -> Tuple[Project, bool]: """Check access, return (project, rw) tuple.""" - project = get_object_or_404(Project, code=project_code) + project = get_object_or_404(Project, code=code) if request.user.is_superuser: return project, True @@ -154,10 +154,10 @@ def _get_project_for_user(request, project_code): return project, membership.is_rw -def _get_rw_project_for_user(request, project_code): +def _get_rw_project_for_user(request: HttpRequest, code: UUID) -> Project: """Check access, return (project, rw) tuple.""" - project, rw = _get_project_for_user(request, project_code) + project, rw = _get_project_for_user(request, code) if not rw: raise PermissionDenied @@ -818,11 +818,15 @@ def uncloak(request, unique_key): @login_required -def transfer(request, code): +def transfer(request: HttpRequest, code: UUID) -> HttpResponse: check = _get_rw_check_for_user(request, code) if request.method == "POST": - target_project = _get_rw_project_for_user(request, request.POST["project"]) + form = forms.TransferForm(request.POST) + if not form.is_valid(): + return HttpResponseBadRequest() + + target_project = _get_rw_project_for_user(request, form.cleaned_data["project"]) if target_project.num_checks_available() <= 0: return HttpResponseBadRequest() @@ -1696,7 +1700,7 @@ def add_victorops(request, code): @require_setting("ZULIP_ENABLED") @login_required -def add_zulip(request, code): +def add_zulip(request: HttpRequest, code: UUID) -> HttpResponse: project = _get_rw_project_for_user(request, code) if request.method == "POST": diff --git a/static/js/add_zulip.js b/static/js/add_zulip.js index 7a4dc018..5a34d97d 100644 --- a/static/js/add_zulip.js +++ b/static/js/add_zulip.js @@ -2,13 +2,15 @@ $(function() { function updateForm() { var mType = $('input[name=mtype]:checked').val(); if (mType == "stream") { - $("#z-to-label").text("Stream Name"); + $("#z-to-label").text("Stream"); $("#z-to-help").text('Example: "general"'); } if (mType == "private") { $("#z-to-label").text("User's Email"); $("#z-to-help").text('Example: "alice@example.org"'); } + + $("#z-topic-group").toggleClass("hide", mType == "private"); } // Update form labels when user clicks on radio buttons diff --git a/templates/integrations/add_zulip.html b/templates/integrations/add_zulip.html index 4819e416..da4c3c0c 100644 --- a/templates/integrations/add_zulip.html +++ b/templates/integrations/add_zulip.html @@ -104,8 +104,6 @@ </div> </div> - - <div id="z-mtype-group" class="form-group {{ form.mtype.css_classes }}"> <label class="col-sm-2 control-label">Post To</label> <div class="col-sm-4"> @@ -135,7 +133,7 @@ {% if form.mtype.value == "private" %} User's Email {% else %} - Stream Name + Stream {% endif %} </label> <div class="col-sm-4"> @@ -156,6 +154,22 @@ </div> </div> + <div id="z-topic-group" class="form-group {% if form.mtype.value == 'private' %}hide{% endif %} {{ form.topic.css_classes }}"> + <label for="z-topic" class="col-sm-2 control-label">Topic</label> + <div class="col-sm-4"> + <input + id="z-topic" + type="text" + class="form-control" + name="topic" + value="{{ form.topic.value|default:"" }}"> + <div class="help-block"> + Example: "Alerts". Leave empty to use auto-generated topics in the form: + "{check name} is {status}". + </div> + </div> + </div> + <div class="form-group"> <div class="col-sm-offset-2 col-sm-10"> <button