diff --git a/hc/api/admin.py b/hc/api/admin.py index f22ed320..196ec69e 100644 --- a/hc/api/admin.py +++ b/hc/api/admin.py @@ -161,6 +161,7 @@ class ChannelsAdmin(admin.ModelAdmin): list_display = ("id", "code", "email", "formatted_kind", "value", "num_notifications") list_filter = ("kind", ) + raw_id_fields = ("user", "checks", ) def email(self, obj): return obj.user.email if obj.user else None diff --git a/hc/api/models.py b/hc/api/models.py index c2d239ea..650f9d2f 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -463,6 +463,21 @@ class Channel(models.Model): def latest_notification(self): return Notification.objects.filter(channel=self).latest() + @property + def sms_number(self): + assert self.kind == "sms" + if self.value.startswith("{"): + doc = json.loads(self.value) + return doc["value"] + return self.value + + @property + def sms_label(self): + assert self.kind == "sms" + if self.value.startswith("{"): + doc = json.loads(self.value) + return doc["label"] + class Notification(models.Model): class Meta: diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 368d40c2..c0d72003 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -477,6 +477,21 @@ class NotifyTestCase(BaseTestCase): self.profile.refresh_from_db() self.assertEqual(self.profile.sms_sent, 1) + @patch("hc.api.transports.requests.request") + def test_sms_handles_json_value(self, mock_post): + value = {"label": "foo", "value": "+1234567890"} + self._setup_data("sms", json.dumps(value)) + self.check.last_ping = now() - td(hours=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.assertEqual(payload["To"], "+1234567890") + @patch("hc.api.transports.requests.request") def test_sms_limit(self, mock_post): # At limit already: diff --git a/hc/api/transports.py b/hc/api/transports.py index 75c49984..69cba040 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -369,7 +369,7 @@ class Sms(HttpTransport): data = { 'From': settings.TWILIO_FROM, - 'To': self.channel.value, + 'To': self.channel.sms_number, 'Body': text, } diff --git a/hc/front/forms.py b/hc/front/forms.py index ad9ba9d0..95307dc9 100644 --- a/hc/front/forms.py +++ b/hc/front/forms.py @@ -105,4 +105,5 @@ phone_validator = RegexValidator(regex='^\+\d{5,15}$', 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]) diff --git a/hc/front/tests/test_add_sms.py b/hc/front/tests/test_add_sms.py index f76db7d6..02f34fe7 100644 --- a/hc/front/tests/test_add_sms.py +++ b/hc/front/tests/test_add_sms.py @@ -22,7 +22,7 @@ class AddSmsTestCase(BaseTestCase): self.assertContains(r, "upgrade to a") def test_it_creates_channel(self): - form = {"value": "+1234567890"} + form = {"label": "My Phone", "value": "+1234567890"} self.client.login(username="alice@example.org", password="password") r = self.client.post(self.url, form) @@ -30,7 +30,8 @@ class AddSmsTestCase(BaseTestCase): c = Channel.objects.get() self.assertEqual(c.kind, "sms") - self.assertEqual(c.value, "+1234567890") + self.assertEqual(c.sms_number, "+1234567890") + self.assertEqual(c.sms_label, "My Phone") def test_it_rejects_bad_number(self): form = {"value": "not a phone number address"} @@ -46,7 +47,7 @@ class AddSmsTestCase(BaseTestCase): self.client.post(self.url, form) c = Channel.objects.get() - self.assertEqual(c.value, "+1234567890") + self.assertEqual(c.sms_number, "+1234567890") @override_settings(TWILIO_AUTH=None) def test_it_requires_credentials(self): diff --git a/hc/front/tests/test_channels.py b/hc/front/tests/test_channels.py index bbea7a60..dbd9a41a 100644 --- a/hc/front/tests/test_channels.py +++ b/hc/front/tests/test_channels.py @@ -74,3 +74,14 @@ class ChannelsTestCase(BaseTestCase): r = self.client.get("/integrations/") self.assertEqual(r.status_code, 200) self.assertContains(r, "(unconfirmed)") + + def test_it_shows_sms_label(self): + ch = Channel(kind="sms", user=self.alice) + ch.value = json.dumps({"value": "+123", "label": "My Phone"}) + ch.save() + + self.client.login(username="alice@example.org", password="password") + r = self.client.get("/integrations/") + + self.assertEqual(r.status_code, 200) + self.assertContains(r, "My Phone (+123)") diff --git a/hc/front/views.py b/hc/front/views.py index 71a43b60..2dc1bfb8 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -896,7 +896,10 @@ def add_sms(request): form = AddSmsForm(request.POST) if form.is_valid(): channel = Channel(user=request.team.user, kind="sms") - channel.value = form.cleaned_data["value"] + channel.value = json.dumps({ + "label": form.cleaned_data["label"], + "value": form.cleaned_data["value"] + }) channel.save() channel.assign_all_checks() diff --git a/templates/front/channels.html b/templates/front/channels.html index c7d52753..7be69291 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -102,6 +102,12 @@ {{ 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 }}) + {% else %} + {{ ch.sms_number }} + {% endif %} {% else %} {{ ch.value }} {% endif %} diff --git a/templates/integrations/add_sms.html b/templates/integrations/add_sms.html index 43d701d2..a6d3e088 100644 --- a/templates/integrations/add_sms.html +++ b/templates/integrations/add_sms.html @@ -9,14 +9,17 @@

SMS

-

Get a SMS message to your specified number when check goes down.

+

+ Get a SMS message to the specified phone number when a + check goes down. +

{% if show_pricing and profile.sms_limit == 0 %}

Paid plan required. SMS messaging is not available on the free plan–sending the messages - costs too much! Please upgrade to a - paid plan to enable SMS messaging. + cost too much! Please upgrade to a + paid plan to enable SMS messaging.

{% endif %} @@ -24,6 +27,29 @@
{% csrf_token %} +
+ +
+ + + {% if form.label.errors %} +
+ {{ form.label.errors|join:"" }} +
+ {% else %} + + Optional. If you add multiple phone numbers, + the labels will help you tell them apart. + + {% endif %} +
+
@@ -39,6 +65,11 @@
{{ form.value.errors|join:"" }}
+ {% else %} + + Make sure the phone number starts with "+" and has the + country code. + {% endif %}