From 21de50d84e1942346b6b2aba4d1c50709c9cfa97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Tue, 20 Nov 2018 23:31:15 +0200 Subject: [PATCH] Add Channel.name field, users can now name integrations. --- CHANGELOG.md | 1 + hc/api/migrations/0043_channel_name.py | 18 ++ hc/api/migrations/0044_auto_20181120_2004.py | 25 ++ hc/api/models.py | 8 +- hc/front/forms.py | 4 + hc/front/tests/test_add_sms.py | 2 +- hc/front/tests/test_channels.py | 13 +- hc/front/tests/test_update_channel_name.py | 54 ++++ hc/front/tests/test_update_name.py | 33 +-- hc/front/urls.py | 1 + hc/front/views.py | 20 +- static/css/channels.css | 77 +++-- static/css/my_checks_desktop.css | 2 +- static/js/channels.js | 20 +- templates/front/channels.html | 293 +++++++++---------- templates/front/update_name_modal.html | 90 +++--- 16 files changed, 385 insertions(+), 276 deletions(-) create mode 100644 hc/api/migrations/0043_channel_name.py create mode 100644 hc/api/migrations/0044_auto_20181120_2004.py create mode 100644 hc/front/tests/test_update_channel_name.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e98488..5d531ea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file. - Add "channels" attribute to the Check API resource - Can specify channel codes when updating a check via API - Added a workaround for email agents automatically opening "Unsubscribe" links +- Add Channel.name field, users can now name integrations ### Bug Fixes - During DST transition, handle ambiguous dates as pre-transition diff --git a/hc/api/migrations/0043_channel_name.py b/hc/api/migrations/0043_channel_name.py new file mode 100644 index 00000000..dd2da8d0 --- /dev/null +++ b/hc/api/migrations/0043_channel_name.py @@ -0,0 +1,18 @@ +# Generated by Django 2.1.3 on 2018-11-20 16:50 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0042_auto_20181029_1522'), + ] + + operations = [ + migrations.AddField( + model_name='channel', + name='name', + field=models.CharField(blank=True, max_length=100), + ), + ] diff --git a/hc/api/migrations/0044_auto_20181120_2004.py b/hc/api/migrations/0044_auto_20181120_2004.py new file mode 100644 index 00000000..c6f10c7b --- /dev/null +++ b/hc/api/migrations/0044_auto_20181120_2004.py @@ -0,0 +1,25 @@ +# Generated by Django 2.1.3 on 2018-11-20 20:04 + +import json + +from django.db import migrations + + +def combine_channel_names(apps, schema_editor): + Channel = apps.get_model('api', 'Channel') + for channel in Channel.objects.filter(kind="sms"): + if channel.value.startswith("{"): + doc = json.loads(channel.value) + channel.name = doc.get("label", "") + channel.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0043_channel_name'), + ] + + operations = [ + migrations.RunPython(combine_channel_names), + ] diff --git a/hc/api/models.py b/hc/api/models.py index aeb77287..5a1a3471 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -231,6 +231,7 @@ class Ping(models.Model): class Channel(models.Model): + name = models.CharField(max_length=100, blank=True) code = models.UUIDField(default=uuid.uuid4, editable=False) user = models.ForeignKey(User, models.CASCADE) created = models.DateTimeField(auto_now_add=True) @@ -240,11 +241,11 @@ class Channel(models.Model): checks = models.ManyToManyField(Check) def __str__(self): + if self.name: + return self.name if self.kind == "email": return "Email to %s" % self.value elif self.kind == "sms": - if self.sms_label: - return "SMS to %s" % self.sms_label return "SMS to %s" % self.sms_number elif self.kind == "slack": return "Slack %s" % self.slack_channel @@ -327,6 +328,9 @@ class Channel(models.Model): return error + def icon_path(self): + return 'img/integrations/%s.png' % self.kind + @property def po_value(self): assert self.kind == "po" diff --git a/hc/front/forms.py b/hc/front/forms.py index 2098f12c..6afead1c 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -108,3 +108,7 @@ class AddSmsForm(forms.Form): error_css_class = "has-error" label = forms.CharField(max_length=100, required=False) value = forms.CharField(max_length=16, validators=[phone_validator]) + + +class ChannelNameForm(forms.Form): + name = forms.CharField(max_length=100, required=False) diff --git a/hc/front/tests/test_add_sms.py b/hc/front/tests/test_add_sms.py index 02f34fe7..0543cd9b 100644 --- a/hc/front/tests/test_add_sms.py +++ b/hc/front/tests/test_add_sms.py @@ -31,7 +31,7 @@ class AddSmsTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.kind, "sms") self.assertEqual(c.sms_number, "+1234567890") - self.assertEqual(c.sms_label, "My Phone") + self.assertEqual(c.name, "My Phone") def test_it_rejects_bad_number(self): form = {"value": "not a phone number address"} diff --git a/hc/front/tests/test_channels.py b/hc/front/tests/test_channels.py index dbd9a41a..32aa23c0 100644 --- a/hc/front/tests/test_channels.py +++ b/hc/front/tests/test_channels.py @@ -33,9 +33,9 @@ class ChannelsTestCase(BaseTestCase): self.assertEqual(r.status_code, 200) # These are inside a modal: - self.assertContains(r, "http://down.example.com") - self.assertContains(r, "http://up.example.com") - self.assertContains(r, "foobar") + self.assertContains(r, "http://down.example.com") + self.assertContains(r, "http://up.example.com") + self.assertContains(r, "foobar") def test_it_shows_pushover_details(self): ch = Channel(kind="po", user=self.alice) @@ -46,7 +46,6 @@ class ChannelsTestCase(BaseTestCase): r = self.client.get("/integrations/") self.assertEqual(r.status_code, 200) - self.assertContains(r, "fake-key") self.assertContains(r, "(normal priority)") def test_it_shows_disabled_email(self): @@ -63,7 +62,7 @@ class ChannelsTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") r = self.client.get("/integrations/") self.assertEqual(r.status_code, 200) - self.assertContains(r, "(bounced, disabled)") + self.assertContains(r, "Disabled") def test_it_shows_unconfirmed_email(self): channel = Channel(user=self.alice, kind="email") @@ -73,7 +72,7 @@ class ChannelsTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") r = self.client.get("/integrations/") self.assertEqual(r.status_code, 200) - self.assertContains(r, "(unconfirmed)") + self.assertContains(r, "Unconfirmed") def test_it_shows_sms_label(self): ch = Channel(kind="sms", user=self.alice) @@ -84,4 +83,4 @@ class ChannelsTestCase(BaseTestCase): r = self.client.get("/integrations/") self.assertEqual(r.status_code, 200) - self.assertContains(r, "My Phone (+123)") + self.assertContains(r, "SMS to +123") diff --git a/hc/front/tests/test_update_channel_name.py b/hc/front/tests/test_update_channel_name.py new file mode 100644 index 00000000..7ed4864e --- /dev/null +++ b/hc/front/tests/test_update_channel_name.py @@ -0,0 +1,54 @@ +from hc.api.models import Channel +from hc.test import BaseTestCase + + +class UpdateChannelNameTestCase(BaseTestCase): + + def setUp(self): + super(UpdateChannelNameTestCase, self).setUp() + self.channel = Channel(kind="email", user=self.alice) + self.channel.save() + + self.url = "/integrations/%s/name/" % self.channel.code + + def test_it_works(self): + payload = {"name": "My work email"} + + self.client.login(username="alice@example.org", password="password") + r = self.client.post(self.url, data=payload) + self.assertRedirects(r, "/integrations/") + + self.channel.refresh_from_db() + self.assertEqual(self.channel.name, "My work email") + + def test_team_access_works(self): + payload = {"name": "Bob was here"} + + # Logging in as bob, not alice. Bob has team access so this + # should work. + self.client.login(username="bob@example.org", password="password") + self.client.post(self.url, data=payload) + + self.channel.refresh_from_db() + self.assertEqual(self.channel.name, "Bob was here") + + def test_it_checks_ownership(self): + payload = {"name": "Charlie Sent This"} + + self.client.login(username="charlie@example.org", password="password") + r = self.client.post(self.url, data=payload) + self.assertEqual(r.status_code, 403) + + def test_it_handles_missing_uuid(self): + # Valid UUID but there is no check for it: + url = "/integrations/6837d6ec-fc08-4da5-a67f-08a9ed1ccf62/name/" + payload = {"name": "Alice Was Here"} + + self.client.login(username="alice@example.org", password="password") + r = self.client.post(url, data=payload) + self.assertEqual(r.status_code, 404) + + def test_it_rejects_get(self): + self.client.login(username="alice@example.org", password="password") + r = self.client.get(self.url) + self.assertEqual(r.status_code, 405) diff --git a/hc/front/tests/test_update_name.py b/hc/front/tests/test_update_name.py index dfccaf5c..31725615 100644 --- a/hc/front/tests/test_update_name.py +++ b/hc/front/tests/test_update_name.py @@ -9,36 +9,35 @@ class UpdateNameTestCase(BaseTestCase): self.check = Check(user=self.alice) self.check.save() + self.url = "/checks/%s/name/" % self.check.code + def test_it_works(self): - url = "/checks/%s/name/" % self.check.code payload = {"name": "Alice Was Here"} self.client.login(username="alice@example.org", password="password") - r = self.client.post(url, data=payload) + r = self.client.post(self.url, data=payload) self.assertRedirects(r, "/checks/") - check = Check.objects.get(code=self.check.code) - assert check.name == "Alice Was Here" + self.check.refresh_from_db() + self.assertEqual(self.check.name, "Alice Was Here") def test_team_access_works(self): - url = "/checks/%s/name/" % self.check.code payload = {"name": "Bob Was Here"} # Logging in as bob, not alice. Bob has team access so this # should work. self.client.login(username="bob@example.org", password="password") - self.client.post(url, data=payload) + self.client.post(self.url, data=payload) - check = Check.objects.get(code=self.check.code) - assert check.name == "Bob Was Here" + self.check.refresh_from_db() + self.assertEqual(self.check.name, "Bob Was Here") def test_it_checks_ownership(self): - url = "/checks/%s/name/" % self.check.code payload = {"name": "Charlie Sent This"} self.client.login(username="charlie@example.org", password="password") - r = self.client.post(url, data=payload) - assert r.status_code == 403 + r = self.client.post(self.url, data=payload) + self.assertEqual(r.status_code, 403) def test_it_handles_bad_uuid(self): url = "/checks/not-uuid/name/" @@ -55,20 +54,18 @@ class UpdateNameTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") r = self.client.post(url, data=payload) - assert r.status_code == 404 + self.assertEqual(r.status_code, 404) def test_it_sanitizes_tags(self): - url = "/checks/%s/name/" % self.check.code payload = {"tags": " foo bar\r\t \n baz \n"} self.client.login(username="alice@example.org", password="password") - self.client.post(url, data=payload) + self.client.post(self.url, data=payload) - check = Check.objects.get(id=self.check.id) - self.assertEqual(check.tags, "foo bar baz") + self.check.refresh_from_db() + self.assertEqual(self.check.tags, "foo bar baz") def test_it_rejects_get(self): - url = "/checks/%s/name/" % self.check.code self.client.login(username="alice@example.org", password="password") - r = self.client.get(url) + r = self.client.get(self.url) self.assertEqual(r.status_code, 405) diff --git a/hc/front/urls.py b/hc/front/urls.py index 20e430b0..be5e65dd 100644 --- a/hc/front/urls.py +++ b/hc/front/urls.py @@ -38,6 +38,7 @@ channel_urls = [ path('add_trello/', views.add_trello, name="hc-add-trello"), path('add_trello/settings/', views.trello_settings, name="hc-trello-settings"), path('/checks/', views.channel_checks, name="hc-channel-checks"), + path('/name/', views.update_channel_name, name="hc-channel-name"), path('/remove/', views.remove_channel, name="hc-remove-channel"), path('/verify//', views.verify_email, name="hc-verify-email"), diff --git a/hc/front/views.py b/hc/front/views.py index b26a7acc..04b7a411 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -22,7 +22,8 @@ from hc.api.models import (DEFAULT_GRACE, DEFAULT_TIMEOUT, Channel, Check, from hc.api.transports import Telegram from hc.front.forms import (AddWebhookForm, NameTagsForm, TimeoutForm, AddUrlForm, AddEmailForm, - AddOpsGenieForm, CronForm, AddSmsForm) + AddOpsGenieForm, CronForm, AddSmsForm, + ChannelNameForm) from hc.front.schemas import telegram_callback from hc.front.templatetags.hc_extras import (num_down_title, down_title, sortchecks) @@ -498,6 +499,21 @@ def channel_checks(request, code): return render(request, "front/channel_checks.html", ctx) +@require_POST +@login_required +def update_channel_name(request, code): + channel = get_object_or_404(Channel, code=code) + if channel.user_id != request.team.user.id: + return HttpResponseForbidden() + + form = ChannelNameForm(request.POST) + if form.is_valid(): + channel.name = form.cleaned_data["name"] + channel.save() + + return redirect("hc-channels") + + def verify_email(request, code, token): channel = get_object_or_404(Channel, code=code) if channel.make_token() == token: @@ -1001,8 +1017,8 @@ def add_sms(request): form = AddSmsForm(request.POST) if form.is_valid(): channel = Channel(user=request.team.user, kind="sms") + channel.name = form.cleaned_data["label"] channel.value = json.dumps({ - "label": form.cleaned_data["label"], "value": form.cleaned_data["value"] }) channel.save() diff --git a/static/css/channels.css b/static/css/channels.css index 3ffdd725..0bd53d47 100644 --- a/static/css/channels.css +++ b/static/css/channels.css @@ -9,6 +9,26 @@ border-top: 1px solid #f1f1f1; } +.channels-table .icon-cell { + width: 40px; +} + +.channels-table .icon-cell img { + margin-left: 16px; + height: 40px; +} + +.channels-table .th-name, +.channels-table .th-checks { + padding-left: 15px; +} + +.channels-table .unnamed { + font-style: italic; + color: #999; +} + + .channels-table .value-cell { max-width: 400px; overflow: hidden; @@ -20,22 +40,10 @@ background: #f5f5f5; } - - table.channels-table > tbody > tr > th { border-top: 0; } -.channels-table .channels-add-title { - border-top: 0; - padding-top: 20px -} - -.channels-table .channels-add-help { - color: #888; - border-top: 0; -} - .word-up { color: #5cb85c; font-weight: bold @@ -58,20 +66,32 @@ table.channels-table > tbody > tr > th { font-size: small; } +.edit-name, .edit-checks { + padding: 12px 6px; + border: 1px solid #FFF; + cursor: pointer; +} -.channels-help-hidden { - display: none; +.edit-name .channel-details-mini { + font-size: 11.7px; + color: #888; + max-width: 500px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; +} + +.channel-details-mini span { + color: #111; +} + +.channel-row:hover .edit-name, +.channel-row:hover .edit-checks { + border: 1px dotted #AAA; } .edit-checks { - display: inline-block; - width: 120px; - padding: 6px; - text-align: center; - line-height: 28px; - border: 1px solid #FFF; color: #333; - } .edit-checks:hover { @@ -79,10 +99,6 @@ table.channels-table > tbody > tr > th { color: #000; } -.channel-row:hover .edit-checks { - border: 1px dotted #AAA; -} - .channel-remove { visibility: hidden; } @@ -91,6 +107,11 @@ table.channels-table > tbody > tr > th { visibility: visible; } +.webhook-details td { + max-width: 300px; + +} + .ai-title { margin-top: 2em; } @@ -212,6 +233,10 @@ table.channels-table > tbody > tr > th { font-style: italic; } +.channel-modal .modal-body { + padding: 15px 40px; +} + /* Add Webhook */ @@ -230,4 +255,4 @@ table.channels-table > tbody > tr > th { .zendesk-subdomain input { border-right: 0; -} \ No newline at end of file +} diff --git a/static/css/my_checks_desktop.css b/static/css/my_checks_desktop.css index 9bd12c23..c47ae8a3 100644 --- a/static/css/my_checks_desktop.css +++ b/static/css/my_checks_desktop.css @@ -36,7 +36,7 @@ #checks-table .integrations, #checks-table .timeout-grace, #checks-table .last-ping { - border: 1px solid rgba(0, 0, 0, 0); + border: 1px solid #FFF; padding: 6px; } diff --git a/static/js/channels.js b/static/js/channels.js index aabc1409..50146146 100644 --- a/static/js/channels.js +++ b/static/js/channels.js @@ -1,30 +1,12 @@ $(function() { - var placeholders = { - email: "address@example.org", - webhook: "http://", - slack: "https://hooks.slack.com/...", - hipchat: "https://api.hipchat.com/...", - pd: "service key" - } - - $("#add-channel-kind").change(function() { - $(".channels-add-help p").hide(); - - var v = $("#add-channel-kind").val(); - $(".channels-add-help p." + v).show(); - - $("#add-channel-value").attr("placeholder", placeholders[v]); - }); $(".edit-checks").click(function() { $("#checks-modal").modal("show"); - var url = $(this).attr("href"); - $.ajax(url).done(function(data) { + $.ajax(this.dataset.url).done(function(data) { $("#checks-modal .modal-content").html(data); }) - return false; }); diff --git a/templates/front/channels.html b/templates/front/channels.html index 74c233ed..da95ce31 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -18,114 +18,94 @@ {% if channels %} - - - + + + + {% for ch in channels %} + {% with n=ch.latest_notification %} - - + + + - + {% endwith %} {% endfor %} {% endif %}
TypeValueAssigned ChecksName, DetailsAssigned ChecksStatus Last Notification
{{ ch.get_kind_display }} - {% if ch.kind == "email" %} - to - {{ ch.value }} - {% if not ch.email_verified %} - {% if ch.latest_notification and ch.latest_notification.error %} - - (bounced, disabled) - + + Slack icon + +
+ {% if ch.name %} + {{ ch.name }} + {% else %} +
unnamed
+ {% endif %} +
+ {% if ch.kind == "email" %} + Email to {{ ch.value }} + {% elif ch.kind == "pd" %} + PagerDuty account {{ ch.pd_account }} + {% elif ch.kind == "pagertree" %} + PagerTree + {% elif ch.kind == "opsgenie" %} + OpsGenie + {% elif ch.kind == "victorops" %} + VictorOps + {% elif ch.kind == "po" %} + Pushover ({{ ch.po_value|last }} priority) + {% elif ch.kind == "slack" %} + Slack + {% if ch.slack_team %} + team {{ ch.slack_team }}, + channel {{ ch.slack_channel }} + {% endif %} + {% elif ch.kind == "webhook" %} + Webhook + {% elif ch.kind == "pushbullet" %} + Pushbullet + {% elif ch.kind == "discord" %} + Discord + {% elif ch.kind == "telegram" %} + Telegram + {% if ch.telegram_type == "group" %} + chat {{ ch.telegram_name }} + {% elif ch.telegram_type == "private" %} + user {{ ch.telegram_name }} + {% endif %} + {% elif ch.kind == "hipchat" %} + HipChat + {% elif ch.kind == "sms" %} + SMS to {{ ch.sms_number }} + {% elif ch.kind == "trello" %} + Trello + board {{ ch.trello_board_list|first }}, + list {{ ch.trello_board_list|last }} {% else %} - - (unconfirmed) - + {{ ch.kind }} {% endif %} - - {% endif %} - {% elif ch.kind == "pd" %} - {% if ch.pd_account %} - account - {{ ch.pd_account}}, - {% endif %} - service key - {{ ch.pd_service_key }} - {% elif ch.kind == "pagertree" %} - URL - {{ ch.value }} - {% elif ch.kind == "opsgenie" %} - API key - {{ ch.value }} - {% elif ch.kind == "victorops" %} - Post URL - {{ ch.value }} - {% elif ch.kind == "po" %} - user key - {{ ch.po_value|first }} - ({{ ch.po_value|last }} priority) - {% elif ch.kind == "slack" %} - {% if ch.slack_team %} - team - {{ ch.slack_team }}, - channel - {{ ch.slack_channel }} - {% else %} - {{ ch.value }} - {% endif %} - {% elif ch.kind == "webhook" %} - {% if ch.url_down %} - down {{ ch.url_down }} - {% endif %} - {% if ch.url_up and not ch.url_down %} - up {{ ch.url_up }} - {% endif %} - {% if ch.url_up or ch.post_data or ch.headers %} - (details) - {% endif %} - - - {% elif ch.kind == "pushbullet" %} - API key - {{ ch.value }} - {% elif ch.kind == "discord" %} - {{ ch.discord_webhook_id }} - {% elif ch.kind == "telegram" %} - {% if ch.telegram_type == "group" %} - chat - {% elif ch.telegram_type == "private" %} - user - {% endif %} - {{ ch.telegram_name }} - {% elif ch.kind == "hipchat" %} - {{ ch.hipchat_webhook_url }} - {% elif ch.kind == "zendesk" %} - {{ ch.zendesk_subdomain }}.zendesk.com - {% elif ch.kind == "sms" %} - {% if ch.sms_label %} - {{ ch.sms_label }} ({{ ch.sms_number }}) +
+
+ 		+
+ {{ ch.n_checks }} checks +
+ 		+ {% if ch.kind == "email" and not ch.email_verified %} + {% if n and n.error %} + Disabled {% else %} - {{ ch.sms_number }} + Unconfirmed {% endif %} - {% elif ch.kind == "trello" %} - board - {{ ch.trello_board_list|first }}, - list - {{ ch.trello_board_list|last }} {% else %} - {{ ch.value }} + Ready to deliver {% endif %} - - {{ ch.n_checks }} of {{ num_checks }} - - - {% with n=ch.latest_notification %} {% if n %} {% if n.error %} @@ -140,7 +120,6 @@ {% if ch.kind == "sms" %}

Used {{ profile.sms_sent_this_month }} of {{ profile.sms_limit }} sends this month.

{% endif %} - {% endwith %}
@@ -364,69 +344,72 @@ {% for ch in channels %} -{% if ch.kind == "webhook" %} -