From 0ea5927b6a1064a343c2f3775b9d65aa0b601882 Mon Sep 17 00:00:00 2001 From: someposer Date: Fri, 3 Nov 2017 13:41:36 -0500 Subject: [PATCH 1/8] Adding Content-Type header to Webhook integrations to work correctly with services like https://ifttt.com/maker_webhooks which require a specific content type, like application/json. --- hc/api/models.py | 6 ++++++ hc/api/transports.py | 7 +++++-- hc/front/forms.py | 4 +++- templates/front/channels.html | 6 ++++++ templates/integrations/add_webhook.html | 16 ++++++++++++++++ 5 files changed, 36 insertions(+), 3 deletions(-) diff --git a/hc/api/models.py b/hc/api/models.py index 3fa9d2ca..eca63700 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -321,6 +321,12 @@ class Channel(models.Model): parts = self.value.split("\n") return parts[2] if len(parts) > 2 else "" + @property + def content_type(self): + assert self.kind == "webhook" + parts = self.value.split("\n") + return parts[3] if len(parts) > 3 else "" + @property def slack_team(self): assert self.kind == "slack" diff --git a/hc/api/transports.py b/hc/api/transports.py index 8293a498..260c9634 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -164,8 +164,11 @@ class Webhook(HttpTransport): url = self.prepare(url, check, urlencode=True) if self.channel.post_data: + headers = {} + if self.channel.content_type: + headers["Content-Type"] = self.channel.content_type payload = self.prepare(self.channel.post_data, check) - return self.post(url, data=payload.encode("utf-8")) + return self.post(url, data=payload.encode("utf-8"), headers=headers) else: return self.get(url) @@ -230,7 +233,7 @@ class Pushbullet(HttpTransport): url = "https://api.pushbullet.com/v2/pushes" headers = { "Access-Token": self.channel.value, - "Conent-Type": "application/json" + "Content-Type": "application/json" } payload = { "type": "note", diff --git a/hc/front/forms.py b/hc/front/forms.py index 1617a5ef..2eb76934 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -65,9 +65,11 @@ class AddWebhookForm(forms.Form): post_data = forms.CharField(max_length=1000, required=False) + content_type = forms.CharField(max_length=1000, required=False) + def get_value(self): d = self.cleaned_data - return "\n".join((d["value_down"], d["value_up"], d["post_data"])) + return "\n".join((d["value_down"], d["value_up"], d["post_data"], d["content_type"])) phone_validator = RegexValidator(regex='^\+\d{5,15}$', diff --git a/templates/front/channels.html b/templates/front/channels.html index 946093ee..8c0cc719 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -80,6 +80,12 @@ {{ ch.post_data }} {% endif %} + {% if ch.content_type %} + + type  + {{ ch.content_type }} + + {% endif %} {% elif ch.kind == "pushbullet" %} API key diff --git a/templates/integrations/add_webhook.html b/templates/integrations/add_webhook.html index 1f388b34..3432bb08 100644 --- a/templates/integrations/add_webhook.html +++ b/templates/integrations/add_webhook.html @@ -105,6 +105,22 @@ {% endif %} +
+ +
+ + {% if form.content_type.errors %} +
+ {{ form.content_type.errors|join:"" }} +
+ {% endif %} +
+
From fdf011aa254480ba91a799f4825e5d6cc013d8a6 Mon Sep 17 00:00:00 2001 From: someposer Date: Fri, 3 Nov 2017 14:06:33 -0500 Subject: [PATCH 2/8] Update Webhook unit tests to use added Content-Type value. --- hc/front/tests/test_add_webhook.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/hc/front/tests/test_add_webhook.py b/hc/front/tests/test_add_webhook.py index a97a97fe..dbda57da 100644 --- a/hc/front/tests/test_add_webhook.py +++ b/hc/front/tests/test_add_webhook.py @@ -18,7 +18,7 @@ class AddWebhookTestCase(BaseTestCase): self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, "http://foo.com\nhttps://bar.com\n") + self.assertEqual(c.value, "http://foo.com\nhttps://bar.com\n\n") def test_it_adds_webhook_using_team_access(self): form = {"value_down": "http://foo.com", "value_up": "https://bar.com"} @@ -30,7 +30,7 @@ class AddWebhookTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.user, self.alice) - self.assertEqual(c.value, "http://foo.com\nhttps://bar.com\n") + self.assertEqual(c.value, "http://foo.com\nhttps://bar.com\n\n") def test_it_rejects_bad_urls(self): urls = [ @@ -59,7 +59,7 @@ class AddWebhookTestCase(BaseTestCase): self.client.post(self.url, form) c = Channel.objects.get() - self.assertEqual(c.value, "\nhttp://foo.com\n") + self.assertEqual(c.value, "\nhttp://foo.com\n\n") def test_it_adds_post_data(self): form = {"value_down": "http://foo.com", "post_data": "hello"} @@ -69,4 +69,14 @@ class AddWebhookTestCase(BaseTestCase): self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, "http://foo.com\n\nhello") + self.assertEqual(c.value, "http://foo.com\n\nhello\n") + + def test_it_adds_content_type(self): + form = {"value_down": "http://foo.com", "post_data": "hello", "content_type": "application/json"} + + self.client.login(username="alice@example.org", password="password") + r = self.client.post(self.url, form) + self.assertRedirects(r, "/integrations/") + + c = Channel.objects.get() + self.assertEqual(c.value, "http://foo.com\n\nhello\napplication/json") From 08ac09ea937c06bcdbbcab310f2ab4f4cf219a29 Mon Sep 17 00:00:00 2001 From: someposer Date: Fri, 3 Nov 2017 15:43:34 -0500 Subject: [PATCH 3/8] Adding additional notify tests to improve coverage. --- hc/api/tests/test_notify.py | 72 +++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 597cccdf..b31c4106 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -5,6 +5,7 @@ import json from django.core import mail from django.utils.timezone import now +from hc.api.transports import Transport from hc.api.models import Channel, Check, Notification from hc.test import BaseTestCase from mock import patch @@ -62,6 +63,14 @@ class NotifyTestCase(BaseTestCase): self.assertFalse(mock_get.called) self.assertEqual(Notification.objects.count(), 0) + @patch("hc.api.transports.requests.request") + def test_webhooks_ignore_down_events(self, mock_get): + self._setup_data("webhook", "\nhttp://example", status="down") + self.channel.notify(self.check) + + self.assertFalse(mock_get.called) + self.assertEqual(Notification.objects.count(), 0) + @patch("hc.api.transports.requests.request") def test_webhooks_handle_500(self, mock_get): self._setup_data("webhook", "http://example") @@ -145,6 +154,22 @@ class NotifyTestCase(BaseTestCase): # unicode should be encoded into utf-8 self.assertTrue(isinstance(kwargs["data"], binary_type)) + @patch("hc.api.transports.requests.request") + def test_webhooks_handle_content_type(self, mock_request): + template = u"http://example.com\n\n{}\napplication/json" + self._setup_data("webhook", template) + self.check.save() + + headers = { + "User-Agent": "healthchecks.io", + "Content-Type": "application/json" + } + + self.channel.notify(self.check) + + mock_request.assert_called_with( + "post", "http://example.com", data=b"{}", headers=headers, timeout=5) + def test_email(self): self._setup_data("email", "alice@example.org") self.channel.notify(self.check) @@ -167,6 +192,17 @@ class NotifyTestCase(BaseTestCase): self.assertEqual(n.error, "Email not verified") self.assertEqual(len(mail.outbox), 0) + @patch("hc.api.transports.emails.alert") + def test_email_missing_profile(self, mock_emails): + self._setup_data("email", "not_alice@example.org") + self.profile.sort = "name" + self.profile.save() + self.channel.notify(self.check) + + args, kwargs = mock_emails.call_args + self.assertEqual(args[0], "not_alice@example.org") + self.assertEqual(args[1]["sort"], "created") + @patch("hc.api.transports.requests.request") def test_pd(self, mock_post): self._setup_data("pd", "123") @@ -276,6 +312,21 @@ class NotifyTestCase(BaseTestCase): payload = kwargs["json"] self.assertIn("DOWN", payload["message"]) + @patch("hc.api.transports.requests.request") + def test_opsgenie_up(self, mock_post): + self._setup_data("opsgenie", "123", status="up") + mock_post.return_value.status_code = 200 + + self.channel.notify(self.check) + n = Notification.objects.first() + self.assertEqual(n.error, "") + + args, kwargs = mock_post.call_args + payload = kwargs["json"] + self.assertEqual(args[0], "post") + self.assertTrue(args[1].endswith("/close")) + self.assertNotIn("message", payload) + @patch("hc.api.transports.requests.request") def test_pushover(self, mock_post): self._setup_data("po", "123|0") @@ -287,6 +338,22 @@ class NotifyTestCase(BaseTestCase): args, kwargs = mock_post.call_args payload = kwargs["data"] self.assertIn("DOWN", payload["title"]) + self.assertNotIn("retry", payload) + self.assertNotIn("expire", payload) + + @patch("hc.api.transports.requests.request") + def test_pushover_emergency(self, mock_post): + self._setup_data("po", "123|2") + 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["title"]) + self.assertIn("retry", payload) + self.assertIn("expire", payload) @patch("hc.api.transports.requests.request") def test_victorops(self, mock_post): @@ -387,3 +454,8 @@ class NotifyTestCase(BaseTestCase): self.channel.notify(self.check) self.assertTrue(mock_post.called) + + def test_transport_notify(self): + self._setup_data("webhook", "http://example") + with self.assertRaises(NotImplementedError): + Transport.notify(self.channel, self.check) From ee0df8be95834eb494f854d84db26fb1b8ede92d Mon Sep 17 00:00:00 2001 From: someposer Date: Fri, 3 Nov 2017 15:57:33 -0500 Subject: [PATCH 4/8] Fixed issue with Transport test --- hc/api/tests/test_notify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index b31c4106..12fc7779 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -458,4 +458,4 @@ class NotifyTestCase(BaseTestCase): def test_transport_notify(self): self._setup_data("webhook", "http://example") with self.assertRaises(NotImplementedError): - Transport.notify(self.channel, self.check) + Transport(self.channel).notify(self.check) From 05c84d7976e47ac7dd83b313d1ce22548fb75811 Mon Sep 17 00:00:00 2001 From: someposer Date: Fri, 3 Nov 2017 19:40:43 -0500 Subject: [PATCH 5/8] Add support for arbitrary headers using a JSON body for webhooks. --- hc/api/models.py | 38 +++++++--- hc/api/tests/test_notify.py | 94 ++++++++----------------- hc/api/transports.py | 16 ++--- hc/front/forms.py | 10 +-- hc/front/tests/test_add_webhook.py | 25 +++---- templates/front/channels.html | 14 ++-- templates/front/log.html | 2 +- templates/integrations/add_webhook.html | 34 ++++----- 8 files changed, 109 insertions(+), 124 deletions(-) diff --git a/hc/api/models.py b/hc/api/models.py index eca63700..05ae3ac6 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -304,28 +304,44 @@ class Channel(models.Model): return user_key, prio, PO_PRIORITIES[prio] @property - def value_down(self): + def url_down(self): assert self.kind == "webhook" - parts = self.value.split("\n") - return parts[0] + if not self.value.startswith("{"): + parts = self.value.split("\n") + return parts[0] + + doc = json.loads(self.value) + return doc["url_down"] + @property - def value_up(self): + def url_up(self): assert self.kind == "webhook" - parts = self.value.split("\n") - return parts[1] if len(parts) > 1 else "" + if not self.value.startswith("{"): + parts = self.value.split("\n") + return parts[1] if len(parts) > 1 else "" + + doc = json.loads(self.value) + return doc["url_up"] @property def post_data(self): assert self.kind == "webhook" - parts = self.value.split("\n") - return parts[2] if len(parts) > 2 else "" + if not self.value.startswith("{"): + parts = self.value.split("\n") + return parts[2] if len(parts) > 2 else "" + + doc = json.loads(self.value) + return doc["post_data"] @property - def content_type(self): + def headers(self): assert self.kind == "webhook" - parts = self.value.split("\n") - return parts[3] if len(parts) > 3 else "" + if not self.value.startswith("{"): + return "" + + doc = json.loads(self.value) + return doc["headers"] @property def slack_team(self): diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 12fc7779..48627710 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -5,7 +5,6 @@ import json from django.core import mail from django.utils.timezone import now -from hc.api.transports import Transport from hc.api.models import Channel, Check, Notification from hc.test import BaseTestCase from mock import patch @@ -63,14 +62,6 @@ class NotifyTestCase(BaseTestCase): self.assertFalse(mock_get.called) self.assertEqual(Notification.objects.count(), 0) - @patch("hc.api.transports.requests.request") - def test_webhooks_ignore_down_events(self, mock_get): - self._setup_data("webhook", "\nhttp://example", status="down") - self.channel.notify(self.check) - - self.assertFalse(mock_get.called) - self.assertEqual(Notification.objects.count(), 0) - @patch("hc.api.transports.requests.request") def test_webhooks_handle_500(self, mock_get): self._setup_data("webhook", "http://example") @@ -155,20 +146,44 @@ class NotifyTestCase(BaseTestCase): self.assertTrue(isinstance(kwargs["data"], binary_type)) @patch("hc.api.transports.requests.request") - def test_webhooks_handle_content_type(self, mock_request): - template = u"http://example.com\n\n{}\napplication/json" - self._setup_data("webhook", template) - self.check.save() + def test_webhooks_handle_json_value(self, mock_request): + self._setup_data("webhook", '{"url_down": "http://foo.com", ' + '"url_up": "", "post_data": "", "headers": ""}') + self.channel.notify(self.check) headers = { - "User-Agent": "healthchecks.io", - "Content-Type": "application/json" + "User-Agent": "healthchecks.io" + } + mock_request.assert_called_with( + "get", "http://foo.com", headers=headers, + timeout=5) + + @patch("hc.api.transports.requests.request") + def test_webhooks_handle_json_up_event(self, mock_request): + self._setup_data("webhook", '{"url_down": "", ' + '"url_up": "http://bar", "post_data": "", "headers": ""}', status="up") + self.channel.notify(self.check) + + headers = { + "User-Agent": "healthchecks.io" } + mock_request.assert_called_with( + "get", "http://bar", headers=headers, + timeout=5) + @patch("hc.api.transports.requests.request") + def test_webhooks_handle_headers(self, mock_request): + self._setup_data("webhook", '{"url_down": "http://foo.com", ' + '"url_up": "", "post_data": "data", "headers": ' + '"{\\\"Content-Type\\\": \\\"application/json\\\"}"}') self.channel.notify(self.check) + headers = { + "User-Agent": "healthchecks.io", + "Content-Type": "application/json" + } mock_request.assert_called_with( - "post", "http://example.com", data=b"{}", headers=headers, timeout=5) + "post", "http://foo.com", data=b"data", headers=headers, timeout=5) def test_email(self): self._setup_data("email", "alice@example.org") @@ -192,17 +207,6 @@ class NotifyTestCase(BaseTestCase): self.assertEqual(n.error, "Email not verified") self.assertEqual(len(mail.outbox), 0) - @patch("hc.api.transports.emails.alert") - def test_email_missing_profile(self, mock_emails): - self._setup_data("email", "not_alice@example.org") - self.profile.sort = "name" - self.profile.save() - self.channel.notify(self.check) - - args, kwargs = mock_emails.call_args - self.assertEqual(args[0], "not_alice@example.org") - self.assertEqual(args[1]["sort"], "created") - @patch("hc.api.transports.requests.request") def test_pd(self, mock_post): self._setup_data("pd", "123") @@ -312,21 +316,6 @@ class NotifyTestCase(BaseTestCase): payload = kwargs["json"] self.assertIn("DOWN", payload["message"]) - @patch("hc.api.transports.requests.request") - def test_opsgenie_up(self, mock_post): - self._setup_data("opsgenie", "123", status="up") - mock_post.return_value.status_code = 200 - - self.channel.notify(self.check) - n = Notification.objects.first() - self.assertEqual(n.error, "") - - args, kwargs = mock_post.call_args - payload = kwargs["json"] - self.assertEqual(args[0], "post") - self.assertTrue(args[1].endswith("/close")) - self.assertNotIn("message", payload) - @patch("hc.api.transports.requests.request") def test_pushover(self, mock_post): self._setup_data("po", "123|0") @@ -338,22 +327,6 @@ class NotifyTestCase(BaseTestCase): args, kwargs = mock_post.call_args payload = kwargs["data"] self.assertIn("DOWN", payload["title"]) - self.assertNotIn("retry", payload) - self.assertNotIn("expire", payload) - - @patch("hc.api.transports.requests.request") - def test_pushover_emergency(self, mock_post): - self._setup_data("po", "123|2") - 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["title"]) - self.assertIn("retry", payload) - self.assertIn("expire", payload) @patch("hc.api.transports.requests.request") def test_victorops(self, mock_post): @@ -454,8 +427,3 @@ class NotifyTestCase(BaseTestCase): self.channel.notify(self.check) self.assertTrue(mock_post.called) - - def test_transport_notify(self): - self._setup_data("webhook", "http://example") - with self.assertRaises(NotImplementedError): - Transport(self.channel).notify(self.check) diff --git a/hc/api/transports.py b/hc/api/transports.py index 260c9634..aeaac2e7 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -147,27 +147,27 @@ class Webhook(HttpTransport): return result def is_noop(self, check): - if check.status == "down" and not self.channel.value_down: + if check.status == "down" and not self.channel.url_down: return True - if check.status == "up" and not self.channel.value_up: + if check.status == "up" and not self.channel.url_up: return True return False def notify(self, check): - url = self.channel.value_down + url = self.channel.url_down if check.status == "up": - url = self.channel.value_up + url = self.channel.url_up assert url url = self.prepare(url, check, urlencode=True) if self.channel.post_data: - headers = {} - if self.channel.content_type: - headers["Content-Type"] = self.channel.content_type payload = self.prepare(self.channel.post_data, check) + headers = {} + if self.channel.headers: + headers = json.loads(self.channel.headers) return self.post(url, data=payload.encode("utf-8"), headers=headers) else: return self.get(url) @@ -233,7 +233,7 @@ class Pushbullet(HttpTransport): url = "https://api.pushbullet.com/v2/pushes" headers = { "Access-Token": self.channel.value, - "Content-Type": "application/json" + "Conent-Type": "application/json" } payload = { "type": "note", diff --git a/hc/front/forms.py b/hc/front/forms.py index 2eb76934..7f65659d 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -1,3 +1,4 @@ +import json from datetime import timedelta as td from django import forms @@ -57,19 +58,18 @@ class AddUrlForm(forms.Form): class AddWebhookForm(forms.Form): error_css_class = "has-error" - value_down = forms.URLField(max_length=1000, required=False, + url_down = forms.URLField(max_length=1000, required=False, validators=[WebhookValidator()]) - value_up = forms.URLField(max_length=1000, required=False, + url_up = forms.URLField(max_length=1000, required=False, validators=[WebhookValidator()]) post_data = forms.CharField(max_length=1000, required=False) - content_type = forms.CharField(max_length=1000, required=False) + headers = forms.CharField(max_length=1000, required=False) def get_value(self): - d = self.cleaned_data - return "\n".join((d["value_down"], d["value_up"], d["post_data"], d["content_type"])) + return json.dumps(self.cleaned_data) phone_validator = RegexValidator(regex='^\+\d{5,15}$', diff --git a/hc/front/tests/test_add_webhook.py b/hc/front/tests/test_add_webhook.py index dbda57da..1f5d09ae 100644 --- a/hc/front/tests/test_add_webhook.py +++ b/hc/front/tests/test_add_webhook.py @@ -11,17 +11,17 @@ class AddWebhookTestCase(BaseTestCase): self.assertContains(r, "Runs a HTTP GET or HTTP POST") def test_it_adds_two_webhook_urls_and_redirects(self): - form = {"value_down": "http://foo.com", "value_up": "https://bar.com"} + form = {"url_down": "http://foo.com", "url_up": "https://bar.com"} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, "http://foo.com\nhttps://bar.com\n\n") + self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "https://bar.com", "post_data": "", "headers": ""}') def test_it_adds_webhook_using_team_access(self): - form = {"value_down": "http://foo.com", "value_up": "https://bar.com"} + form = {"url_down": "http://foo.com", "url_up": "https://bar.com"} # Logging in as bob, not alice. Bob has team access so this # should work. @@ -30,7 +30,7 @@ class AddWebhookTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.user, self.alice) - self.assertEqual(c.value, "http://foo.com\nhttps://bar.com\n\n") + self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "https://bar.com", "post_data": "", "headers": ""}') def test_it_rejects_bad_urls(self): urls = [ @@ -45,7 +45,7 @@ class AddWebhookTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") for url in urls: - form = {"value_down": url, "value_up": ""} + form = {"url_down": url, "url_up": ""} r = self.client.post(self.url, form) self.assertContains(r, "Enter a valid URL.", msg_prefix=url) @@ -53,30 +53,31 @@ class AddWebhookTestCase(BaseTestCase): self.assertEqual(Channel.objects.count(), 0) def test_it_handles_empty_down_url(self): - form = {"value_down": "", "value_up": "http://foo.com"} + form = {"url_down": "", "url_up": "http://foo.com"} self.client.login(username="alice@example.org", password="password") self.client.post(self.url, form) c = Channel.objects.get() - self.assertEqual(c.value, "\nhttp://foo.com\n\n") + self.assertEqual(c.value, '{"url_down": "", "url_up": "http://foo.com", "post_data": "", "headers": ""}') def test_it_adds_post_data(self): - form = {"value_down": "http://foo.com", "post_data": "hello"} + form = {"url_down": "http://foo.com", "post_data": "hello"} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, "http://foo.com\n\nhello\n") + self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "", "post_data": "hello", "headers": ""}') - def test_it_adds_content_type(self): - form = {"value_down": "http://foo.com", "post_data": "hello", "content_type": "application/json"} + def test_it_adds_headers(self): + form = {"url_down": "http://foo.com", "headers": '{"test": "123"}'} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, "http://foo.com\n\nhello\napplication/json") + self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "", "post_data": "", "headers": "{\\\"test\\\": \\\"123\\\"}"}') + diff --git a/templates/front/channels.html b/templates/front/channels.html index 8c0cc719..141bd569 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -62,16 +62,16 @@ {% endif %} {% elif ch.kind == "webhook" %} - {% if ch.value_down %} + {% if ch.url_down %} - + {% endif %} - {% if ch.value_up %} + {% if ch.url_up %} - + {% endif %} {% if ch.post_data %} @@ -80,10 +80,10 @@ {% endif %} - {% if ch.content_type %} + {% if ch.headers %} - - + + {% endif %}
down {{ ch.value_down }}{{ ch.url_down }}
up {{ ch.value_up }}{{ ch.url_up }}
{{ ch.post_data }}
type {{ ch.content_type }}headers {{ ch.headers }}
diff --git a/templates/front/log.html b/templates/front/log.html index fbd1e60f..42422737 100644 --- a/templates/front/log.html +++ b/templates/front/log.html @@ -96,7 +96,7 @@ {% elif event.channel.kind == "po" %} Sent a Pushover notification {% elif event.channel.kind == "webhook" %} - Called webhook {{ event.channel.value_down }} + Called webhook {{ event.channel.url_down }} {% else %} Sent alert to {{ event.channel.kind|capfirst }} {% endif %} diff --git a/templates/integrations/add_webhook.html b/templates/integrations/add_webhook.html index 3432bb08..1aacfee0 100644 --- a/templates/integrations/add_webhook.html +++ b/templates/integrations/add_webhook.html @@ -57,34 +57,34 @@
{% csrf_token %} -
+
- {% if form.value_down.errors %} + value="{{ form.url_down.value|default:"" }}"> + {% if form.url_down.errors %}
- {{ form.value_down.errors|join:"" }} + {{ form.url_down.errors|join:"" }}
{% endif %}
-
+
- {% if form.value_up.errors %} + value="{{ form.url_up.value|default:"" }}"> + {% if form.url_up.errors %}
- {{ form.value_up.errors|join:"" }} + {{ form.url_up.errors|join:"" }}
{% endif %}
@@ -105,18 +105,18 @@ {% endif %}
-
- +
+
- {% if form.content_type.errors %} + name="headers" + placeholder='{"Content-Type": "application/json"}' + value="{{ form.headers.value|default:"" }}"> + {% if form.headers.errors %}
- {{ form.content_type.errors|join:"" }} + {{ form.headers.errors|join:"" }}
{% endif %}
From 077bc45b12c7ae2a32d60072e0a1cf13f22812e7 Mon Sep 17 00:00:00 2001 From: someposer Date: Fri, 3 Nov 2017 19:56:38 -0500 Subject: [PATCH 6/8] Sorting keys on Webhook JSON value for consistent unit testing. --- hc/front/forms.py | 2 +- hc/front/tests/test_add_webhook.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hc/front/forms.py b/hc/front/forms.py index 7f65659d..babae8b0 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -69,7 +69,7 @@ class AddWebhookForm(forms.Form): headers = forms.CharField(max_length=1000, required=False) def get_value(self): - return json.dumps(self.cleaned_data) + return json.dumps(self.cleaned_data, sort_keys=True) phone_validator = RegexValidator(regex='^\+\d{5,15}$', diff --git a/hc/front/tests/test_add_webhook.py b/hc/front/tests/test_add_webhook.py index 1f5d09ae..5db65e86 100644 --- a/hc/front/tests/test_add_webhook.py +++ b/hc/front/tests/test_add_webhook.py @@ -18,7 +18,7 @@ class AddWebhookTestCase(BaseTestCase): self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "https://bar.com", "post_data": "", "headers": ""}') + self.assertEqual(c.value, '{"headers": "", "post_data": "", "url_down": "http://foo.com", "url_up": "https://bar.com"}') def test_it_adds_webhook_using_team_access(self): form = {"url_down": "http://foo.com", "url_up": "https://bar.com"} @@ -30,7 +30,7 @@ class AddWebhookTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.user, self.alice) - self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "https://bar.com", "post_data": "", "headers": ""}') + self.assertEqual(c.value, '{"headers": "", "post_data": "", "url_down": "http://foo.com", "url_up": "https://bar.com"}') def test_it_rejects_bad_urls(self): urls = [ @@ -59,7 +59,7 @@ class AddWebhookTestCase(BaseTestCase): self.client.post(self.url, form) c = Channel.objects.get() - self.assertEqual(c.value, '{"url_down": "", "url_up": "http://foo.com", "post_data": "", "headers": ""}') + self.assertEqual(c.value, '{"headers": "", "post_data": "", "url_down": "", "url_up": "http://foo.com"}') def test_it_adds_post_data(self): form = {"url_down": "http://foo.com", "post_data": "hello"} @@ -69,7 +69,7 @@ class AddWebhookTestCase(BaseTestCase): self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "", "post_data": "hello", "headers": ""}') + self.assertEqual(c.value, '{"headers": "", "post_data": "hello", "url_down": "http://foo.com", "url_up": ""}') def test_it_adds_headers(self): form = {"url_down": "http://foo.com", "headers": '{"test": "123"}'} @@ -79,5 +79,5 @@ class AddWebhookTestCase(BaseTestCase): self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, '{"url_down": "http://foo.com", "url_up": "", "post_data": "", "headers": "{\\\"test\\\": \\\"123\\\"}"}') + self.assertEqual(c.value, '{"headers": "{\\\"test\\\": \\\"123\\\"}", "post_data": "", "url_down": "http://foo.com", "url_up": ""}') From 5781ddfe4dccc3cb05cf35413e7d7c3ce8efb1f5 Mon Sep 17 00:00:00 2001 From: someposer Date: Sun, 5 Nov 2017 19:10:19 -0600 Subject: [PATCH 7/8] Created an improved interface for arbitrary headers and simplified header storage. --- hc/api/tests/test_notify.py | 4 +-- hc/api/transports.py | 2 +- hc/front/forms.py | 16 +++++++++-- hc/front/tests/test_add_webhook.py | 12 ++++----- hc/front/views.py | 5 +++- static/js/webhook.js | 26 ++++++++++++++++++ templates/integrations/add_webhook.html | 36 ++++++++++++++++++------- 7 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 static/js/webhook.js diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 48627710..ce1c0565 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -174,8 +174,8 @@ class NotifyTestCase(BaseTestCase): @patch("hc.api.transports.requests.request") def test_webhooks_handle_headers(self, mock_request): self._setup_data("webhook", '{"url_down": "http://foo.com", ' - '"url_up": "", "post_data": "data", "headers": ' - '"{\\\"Content-Type\\\": \\\"application/json\\\"}"}') + '"url_up": "", "post_data": "data", ' + '"headers": {"Content-Type": "application/json"}}') self.channel.notify(self.check) headers = { diff --git a/hc/api/transports.py b/hc/api/transports.py index aeaac2e7..68ae002e 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -167,7 +167,7 @@ class Webhook(HttpTransport): payload = self.prepare(self.channel.post_data, check) headers = {} if self.channel.headers: - headers = json.loads(self.channel.headers) + headers = self.channel.headers return self.post(url, data=payload.encode("utf-8"), headers=headers) else: return self.get(url) diff --git a/hc/front/forms.py b/hc/front/forms.py index babae8b0..3308787b 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -66,10 +66,22 @@ class AddWebhookForm(forms.Form): post_data = forms.CharField(max_length=1000, required=False) - headers = forms.CharField(max_length=1000, required=False) + def __init__(self, *args, **kwargs): + self.headers = {} + if all(k in kwargs for k in ("header_keys", "header_values")): + header_keys = kwargs.pop("header_keys") + header_values = kwargs.pop("header_values") + + for i, (key, val) in enumerate(zip(header_keys, header_values)): + if key: + self.headers[key] = val + + super(AddWebhookForm, self).__init__(*args, **kwargs) def get_value(self): - return json.dumps(self.cleaned_data, sort_keys=True) + val = dict(self.cleaned_data) + val["headers"] = self.headers + return json.dumps(val, sort_keys=True) phone_validator = RegexValidator(regex='^\+\d{5,15}$', diff --git a/hc/front/tests/test_add_webhook.py b/hc/front/tests/test_add_webhook.py index 5db65e86..744f5686 100644 --- a/hc/front/tests/test_add_webhook.py +++ b/hc/front/tests/test_add_webhook.py @@ -18,7 +18,7 @@ class AddWebhookTestCase(BaseTestCase): self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, '{"headers": "", "post_data": "", "url_down": "http://foo.com", "url_up": "https://bar.com"}') + self.assertEqual(c.value, '{"headers": {}, "post_data": "", "url_down": "http://foo.com", "url_up": "https://bar.com"}') def test_it_adds_webhook_using_team_access(self): form = {"url_down": "http://foo.com", "url_up": "https://bar.com"} @@ -30,7 +30,7 @@ class AddWebhookTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.user, self.alice) - self.assertEqual(c.value, '{"headers": "", "post_data": "", "url_down": "http://foo.com", "url_up": "https://bar.com"}') + self.assertEqual(c.value, '{"headers": {}, "post_data": "", "url_down": "http://foo.com", "url_up": "https://bar.com"}') def test_it_rejects_bad_urls(self): urls = [ @@ -59,7 +59,7 @@ class AddWebhookTestCase(BaseTestCase): self.client.post(self.url, form) c = Channel.objects.get() - self.assertEqual(c.value, '{"headers": "", "post_data": "", "url_down": "", "url_up": "http://foo.com"}') + self.assertEqual(c.value, '{"headers": {}, "post_data": "", "url_down": "", "url_up": "http://foo.com"}') def test_it_adds_post_data(self): form = {"url_down": "http://foo.com", "post_data": "hello"} @@ -69,15 +69,15 @@ class AddWebhookTestCase(BaseTestCase): self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, '{"headers": "", "post_data": "hello", "url_down": "http://foo.com", "url_up": ""}') + self.assertEqual(c.value, '{"headers": {}, "post_data": "hello", "url_down": "http://foo.com", "url_up": ""}') def test_it_adds_headers(self): - form = {"url_down": "http://foo.com", "headers": '{"test": "123"}'} + form = {"url_down": "http://foo.com", "header_key[]": ["test", "test2"], "header_value[]": ["123", "abc"]} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) self.assertRedirects(r, "/integrations/") c = Channel.objects.get() - self.assertEqual(c.value, '{"headers": "{\\\"test\\\": \\\"123\\\"}", "post_data": "", "url_down": "http://foo.com", "url_up": ""}') + self.assertEqual(c.value, '{"headers": {"test": "123", "test2": "abc"}, "post_data": "", "url_down": "http://foo.com", "url_up": ""}') diff --git a/hc/front/views.py b/hc/front/views.py index 22853577..972ef126 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -437,7 +437,10 @@ def add_email(request): @login_required def add_webhook(request): if request.method == "POST": - form = AddWebhookForm(request.POST) + header_keys = request.POST.getlist('header_key[]') + header_values = request.POST.getlist('header_value[]') + form = AddWebhookForm(request.POST or None, + header_keys=header_keys, header_values=header_values) if form.is_valid(): channel = Channel(user=request.team.user, kind="webhook") channel.value = form.get_value() diff --git a/static/js/webhook.js b/static/js/webhook.js new file mode 100644 index 00000000..7a07435e --- /dev/null +++ b/static/js/webhook.js @@ -0,0 +1,26 @@ +$(function() { + $("#webhook_headers").on("click", ".webhook_header_btn.btn-danger", function(e) { + e.preventDefault(); + $(this).closest("div.row").remove(); + }); + + $("#webhook_headers").on("click", ".webhook_header_btn.btn-info", function(e) { + e.preventDefault(); + + // Add new header form + $("#webhook_headers").append( +'
\ +
\ + \ +
\ +
\ +
\ + \ + \ + \ + \ +
\ +
\ +
'); + }); +}); \ No newline at end of file diff --git a/templates/integrations/add_webhook.html b/templates/integrations/add_webhook.html index 1aacfee0..c9d3c338 100644 --- a/templates/integrations/add_webhook.html +++ b/templates/integrations/add_webhook.html @@ -106,20 +106,28 @@
- -
- Headers +
+
+
+ +
+
+
+ + + + +
+
+
+
+ - {% if form.headers.errors %} -
- {{ form.headers.errors|join:"" }} -
- {% endif %} -
+
@@ -130,3 +138,11 @@
{% endblock %} + +{% block scripts %} +{% compress js %} + + + +{% endcompress %} +{% endblock %} From 602ad1dea81f457fe506718fb8bad9a676009b65 Mon Sep 17 00:00:00 2001 From: someposer Date: Sun, 5 Nov 2017 21:24:02 -0600 Subject: [PATCH 8/8] Improved handling of webhook header values when form has errors --- static/js/webhook.js | 3 +++ templates/integrations/add_webhook.html | 24 ++++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/static/js/webhook.js b/static/js/webhook.js index 7a07435e..9b22b380 100644 --- a/static/js/webhook.js +++ b/static/js/webhook.js @@ -1,4 +1,7 @@ $(function() { + $(".webhook_header_btn:first").addClass("btn-info").text("+") + $(".webhook_header_btn:not(:first)").addClass("btn-danger").text("X") + $("#webhook_headers").on("click", ".webhook_header_btn.btn-danger", function(e) { e.preventDefault(); $(this).closest("div.row").remove(); diff --git a/templates/integrations/add_webhook.html b/templates/integrations/add_webhook.html index c9d3c338..4291a041 100644 --- a/templates/integrations/add_webhook.html +++ b/templates/integrations/add_webhook.html @@ -108,6 +108,23 @@
+ {% if form.headers %} + {% for k,v in form.headers.items %} +
+
+ +
+
+
+ + + + +
+
+
+ {% endfor %} + {% else %}
@@ -121,13 +138,8 @@
+ {% endif %}
- -